PID PID_LOCAL_SENSOR_NAME doesn't seem to update Pv #22094
Comments
|
OK, found the hint in the PID_USE_LOCAL_SENSOR #define comment. By dropping TelePeriod down from 300 to 10 (minimum, not really documented), I get it to do the PID loop more timely. But why should it be limited to the TELE_PERIOD? We're explicitly servoing on a local parameter that isn't obtained set with mqtt. As #527 explains, we don't want TELE_PERIOD < 10, but it's perfectly acceptable to have PID recalculating on a 1 second loop. But I still don't understand PIDShowSensor() - by testing for parser.getRootObject, we're bailing out of every loop every second except for the one at the end of TELE_PERIOD. This change was introduced in 9eb184c which introduced local sensor handling in the first place, so it seems unlikely it doesn't work at all. @blacknell , any ideas what the code was trying to achieve or what I'm doing wrong? |
|
As I recall, the sensor data only updated every 10s (I used TELEPERIOD 10) and my PID system times were much longer than that (in the order of minutes). So I suspect I wouldn't have seen the behaviour you are. I believe PIDShowSensor works to update sensor values when there are made available (from the main tasmota loops) but sometimes the values were erroneous - hence the testing. Sorry my recollection is a bit ropey as it was about a year ago. Does yours work with slightly longer than 10s for the PID loop? |
|
Mine will work with TelePeriod 10 seconds, but it does respond slower than it should. Take a look at my trace above with "Got isNum" vs "no root" (failing the test on value_token.isNum, line 277). So when fed with local sensor, every call to PidShowSensor is not been given a valid ResponseData except for the one call triggered by the MQTT TELE_PERIOD poll update - unsurprising since we're deliberately not getting data from MQTT when reading a local sensor. Since I don't quite understand the data flow, I can't work out how to convince it to use the most recent local sensor value that ought to have been obtained the last second (since it is being accurately updated in cmnd "Status 8" and the main webpage). Looks like there was an attempt before this commit was made in response to #17636 to fix a similar problem, but I think that one actually dealt with values being updated by MQTT rather than local sensor. |
|
The problem was and is fixed. The PID algorithm is called at the time intervals specified by the PID-command PidUpdateSecs. The temperature for the algorithm is updated by TelePeriod time slice. All values were processed from the local sensor. I do not use MQTT because the controller has been running continuously without any external influences since then. |
|
Mhm, you remove line 255 in your changes of the PID controller.
Refer to: We are back to outdated temperature values. |
|
Yes @Wintespe, the old PIDShowSensor, whose job it was to copy the current values to PV (the function was misnamed, I believe, hence me renaming it to PIDProcessSensor() (FUNC_SHOW_SENSOR, "When FUNC_JSON_APPEND completes", https://tasmota.github.io/docs/API/#driver-sensor-energy-and-light-callback-ids ) was also called by PIDEverySecond, except that it didn't have a valid JSON structure when doing so, so this achieved nothing. The only time that structure was valid was at the end of FUNC_JSON_APPEND, so only that one single call to FUNC_SHOW_SENSOR resulted in PV being copied across. However, both PIDShowValuesWeb and PIDShowValuesWeb, whose job it is to relay the current status of the PID loop to the user and MQTT, was abused to also run pick.tick. Since these calls can happen at every time, they should never have resulted in such a fundamental side-effect. That setPV has moved to PIDProcessSensor, called by PIDEverySecond. The mechanism to reliably access the valid JSON data is discussed in the link linked in the code: #18328. Calling MqttShowSensor actually ends in that call to FUNC_SHOW_SENSOR. By that stage, we now have valid JSON data, but there's no advantage calculating the PID loop at that point rather than carrying on after the call to MqttShowSensor() and setting run_pid_now then so the PID can be ticked as soon as PIDPRocessSensor returns and we immediately call PIDRun(). The code flow is now much clearer now that it's not jumping through the callback state machine. And it's using current sensor values immediately, with latency of no more than 1 second from the actual physical process change, rather than calling pid.tick at essentially random times using PV values that are up to TELE_PERIOD old (imagine a scenario where a physical process is changing smoothly between 10 and 20 degrees, but you'd PID looping on the assumption that the physical value is 10, 10, 10, 10, 10, 10, 10, 20, 20, 20, 20, 20, so I will wind up and D will suddenly jump and P will suddenly jump in sign). As for testing - it demonstrably isn't copying stale data, and I had it reliably using current values within 1 second both in the LOCAL_SENSOR case and when pushed PidPV values. I tested also in the circumstance that tasmota lost contact with the MQTT server - the process carried on updating with the real physical sensor values. Are you seeing different behaviour? |
|
Thanks for the detailed explanation. I don't use MQTT because my PID controller has to run completely independently. I can't even judge whether the new code can do this. My system is very small and needs the PID value to be updated every second. However, I don't have any problems at the moment and I follow the rule of "never change a running system". The result look like this:
|
and others
PROBLEM DESCRIPTION
Documentation is lacking for how to use PID, other than https://tasmota.github.io/docs/PID-Control/, so it's not clear I am using it correctly, but setting
USE_PID
PID_USE_LOCAL_SENSOR
PID_LOCAL_SENSOR_NAME "ANALOG"
PID_LOCAL_SENSOR_TYPE "Temperature1"
results in Pv not actually updating in PIDShowSensor every second when called by PIDEverySecond
REQUESTED INFORMATION
Make sure your have performed every step and checked the applicable boxes before submitting your issue. Thank you!
Backlog Template; Module; GPIO 255:Backlog Rule1; Rule2; Rule3:Status 0:weblogto 4 and then, when you experience your issue, provide the output of the Console log:("PID: no root" and "PID: Got isNum" are my debug traces I inserted on lines 279 and 282 of xdrv_49_pid.ino respectively)
TO REPRODUCE
Steps to reproduce the behavior:
Provide an analogue voltage to GPIO35, set the #defines listed above, set GPIO35 to ADC temperature, and set adcparam if necessary (not really necessary for this test)
and notice how Status PidPv doesn't change when the voltage (measured through Status ANALOG.Temperature1) changes, except for once every 3 minutes (itself a fault - you can adjust PidMaxInterval and this doesn't change - you can see status pidinterval still incrementing to 300 before resetting, implying Pid.max_interval is not being updated).
Putting a trace in PIDShowSensor shows that root is false for every call to PIDEverySecond and only gets set to true at that time when pidinterval gets reset to 0 from 300. Only at that moment does Pv finally take on the current value that ANALOG.Temperature1 had been set to that whole time. And then we have to wait another 5 minutes before it's reset again.
EXPECTED BEHAVIOUR
A clear and concise description of what you expected to happen.
With PID told to use
PID_USE_LOCAL_SENSOR
PID_LOCAL_SENSOR_NAME "ANALOG"
PID_LOCAL_SENSOR_TYPE "Temperature1"
I would expect it to be polling Status ANALOG.Temperature1 every second and using that as the current Pv value in the PID loop.
SCREENSHOTS
If applicable, add screenshots to help explain your problem.
ADDITIONAL CONTEXT
Add any other context about the problem here.
(Please, remember to close the issue when the problem has been addressed)
The text was updated successfully, but these errors were encountered: