-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
Introduce M108 cancel wait for heatup/cooldown of the hotend and bed #3611
Introduce M108 cancel wait for heatup/cooldown of the hotend and bed #3611
Conversation
This G-code is asynchronously handled in the get_serial_commands() parser.
Nice idea, but when a script (g-code program) is running as useless as M112. The buffers are full, no further command is read from the serial until M109/M190 has finished. |
@Blue-Marlin the command can still enter the The only situation where M108 would not be executed/enter the buffer is |
Test: |
@Blue-Marlin if a script is spamming Marlin with G-code then even the Take a look at alephobjects@dad9017. alephobjects@65e4ef9 and alephobjects@93b6bff fix a couple mistakes that slipped through the cracks. |
The host only "spams" if it receives an "OK" from Marlin, but in a printing situation, Marlin should have its buffer full as @Blue-Marlin is pointing out. |
@jbrazio the host should not keep It's the job of the host to maintain a certain number of lines inside |
There are two parts to this, first there's Marlin and how it handles commands it receives ( As for the issue of the Also, I believe that Pronterface sends only 1 command at a time to Marlin and only sends the next when it receives the 'ok'. The problem is that it will timeout a command after X seconds (5 or 10 or 30, I'm not sure), and will force the next command to be sent. The problem with that is that a M109 or M190 will usually last more than a few seconds, which causes Pronterface to start flooding Marlin with commands every X seconds until it receives the 'ok'. I think that's also a bug in the host because it interprets all commands as being equals. Cura LE has a different timeout depending on the command being executed, a G28, G29, M109 or M190 is expected to take longer than a G1, and the timeout is therefore different for those commands. This allows us to avoid unnecessarily assuming that the 'ok' was lost and causing the behavior that we wanted to avoid in the first place. All of these are host issues/bugs (that hopefully we fixed properly in Cura LE), and should not affect the decision making in implementing a feature in Marlin, as I don't think it's the job of Marlin to limit itself in order to work around bugs in other people's software. |
I'll try a crack at explaining this too. Sorry if I repeat anything already stated! The host sends commands as fast as it can, while Marlin buffers commands as long as there is space in the buffer. Once the buffer is full, the host is "held off" and must wait for Marlin to have a space in the queue before its commands can be accepted. The buffer has 4 slots, by default. So the host might send: M109
G1 X100 Y100 F4000
G1 X110 Y100 F4000
G1 X120 Y110 F4000 …and now the buffer is full, so the host is "held off." Any serial data sent by the host after these commands is still in the serial buffer. You might think it's still be possible to intercept |
P.S. The solution we are looking for is called "handshaking" and depends on both host and firmware following extra protocols.
Then if the buffer is full, the host will not try to send more than one command. A user could then send I don't believe that hosts currently do any waiting for an "Acknowledging" reply before sending the next command. |
@thinkyhead that's the holly grail, but host people don't give a shit about that.. you saw what happen with the "busy" feature. Nevertheless I do believe that they wait for an "OK" before sending the next command. |
@jbrazio Anytime I make changes related to "hosts and protocols" I make sure to involve the host people. Compared to some earlier changes I've made (naïvely), I thought the "busy" feature was received pretty well, and got a lot of good feedback. Which reminds me, we need to document our new codes (like |
Instead of this feature, I suggest making an alteration to the "kill button" feature. When pressed, if heating is active just cancel heating. If heating isn't active (as on the next press) then do |
@thinkyhead kill() is not the functionality we're seeking here though; this G-Code simply adds the ability to stop a wait for target temp to be reached (and doesn't touch the target temp). Please reconsider adding this G-Code. |
For me this type of "immediate" commands require a reformulation of the parser thus it will be something we will need to have in mind when rewriting the new parser. |
TL;DR:
@thinkyhead This pull request is not effecting how the host and Marlin communicate, it's about adding a new g-code to allow cancelling a heat&wait command (M109/M190). Any issues with the
I disagree because not only will it become a mess with docs (a single command doing 2 different things dependent on the current state) and confusing to users (does it cancel the heatup or just cancel the blocking wait of the command), but it will also cause huge headaches as it is prone to race conditions :
Please reconsider or at least reopen the issue until it is discussed properly instead of dismissing it because of an entirely unrelated issue (host communication issues). And in response to @jbrazio
I do care about that, and I've spent a considerable amount of time resolving those specific issues from the host side. As for what you do believe the host does, that's right in the case of pronterface, but in the case of Cura, it fills the queue with 3 commands THEN it waits for 'ok' before sending the next, but the command timeout issue is what causes the problems with pronterface and what used to cause problems with Cura (see previous comment). I don't know about the behavior of the other host softwares. |
@kakaroto The code in this PR won't work. Because Marlin (MARLIN!) isn't listening to serial input during As @Blue-Marlin correctly pointed out, That is why I suggested altering the hardware kill button, because it is one way that you can talk to Marlin while the queue is full. Also, a "Cancel Heating" menu item can bypass the GCode queue. So these are the only options available for a user to cancel waiting for heating. The machine should of course still wait for heating if the temperature is below minimum extrusion temperature, otherwise it will start to "print" but it will not move the E axis, and it will spit out errors the whole time. |
@thinkyhead this is actually not the case, referring to my response to @Blue-Marlin:
We have tested this many times and it does work. We have a good understanding of how the Please test this PR by sending your machine an M109 R# and/or M190 R# then send G28 then M108. I suggest something like:
|
That GCode doesn't fill the command buffer. Add a few more commands to the end and test it again. |
@thinkyhead certainly if the command buffer is full then you cannot perform a command asynchronously, which is exactly why it is the job of the host software to ensure that there is available volatile memory (buffer array element) on the MCU to execute an asynchronous command.
This is because Cura LE is properly maintaining Marlin'a command buffer, as it should. The MCU will otherwise just accept any command it is given over USART. Choosing to discard certain commands on the firmware side is not the correct solution. The host software needs to be aware of which commands would be executed asynchronously and only fill the reserved element(s) of the command buffer with any one of those given async G-Codes and not the synchronous ones. This is the proper way to handle this issue, Marlin cannot simply do it alone, that is a huge reason why we return an 'ok' after processing a command to begin with; the host has a responsibility for properly managing the USART line and to not congest the firmware's buffer. This is also why host softwares maintain a buffer of their own as well, so that G-Codes are processed in their anticipated manner. Marlin needs to be able to execute certain G-Codes asynchronously and M108 is a perfect example of such a desired functionality. |
No.. the host's job is to send commands until the firmware is full, for instance the "look ahead" i.e. the feature to produce a smooth extrusion is dependent on a "full" buffer. The host couldn't care less for firmware's SRAM or "volatile memory" as you called it, and it shouldn't as long as the "OK" commands keep being sent by the firmware. I feel we are iterating over the same point here. Nevertheless did you even test your code ? Did it behave as you expect ? Which were the test conditions ? |
@jbrazio currently 'ok' responses are only sent once a command is finished processing, not when it is received, with the exception of when G0/G1 enters the block buffer. Therefore, currently, the host doesn't know if the data was written to the SRAM until it is processed; and even if it did know it was received successfully how can the host force an asynchronous G-Code line to Marlin (and Marlin execute it async) if Marlin's command buffer is congested? We've tested this several times, since Cura LE properly maintains Marlin's buffer, M108 is guaranteed to cause |
Exactly, when the buffer is full, the next "ok" response from Marlin will mean a slot in the buffer just got free. |
@jbrazio correct, but if there are no free elements in |
Marlin is listening to serial input during M109 and of course this was tried and tested for months in all use cases before the pull request was sent.
M112 is not broken if the host acts properly.
The code in this PR will work, as long as the host doesn't do something stupid. If for the same reasons you have, M112 wouldn't work, then why didn't you remove that command ?
Yes, the code @scalez put in his comment wouldn't fill the command buffer, and that's because he didn't know which host you are using, and because you said serial input is not read. But it proves that serial input is actually read and if you download and use the latest version of Cura LE (19.12) from https://www.lulzbot.com/cura and try whichever gcode you want which would actually fill the command buffer, then that would still work because Cura will do the right thing.
The look-ahead feature will still work because the host will fill the command buffer of Marlin during moves, During a M109 or M190, there are no moves, and there is no look-ahead, which is why it will not affect print speed if the buffer is empty for a few milliseconds after heating is done. During moves, we do fill the command queue but we also need to keep at least one slot empty in the queue for async commands, so when M112 is sent, it will be sent and processed asynchronously, so it would work. Read my commit log here from Cura : https://code.alephobjects.com/rCURA397d17dadb66de92fcd01f7155e19224bc411c47
Yes, the code has been tested many, many times and has been tested for months now without issues in all possible use cases we could come up with. Test conditions are to use Cura LE 19.12+. |
IMHO, I think if you see Marlin as a command processor, this is adding unneeded complexity and the "wait for heatup" logic should be handled in the host. |
@daid that may be the most ideal way to handle such situations, although any input given to the digital pins would also have to be able to wait in queue (including a GLCD), so the MCU needs to somehow be aware that it is in a wait state. @jbrazio @thinkyhead @Blue-Marlin: here's a video of a Mini receiving an M109 S200, G28, then spammed many M105s (definitely enough to potentially clog the command buffer). Notice that the expected behavior still occurs when using Cura LE 19.12: https://goblinrefuge.com/mediagoblin/u/kuzmenko/m/marlin-m108-example/ |
"Noone" is not a word. Sometimes I wish it was. The weirder word is "weird" because it places "e" before "i" in contravention of the "usual rule." |
If I recall correctly more words break that rule than abide by it. |
Indeed. My last name, for example: Lahteine. But then, it is a Finnish name… |
Yeah, it realized it should be "no one" but thought it is funny 😄 And god, I made a spelling mistake in comment of spelling... weird :) |
And me please, as I'm one of the main developers of Cura LE! |
@daid, @foosel, @repetier, @kakaroto you'll receive and invitation to join the organization so I can made you part of a team. What is Kliment's github username ? As for the developers I suggest @thinkyhead, @AnHardt and @CONSULitAS as our trusty advisor. |
The team is created and people invited, is just a question of accepting the requests. To use the team the tag is @MarlinFirmware/host-software-team |
Working or no, here ya go y'all! You can always try |
Incidentally, we are totally open to choosing a different code number. The sooner the better, of course. |
@thinkyhead I think |
Nice idea! Have G29 same "cancel" command? If it does not.. should we have separate cancel commands for each long-time action inside Marlin or one universal command with parameter (describe what we want to stop) ? |
I just added support for |
@thinkyhead @AnHardt what about handling M18/M84 in |
I vote no on M18/M84. For one thing, the stepper ISR or planner would immediately power the steppers right up again. |
@thinkyhead ah you're right, I just realized we have |
|
@thinkyhead ah yes the need for a flag to escape from I'm not sure if the same flag should be shared between |
Get to know the Marlin architecture. You can't have more than one command running at once. Marlin will either be inside of |
@thinkyhead I understand it couldn't inject in the middle of handing an ISR such as Try selecting the "Stop print" whilst in the middle of |
IMHO quick_stop will not help a lot. The most time G29 is waiting for a move to finish. If the motion Q is emptied, G29 happily will continue with the next move - and a corrupted position. M108 and asking the flag in G29, at a couple of places, has a much better chance to succeed. |
@Blue-Marlin you could use a flag to escape from |
I wasn't talking about the ISR, but the main thread and the command parser. When a command like
By setting a flag that, when set, causes the ISR to flush itself.
Bad Idea!™ Setting a flag is safe. But calling a function from the RX interrupt —like
Yes, the LCD can take direct actions, but if an LCD menu item is only injecting a GCode command (as some of them do) they just go into the queue, and will not interrupt other GCode commands. And actually, the "Stop Print" command doesn't cause
I implemented methods to recover the current position directly from the stepper counters after a forcible interruption. Anyway, I believe all the functions used by G29 now use |
This G-code is asynchronously handled in the get_serial_commands() parser.