openbmc / phosphor-pid-control

OpenBMC PID-based Thermal Control Daemon
Apache License 2.0
16 stars 21 forks source link

Should we use property "isCeiling" to specify zone ceiling support instead of "Class"? #37

Open blackcatevil opened 1 year ago

blackcatevil commented 1 year ago

I have a little question about an old commit https://gerrit.openbmc.org/c/openbmc/phosphor-pid-control/+/18650

Should we use property " isCeiling " instead of property "Class" to specify if this stepwise controller supports ceiling feature? The property “isCeiling” had been described in phosphor-pid-control configure.md before.

In my understanding, “Class” aims to describe the thermal type of the inputs. For example: temperature, power reading, margin and so on.

blackcatevil commented 1 year ago

I would like to say that the "Ceiling" should be a feature category instead of a sub-type of controller. A controller with "Ceiling" feature enablement means its output is the max RPM of a zone it contribute to, but it still can be a margin controller at the same time while its inputs are provided by margin sensors. Same for power/powersum controller as well Then this "Ceiling" feature can be applied to PID controller as well if needed.