MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.05k stars 19.14k forks source link

[FR] Support out of order execution #20161

Open repetier opened 3 years ago

repetier commented 3 years ago

I have just added this feature to Repetier-Server 0.94.4 beta. You can already download it for testing (linux versions only). Also implemented it into Repetier-Firmware 2 so I could do a full test. Main reason was a problem with emergency parser and host prompt support I had when implementing it into server. When I simulated a filament change and selected purge more often enough the firmware stopped responding due to full input buffer from all emergency commands. Other dialogs can also get that problem. Problem is that input buffer does not get cleared until executed, but here communication in general is stopped so it will not happen. So with this small protocol change that needs active activation from host side this would be easy to solve. Maybe more easily in repetier-firmware due to the way we handle emergency parsing, but I think it still would be possible also in marlin.

My notes on implementation (same as described on reprap https://reprap.org/wiki/Firmware_Capabilities_Protocol#Cap:OUT_OF_ORDER :

Currently all commands are parsed in order with the exception of some emergency commands. These are executed as fast as they get detected. The problem is that they stay in queue until they get handled in correct order to provide the "ok" for host in right order to not confuse host.

Now there are cases where you need to send multiple "emergency" commands like M876 to navigate through e.g. filament change. When you need to purge several times the buffer is full and firmware does not react any more on host navigation.

There are also commands where it would be nice if they work out of order as well because we do not want to wait like babystepping or which are just informational and do not need to be executed in order like M105. Imagine being able to send M115 during slow homing and get updates.

So this is where this extension kicks in. It is designed to be fully backwards compatible to existing hosts. The idea is to have a list of codes that can be executed out of order. These commands get removed from input queue once executed and hence free the input buffer.

Sequence is as follows.

  1. Report in M115 the capability for out of order execution: Cap:OUT_OF_ORDER:1

  2. Wait for host to confirm it understands the extension and send supported codes: M576 S1 out_of_order:M108 M112 M290 M876

Mark feature enabled for the serial where we got the command from. For multiple connections not all might support it. Host now has a list of all codes that are handled out of order. That way the list can be extended any time without requiring host changes!

  1. When a out of order commands on supported connection execute it and confirm it: ooo M290

Out of order commands should have no line numbers. That would make problems with the sequential line numbering of regular commands. With the "ooo" instead of "ok" host knows the emergency buffer is freed and it is safe to send next out of order command.

Race case: When a connect does not reset the connection the firmware might have out of order enabled while the connecting host does not have it enabled and expects an "ok" instead of "ooo" response. To help solve the issue a call of M115 should disable out of order automatically until host reenables it on seeing the OUT_OF_ORDER capability. As long as host sends M115 before any out of order commands this will solve the race condition.

thinkyhead commented 2 years ago

The problem for you is that there is only a single command queue, but you want to command certain things to happen right away without regard for whatever else is going on. So let's just start there.

Instead of forcing firmware to allow the hacking of the main command queue, maybe what you really want is a second command queue that doesn't care about line numbers or checksums, or maybe you want to be able to tell the firmware to start running commands immediately and then tell the firmware to stop running commands immediately — effectively switching from one queue to the other. Maybe you want to be able to put a prefix on lines that should be executed in parallel, like $M115 to get an M115 report without waiting for the main queue.

The first implementation sent to us at MarlinFirmware/Marlin#23691 is problematic. I do not agree with the concept of making the firmware have a series of commands that it will always run immediately. This continues the bad precedent of introducing printers in the wild that don't behave in predictable ways according to established standards.

This is not backward-compatible if it changes command queue execution as a default behavior. Command queue behavior should remain just the same as it has always been, for all commands, executing in the order received. Some commands would be quite problematic to execute in the middle of another command, as this proposes to do, and this may turn out to be especially true for things like babystepping in spite of the first blush.

repetier commented 2 years ago

@thinkyhead No you missed the point. It is not to allow all commands out of order. I agree that this is fatal and should not be allowed. As you see the firmware propagates which commands can be run out of order. So only these are allowed that way. And these commands are more or less the same you would run as emergency commands.

Reason I added them are actually these emergency commands making problems. Especially M876 with filament run out. If your buffer is 127 byte, but lets say 80 byte are used also it does not matter. In filament out you can choose it is not extruding and it shows next prompt requiring new M876 answer. At some point any queue is full since firmware stops executing commands except emergency buffer and if buffer reaches end I can not say extrude more and are stuck or hopefully have a display to select it there.

So what OOO does is do not put line numbers to them and respond with ooo M876 to let host know it is executed. Now firmware can free that buffer so new commands can be parsed. Same might happen with babysteps where a long lag until execution is unwanted and that is also allowed out of order in your display.

And since the communication channel MUST enable OOO explicitly, it is backward compatible. So either emergency parser solution or OOO solution which does the same except rewinding serial buffer and maybe copy already received later data (you know non ping-pong mode in our hosts:-)

thinkyhead commented 2 years ago

Apologies for the long delay in evaluating this concept. There has been an enormous amount of submissions of PRs in recent months, and Feature Requests have fallen by the wayside.

I do want to revisit this concept, but I want to make sure we have good systematic support for it. So much of the G-code parsing machinery in Marlin is ad hoc (or odd hack) that I fear introducing another inlet for commands is going to require a little refactoring. I tried to get around to explaining as much to someone who submitted a solution, but was not able to get that far into the discussion. Importantly, Marlin has some constraints on where G-code is executed, so it cannot be from any arbitrary point in the program flow. That might not be perfectly apparent. The "emergency parser" has been cited, but it just sets some flags when it recognizes a command, and then those flags are handled within the regular execution flow. It wasn't set up to do real parsing.

So… to get support for this concept into Marlin, I'd need to either re-use the "immediate queue" that we use for things like menu items –which will probably work okay– or add an extra "immediate queue" for non-printjob commands — and only apply it when the feature is enabled. It should be turned off by default unless a host (or serial-based controller) requests it. And, it can only apply when the host is using full handshaking with line numbers. Not all print job runners are necessarily going to use handshaking, so in those cases it would be bad to go out of order.

Anyway, things are still on the busy side and the TODO list is long, but I will try and put together a basic PR based on your description, and we'll see how this rickety old firmware holds up.

repetier commented 2 years ago

It does not depend on line numbers at all and as you see it must be explicitly enabled by host. Actually if you only add same commands you already do in emergency parser I see a quite simple solution maybe. The important part is that it must be parsed before it gets to your full g-code parse so it must be completely in your incoming buffer. Once you detect emergency command in your incoming buffer you execute it as always. Now if no OOO is enabled leave it for full parser to send "ok" response. If enabled send "ooo" and remove it from input buffer. That way you free input buffer and you can get more commands like babystep/M108 without overflowing. Only tricky is to send "ooo" without doing it in the middle of and running output. Not sure if that is an issue as I'm not so familiar with structure. Otherwise add a counter for open "ooo" messages in some good place that runs between outputs.

If you can not move back in incoming buffer it gets a bit more complicated. In case of repetier-firmware I had to add an extra buffer from input -> watch so I had control of a buffer.

thinkyhead commented 2 years ago

Thanks for the feedback!

It does not depend on line numbers at all and as you see it must be explicitly enabled by host.

Well, we must be able to distinguish between a command coming from a print job, which must be run in place, and one which comes from elsewhere (with no N parameter or * checksum on it). But, are you suggesting that every immediate command would be preceded by 'enable' and then followed by 'disable'? Ideally, that could work, but we'd have to flag it per-serialport.

The important part is that it must be parsed before it gets to your full g-code parse so it must be completely in your incoming buffer.

The previously-submitted PR got it pretty close, so I've got a good idea where it'll get copied over to the immediate dispatch buffer. Although Marlin can parse a command and run it from anywhere, like a function call, commands should reasonably expect to be running out of the main loop so that they can block, if needed, and maybe prevent blowing up the stack.

So the main thing is to be able to reliably tell which commands will be dispatched immediately, and keep other commands from getting through. I was thinking of adding an extension to the parser so that a command like M100! S8 will be dispatched as a "now!" command. That should be backwards-compatible, as our parser just ignores the ! currently.

Once you detect emergency command in your incoming buffer you execute it as always.

Not in the case of Marlin, alas. The EP is a state machine, and all recognized G-codes are specially handled, not just dispatched to the parser.

if (ISEOL(c)) {
  if (enabled) switch (state) {
    case EP_M108: wait_for_user = wait_for_heatup = false; break;
    case EP_M112: killed_by_M112 = true; break;
    case EP_M410: quickstop_by_M410 = true; break;
    #if ENABLED(HOST_PROMPT_SUPPORT)
      case EP_M876SN: hostui.handle_response(M876_reason); break;
    #endif
    #if ENABLED(REALTIME_REPORTING_COMMANDS)
      case EP_GRBL_STATUS: report_current_position_moving(); break;
      case EP_GRBL_PAUSE: quickpause_stepper(); break;
      case EP_GRBL_RESUME: quickresume_stepper(); break;
    #endif
    #if ENABLED(SOFT_RESET_VIA_SERIAL)
      case EP_KILL: HAL_reboot(); break;
    #endif
    default: break;
  }
  state = EP_RESET;
}
thinkyhead commented 2 years ago

I finally gathered the wherewithal to review the protocol (again?) and it is straightforward. The previous submission threw me for a loop. I'll post something for testing very soon.

rabirland commented 4 months ago

Is there any update to this? Priority commands would be rather helpful for writing external control programs that can immediately have an effect on the machines, e.g.: changing Z offset during a movement, not at the end of the queue.