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

[BUG] Is PID_FAN_SCALING working/implemented? #25091

Closed
1 task done
MarkMan0 opened this issue Dec 12, 2022 · 4 comments
Closed
1 task done

[BUG] Is PID_FAN_SCALING working/implemented? #25091

MarkMan0 opened this issue Dec 12, 2022 · 4 comments

Comments

@MarkMan0
Copy link
Contributor

MarkMan0 commented Dec 12, 2022

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

PID_FAN_SCALING doesn't seem to be working, and the PIDRunner doesn't use the Kf parameter of the PIDF struct. This lead me to believe PID_FAN_SCALING is not implemented into Marlin. Setting the Kf to high values (100) has no effect.

Bug Timeline

12.12.2022

Expected behavior

The compensation to work. Or when the Kf parameter is set too high, the temperature to overshoot the set value when turning on the fan

Actual behavior

The kf parameter had no effect on the temperature

Steps to Reproduce

  1. Enable PID_FAN_SCALING
  2. Observe temperature graph with Kf set to 0
  3. Observe temperature with Kf set to 100

Version of Marlin Firmware

Marlin bugfix-2.1.x (Dec 12 2022 21:15:52)

Printer model

Ender 3

Electronics

SKR v1.3

Add-ons

No response

Bed Leveling

UBL Bilinear mesh

Your Slicer

Prusa Slicer

Host Software

None

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Configuration.zip

@ellensp
Copy link
Contributor

ellensp commented Dec 13, 2022

It looks like this feature was lost in #24389 from Marlin/src/module/temperature.cpp
PID_FAN_SCALING just vanished...
As did PID_EXTRUSION_SCALING...

@MarkMan0
Copy link
Contributor Author

MarkMan0 commented Dec 13, 2022

I'll try to reimplement it then. Guess the cleanest way would be to add a calculate() method to PID_t, PIDC_t, PIDF_t, PIDCF_t.

Btw is PID_EXTRUSION_SCALING working? Because the contant for it is in the PID struct too.

MarkMan0 added a commit to MarkMan0/Marlin that referenced this issue Dec 13, 2022
Refactor PID classes for localized calculation
MarkMan0 added a commit to MarkMan0/Marlin that referenced this issue Dec 13, 2022
Refactor PID classes for localized calculation
@thisiskeithb
Copy link
Member

Closing since PR #25096 has been opened.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants