Re: [RFC] Issues with Zephyr Sensors API and ways to resolve them


Nashif, Anas
 

Hi,
If I remember correctly, the PCM units for humidity are coming from Linux sysfs. When this interface was originally proposed, it tried to follow a few things from the Linux world.

See https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

In general and for other units, you really need to think about high accuracy sensors.
I do not think it is terribly bad how the units are defined as long as they are defined and are consistent. So this is basically not a bug, but if there is a general agreement that it should be changed and for a good reason other than avoiding additional calculation, I am fine with that.

Anas

On 25/01/2018, 02:43, "zephyr-devel-bounces@lists.zephyrproject.org on behalf of Paul Sokolovsky" <zephyr-devel-bounces@lists.zephyrproject.org on behalf of paul.sokolovsky@linaro.org> wrote:

Hello,

Having tried Sensors API recently, I found few issues which I think
worth discussing and maybe resolving. A summary of them is:

1. Humidity is defined in milli-percents, which is somewhat confusing.
2. By extension, we may want to re-review unit of measurements used
for other values too.
3. Definition of struct sensor_value is vague, leading to confusion.

Details:

1. Humidity is defined in milli-percents, which is somewhat confusing.

Tracked as https://github.com/zephyrproject-rtos/zephyr/issues/5693

SENSOR_CHAN_HUMIDITY is defined as returning value in milli-percents.
Normal unit of measuring (relative) humidity is percent (that's what's
used "everywhere", not milli-percents). Zephyr Sensor subsystem has
provision for returning fractional values. Normal precision/error of
humidity sensors revolves around ones on decimals of a percent. Most of
other Zephyr sensor channels measure in main units, not their fractions
or multiples. The ticket above shows that there're samples which tell
user that they show %RH, but instead show number like "54500".

Proposal: change SENSOR_CHAN_HUMIDITY to return value in percents.

2. Taking a chance to resolve possible similar issue with other
readings (especially in the wake of coming LTS release), make sense to
look at other values too. Again, in majority of cases we use main SI
unit, like radian/s (not even angle degree!), Gauss, degree Celsius,
lux, etc. Exceptions:

SENSOR_CHAN_PRESS - kilopascal

Commentary: https://en.wikipedia.org/wiki/Pascal_(unit) says that
kilopascal is the "most used" of well-formed of SI units, in relation
to atmospheric pressure.

SENSOR_CHAN_PM_1_0, etc. - ug/m^3

Commentary: Well, that's pretty specific unit, almost an opaque
one, unlikely makes sense to redefine it.

SENSOR_CHAN_DISTANCE - millimeters

Commentary: Of all 3 above, this makes the most sense to redefine to be
in meters IMHO. Really, there're ultrasonic and laser distance sensor
which can easily go tens of meters and more, and using millimeters for
these is ... weird.


My summary: depending on success with humidity, I might go forward with
a proposal on redefining SENSOR_CHAN_DISTANCE, but not the other 2.


3. Definition of struct sensor_value is vague, leading to confusion.

Tracked as https://github.com/zephyrproject-rtos/zephyr/issues/5692

We have it defined as:

/**
* @brief Representation of a sensor readout value.
*
* The value is represented as having an integer and a fractional part,
* and can be obtained using the formula val1 + val2 * 10^(-6).
*/
struct sensor_value {
/** Integer part of the value. */
s32_t val1;
/** Fractional part of the value. */
s32_t val2;
};


Question to a reader: what can you say about negative values, based on
that definition?

For reference, we have 2 types of samples using it: a) ones which use
helper function to convert that value to float; b) the rest almost
universally get negative values wrong.

More detailed discussion is in the ticket above. Proposed solutions are:

1. Leave semantics as is, describe thoroughly, leave samples doing it
in adhoc manner, but fix them of course.
2. Leave semantics as is, describe thoroughly, add convenience func to
convert sensor value to string, switch samples to it.
3. Make fractional part unsigned - doesn't seem to make much sense.
4. Make both integer and fractional parts be absolute, separate sign to
own field - a bit more sense than previous, still not enough
apparently.
5. Get away from "binary coded decimal word" approach, use 32.32 binary
encoding, augmented with convenience func for str conversion. This is
the best "scientific" choice, but may be somewhat far-fetching.


Thanks for your consideration.

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog
_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@lists.zephyrproject.org
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel

Join devel@lists.zephyrproject.org to automatically receive all group messages.