dbus-sensors

[Bills, Jason M]

On 4/22/2020 9:19 AM, Matt Spinler wrote:
> Great!  Sounds like we should be able to make thing work.
> A few comments below.
> 
> On 4/21/2020 4:54 PM, James Feist wrote:
>> On 4/21/2020 12:35 PM, Matt Spinler wrote:
>>> Hi James,
>>>
>>> We're looking into using dbus-sensors(HwmonTemp and PSU) in the future,
>>> but would need to make a few changes to fit our requirements. Was 
>>> wondering
>>> what you'd think of the following:
>>>
>>> 1. Check if a sensor has a _fault sysfs file, and if it does and it
>>>    is nonzero, set the Functional property on the OperationalStatus
>>>    interface to false (and/or maybe 6 below)
>> Sounds ok.
>>
>>>
>>> 2. After the 10 failed reads, instead of just setting the sensor to 0
>>>    also make a D-Bus call to create a phosphor-logging event log and set
>>>    the OperationalStatus sensor to false.
>>
>> Sounds ok.
>>
>>>
>>> 3. After creating this event log, make sure not to do it again until
>>>    main power is cycled.
>>
>> I'd rather this be until the status goes OK again.
> 
> As suggested by Patrick, I agree the throttling can be done elsewhere, 
> so we
> would just create the logs as you state here.
> 
I'm not familiar with the sensors, but for this specific case, would it 
work to base the log on OperationalStatus?  It seems logical to not log 
events for sensors that are not operational, and since it will be set to 
false after the 10 failures, it would stop the log from nagging.

>>
>>>
>>> 4. If not already supported (was unsure), be able to find an
>>>    _input file based on a value it has in the corresponding _label file.
>>
>> PSU sensor does this, hwmontemp does it by index.
> 
> Would you be OK with us also adding this to PSUSensor?
> 
>>>
>>> 5. We have a case where a driver isn't loaded with power off, so somehow
>>>    we still need the sensors to stay on D-Bus when off (and show them
>>>    as not available).
>>
>> All sensors are on d-bus all the time, its based on the EM config.
> 
> Perfect!
> 
>>
>>>
>>> 6. Maybe add a new property to Sensor.Value on the validity
>>>    of the value property, for when driver is unloaded or there is an
>>>    error or the sensor reading is otherwise not valid.  We could add
>>>   this to phosphor-hwmon at the same time.
>>>   (I think this was mentioned on the list before).
>>
>> Yes, this is where we've used std::nan, I'm not sure if that made it 
>> to all sensors as it's not tested very much. I know the fans do this.
>>
>>>
>>> We would definitely of course work with you on the best way to
>>> accomplish these, and I know #6 needs more discussion on if
>>> this is something we want to do in OpenBMC, though I thought
>>> I remembered an earlier discussion where it was popular.
>>>
>>> Thanks,
>>> Matt
>