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

[i.kononenko]

On 16.09.2020 19:24, Ed Tanous wrote:
> On Wed, Sep 16, 2020 at 8:23 AM James Feist <james.feist at linux.intel.com> wrote:
>>
>> On 9/16/2020 6:28 AM, Andrei Kartashev wrote:
>>> Hi,
>>>
>>> We noticed that dbus-sensors doesn't fully implement
>>> xyz.openbmc_project.Sensor.Value interface: there is no Unit property
>>> for all the sensors, defined by dbus-sensors.
>>> I believe it was intentionally, but I still wondering what was the
>>> reason?
>>
>> It was originally as the information seemed redundant. If the
>> information is needed I'm fine with someone adding it, it just hasn't
>> seemed to be a high priority.

I have a number of patches which adds the "Unit" dbus-property to each of 
dbus-sensors (adcsensor, pwmsensor, etc.),
Also this changes append dependency from phosphor-dbus-interfaces to dbus-sensors recipe. 
Does the dbus-sensors package intentionally lack the phosphor-dbus-interfaces? 
Or can I safely add this dependency?

> 
> Considering we've gone this long with no impact (considering the path
> can be used to lookup the unit) I wonder if we should consider
> removing unit from the sensor Value API?  It doesn't seem used.
> 

Ed, I'm sure that each sensor's unit may be obtained by analyzing a dbus-path in each service, 
but is it really any better than using well-conforming dbus-interface's? 
Setting the Unit once at the single centralized place (I'd suggest it should be dbus-sensors, 
which defenetly knows better about units) we'll prevent any further code duplication and 
re-implementing of this feature in the future.

>>
>>> I noticed that in intel-ipmi-oem units are determined based on object
>>> paths, but that looks ugly since there is well-defined natural
>>> interface for units in dbus.
>>> Lack of the "Unit" property in the interface breaks some existing
>>> logic.
>>>
> 
> Technically the way the interfaces define it, both are valid to use to
> determine the Units, and both would need to be lookup tables.  Is
> using the path any more ugly than using the property?
> 

-- 
best,

Igor Kononenko