prusa3d / Prusa-Firmware

Firmware for Original Prusa i3 3D printer by PrusaResearch
GNU General Public License v3.0
2k stars 1.05k forks source link

Use M117 to set label for the filament to load at the M600 Insert filament screen #4548

Closed Panayiotis-git closed 1 week ago

Panayiotis-git commented 7 months ago

The M600 G-code command is often used during a print job, to trigger a filament change to a different material/color, without the need of additional specialized hardware. This pull request (PR) extends the command's functionality, by allowing the users to specify which filament to load, adding more flexibility to the multi-color printing process.

In contrast with the solution implemented at the https://github.com/prusa3d/Prusa-Firmware/pull/2380, this PR tries to not modify the command's parameters, but rather use the M117 command to set the next filament's label:

M117 Deep Blue
M600

The Insert filament screen will then show the custom text defined with the M117 command:

Insert filament
Deep Blue
and press the knob

With this PR, the M117 content is also stored in a new text buffer that is updated only by the M117 command. The content remains in the buffer until either of the following happens:

There are two drawbacks to this solution:

  1. The M117 command will immediately update the status screen with the custom message. It will remain visible during the M600's initial printing head movement. This is a rather cosmetic side-effect and it will not affect the command's functionality.
  2. The M600 will consume any message that was previously set by an M117 command. To clear unwanted messages, an empty M117 command should be issued before the M600:
    M117
    M600
github-actions[bot] commented 7 months ago

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG 48 21 247832 5674 6120 2518
MK3_MULTILANG 62 21 247144 5683 6808 2509
3d-gussner commented 7 months ago

@Panayiotis-git First test looks good. The M117 message is shown correctly during M600. :tada:

3d-gussner commented 7 months ago

Here some test pictures with my changes in French as it is one of the messages that "normally" exceeds the limit of 20 char. First Change with M117 Deep Blue followed by M600 Fri Dec 22 11:16:26 2023 Second M600 Change without M117. This is the same as before. Fri Dec 22 11:16:58 2023

Only minor issue I see is if someone sends a different M117 long time before the M600it will also be shown, but that is something we can't prevent from happening without an extra parameter to M117 or M600

3d-gussner commented 7 months ago

@Panayiotis-git Thank you so much to provide a new PR for your initial awesome idea from 19 December 2019. Impressive that you are still willing to work on this 4 years later :hugs:

3d-gussner commented 7 months ago

@gudnimg Do you have an idea how we can not spend 21 bytes of RAM?

sarusani commented 7 months ago

Two (maybe stupid) ideas to mitigate the mentioned drawbacks. Feel free to ignore :)

1) Use a dedicated prefix in the M117 msg when it's intended for M600. Have M117 not print that msg but store it for later use in M600.

2) Change the order and have M600 only consume the M117 msg when it's received between the start of M600 and the rendering of the msg. (Basically the time it takes to do the initial printing head movement) I don't know at all how the receive buffer works or if a host would even send commands after a M600. So this might not be technically possible....

Panayiotis-git commented 7 months ago

@3d-gussner, big thanks for your review! I've integrated the patch with your changes and fixes.

After a bit of tweaking, the PR now only needs 62 bytes of ROM.

Hey @sarusani, love your suggestions! Sadly, the second one can't be done, but the first one could be a great solution. @3d-gussner, could you share your thoughts on whether this change is desirable, and if so, what m117 prefix should we use?

3d-gussner commented 7 months ago

@Panayiotis-git and @sarusani The previous PR #2380 has been rejected as it would need changes in PrusaSlicer by adding a parameter to M600 . So not sure if a change to M117 with a parameter will be approved.

Panayiotis-git commented 7 months ago

In order to mitigate the first drawback mentioned at the opening post, the M600, when issued, initializes the status message to the MSG_FILAMENTCHANGE.

3d-gussner commented 7 months ago

@gudnimg Please have a look if we can save any RAM. I guess not possible without modifying M600 or M117

gudnimg commented 7 months ago

I don't see any good way to free up the RAM. I think the 20 bytes is OK since we have plenty available (2500 bytes). 👍

3d-gussner commented 7 months ago

@Panayiotis-git The PR looks good but i found while testing again a minor issue. Steps:

  1. Connect with a serial terminal
  2. Start a print
  3. Send M117 Deep Blue
  4. Send M600
    1. The Status line shows for a moment a N
    2. This only happens when a M117has been sent before. A M600 on its own works as before and has no weird N Status line.

Please check your PR. I would guess that

