pidloop in phosphor-pid-control is running significantly slower than expected

[gauravgandhi70]

Currently default values for cycleIntervalTimeMS and updateThermalsTimeMS are 100 and 1000. These values provide the counter for how often processThermals runs in pidControlLoop. getUpdateThermalsCycle gets updated in buildPIDsFromJson function but this is not done in dbusconfiguration. Our platform does not use buildjson; it depends on dbusconfiguration method. Becasue of this by default pidControlLoop runs every 100ms and cycleCnt gets increamented by 1 (every 100ms) and processThermal gets called when cycleCnt is 1000 causing 100 Sec delay.

CC: @Krellan

[gauravgandhi70]

Original commit causing this issue: https://github.com/openbmc/phosphor-pid-control/commit/0e8fc3980a6690d1b2be2b4e96a8018aa1eacc96

@BonnieLo

[Krellan]

I have an idea as to how to fix this. Let me see if it works, then I'll put it up for review.

[harveyquta]

I commit a PR to implement cycle time setting in dbusconfiguration https://gerrit.openbmc.org/c/openbmc/phosphor-pid-control/+/58797

[Krellan]

Here's my intended fix: https://gerrit.openbmc.org/c/openbmc/phosphor-pid-control/+/59721

• Cleaned up D-Bus initialization path (to match the old static JSON initialization path)
• Redid the timer logic to remove the need for the division that was done earlier
• Also noticed and fixed a bug in timer logic that was causing it to run slower than intended

[Krellan]

As for https://gerrit.openbmc.org/c/openbmc/phosphor-pid-control/+/58797

Good idea, I like how both of the settings paths were consolidated into a single function. I ended up having to do it slightly differently, as the old JSON path differs a little in the types of the variables from the new D-Bus path.

[Krellan]

It should be fixed now! https://gerrit.openbmc.org/c/openbmc/phosphor-pid-control/+/59721 is merged.

[gauravgandhi70]

Thanks @Krellan , @harveyquta . Closing the issue!