RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.9k stars 1.98k forks source link

lpsxxx: float arithmetics #17486

Closed a-podshivalov closed 1 year ago

a-podshivalov commented 2 years ago

What is the need for using floats in lpsxxx driver? Currently, we have TEMP_BASE defined as

define TEMP_BASE (42.5f)

define TEMP_DIVIDER (480U)

and the calculation using it as follows (temp and val are int16_t):

float res = TEMP_BASE;      /* reference value -> see datasheet */
/* skipped some code */
/* compute actual temperature value in °C */
res += ((float)val) / TEMP_DIVIDER;
/* return temperature in c°C */
*temp = (int16_t)(res * 100);

Basically all the same calculations can be done without floats, just like this in a previous version of the driver:

#define TEMP_BASE           (42500L)
#define TEMP_DIVIDER        (480L)
/* return temperature in m°C */
*temp = (int16_t)(TEMP_BASE + (1000*(int)val)/TEMP_DIVIDER);

Also, as many of RIOT's target platforms do not have an FPU, the driver design guide recommends to avoid using floats: https://doc.riot-os.org/driver-guide.html . It definitely seems unnecessary here.

Another strange "optimization" in this driver was replacing division in pressure reading with bit shift. Any decent optimizing compiler would replace division by a power of two by a shift, but the macros and code specifying an implicit division are often considered more readable.

aabadie commented 2 years ago

What is the need for using floats in lpsxxx driver?

IIRC, the previous version of the calculation was not working with new variants of the driver. It was initially written for lps331ap, then there was lps25hb and more recently lps22hb/hh/etc. Maybe it's possible to avoid float with all variants.