intelligent-agent / redeem

Firmware for Replicape
http://wiki.thing-printer.com/index.php?title=Redeem
GNU General Public License v3.0
36 stars 44 forks source link

Conforming M27 response to Octoprint expections #145

Closed goeland86 closed 5 years ago

goeland86 commented 5 years ago

The if/else logic for what message to send back to the printer was inconsistent for M27, causing issues with the Print from SD functionality.

This should resolve the issue #142 when running with Octoprint 1.3.10 RC1+

goeland86 commented 5 years ago

As far as I can tell, the SD printing state from OctoPrint's purview is "SD printing active" or "Printer is interactively responding".

I would avoid introducing additional state management if there's no gain to the use cases.

goeland86 commented 5 years ago

@Wackerbarth Are the last changes not to your liking?

Wackerbarth commented 5 years ago

As far as they go, the changes seem to be an improvement. However, there are two things holding this up. (1) I still believe that the code fails to meet the "specification"

no file selected: Not SD Printing or SD printing byte 0/0 (the latter shouldn't exist but I've seen it in the wild)
file selected but not yet printing: SD printing byte 0/235422
file selected and printing: SD printing byte 2134/235422

and (2) I have seen no evidence that this has been tested by anyone.

goeland86 commented 5 years ago

Well my printer is out of commission until the weekend (for mechanical reasons) so I can't test the changes "live" until then, but it should be tested by someone else as well.

Regarding the spec, OctoPrint doesn't start issuing M27 until you actually set OctoPrint in print mode - and I'm still weary of sending out SD printing byte 0/235422 to OctoPrint without validating it yet. If you have access to a printer, can you please check if removing the skip 0 step functions or crashes either Redeem or OctoPrint?

goeland86 commented 5 years ago

@Wackerbarth I have tested the current fix as it is now successfully.

goeland86 commented 5 years ago

@Wackerbarth so, further testing this morning after the print finished. Here's the OctoPrint terminal log for you to see what's happening after I issue M23, but not M24. I suspect that the active flag is only true when the print has begun (i.e. M24). If so, that's a flaw in the state management, to some extent, but it doesn't break the current expected responses from Redeem. It does, however allow one to send M24 and not know what file is loaded.

Send: M27
Recv: Not SD printing.
Recv: ok
[...]
Send: M27
Recv: Not SD printing.
Recv: ok[...]
Send: M23 /lcl/calibration-cube.gcode
Recv: File opened:/usr/share/models/calibration-cube.gcode Lines:16720 Size:443908B
Recv: File selected
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
[...]
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
[...]
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok
[...]
Send: M27
Recv: Not SD printing.
Recv: ok
Send: M27
Recv: Not SD printing.
Recv: ok

Note the [...] are filtered out M105 & responses.

Wackerbarth commented 5 years ago

@goeland86 -- As I expected, the log that you provide indicates a deviation from the expected behavior. But the important thing that you have omitted is what responses we get DURING and AFTER a print. (BTW, "..." is sufficient for repeating M27 responses which are of the same form as the prior one.)

Wackerbarth commented 5 years ago

This functionality was included as a part of PR#158.