jschuh / klipper-macros

A collection of useful macros for the Klipper 3D printer firmware
GNU General Public License v3.0
967 stars 173 forks source link

LOAD_FILAMENT and UNLOAD_FILAMENT not showing up in extruder menu on Mainsail #64

Closed agreen closed 1 year ago

agreen commented 1 year ago

Mainsail has a kebab menu if it detects the LOAD_FILAMENT and UNLOAD_FILAMENT macros, however this doesn't seem to work using these macros. I've reviewed the code, and there are two things going on. #1 - the code will wait for the printer to be at the correct temperature, or if the macro has a heat_wait command, allow executing immediately.

It checks for one of the following gocdes within the macro for the heat_wait check - "printer.extruder.can_extrude", "TEMPERATURE_WAIT", "M109"

The bigger issue (as dumb as it is...) is the macro specifically checks for an uppercase "LOAD_FILAMENT" for the macro name... where in the macros here are lowercase.

agreen commented 1 year ago

This almost seems like a Mainsail bug as macros aren't case sensitive within Klipper. Opening up a bug within Mainsail as well. Will comment with the bug # on the Mainsail repo.

jschuh commented 1 year ago

Ah thanks. I verified that it works if I change the case of the command and add an M109 line to the dummy block that's already there to trigger the parameter hinting. It's a tiny change, so maybe I'll just land it with a TODO explaining the case difference.

There's a growing number of these things where Mainsail/Fluidd implemented some UX functionality that gets enabled when a macro tickles the right behavior. I also saw that they recently added pausing at layer changes after I suggested it last year, but I find the implementation significantly lacking compared to the one I already have. So, I was thinking about checking the code out and writing a PR to improve it and make it more compatible with mine. Maybe I'll start with a PR for the casing bug while I'm at it.

agreen commented 1 year ago

FYI Looks like this was fixed in the mainsail codebase today, from the bug I opened there.

They didn't fix the bug I found in the set layer logic though. I do agree yours seems a bit more advanced. I like that yours works in Klipper. Their implementation seems to rely on Mainsail to do the work of pausing at exactly the right time.

jschuh commented 1 year ago

Thanks for the heads up. Also, the logic for their layer implementation is actually in their SET_PRINT_STATS_INFO override macro, which you can pull from the mainsail-config repo. So, it will work from the console, but it just provides one command for a scheduled layer and another one for the next layer. Whereas my implementation supports any number of commands at arbitrary layers and heights, either before or after the layer change.

I'm not sure they'd want to expose that level of complexity, but it would be nice to not be limited to just two pending commands. Of course, if I get them to accept the PR I'd have to do it in multiple steps across four repos (to cover both Mainsail and Fluidd) which is why I've procrastinated a bit.