[BUG] The PID routine does not follow pid algorith, the I term cannot be negative

[feldi12]

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

Yes, and the problem still exists.

Bug Description

The temp_iState is defined by call to constrained in:

Marlin/Marlin/src/module/temperature.h

Lines 225 to 226 in ee35929

	const float max_power_over_i_gain = float(MAX_POW) / Ki - float(MIN_POW);
	temp_iState = constrain(temp_iState + pid_error, 0, max_power_over_i_gain);

This call makes it impossible for the temp_iState to be negative. In theory of operation of PID controller this should be an integral over time of error, and there is no reason for it to be only positive. As it is now, this does not allow for the I term to act in opposition to d term in some cases, and in general deviates from expected behaviour

Bug Timeline

The constrain call was introduced all the way back in commit 1db7013

Expected behavior

Currently we are introducing constraints to maximum and minimum value of integral. This can be done and can help PID stabilize faster, but should not be done behind user back. Ideally, this would be controlled by parameters to be configured by user based on their use case, with separate max and min constraints. By default some negative value should be allowed.

Actual behavior

The PID constraints the integral from becoming negative.

Steps to Reproduce

N/A

Version of Marlin Firmware

2.1.2.2

Printer model

No response

Electronics

No response

LCD/Controller

No response

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

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

Additional information & file uploads

As the PID does not follow actual PID behaviour, some PID tuning techniques are not applicable to this implementation. It also may negatively affect stability of temperature in some configurations.
The bug is not dependant on Configuration.h and Configuration_adv.h., so the files are not attached.

P.S: Because ideal solution would provide new features ( control over integral limits ) this could be also considered feature request. Please move as you see fit.

[feldi12]

I made some improvements to the code and tested them. The graphs below show heating process for my Anycubic Mega i3 S, tha improvement is more visible when heating to 230C. The temps stabilize faster and I believe during printing it also tracks better. I feel like my prints look a bit better. I'll make a pull request for the changes in a moment and link this issue to it.

The improvements:

• When heating up, handover to PID with maximum value of integral term - after all we are heating from way lower temp. This way at handoff we stay at max heater power and there is no kink at handover. I did not make similar change for when we cool from higher temperature, as heater power cannot go below zero anyway.
• Increased handover distance to 20 - again to smooth things up, particularly when cooling down.
• Allow integral term to become negative. As usually when cooling up making this too large will actually slow down stabilizing, I limit the term from integration to 1/4 of max power. I believe this helps during printing when there are sudden changes in filament flow.

Cheers!

[thinkyhead]

Moving discussion to #27740.

[github-actions]

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.