search

fluidd-core / fluidd

Fluidd, the klipper UI.
https://docs.fluidd.xyz
GNU General Public License v3.0
1.39k stars 424 forks source link

feat(spoolman): multi-tool support #1324

Closed matmen closed 8 months ago

matmen commented 8 months ago

Closes https://github.com/fluidd-core/fluidd/issues/1269

Adds a new tool selection dropdown to the spoolman card when one or more gcode_macros with a spool_id variable are detected. Emits a SET_GCODE_VARIABLE command to update the tools associated spool.

screenshot

Docs

When a tool has a spool ID set, the tools "color dot" will be replaced by a filament spool icon in the corresponding filaments color (like in the spool selection modal). Persisting settings available using Klipper's [save_variables] module.

pedrolamas commented 8 months ago

When a tool has a spool ID set, the tools "color dot" will be replaced by a filament spool icon in the corresponding filaments color (like in the spool selection modal).

The only issue I see with this is if the spool color is gray, it won't show up if Fluidd is using Dark theme (same for white spool in Light theme), hence why I was using that "dot" thing!

matmen commented 8 months ago

The same issue exists in the spool selection dialog, I think this should be an easy fix by just adding an outline to the spool icon

pedrolamas commented 8 months ago

...adding an outline to the spool icon

not sure if that is possible, but if you manage to do it, I am very interested! 😁

matmen commented 8 months ago

I think this should work:

image image image image

pedrolamas commented 8 months ago

I think this should work:

Sure, I personally can see it fine, though I am not sure that we can claim that it is "accessible" to every seeing user!

Also, a 16-bit screen or lower might be problematic... but granted, nowadays these are extremely rare! 😁

matmen commented 8 months ago

Yeah, that was my thought too, but I couldn't really figure out how to make it accessible and have it look decent. Even the current solution is kinda meh (especially on smaller screens), I think ideally we'd want a light or dark background color on the spool icon depending on the lightness of the filament color - or maybe even a multi-tone icon?

fweber3 commented 8 months ago

Yeah, that was my thought too, but I couldn't really figure out how to make it accessible and have it look decent. Even the current solution is kinda meh (especially on smaller screens), I think ideally we'd want a light or dark background color on the spool icon depending on the lightness of the filament color - or maybe even a multi-tone icon?

I'm not a big designer, but here's two triggers outside of your box: How about using a contrasting color for accessibility BUT make it dashed/dotted (lines) or striped/polkadots (areas)? How about a strikethrough? ("big red cross", or something less in-your-face).

pedrolamas commented 8 months ago

I am not sure what is the best course of action here in terms of layout (I too am not a designer), but I am happy to review this in terms of functionality and we can revisit this topic in a later change!

pedrolamas commented 8 months ago

I admit I am troubled with the whole SET_GCODE_VARIABLE...

I honestly don't think we should be doing this from Fluidd side because if the user has a KlipperScreen or also uses Mainsail, it will not work until they too make the same changes...

In my view, this should either be done on Moonraker side, or somehow directly from Klipper side.

matmen commented 8 months ago

Any particular reason for that macro.name.toUpperCase()?

That's just so the macro name is displayed in upper case in the modals, in theory we can do this with CSS (or a .toUpperCase() call on every reference) instead. Either way should be fine I think, as currently only the "change spool" dropdown and spool selection modal consume the targetableMacros result. After all, Klipper reports the command names in upper case too, but gcode macro names are returned in lower case.

I admit I am troubled with the whole SET_GCODE_VARIABLE...

I honestly don't think we should be doing this from Fluidd side because if the user has a KlipperScreen or also uses Mainsail, it will not work until they too make the same changes...

In my view, this should either be done on Moonraker side, or somehow directly from Klipper side.

Well, having an API to set gcode macro variables directly would be optimal, but AFAIK there isn't one right now. I think the spool_id variable name should be fine across frontends, and if the functionality to set gcode macro variables via Moonraker gets added in the future this can still be refactored. IMO it's the same for all the other gcode commands we send (SET_PIN, EXCLUDE_OBJECT, SET_VELOCITY_LIMIT, ...) - they all modify the printer's state. If there was an option to do it directly via an API call, that'd be preferable, but there isn't.