mmone / OctoprintKlipperPlugin

A plugin for a better integration of Klipper into OctoPrint.
GNU General Public License v3.0
83 stars 62 forks source link

Macros with multiple lines don't work anymore #19

Open oderwat opened 6 years ago

oderwat commented 6 years ago

From 0.2.5 on my macros which are using multiple lines are not working anymore. Instead of sending multiple commands it all ends up in one line with spaces instead of newlines.

oderwat commented 6 years ago

Here some examples of what I used before:

image image image

mmone commented 6 years ago

I do strip newlines from the macros. It was indeed added with the parameterised macro feature to allow for better formatting in the editor, with each parameter in a new line, forgot that this would break having multiple commands.

I will try to fix this tonight. For now the only fix is to downgrade to 0.2.4 by installing from this url: https://github.com/mmone/OctoprintKlipperPlugin/archive/0.2.4.zip

oderwat commented 6 years ago

TY. I changed the code in 2.5.

mmone commented 6 years ago

I don't want to remove the ability to format a command in the editor across multiple lines. So I need to come up with a regex that can split gcode commands. If someone has something ready suggestions are welcome. It obviously also needs to cover the klipper style commands with parameters in the form var=value.

oderwat commented 6 years ago

What is the advantage to allow newlines? The field does simply wrap the text anyway.

I don't see any way to reliably split on commands and esp. not with a regular expression (which is not good for parsing anything).

If you insist on having non standard handling of newlines in the macros I would suggest either:

Double newline

M117 This is all
one line

M117 This the next

Or continuation with a space as the first character.

M117 This is all
 one line
M117 This is the next

I was tempted to also suggest using ; for command limiter, but that is not really ideal (M117 G28;G1 X999 is a valid GCode line)

M117 This is all
one line;
M117 This is the next
mmone commented 6 years ago

I like multiline particularly for the parameterised macros eg.:

PID_CALIBRATE
HEATER={label:Heater, default:extruder, options:extruder|extruder1}
TARGET={label:Target Temperature, unit:°C, default:190}
WRITE_FILE={label:Write to File, default:0, options:0|1}

it's just easier to read with each param in a separate line

mmone commented 6 years ago

Double newline

I think that needs to much explaining. It shouldn't break things either for sure.

Or continuation with a space as the first character.

Klipper config does that and I responded to a least one issue on the klipper github where someone wondered why his config was not parsing anymore because he didn't have leading whitespace in front of a Macro. Probably only makes sense to Python devs :-)

Ideally there should be a solution that needs no explanation at all. That would be to detect the boundaries between commands and then remove newlines if necessary. I think it should be doable to come up with a suitable regex. A quick google brings up this solution. Haven't tested it yet and it doesn't yet cover var=value param form.

oderwat commented 6 years ago

I still think it is not worth to break functionality for having code which is getting wrapped anyway "formatted".

image

vs

image

I don't think that there is any way you could do that universally. M117 G28 "breaks" everything I can think of. The gcode command separator is the newline, thats just how it is. You could also:

End of line continuation:

M117 This is\
one line
M117 This is the next line

That said: I can modify the mod for my systems as I want it.

aeropic commented 5 years ago

@oderwat , I have the same problem, multi lines macros are not working anymore. What is the modification you applied in the 2.5 to make it work again ?

oderwat commented 5 years ago

@aeropic I wonder that this was not getting fixed by now. Seems like not many people are using macros for anything real. You can simply fix it by undoing what I commented here: https://github.com/mmone/OctoprintKlipperPlugin/commit/769b8a53ed5fa38202aaaeaaa107b176749a8ff1#r30293148

Just remove .replace(/(?:\r\n|\r|\n)/g, " ") from the line in the file ~/OctoPrint/venv/lib/python2.7/site-packages/octoprint_klipper/static/js/klipper.js when you are logged in as pi

aeropic commented 5 years ago

Arghh, in the Octoprint directory I do not get any "venv" dir ... octoprint

aeropic commented 5 years ago

I found the file in /home/pi/oprint/lib .... I did patch it but ocotklipper seems to not like it... I had to reinstall the plugin

aeropic commented 5 years ago

I found another way to workaround this bug : I declare all my macros inside the printer.cfg file of klipper so that any macro seen by octoklipper is now a single line macro ;-)

eg : [gcode_macro TEMPsOFF] gcode: M140 S0 M104 S0

arminth commented 5 years ago

I ideed stumbled over the same issue because I wanted too set bed temperature and Extruder at one button-click. It's a bit annoying that it's not working out of the box...

