Proposal to change sensor interface to use double values

Proposal to change sensor interface to use double values

[James Feist]

I've submitted a change to modify the sensor interface to remove scale, 
and make the sensor value a double. The main reason for doing this is 
simplification of the interface. In most usages, daemons pull the value, 
and the scale, then immediately translate this value to a double. This 
could easily be done once by the sensor producer instead of all daemons. 
The counter example that was given was phosphor-fan-presence, but doing 
a quick grep I can't find scale being used, so this might mean for a 
scaled fan you have to factor that in to all configuration files, or 
scaled fans aren't supported. The threshold interface is not usable 
independent from the value interface, as the thresholds are affected by 
the same scale. Using a fixed scale also won't work well for a 
logarithmic sensor, while floating point would. Realistic precision 
should not be affected for any common sensor. We also in the future may 
be interested in a daemon producing a threshold for a sensor that 
another daemon owns. With the scale existing elsewhere, this would not 
be trivial to map.

Review for context: https://gerrit.openbmc-project.xyz/#/c/11739/

This change is not ready to be submitted as is, but we would like to 
hear if there are any major objections. If not, we would like to start 
an upgrade path to support both interfaces until it is possible to make 
the switch. Bmcweb already supports both. In most cases this is just 
pulling the value using a visitor, and setting the scale to 0 if it does 
not exist. Max and Min can also be used for IPMI scaling.

Thanks,

- James Feist

Re: Proposal to change sensor interface to use double values

[Patrick Venture]

On Mon, Aug 6, 2018 at 12:10 PM, James Feist
<james.feist@linux.intel.com> wrote:
> I've submitted a change to modify the sensor interface to remove scale, and
> make the sensor value a double. The main reason for doing this is
> simplification of the interface. In most usages, daemons pull the value, and
> the scale, then immediately translate this value to a double. This could
> easily be done once by the sensor producer instead of all daemons. The
> counter example that was given was phosphor-fan-presence, but doing a quick
> grep I can't find scale being used, so this might mean for a scaled fan you
> have to factor that in to all configuration files, or scaled fans aren't
> supported. The threshold interface is not usable independent from the value
> interface, as the thresholds are affected by the same scale. Using a fixed
> scale also won't work well for a logarithmic sensor, while floating point
> would. Realistic precision should not be affected for any common sensor. We
> also in the future may be interested in a daemon producing a threshold for a
> sensor that another daemon owns. With the scale existing elsewhere, this
> would not be trivial to map.
>
> Review for context: https://gerrit.openbmc-project.xyz/#/c/11739/
>
> This change is not ready to be submitted as is, but we would like to hear if
> there are any major objections. If not, we would like to start an upgrade
> path to support both interfaces until it is possible to make the switch.
> Bmcweb already supports both. In most cases this is just pulling the value
> using a visitor, and setting the scale to 0 if it does not exist. Max and
> Min can also be used for IPMI scaling.

I'm fine with this.  It saves everybody converting it to a double
before using.  Further scaling, which can be necessary can be further
done against the double.  For instance, with ipmi, to fit a value we
sometimes scale again.

>
> Thanks,
>
> - James Feist
>
>

Re: Proposal to change sensor interface to use double values

[Emily Shaffer]

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

Same, I'm ok with this.

On Mon, Aug 6, 2018 at 1:09 PM Patrick Venture <venture@google.com> wrote:

