Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meatpack::report_state on serial port init #20903

Conversation

ellensp
Copy link
Contributor

@ellensp ellensp commented Jan 28, 2021

Description

Currently meatpack report_state only replies on serial_port. This effectively disables meatpack for all but the primary serial port.
This PR makes report_state reply to the initiating serial port.

Requirements

#define meatpack
octoprint running meatpack plugin.

Benefits

Any serial port can run meatpack.

Configurations

Related Issues

Was mentioned in Marlin Discord by cass-e

Copy link
Contributor

@CRCinAU CRCinAU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks ok, but I'm not sure I could replicate the setup required to do functional tests...

@cass-e-design
Copy link

It handshakes for me succesfully on both ports now, and I have it running through an octolapse fake print to confirm and everything seems to be in order. I can run through other functional tests if you need?

@CRCinAU
Copy link
Contributor

CRCinAU commented Jan 28, 2021

It handshakes for me succesfully on both ports now, and I have it running through an octolapse fake print to confirm and everything seems to be in order. I can run through other functional tests if you need?

When doing testing, I normally just cycle through options in different combos and run a print (usually the 20mm XYZ calibration cube) and see that it all works well...

I doubt that's really required here other than verifying that your other serial ports work as expected....

@thinkyhead thinkyhead changed the title Meatpak update report_state to reply on initiating serial port. Meatpack::report_state on serial port init Jan 28, 2021
@thinkyhead
Copy link
Member

thinkyhead commented Jan 28, 2021

I applied one small change here, which was to relocate the redirect ahead of the command processing so that the redirect only needs to be set once and will stick until the command returns.

Also, the 'reset' command was causing the 'report' to get called twice, so that's fixed too.

…Aaaand, added a serial_index_t so we have a standard type to refer to a serial port index.

@thinkyhead thinkyhead merged commit c929fb5 into MarlinFirmware:bugfix-2.0.x Jan 28, 2021
@rado79
Copy link
Contributor

rado79 commented Jan 28, 2021

Connection doesn´t work for me with latest changes
Configuration.zip
Configuration_adv.zip
meatpack 1.5.21

octoprint terminal:
Changing monitoring state from "Offline" to "Detecting serial connection"
Performing autodetection with 1 port/baudrate candidates: /dev/ttyACM0@115200
Trying port /dev/ttyACM0, baudrate 115200
Handshake attempt #1 with timeout 2.0s
Connected to: PackingSerial<id=0x6cc47f50, open=True>(port='/dev/ttyACM0', baudrate=115200, bytesize=8, parity='N', stopbits=1, timeout=2.0, xonxoff=False, rtscts=False, dsrdtr=False), starting monitor
Send: N0 M110 N0125
[...]
Handshake attempt #2 with timeout 2.0s
Send: N0 M110 N0
125
[...]
Handshake attempt #3 with timeout 2.0s
Send: N0 M110 N0*125
[...]
Changing monitoring state from "Detecting serial connection" to "Error: No more candidates to test, and no working port/baudrate combination detected."
Changing monitoring state from "Error: No more candidates to test, and no working port/baudrate combination detected." to "Offline (Error: No more candidates to test, and no working port/baudrate combination detected.)"
Connection closed, closing down monitor

@CRCinAU
Copy link
Contributor

CRCinAU commented Jan 28, 2021

Performing autodetection with 1 port/baudrate candidates: /dev/ttyACM0@115200

What if you just set the port and baud rate manually?

@rado79
Copy link
Contributor

rado79 commented Jan 28, 2021

Performing autodetection with 1 port/baudrate candidates: /dev/ttyACM0@115200

What if you just set the port and baud rate manually?

tried a few times without success

@ellensp
Copy link
Contributor Author

ellensp commented Jan 28, 2021

Is the pi on serial 0 or -1 ?

@rado79
Copy link
Contributor

rado79 commented Jan 28, 2021

Performing autodetection with 1 port/baudrate candidates: /dev/ttyACM0@115200

What if you just set the port and baud rate manually?

