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.68k stars 349 forks source link

fix(ExtruderPanel): restore mode after extruding/retracting #1965

Closed mryel00 closed 2 weeks ago

mryel00 commented 1 month ago

Description

This PR restores the gcode state to the state before the extrusion, triggered through the UI, happenend.

The old behavior changes the extrusion mode and can create problems with absolute extrusion setups. The new behavior saves the state with SAVE_GCODE_STATE an unique name and restores this state with RESTORE_GCODE_STATE after the extrusion happened.

Related Tickets & Documents

This extends on the idea of #1964 but uses only Klipper built in commands to make this more generic.

Mobile & Desktop Screenshots/Recordings

none

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

none

kraftaksvk commented 1 month ago

Hi @mryel00, thanks for extending on my PR, I will test it soon. I just have a question; is the counter really needed? Tell me if I'm wrong, but doesn't SAVE_GCODE_STATE overwrite the save?

EDIT: dude I was just writing this and I noticed the commit 🤦‍♂️😆

kraftaksvk commented 1 month ago

Hello, seems to work fine for me, however i suggest you replace the server messages: this.$store.dispatch('server/addEvent', { message: gcode, type: 'command' }) this.$store.dispatch('server/addEvent', { message: gcode, type: 'command' }) with: this.$store.dispatch('server/addEvent', { message: `G1 E-${this.feedamount} F${this.feedrate * 60}\n`, type: 'command' }) this.$store.dispatch('server/addEvent', { message: `G1 E${this.feedamount} F${this.feedrate * 60}\n`, type: 'command' }) ...or similar to declutter the console (4 lines of messages for one extrusion feels like a bit too much).

Goes from this: before To this: after

edit: ....idk for me when i refresh the interface the console is cleared, but yeah, it would be a lie...

meteyou commented 1 month ago

@kraftaksvk this would be just useless. if you refresh the interface, Mainsail will reload the console history from Moonraker and you will see 4 lines.

furthermore, it would be a lie to display only 1 line...

edit: ....idk for me when i refresh the interface the console is cleared, but yeah, it would be a lie...

the console have to be correct. not "clear"...

kraftaksvk commented 1 month ago

Very true, but can't it be somehow improved?

meteyou commented 1 month ago

it needs 4 lines of gcodes to fix your issue, so it have to be 4 lines of gcodes. this is the shortest way without external sources and i se absolutly no issue with 4 lines of gcodes.

mryel00 commented 1 month ago

Altering the output on the console will be very confusing.

  1. If someone got the printer in absolute extrusion mode and then only see a series of G1 E1 F300, the person could wonder why it's even extruding, as the console only shows movement to the same e coordinate.
  2. If we would only show M83 and G1 E1 F300 the same person could think that the extrusion mode got changed, as the old behavior did.
  3. If we would show all the commands in the same line, without line breaks, the output would look like a non functional gcode and might be even worse than 4 separate lines.

Those 3 points should get you the idea of why altering the output, is a pretty bad idea.

but can't it be somehow improved?

Only if you find something with less commands without the need of adding new macros. The solution has to be generic that everyone can use it right out of the box.

kraftaksvk commented 1 month ago

thanks for the explanation, it was somewhat late for me but now I realize the possible consequences, I apologize. I also remembered if I'm not wrong, that there is an option for text size in the console..? Which could in turn look better.

pedrolamas commented 1 week ago

Probably an overkill, but I do wonder if the same "save & restore" approach should be done for printhead moves (X, Y, and Z) initiated from the UI... 🤔

meteyou commented 1 week ago

Probably an overkill, but I do wonder if the same "save & restore" approach should be done for printhead moves (X, Y, and Z) initiated from the UI... 🤔

Thank you for your comment. I thought Klipper would return to the original position with the RESTORE_GCODE_STATE command, but it doesn't. I've also pinged you in my test PR. I'll ask Alexz what he thinks about this PR. I trust his opinion in terms of macros.

zellneralex commented 1 week ago

@pedrolamas my answer to it can be found at https://github.com/mainsail-crew/mainsail/pull/1988.