mainsail-crew / mainsail

Mainsail is the popular web interface for managing and controlling 3D printers with Klipper.
https://docs.mainsail.xyz
GNU General Public License v3.0
1.74k stars 376 forks source link

fix(extruderPanel): Modified ExtruderControlPanelControl to use _CLIENT_EXTRUDE and _CLIENT_RETRACT for correct extrusion mode after operation #1964

Closed kraftaksvk closed 3 months ago

kraftaksvk commented 3 months ago

Description

This PR changes the way how Extruder Control Panel sends motion requests to the extruder. Originally, when pressing EXTRUDE, the panel would send out this command:

M83
G1 E${this.feedamount} F${this.feedrate * 60}

RETRACT command:

M83
G1 E-${this.feedamount} F${this.feedrate * 60}

This puts the extruder movements into Relative mode and stays like that until M82 is emmited, resulting in unwanted behavior if using Absolute mode otherwise.

My Pull Request replaces that EXTRUDE command with:

_CLIENT_EXTRUDE LENGTH=${this.feedamount} SPEED=${this.feedrate}

...and RETRACT command with:

_CLIENT_RETRACT LENGTH=${this.feedamount} SPEED=${this.feedrate}

By utilizing _CLIENT_EXTRUDE and _CLIENT_RETRACT defined in mainsail.cfg, we address the issue of movement modes, as these macros take care of retaining the same movement mode, and as a bonus it also handles firmware retract and cold extrusion protection.

Related Tickets & Documents

none

Mobile & Desktop Screenshots/Recordings

Before:

before

After:

after

(PROBE_EXTRUSION checks whether Absolute mode is present)

[optional] Are there any post-deployment tasks we need to perform?

none

Signed-off-by: Marek Rozemberg marek.rozembergsk@gmail.com

meteyou commented 3 months ago

hey @kraftaksvk!

Thank you very much for the PR, but I have some questions about it.

Can you also describe the exact issue you want to fix with that? Maybe we can find a better solution to fix your issue.

kraftaksvk commented 3 months ago

Hi @meteyou. Well, those are some good points 😅😅. I figured that mainsail.cfg would be the same in every instance as it is read-only. My issue is as described, the buttons on the panel force M83 every time, so if the user doesn't notice, and forgets to send specifically M82 (G90 doesn't seem to affect) manually or in a start macro, bad things could happen (personal experience). I would like to see similar behaviour as with _CLIENT_EXTRUDE, which tolerates the previous movement mode and applies it after move. I hope we can find another way then!

meteyou commented 3 months ago

To be honest, an M82 in the start marco is an essential requirement for an absolute extrude print. Even if a relative retract is made in the end gcode (which is normal), this is required at print_start. Furthermore, this is not a fix if a macro has been copied from somewhere else and uses relative extrusion. To be sure that a print with absolute extrusion works, you have to set it in your start gcode.

kraftaksvk commented 3 months ago

I agree. I'm going to close this PR. Have a nice day!

meteyou commented 3 months ago

Feel free to contact us if you find another problem with relative extrusion outside of print start.

@mryel00 also wants to test something without _client_extrude, maybe this would also be a possible fix for this issue without "external macros".

mryel00 commented 3 months ago

@kraftaksvk My PR #1965 is up. The code about the names is just rudimentary and will be fixed by @meteyou. I'm not a web dev 😅 I would appreciate if you can test it. This approach should be more generic and hopefully work on every setup, without messing stuff up.