fab33 commented 5 years ago

Solution with macro in printer.cfg is not suitable in all case, because we can't use octoprint event. For example I want M80 in a macro but with macro in printer.cfg I don't recieve PowerOn event.

oderwat commented 5 years ago

@fab33 of course it is not. At least I didn't even consider that as one. I wonder why @mmone is silent about this problem, which seems to be a regression to me.

aeropic commented 5 years ago

Sure it is just a workaround, and As Oderwat says, this is a regression !

jorgempy commented 5 years ago

+1 to this issue, Maybe you can "escape" the new line, as in python? (like @oderwat sugggested) So if you want to have one command in multiple lines, you add a "\" at the end of each? It can still mess up with the M117, but i dont think there's a good solution anyway

ufoDziner commented 5 years ago

I just ran across this issue as well and am going to use the printer.cfg workaround for now. Is there any updates regarding the progress of a fix? Thanks

oderwat commented 5 years ago

@vmarunin that was one of my proposals from August 28. I think we should face it, that @mmone de facto stopped developing this plugin.

vmarunin commented 5 years ago

@vmarunin that was one of my proposals from August 28. I think we should face it, that @mmone de facto stopped developing this plugin.

Yep, sorry for missing it. Anyway, +1 for any way for several commands in Macro

Regexp is not a solution, because Klipper have own "long" useful commands.

jameseleach commented 5 years ago

I've realized that the same fix for #31 applies here. The bug here is that a single sendGcode command is being used to send multiple commands. When there are multiple commands of the same type, like move or heat commands, things get really confused. It's better to send all the commands, individually, but with one sendGcode command, as an array of strings.

I've committed a change to my branch but I'm not 100% sure about it yet. One concern is how it will handle macros with a single command. If someone is willing to test this and let me know that'd be great as I'm not where I can test with my printer, currently.

oderwat commented 5 years ago

@jameseleach I don't think that this issues original problem has to do with the command queue. The problem is that @mmone choose to interpret a LF character as formatting instead of an line ending. Therefor you can't have more than one line in a macro. Your fix would still depend on what is considered lines. As there is currently not even the possibility to have more than one line, there is also no problem with the queue.

jameseleach commented 5 years ago

You could very well be right regarding the parameterised macros, at least. I'm not in a spot where I can test right now as I am using my OctoPrint Pi for something else but should be back up and running tonight or tomorrow.

In any case, with the current codebase a multi-line macro like this:

Name: Preheat PLA
Command:
      M140 S60
      M104 S180

Is received by the printer like this (viewed in the Terminal):

Send: M140 S60 M104 S180

With the change I made the output is two separate commands:

Send: M140 S60
Send: M104 S180

So, this might not resolve this issue but I think it might do the job for #32

I'll do some testing in the next couple of days and post an update.

oderwat commented 5 years ago

@jameseleach Don't get me wrong I think your work is fine! Your change is OK with me. It simply removes the last change of @mmone which destroyed the real multiline macro functionality. As you show, the current result for a multiline macro would be an illegal command. I only wanted to put attention to what was the real problem here. As @mmone is silent for a long time now there is little chance that your changes will be added to the plugin soon.

I already use my own version of it and would love to have your changes in there too. I may switch to your repository because of this reason 👍

jameseleach commented 5 years ago

I've released my changes on my repository and would be interested in hearing feedback - specifically testing the parameterized macros, which I don't use. I did test the 'regular' multi-line macros, which work as expected.

It's available here: https://github.com/jameseleach/OctoprintKlipperPlugin/releases

arminth commented 5 years ago

@jameseleach You should edit the read.me in your version!

Thx for revitalizing! Cheers Armin

jameseleach commented 5 years ago

@jameseleach You should edit the read.me in your version!

Thx for revitalizing!

I thought I had but I went ahead and did a bit more editing, adding installation instructions.

Let me know if you have any luck with the parameterized macros.

ghost commented 4 years ago

What's currently the best way to write a macro that uses two commands in two lines? I'm not getting it to work.

arminth commented 4 years ago

Create a custom GCODE command in printer.cfg with your multiple lines. Then use this single line command in the macros.

OldhamMade commented 4 years ago

Any idea when this will be resolved? I'm switching over to Klipper and being able to quickly throw together macros is useful… but if one has to managed the macro details in 2 places (macro + printer.cfg) it makes the workflow more error prone. I might edit macros regularly but want to "lock down" my printer.cfg once it's stable.