Skip to content
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

Fix fan behavior with Danger Klipper #1457

Open
fbeauKmi opened this issue Jun 21, 2024 · 2 comments
Open

Fix fan behavior with Danger Klipper #1457

fbeauKmi opened this issue Jun 21, 2024 · 2 comments
Labels
FR - Enhancement New feature or request Upstream - DK For everything Danger Klipper related

Comments

@fbeauKmi
Copy link

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

const speed = this.fan.speed / (this.fan.config.max_power || 1)

Thanks,

Describe alternatives you've considered

No response

Additional information

No response

@fbeauKmi fbeauKmi added the FR - Enhancement New feature or request label Jun 21, 2024
@fbeauKmi fbeauKmi changed the title Fix fan behavior in Danger Klipper Fix fan behavior with Danger Klipper Jun 21, 2024
@pedrolamas
Copy link
Member

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 pedrolamas added the Upstream - DK For everything Danger Klipper related label Jun 21, 2024
@fbeauKmi
Copy link
Author

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. DangerKlippers/danger-klipper#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 :/).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FR - Enhancement New feature or request Upstream - DK For everything Danger Klipper related
Projects
None yet
Development

No branches or pull requests

2 participants