> On Mon, Aug 6, 2018 at 12:10 PM, James Feist
> <james.feist@linux.intel.com> wrote:
> > I've submitted a change to modify the sensor interface to remove scale,
> and
> > make the sensor value a double. The main reason for doing this is
> > simplification of the interface. In most usages, daemons pull the value,
> and
> > the scale, then immediately translate this value to a double. This could
> > easily be done once by the sensor producer instead of all daemons. The
> > counter example that was given was phosphor-fan-presence, but doing a
> quick
> > grep I can't find scale being used, so this might mean for a scaled fan
> you
> > have to factor that in to all configuration files, or scaled fans aren't
> > supported. The threshold interface is not usable independent from the
> value
> > interface, as the thresholds are affected by the same scale. Using a
> fixed
> > scale also won't work well for a logarithmic sensor, while floating point
> > would. Realistic precision should not be affected for any common sensor.
> We
> > also in the future may be interested in a daemon producing a threshold
> for a
> > sensor that another daemon owns. With the scale existing elsewhere, this
> > would not be trivial to map.
> >
> > Review for context: https://gerrit.openbmc-project.xyz/#/c/11739/
> >
> > This change is not ready to be submitted as is, but we would like to
> hear if
> > there are any major objections. If not, we would like to start an upgrade
> > path to support both interfaces until it is possible to make the switch.
> > Bmcweb already supports both. In most cases this is just pulling the
> value
> > using a visitor, and setting the scale to 0 if it does not exist. Max and
> > Min can also be used for IPMI scaling.
>
> I'm fine with this.  It saves everybody converting it to a double
> before using.  Further scaling, which can be necessary can be further
> done against the double.  For instance, with ipmi, to fit a value we
> sometimes scale again.
>
> >
> > Thanks,
> >
> > - James Feist
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 2752 bytes --]

RE: Proposal to change sensor interface to use double values

[Tanous, Ed]

+1

The existing interfaces are error prone to implement to.  This change makes them easier to use, and clearer.

-Ed

Re: Proposal to change sensor interface to use double values

[Vernon Mauery]

On 06-Aug-2018 01:24 PM, Emily Shaffer wrote:
>Same, I'm ok with this.
>
>On Mon, Aug 6, 2018 at 1:09 PM Patrick Venture <venture@google.com> wrote:
>
>> On Mon, Aug 6, 2018 at 12:10 PM, James Feist
>> <james.feist@linux.intel.com> wrote:
>> > I've submitted a change to modify the sensor interface to remove scale,
>> and
>> > make the sensor value a double. The main reason for doing this is
>> > simplification of the interface. In most usages, daemons pull the value,
>> and
>> > the scale, then immediately translate this value to a double. This could
>> > easily be done once by the sensor producer instead of all daemons. The
>> > counter example that was given was phosphor-fan-presence, but doing a
>> quick
>> > grep I can't find scale being used, so this might mean for a scaled fan
>> you
>> > have to factor that in to all configuration files, or scaled fans aren't
>> > supported. The threshold interface is not usable independent from the
>> value
>> > interface, as the thresholds are affected by the same scale. Using a
>> fixed
>> > scale also won't work well for a logarithmic sensor, while floating point
>> > would. Realistic precision should not be affected for any common sensor.
>> We
>> > also in the future may be interested in a daemon producing a threshold
>> for a
>> > sensor that another daemon owns. With the scale existing elsewhere, this
>> > would not be trivial to map.
>> >
>> > Review for context: https://gerrit.openbmc-project.xyz/#/c/11739/
>> >
>> > This change is not ready to be submitted as is, but we would like to
>> hear if
>> > there are any major objections. If not, we would like to start an upgrade
>> > path to support both interfaces until it is possible to make the switch.
>> > Bmcweb already supports both. In most cases this is just pulling the
>> value
>> > using a visitor, and setting the scale to 0 if it does not exist. Max and
>> > Min can also be used for IPMI scaling.
>>
>> I'm fine with this.  It saves everybody converting it to a double
>> before using.  Further scaling, which can be necessary can be further
>> done against the double.  For instance, with ipmi, to fit a value we
>> sometimes scale again.

+2

And for anyone complaining about a loss of precision, keep in mind we 
are reading low-precision ADCs and stuff like that anyway. The original 
design may have had a ton of precision (with a 64-bit value and a 64-bit 
exponent), but it was not being used to the point that demotion to a 
double will cause a loss of significant figures. This will maintain 
every bit of precision that was there from the hardware, but in a much 
more efficient manner.

--Vernon