fluidd icon indicating copy to clipboard operation
fluidd copied to clipboard

Fix fan behavior with Danger Klipper

Open fbeauKmi opened this issue 1 year ago • 2 comments

Is your feature request related to a problem? Please describe

There is a different behavior of fan.py between klipper and danger-klipper. klipper scales input between 0 and max_power and report pwm as speed set_speed in klipper

danger-klipper scales input between min_power and max_power and report input value as speed set_speed in danger-klipper For now, while using Danger-klipper, the display value in fluidd is inaccurate while using min_power/max_power, sliders gives unwanted behavior.

Describe the solution you'd like

As support to DK in Fluidd was recently added. Proposal : while using DK, do not scale fan.speed value, I guess it is this lines https://github.com/fluidd-core/fluidd/blob/37617b517c8c932a003c8543684f54e76006054f/src/components/widgets/outputs/OutputFan.vue#L60

Thanks,

Describe alternatives you've considered

No response

Additional information

No response

fbeauKmi avatar Jun 21 '24 11:06 fbeauKmi

Hi @fbeauKmi, thank you for sending this feature request.

Ideally the existing Klipper API surface & format should not change in Danger Klipper as that will lead to breaking expected behavior and eventually introduce issues in Fluidd, Mainsail, KlipperScreen and other UIs - which is what we are seeing here.

The rule of thumb is to not change existing endpoints/property naming or data formats in the Klipper API, but instead add new endpoints and/or properties that will enrich the existing API.

We will coordinate this matter with the Danger Klipper development team and update this ticket accordingly, but I am hoping that we will not need to do any changes in Fluidd side.

pedrolamas avatar Jun 21 '24 14:06 pedrolamas

Thanks for your interest. I opened a related draft PR in DK about changes in fan.py few month ago which did not reach much interest. https://github.com/DangerKlippers/danger-klipper/pull/190. Some changes can be done in this PR to not break the API, the PR introduce a new property (but still change the klipper speed value to keep DK behavior :/).

fbeauKmi avatar Jun 21 '24 15:06 fbeauKmi