The type of sensor value in redfish should be double or int64?

[Patrick Venture]

On Mon, Oct 14, 2019 at 7:57 PM Carol Wang <karo33bug at gmail.com> wrote:
>
> Hi,
>
> I tried to override sensor values with redfish Patch, referring to the Voltages example of https://github.com/openbmc/bmcweb/search?q=413961de&type=Commits, but met an error "Invalid argument". Comparing the code https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/sensors.hpp#L2192 and the doc https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Sensor/Value.interface.yaml#L21, the current code uses double type, but the doc says the value type should be int64. It seems a scale difference. Why the code uses double here?

The short answer is, code generally authored by Intel uses doubles for
the sensor values, whereas the original OpenBMC sensor models used
int64.

A while back there was a bit of a debate about this, and it looks like
we never truly resolved it.  Brad, perhaps this is a good time to?
The idea is, that with the int64 storage, we store the raw value and
we also store the scaling factor, so that one can scale the number if
they choose to.  Sort of keeping the values as they are -- instead of
operating on them before publishing them to dbus.  We do notably
however, modify values in phosphor-hwmon before publishing them as
often there are scaling factors and offsets, beyond the conversion
from mC to C.

phosphor-host-ipmid's sensor YAML requires the type to be specified
for the sensor value interface already, and does validly support both
types: int64_t and double (I tested it when I briefly had a mixed
environment).
phosphor-hwmon reports values without applying the scaling factor, but
it is aware of the scaling factor, so it could.
phosphor-pid-control works with both types correctly.

I'm not sure about the phosphor-fan stuff, but I imagine we'll find
some arithmetic we can drop where it ingests values.

This change from int64 to double is big enough though that many
configuration files would need to be adjusted, which is non-trivial.
I argue however that having code that doesn't have a common interface
or interfaces for sensors is less than ideal.  Having multiple options
for talking to sensors is fine, but in my opinion this is only true if
they implement different interfaces, or the same interface.  In this
case, we have dbus-sensors and phosphor-hwmon which both implement the
same interface, but differently: int64 vs double.

I think using doubles makes sense at the dbus level because generally
one wants that version of the value.  Therefore you end up with code
in each daemon that reads the sensor value and the scale so that it
can perform the same operation that another daemon is also going to
perform, etc.  Instead of just doing it once.

I'll climb off my debate box now and climb onto my voting box and say,
I'd like to make phosphor-hwmon report the value as double and I'm
willing to review incoming patches that address other aspects of the
codebase to bring it all together -- since they'll need to be in a
locked step-forward.

>
> Thanks!