Here a picture: Selection_016

3d-gussner commented 7 months ago

Issue with PrusaLink active and this PR. Mon Jan  8 16:53:47 2024

3d-gussner commented 7 months ago

We need to rethink this PR as M117 is used by PrusaLink to show the status. At this moment I don't see any other option than adding a parameter to M117 or M600. I will discuss this within our team. Sorry for the delay @Panayiotis-git as I really like your idea.

Panayiotis-git commented 7 months ago

@Panayiotis-git The PR looks good but i found while testing again a minor issue. Steps:

1. Connect with a serial terminal

2. Start a print

3. Send `M117 Deep Blue`

4. Send `M600`

   1. The Status line shows for a moment a `N                       `

@3d-gussner, I was not able to reproduce this behavior. Whenever I issue the M600 command, the status line changes immediately to Change filament. Either, I've sent an M117 previously or not.

3d-gussner commented 7 months ago

@Panayiotis-git

Whenever I issue the M600 command, the status line changes immediately to Change filament. Either, I've sent an M117 previously or not.

The MK404 shows quite often LCD issues which aren't seen in real hardware. But as you can see there is a N in the status line running this PR test. It is repeatable so there must be somewhere a minor issue.

3d-gussner commented 7 months ago

My biggest issue with this PR now is that the combination of PrusaLink, which uses the Status line M117 very often to show the state of PrusaLink and PrusaConnect "break" the M600 gcode. Sorry have to move it to FW3.14.1 until we decide if we can use some parameters or find a different solution.

Panayiotis-git commented 6 months ago

While testing with an actual SD card print, I've noticed something about the command M117. Unlike the 'M600' command, the M117 does not wait for the planner to complete the pending moves. It immediately sets the status message with the filament name, even if the printer still executes earlier move commands. On the other hand, the command M600 waits for the planner to complete the moves and then sets the status to Change filament and initiates the change filament sequence. This results in the filament name being visible at the status line for a much longer time.

To address this, we can move the lcd_setstatuspgm(MSG_FILAMENTCHANGE); as the first line of code after the case 600: statement. However, this will cause the 'Change filament' message to be shown much sooner, potentially before the planner completes the moves. This may affect the user experience by displaying the message prematurely.

One solution to this situation could be to add a G4 or a M400 command before the M117. Something like this:

M400
M117 Deep Blue
M600

Another solution could be to set the filament name using some specific M117 parameter.

Not changing anything in the code, until a decision is made.

3d-gussner commented 6 months ago

@Panayiotis-git Thanks looking into this deeper. Yes please wait until we decide how to continue.

3d-gussner commented 2 months ago

@Panayiotis-git Your PR isn't forgotten and we will look into it again in few weeks.

3d-gussner commented 1 month ago

@Panayiotis-git Sorry for the late response. We discussed this PR and the issue that PrusaLink uses M117 to update the LCD info screen. So we gonna need some other buffered message for this feature. Try to make it "open" so other features can use the 2nd buffered message as well. Yes it will cost at least 20 bytes (please limit it to one LCD line) but that is okay (approved by the team).

There are also few other Gcodes in RepRap wiki we could re-use for that

If you find the time please implement something and I promise to review it asap and finally merge it.

Panayiotis-git commented 1 month ago

Hey @3d-gussner, no worries!

I propose creating a command to store a generic text that can be utilized later by another command. For G-code, we could either define a new command or repurpose an existing one, such as M70 or M291. These commands are currently not implemented in the Prusa firmware, making them available for this purpose.

3d-gussner commented 1 month ago

Hey @3d-gussner, no worries!

I propose creating a command to store a generic text that can be utilized later by another command. For G-code, we could either define a new command or repurpose an existing one, such as M70 or M291. These commands are currently not implemented in the Prusa firmware, making them available for this purpose.

Thanks, I try to avoid to create new Gcodes and reuse existing one. For me the RepRap Gcode Wiki covers most platforms and use it as a reference.

3d-gussner commented 1 month ago

@Panayiotis-git Please contact me via e-mail.

3d-gussner commented 1 week ago

@Panayiotis-git Any update on this PR using a different Gcode M70 for the message?

Panayiotis-git commented 1 week ago

@3d-gussner , I opened a new PR (https://github.com/prusa3d/Prusa-Firmware/pull/4731) for the implementation of the feature using the M70 command.

Panayiotis-git commented 1 week ago

Closed in favor of https://github.com/prusa3d/Prusa-Firmware/pull/4731