br3ttb / Arduino-PID-Library

1.93k stars 1.11k forks source link

Min and Max output limit issue with flexible min/max values #101

Closed gryzli133 closed 3 years ago

gryzli133 commented 4 years ago

Hi, first of all thanks for your amazing library! :-)

I am currently applying it to floor heating thermostat. The way, that if the temperature is out of allowed range, the min and max of the PID are going to be higher and lower linear depending on the error (gap). The problem is, that if the temperature is too small, then the wish min and max are the same (100%), so the controller is forced to 100% - same for too high (0%). I could change it in the logic, but I think it is not wrong if minOut = maxOut. Could you change it in this part of logic:

void PID::SetOutputLimits(double Min, double Max) { if(Min >= Max) return; // could you change it to: if(Min > Max) return; outMin = Min; outMax = Max; (...) }

br3ttb commented 4 years ago

if min= max, there's nothing the pid can do to control, and might as well not even be used; you could just set it to manual and fix the output. This was my reasoning behind that line of code.

I understand your thought process, but from a general standpoint, I still feel that >= makes the most sense.

The good news is that the library is open source. There's nothing stopping you from making that change in your pid_v1.cs file.

On Mon, Mar 16, 2020, 3:56 AM gryzli133 notifications@github.com wrote:

Hi, first of all thanks for your amazing library! :-)

I am currently applying it to floor heating thermostat. The way, that if the temperature is out of allowed range, the min and max of the PID are going to be higher and lower linear depending on the error (gap). The problem is, that if the temperature is too small, then the wish min and max are the same (100%), so the controller is forced to 100% - same for too high (0%). I could change it in the logic, but I think it is not wrong if minOut = maxOut. Could you change it in this part of logic:

void PID::SetOutputLimits(double Min, double Max) { if(Min >= Max) return; // could you change it to: if(Min > Max) return; outMin = Min; outMax = Max; (...) }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/br3ttb/Arduino-PID-Library/issues/101, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACYX4SUZA6YBUDVIN57BI3RHXLQ5ANCNFSM4LL4QF4A .

gryzli133 commented 4 years ago

Thanks for your reply. I just now PIDs from Automation PLC (Like simens PCS7). We often use the min/max properties to force to controller to work in specific range. If the min/max is a result from calculation (prediction logic, environment changes etc.), there would be unnecessary logik between logic and PID. This is still not a fault, to force the controller using min/max set to same value. I have already checked, that you are not using the output range in any division, so there is no risk to do this small change (div/0).

br3ttb commented 4 years ago

I'm not saying you're wrong, just that I see it differently. In the context of the arduino, with a large number of beginner users, I stand by my point of view.

In an industrial setting, your limiting technique is intriguing. It's a way to safely implement a rudimentary feed-forward in a controller that might not otherwise support it.

On Mon, Mar 16, 2020, 7:20 AM gryzli133 notifications@github.com wrote:

Thanks for your reply. I just now PIDs from Automation PLC (Like simens PCS7). We often use the min/max properties to force to controller to work in specific range. If the min/max is a result from calculation (prediction logic, environment changes etc.), there would be unnecessary logik between logic and PID. This is still not a fault, to force the controller using min/max set to same value. I have already checked, that you are not using the output range in any division, so there is no risk to do this small change (div/0).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/br3ttb/Arduino-PID-Library/issues/101#issuecomment-599482030, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACYX4T47VGNPJRSBPQYR4TRHYDPZANCNFSM4LL4QF4A .