search

fluidd-core / fluidd

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

Fix fan behavior with Danger Klipper #1457

Open fbeauKmi opened 5 months ago

fbeauKmi commented 5 months ago

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

pedrolamas commented 5 months ago

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.

fbeauKmi commented 5 months ago

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 :/).