sensors: SENSOR_CHAN_HUMIDITY confusingly defined in "milli percent", SENSOR_CHAN_DISTANCE inconsistently defined in millimeters

[pfalcon]

Let's compare:

        /** Temperature in degrees Celsius. */
        SENSOR_CHAN_TEMP,

But a half a degree Celsius is quite a noticeable change, easily discernible in most cases by a human. And yet Zephyr defined it in degrees (thus requiring fractional part).

And yet

        /** Humidity, in milli percent. */
        SENSOR_CHAN_HUMIDITY,

But half a percent of relative humidity is nothing, clearly not noticeable by a human, and hardly there're sensors which have enough precision/error-freeness even of 0.5%. And yet suddenly "milli percent".

It's not too late to fix this goof yet.

[nashif]

@bogdan-davidoaia you know the background for this?

[pfalcon]

@bogdan-davidoaia you know the background for this?

Thanks for hint @nashif , but seems such definition for humidity was introduced in d1c7c7b7efe54beb2bc269a2c3442ce52cc5a608, by @ddvlad. @ddvlad , any comments?

[pfalcon]

Looking at the available drivers, a number support SENSOR_CHAN_HUMIDITY:

bme280, hts221, dht, sht3xd, hdc1008, th02

At the same time, a few to extra leg work to calculate "milli-percents":

bme280.c:

                val->val1 = (data->comp_humidity >> 10);
                val->val2 = (((data->comp_humidity & 0x3ff) * 1000 * 1000) >> 10);
                val->val1 = val->val1 * 1000 + (val->val2 * 1000) / 1000000;
                val->val2 = (val->val2 * 1000) % 1000000;

dht.c:

                val->val1 = drv_data->sample[0] * 1000;
                val->val2 = 0;

[pfalcon]

Now samples. There're 5:

samples/sensor/bme280:

                sensor_channel_get(dev, SENSOR_CHAN_HUMIDITY, &humidity);

                printf("temp: %d.%06d; press: %d.%06d; humidity: %d.%06d\n",
                      temp.val1, temp.val2, press.val1, press.val2,
                      humidity.val1, humidity.val2);

Well, such cases started it all. Seeing humidity: 54352.134000, a user may only think there's something wrong with the sample or their board/sensor.

samples/sensor/hts221:

                printf("Relative Humidity:%.0f%%\n",
                       sensor_value_to_double(&hum) / 1000);

Legwork of converting back to percents.

samples/sensor/th02:

                sprintf(row, "RH:%.0f%c", sensor_value_to_double(val + 1),
                        37 /* percent symbol */);

samples/boards/arduino_101/environmental_sensing:

               /* resolution of 0.01 percent */
                humidity_value = val->val1 / 10;

"resolution of 0.01 percent" - no, it is not.

samples/boards/arduino_101/environmental_sensing/sensor:

                sprintf(row, "RH:%d%c", val[1].val1/1000,
                        37 /* percent symbol */);

Legwork of converting back to percent.

[pfalcon]

Based on the above, I think it's worth pursuing switchover to just percent values, and then do this for 1.11, to offer stability by the LTS release.

[pfalcon]

@grgustaf, @jimmy-huang: Let me know how you'd like/don't like this in Zephyr.js context.

[grgustaf]

A little research: This sensor claims +/- 0.5% accuracy. Most out there are +/- 2-5%. So using percent would seem pretty adequate. Sure fine with us!

[pfalcon]

Posted RFC to the mailing list: https://lists.zephyrproject.org/pipermail/zephyr-devel/2018-January/008768.html

[pfalcon]

Additionally, th02.c driver already returned values in percent, not milli-percent.

[ddvlad]

The only reason I can think of for having milli percent for humidity readouts is that Linux also uses these units: http://elixir.free-electrons.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-iio#L263

But your arguments for switching to percent make sense.

[pfalcon]

@ddvlad: Aarrgh, thanks for hint! But yeah, Linux also specifies temperature in milli-degrees, http://elixir.free-electrons.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-iio#L164 , but Zephyr already uses just degrees. So, from that point of view, it makes sense to be consistent with the scale of majority of Zephyr's units.

[ddvlad]

Alas, for that I do not have an explanation :)

I have no objections against the change, especially since I haven't been involved in Zephyr for a while now. Thanks for working on making this more consistent.

[pfalcon]

@ddvlad: Thanks for the prompt reply to this ticket! I'm in the process of preparing a patch...

[pfalcon]

So, as discussed on the mailing list, I'd like to try to perform the similar analysis, and issue the similar proposal, for SENSOR_CHAN_DISTANCE, which is defined in millimeters.

So, facts:

1. We have SENSOR_CHAN_ALTITUDE, which is "Altitude, in meters", and yet SENSOR_CHAN_DISTANCE is in millimeters. Hardly a consistency example.
2. There's only one driver which delivers SENSOR_CHAN_DISTANCE, vl53l0x.c.
3. That driver was actually merged less than a week ago, and SENSOR_CHAN_DISTANCE was introduced as part of it, https://github.com/zephyrproject-rtos/zephyr/pull/829. So, it would be pretty good timing to fix it up.
4. vl53l0x itself is "Time-of-Flight (ToF) laser-ranging module", "can measure absolute distances up to 2m". As can be seen, even this device actually operates with distances on the order of a meter, and bigger laser distance modules can work with distances of 10m, 100m, or 1km (based on off the shelf laser rangefinders used e.g. in building engineering). Using millimeters as a base unit for them would be quite cumbersome, while using meters for vl53l0x shouldn't be a problem.

@VVESTM : Can you please comment what were the reasons to define SENSOR_CHAN_DISTANCE in millimeters, and how would you feel re: changing it to meters? Thanks.

[VVESTM]

Hi @pfalcon, I see no issue if you want to update the unit from millimeter to meter for SENSOR_CHAN_DISTANCE. In fact, I used millimeters because it was used in the default implementation from ST for this sensor. No problem to switch it to meters. Just need to see how to report the decimal value.

[pfalcon]

@VVESTM : Thanks for the reply and being positive about it! Sure, I'll update the sample to report the decimal value, the easiest way to do that is to use sensor_value_to_double(): https://github.com/zephyrproject-rtos/zephyr/blob/master/include/sensor.h#L492 . As a part of https://github.com/zephyrproject-rtos/zephyr/issues/5692 , I also plan to add func which formats value as a string without involving floating-point.