Proposal to change sensor interface to use double values

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

Quoted 1 line · James Feist

I've submitted a change to modify the sensor interface to remove scale,

and

Quoted 2 lines

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

Quoted 3 lines

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

Quoted 1 line

grep I can't find scale being used, so this might mean for a scaled fan

you

Quoted 2 lines

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

Quoted 1 line

interface, as the thresholds are affected by the same scale. Using a

fixed

Quoted 2 lines

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

Quoted 1 line

also in the future may be interested in a daemon producing a threshold

for a

Quoted 6 lines

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

Quoted 3 lines

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

Quoted 2 lines

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.

Quoted 6 lines

Thanks,

- James Feist

Same, I'm ok with this.

Quoted 41 lines · Patrick Venture

Quoted 1 line · James Feist

I've submitted a change to modify the sensor interface to remove scale,

and

Quoted 2 lines

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

Quoted 3 lines

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

Quoted 1 line

grep I can't find scale being used, so this might mean for a scaled fan

you

Quoted 2 lines

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

Quoted 1 line

interface, as the thresholds are affected by the same scale. Using a

fixed

Quoted 2 lines

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

Quoted 1 line

also in the future may be interested in a daemon producing a threshold

for a

Quoted 6 lines

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

Quoted 3 lines

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

Quoted 2 lines

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.