External Sensors - phosphor-pid-control

[Patrick Venture]

On Fri, Aug 7, 2020 at 3:44 PM Ed Tanous <ed at tanous.net> wrote:
>
> On Fri, Aug 7, 2020 at 1:12 PM Patrick Venture <venture at google.com> wrote:
> >
> > As you know
>
> Hah.  I totally didn't know it did this, but looking at the code, it sure does!
>
> > the json configuration of phosphor-pid-control supports
> > "external sensors" which are sensors that phosphor-pid-control creates
> > itself and publishes to dbus, expecting IPMI (or other) to set the
> > value.  This is used for cases where the host can read sensors or
> > information that the BMC cannot, and basically give it some missing
> > information.
> >
> > From the query phosphor-pid-control does in dbusconfiguration, I don't
> > see a way for it to find these sensors that it must create.  i was
> > thinking, something in the profile that entity-manager publishes could
> > tell it this.  But, since it doesn't show up in the sensor query, I
> > think it'll be something more.
> >
>
> I don't think we ever used this for anything.  Looking at the examples
> in the repo, it looks like it was used for reading fan tachs and
> monitoring the results of margin sensors from the host (ie the
> fleeting sensor)?
>
> In an entity-manager + dbus-sensors setup, we would ordinarily use
> TachSensor, which comprehends the idea of what a "fan" is, and adds
> its tach as a sensor.
> https://github.com/openbmc/dbus-sensors/blob/master/include/TachSensor.hpp
>
> And PWM sensor
> https://github.com/openbmc/dbus-sensors/blob/master/include/PwmSensor.hpp
> Which understands the idea of what a PWM output is, and adds it as a sensor.
>
> Breaking these out into individual applications makes them _much_
> easier to remove, and replace with other implementations, which allows
> doing things like simulating thermals without necessarily having a
> physical fan available.
>
> I don't think entity manager has anything that covers the "host has in
> band sensors it wants to expose" use cases yet, but it would be a
> relatively easy sensor type to write.  I'm a bit surprised nobody has
> written it yet.  I suspect a new sensor type is more in line with the
> separation of concerns model that dbus sensors currently takes, rather
> than putting it directly in phosphor-pid-control.  With that said, I
> wouldn't be against adding a config option for the latter if that's
> what you're inclined to do to minimize the changes, it will just look
> a little different than the rest of the sensors.
>
> Most (maybe all these days?) of the sensor types support the concept
> of an "override".  So calling SetProperty on the Value interface of
> any individual sensor will force it to "stick" to that value, stop
> normal scanning operations, and send out a PropertiesChanged event as
> if it had just read that value from its hardware interface.
> Here's the logic that PWM sensor uses for its set interface.
> https://github.com/openbmc/dbus-sensors/blob/8b8bcc8700af5ba1d435c7cf0f6467b2b6a8c679/src/PwmSensor.cpp#L53
> It's slightly special because it actually changes the hardware value.
> Most other sensors derive from the common sensor class, that just
> masks the value directly.
> https://github.com/openbmc/dbus-sensors/blob/1a540b81a3697202f69771785cd22a31294154a7/include/sensor.hpp#L58
>
> A host sensor would be exactly that, just with the normal scanning
> logic deleted, and a reasonable default value put in place.  I'm
> assuming you probably also need a power on/off check in it to check if
> the host is off, and revert the value to NAN.

Ok, so someone would need to basically add a new daemon to
dbus-sensors that provides ownership to host-provided sensors, and
then it would just be a normal sensor for phosphor-pid-control.  One
thing, I haven't checked is, with the json sensors there are timeouts
set, so we track how often a sensor is updated -- is this supported
with dbus configured sensors?