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

Add M400 dummy command for Octoprint workaround #391

Open
wants to merge 1 commit into
base: edge
Choose a base branch
from

Conversation

vegetablejedi
Copy link

  • Octoprint calls M400 every printing job is ended.
  • Fixed an issue where the M400 command was not implemented and disconnected each time.

- Octoprint calls M400 every printing job is ended.
- Fixed an issue where the M400 command was not implemented and disconnected each time.
@vegetablejedi
Copy link
Author

I remove my PR that I did not want, and I work again to request a pull. :-)

@justinclift
Copy link
Member

@ril3y @giseburt @aldenhart This should be ok to merge as-is yeah? 😄

@ril3y
Copy link
Member

ril3y commented Dec 29, 2018 via email

@justinclift
Copy link
Member

No worries. 😄

@giseburt
Copy link
Member

Hello @vegetablejedi ,

Thanks for this PR. I appreciate you taking the time on this. This version is also cleaner than the last so thanks for that as well.

The only suggestion I have for this PR is that I’d prefer it not be a non-op when it’s a little more work to make it do what M400 is supposed to do.

According to the RepRap wiki:

Finishes all current moves and and thus clears the buffer.

I feel that’s slightly vague, but I interpret that as meaning that after an M400 is issued, no other gcodes will be interpreted until all previously issued commands are executed and motion is stopped. Also, since it’s Marlin, we don’t want an ok on parse but when the command is done executing. We already do similar (but not the response part) in a few other commands. More specifically, in Marlin compatibility we have the temperature commands that wait until the temperature has been reached.

This breaks down to three parts:

  1. Handle the M400 command in the parser and pass it to the marlin compatibility code. Suppress the ok response.
  2. Stop reading new gcode until the condition is met. The condition is two-part:
    A. All other commands issued and read up until this one have been executed.
    B. All motion is stopped.
  3. Issue the ok response.

Part 1 has plenty of examples near where you already edited, so I won’t go into detail there (but feel feee to ask if it’s not clear). I would suggest using an enum in mst (see MarlinStateExtended) that is a state engine for M400 with states like idle/off, waiting, and done (response pending).

The exception to those examples being suppressing the ok, which is a little more involved and not currently handled by anything else. (Also we need to confirm that Marlin does hold the ok until motion has stopped, or perhaps it does something else?) The ok comes from marlin_response() which is called (roughly) like: marlin_response(gcode_parser(...), ...); I looked and we have the status of STAT_NO_DISPLAY reserved for this (but not used). So gcode_parser() (actually _execute_gcode_block_marlin()) would return STAT_NO_DISPLAY and marlin_response() would be modded to treat it the same as cs.responses_suppressed.

Part 2 is a little less obvious. To stop interpreting new gcode, marlin_callback() responds STAT_EAGAIN until the conditions are met.

Part 2a and 2b can be handled like homing handles the same situation, by queueing a command that calls a function that sets a flag that tells the callback that the queue is empty up to where the M400 was issued and that all previous motion has stopped. (Note the callback cannot take direct action such as writing output, it will lock up the system.)

Part 3 is just calling marlin_response() from marlin_callback() to write the ok (Which ignores the buffer it’s handed and uses it’s own, so you can pass nullptr): marlin_response(STAT_OK, nullptr); Don’t forget to reset the state engine (enum mentioned above for step 1) back to idle/off.

Do you feel like giving that a shot? Either way let us know.

Thank you!
-Rob

@justinclift
Copy link
Member

@vegetablejedi Still interested in having this merged?

@solhuebner
Copy link

solhuebner commented Jan 27, 2020

@vegetablejedi I would appreciate if we could get M400 implemented as it is also related to #138 and I would like to see G2 as a viable option for OpenPNP :)

@justinclift
Copy link
Member

@giseburt Would it be workable to merge this now, to get things working... and improvements for the M400 behaviour could optionally be done at a later point if it turns out to be needed?

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