dbus-sensors: Unit property for xyz.openbmc_project.Sensor.Value interface

[Ed Tanous]

On Wed, Sep 16, 2020 at 10:29 AM Patrick Williams <patrick at stwcx.xyz> wrote:
>
> On Wed, Sep 16, 2020 at 10:19:56AM -0700, Ed Tanous wrote:
> > On Wed, Sep 16, 2020 at 10:08 AM Patrick Williams <patrick at stwcx.xyz> wrote:
> > > I think we only "got away" with "no impact" from the lack of
> > > implementation because the particular implementations that work well
> > > with dbus-sensors also didn't implement it because dbus-sensors didn't
> > > provide it.  That's kind of circular logic as a reason to eliminate it.
> >
> > Are there any implementations that people use that rely on Units?
>
> Yes.  Originally the IPMI hanlders did and they seem to still.
>
> https://github.com/openbmc/phosphor-host-ipmid/blob/master/sensorhandler.cpp#L652
>
> I don't know what happens to this code when a dbus-sensors sensor is
> read.
>
> > > There isn't really a programatic way to define and enforce object paths
> > > presently.  There is a programatic way to define and enforce
> > > enum values.  The enums don't actually need a lookup table, if you're
> > > using the sdbusplus generated headers and bindings.
> >
> > The lookup table I was referring to was enum-value -> IPMI value.  I
> > don't think sdbusplus has the ability to generate that for you.
>
> Agreed.  But, a C-enum to C-enum map is easier to maintain than a path
> segment extraction and string to C-enum map.

Fair point, although the segment extraction is usually 2 lines of
code, so it's in the noise a bit.


>
> > > If I were to guess which of the requirements would be more likely to be
> > > changed it would be the object path.  There are very few other places
> > > where we have such strict requirements on object paths (though we do
> > > have places where the object path has meaning).  The current definition
> > > is a bit ambiguous by what is meant by "the correct hierarchy within the
> > > sensors namespace[2]", but the current implementations seem to take this
> > > to mean `/xyz/openbmc_project/sensors/...`[3].  I don't know that this is
> > > particular convenient for a multi-host system or any case where a BMC
> > > is aggregating sensors from other BMCs.
> >
> > I'd be fine with this solution too.  The question I should've asked is
> > "is there a way to get rid of the redundant information".
>
> We could also remove the path segment requirement.  I'm not sure the
> value of it in one way or another.  It is only redundant to a human,
> isn't it?


I'd be in support of that too, but that's probably more work.  I have
no problem adding Unit to the sensors, just curious if it was needed.

>
> --
> Patrick Williams