OctoprintKlipperPlugin icon indicating copy to clipboard operation
OctoprintKlipperPlugin copied to clipboard

Macros with multiple lines don't work anymore

Open oderwat opened this issue 5 years ago • 31 comments

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 avatar Aug 27 '18 00:08 oderwat

Here some examples of what I used before:

image image image

oderwat avatar Aug 27 '18 00:08 oderwat

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

mmone avatar Aug 27 '18 07:08 mmone

TY. I changed the code in 2.5.

oderwat avatar Aug 27 '18 11:08 oderwat

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.

mmone avatar Aug 28 '18 09:08 mmone

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

oderwat avatar Aug 28 '18 10:08 oderwat

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 avatar Aug 28 '18 10:08 mmone

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.

mmone avatar Aug 28 '18 10:08 mmone

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.

oderwat avatar Aug 28 '18 11:08 oderwat

@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 ?

aeropic avatar Sep 23 '18 13:09 aeropic

@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

oderwat avatar Sep 23 '18 13:09 oderwat

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

aeropic avatar Sep 23 '18 16:09 aeropic

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 avatar Sep 23 '18 16:09 aeropic

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

aeropic avatar Sep 23 '18 17:09 aeropic

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...

arminth avatar Sep 25 '18 18:09 arminth

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.

fab33 avatar Sep 27 '18 14:09 fab33

@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.

oderwat avatar Sep 27 '18 14:09 oderwat

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

aeropic avatar Sep 27 '18 18:09 aeropic

+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

jorgempy avatar Oct 05 '18 16:10 jorgempy

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

ufoDziner avatar Oct 19 '18 01:10 ufoDziner

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

oderwat avatar Dec 13 '18 17:12 oderwat

@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.

vmarunin avatar Dec 13 '18 17:12 vmarunin

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.

jameseleach avatar Feb 12 '19 00:02 jameseleach

@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.

oderwat avatar Feb 12 '19 01:02 oderwat

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.

jameseleach avatar Feb 12 '19 16:02 jameseleach

@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 👍

oderwat avatar Feb 12 '19 16:02 oderwat

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

jameseleach avatar Feb 13 '19 19:02 jameseleach

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

Thx for revitalizing! Cheers Armin

arminth avatar Feb 13 '19 20:02 arminth

@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.

jameseleach avatar Feb 14 '19 02:02 jameseleach

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

ghost avatar Nov 10 '19 14:11 ghost

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

arminth avatar Nov 10 '19 20:11 arminth