-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Arc welder changes the case of things, including comments. #27
Comments
Hey, I think I have solved this actually, but have not released the changes. I'll get back to you ASAP about this. |
Hmm, that was easier than I though it would be to check :) See this commit. If you'd like to try it out, you can install from the plugin manager using this URL: https://github.com/FormerLurker/ArcWelderPlugin/archive/devel.zip |
Thanks for the quick response! downloading to test now |
Anytime! I'm excited to hear how it goes. I'm planning to release this very soon, even though i know some OS/Compilers have issues with it. Fortunately not RPI, and most all modern compilers will work when using python 3. Good luck! |
I was taking a look to the approach for preserving capitals (since you're using C for the parsing which is not exciting for string handling) and it looks like your excluding certain commands from the upper case-ification. Unfortunately, there is an additional area of case sensitivity that is much harder to detect: Klipper macros. I have a macro in my start GCode that loads a bed mesh profile for "ender3", which Arc-Welder dutifully makes uppercase for me and Klipper balks at loading that named profile. Not every macro cares about case by any means, but some that expect text parameters can. Since a Klipper macro could be named absolutely anything (Klipper doesn't restrain GCode to M*, G*, etc commands) it would be impossible to manage with the current approach as far as I can see. The only solution I can offer is not to change case for ANY command, but I imagine that would make processing more complicated. I will take a look and see if there is some easy approach I am missing (or you could correct me if I'm way off-base after a quick glance through the code) - but I have to add that I love the plugin and it works fantastically with Klipper firmware with a simple config line to enable G2/G3. Keep up the great work! |
Thanks for the observations. Yes, string handling isn't fun at all. It was written for Octolapse because python was excruciatingly slow, but at a high cost (my sanity for one). Octolapse only needed to interpret the gcode, but arcwelder needs to rewrite it, so perhaps I can make some changes with that in mind. Eventually Id like to publish the parser and printer state tracking as a python package, so maybe it is worth dealing with this. I will take a look myself, but let me know if you have ideas. I haven't used c/c++ in over 20 years, and was relearning as I went, which you will definitely see, lol!! Also, thanks for your kind words! |
OK, I've made some additional changes to preserve most gcode formatting, including case and spacing. It's currently available in the dev branch, which can be installed with this URL: https://github.com/FormerLurker/ArcWelderPlugin/archive/70c2929bdaf5ee84d59c978979116c8354076664.zip Please let me know how this works for you. I'm also interested in how it handles Klipper Macros. However, I think the processor will attempt to interpret the macros as typical gcode, so I might have to program in bits to detect these macros, which is most unfortunate for the printer state detection. |
I haven't had a chance to test your changes yet. I went the other direction and attempted to remove case changing by being a bit more inclusive in testing for commands. You should be able to see this in my fork repo. It works perfectly on my G-Code generated by Prusa Slicer (which always uses capitals for the G/M commands so it is not the greatest test case) and it works perfectly to preserve case on non-move commands. Also, I should clarify that I believe Klipper macros are case-insensitive themselves but it was the parameter I was sending that was case-sensitive for the mesh loading. It was a bit of an edge case but I thought it was worth mentioning. |
@fiferboy, no problem. I think it's still possible that some macro names might cause issues, but I haven't tested everything yet. I need another printer so I can experiment. I think if you named your macro something that looked like a valid G0/G1 gcode ArcWelder might mess things up. Hopefully Klipper has some specific rules about the names that can be used for macros. ArcWelder doesn't care if there are any spaces in the gcode, so commands like G1X1Y1 will work (and will on many 3d printers too, which is why I support it). Because of this anything that COULD be interpreted as a gcode will be. ArcWelder only rewrites commands that can be converted into G2/G3, so it should output these lines correctly, but it could mess up the position tracking IF the name can be interpreted as a valid gcode. Also, in the latest dev commit (linked above in my last comment) all case should be preserved EXCEPT for G2 G3 commands generated by ArcWelder, for obvious reasons. I added a bool parameter to parsing that will preserve case, which is always true. However, rewriting the gcodes, for example if parameters are changed, will result in case changes for the rewritten command, but comment case should be preserved anyway. This should work for both supported and unsupported gcodes. When you have time perhaps you could compare the results of the new version to your fork so we can learn the pros and cons of both methods? |
running in to this also, would be great if one of these solutions could be merged |
@tobyfu, working on this today. You are welcome to install from the archive link in my last post. It will be virtually identical to the release version. Edit:. Here is the link |
I am about to push the next version that should include this fix, as well as a bunch of other stuff. Going to consider this closed, but please open a new issue if you find any other problems! |
When I use arcwelder, the
@Object
markers that cancel-print uses to identify objects get converted to@OBJECT,
which then breaks cancel-object.I've also got a bug going on the cancel-object side,
( paukstelis/Octoprint-Cancelobject#55 )
and I think they're going to remove the case sensitivity at some point. But I think it would be good practice in general for ArcWelder not to make extraneous changes to the file.
The text was updated successfully, but these errors were encountered: