openbmc / phosphor-pid-control

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

Support Name field as primary thermal zone identifier and deemphasize Zone Index number #42

Open Krellan opened 8 months ago

Krellan commented 8 months ago

It would be nice if thermal zones were primarily identified by the Name string, using the Zone Index number merely as a fallback.

Currently, it is the other way around.

When IPMI was the only external interface, zone index numbers were required, and the only way to access our IPMI thermal zone extensions: https://github.com/openbmc/phosphor-pid-control/blob/master/ipmi.md

However, now that we have Redfish, this limitation no longer exists, and we should be using names, not numbers, with Redfish.

The good news is that phosphor-pid-control cleanly takes care of a conflicting or missing zone number, and already auto-assigns the next available free zone number in sequence, starting with zero and going up from there:

https://github.com/openbmc/phosphor-pid-control/blob/3f0f7bc35831d467d912698dc5dbd9dcf9ad10f3/dbus/dbusutil.cpp#L73

As an example of how the reliance on zone numbers causes problems elsewhere, I made this fix to bmcweb in order to avoid an error when the ZoneIndex field did not appear in the Entity Manager JSON:

https://gbmc-review.googlesource.com/c/gbmcweb/+/13314

Being able to use zone names, instead of relying on zone index numbers, will go a long way towards gaining acceptance of a standard Redfish equivalent to our existing IPMI thermal zone extensions.