there is a difference between autodetection and manual:
with autodetection - see above
with manual - temperatur reading works, but no moves or heating or anything else available

Changing monitoring state from "Offline" to "Opening serial connection"
Changing monitoring state from "Opening serial connection" to "Connecting"
Connected to: PackingSerial<id=0x6cc47f50, open=True>(port='/dev/ttyACM0', baudrate=115200, bytesize=8, parity='N', stopbits=1, timeout=10.0, xonxoff=False, rtscts=False, dsrdtr=False), starting monitor
Send: N0 M110 N0*125
Recv: T:24.38 /0.00 B:24.38 /0.00 @:0 B@:0
Recv: T:24.38 /0.00 B:24.38 /0.00 @:0 B@:0
Recv: T:24.38 /0.00 B:24.38 /0.00 @:0 B@:0
Recv: T:24.38 /0.00 B:24.53 /0.00 @:0 B@:0

@rado79
Copy link
Contributor

rado79 commented Jan 28, 2021

Is the pi on serial 0 or -1 ?

how can i check this?
Marlin setting are #define SERIAL_PORT 0 and #define SERIAL_PORT_2 -1

@ellensp
Copy link
Contributor Author

ellensp commented Jan 28, 2021

how do you have the controller connected to the pi? usb cable from the controller to the PI, or did you add wires between tx0/rx0 and the pi, the device name ttyACM0 would indicate the usb cable.

@rado79
Copy link
Contributor

rado79 commented Jan 28, 2021

connection is usb cable from the controller to the PI

@CRCinAU
Copy link
Contributor

CRCinAU commented Jan 28, 2021

Tested and confirmed broken in the current bugfix-2.0.x using:

Ender 3 Pro + SKR Mini E3 v2.0 mainboard
Plugin version 1.5.21

I know the Ender 3 V2 was working as of 8 hours ago, I'm just going to roll a new firmware for that and test to see if thats broken too...

@CRCinAU
Copy link
Contributor

CRCinAU commented Jan 28, 2021

Ender 3 V2 works ok still.

SKR Mini E3 v2.0 does not work. Seems like the TX chain to the printer is broken.

Configs as per:
https://github.com/MarlinFirmware/Configurations/tree/bugfix-2.0.x/config/examples/Creality/Ender-3%20Pro/BigTreeTech%20SKR%20Mini%20E3%202.0

Wondering if this is something to do with before MEATPACK mode is enabled?

Interestingly, if I disable the MeatPack plugin in OctoPrint, I can successfully connect to the printer....

Thinking this could be because the config is:

#define SERIAL_PORT 2
#define SERIAL_PORT_2 -1

@CRCinAU
Copy link
Contributor

CRCinAU commented Jan 28, 2021

@thinkyhead - Would this work with -1 being the USB emulated serial port?

Given that its an emulated port and iirc, baud rates don't really matter, hence in theory, Meatpack won't help in this case, is it still supposed to work?

@rado79
Copy link
Contributor

rado79 commented Jan 28, 2021

Interestingly, if I disable the MeatPack plugin in OctoPrint, I can successfully connect to the printer....

confirm

@cass-e-design
Copy link

cass-e-design commented Jan 29, 2021

Can confirm on SKRv1.4 that it worked for me on the previous version (49eaf45), but doesn't work now.

using minicom, it seems like when I'm trying to contact on SERIAL_PORT_2, it's responding on SERIAL_PORT, and when I contact on SERIAL_PORT, I have no evidence it is responding on either.

Because I'm presently using minicom on the same pi (and it seems that octoprint and minicom cannot monitor/use one port simultaneously) it's difficult to 100% verify a lack of response on the same port octoprint is using.

Edit: Without plugin, everything works fine, as in other reports

Jyers pushed a commit to Jyers/Marlin that referenced this pull request Feb 3, 2021
kpishere pushed a commit to kpishere/Marlin that referenced this pull request Feb 19, 2021
zillarob pushed a commit to zillarob/Marlin that referenced this pull request Feb 25, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Apr 29, 2021
thinkyhead added a commit that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants