garrettluu
commented
5 years ago Interested! I would like to work on this issue.
Closed jcarrano closed 1 year ago
garrettluu
commented
5 years ago Interested! I would like to work on this issue.
aabadie
commented
5 years ago Thanks @garrettluu! Looking forward for your PRs. A good approach would be to open a single PR for each module fixed/updated. And don't forget to have a look at our contributor guide.
garrettluu
commented
5 years ago New here, what is considered a module? For example, is the whole drivers folder a module, or is each folder within the drivers folder a module? @aabadie
aabadie
commented
5 years ago is each folder within the drivers folder a module?
Mostly yes. If you open a PR for each driver it will be easier to verify nothing is broken: only one driver to review/test per PR.
garrettluu
commented
5 years ago Draft PR created for sx127x drivers, but I'm unsure of how to test or at least compile my changes without a proper board.
maribu
commented
1 year ago IMO this is more a rant than an issue. I was so free to create a tracking issue: https://github.com/RIOT-OS/RIOT/issues/19614
I would like to close this in favor of that tracking issue. Please reopen, if anyone disagrees.
Description
Most platforms do not have HW support for single-precisions floating point and none (aside from native) have support for double precision.
greping through the source code I can find instances of float and double. These are a problem because:sx127xdriver is using doubles.From what I can see, most of these usages are unjustified. In particular all uses of double could be replaced by float.
Steps to reproduce the issue
A quick way to get an approximate search is to use these regexes:
They will catch some false positives and probably miss some false negatives.
Actual results
After some manual cleanup, here are the results:
Double trouble
``` drivers/bmp180/bmp180.c: return (int16_t)(44330.0 * (1.0 - pow((double)p / pressure_0, 0.1903)));; drivers/bmp180/bmp180.c: return (uint32_t)((double)p / pow(1.0 - (altitude / 44330.0), 5.255));; drivers/sx127x/sx127x_getset.c: channel = (uint32_t)((double) channel / (double)LORA_FREQUENCY_RESOLUTION_DEFAULT); drivers/sx127x/sx127x_getset.c: double bw = 0.0; drivers/sx127x/sx127x_getset.c: double rs = bw / (1 << dev->settings.lora.datarate); drivers/sx127x/sx127x_getset.c: double ts = 1 / rs; drivers/sx127x/sx127x_getset.c: double t_preamble = (dev->settings.lora.preamble_len + 4.25) * ts; drivers/sx127x/sx127x_getset.c: double tmp = drivers/sx127x/sx127x_getset.c: / (double) (4 * dev->settings.lora.datarate drivers/sx127x/sx127x_getset.c: double n_payload = 8 + ((tmp > 0) ? tmp : 0); drivers/sx127x/sx127x_getset.c: double t_payload = n_payload * ts; drivers/sx127x/sx127x_getset.c: double t_on_air = t_preamble + t_payload; drivers/sx127x/sx127x_internal.c: initial_freq = (double) (((uint32_t) sx127x_reg_read(dev, SX127X_REG_FRFMSB) << 16) drivers/sx127x/sx127x_internal.c: | ((uint32_t) sx127x_reg_read(dev, SX127X_REG_FRFLSB))) * (double)LORA_FREQUENCY_RESOLUTION_DEFAULT; drivers/tmp006/tmp006.c: *tamb = (double)rawt / 128.0; drivers/tmp006/tmp006.c: double tdie_k = *tamb + 273.15; drivers/tmp006/tmp006.c: double sens_v = (double)rawv * TMP006_CCONST_LSB_SIZE; drivers/tmp006/tmp006.c: double tdiff = tdie_k - TMP006_CCONST_TREF; drivers/tmp006/tmp006.c: double tdiff_pow2 = pow(tdiff, 2); drivers/tmp006/tmp006.c: double s = TMP006_CCONST_S0 * (1 + TMP006_CCONST_A1 * tdiff drivers/tmp006/tmp006.c: double v_os = TMP006_CCONST_B0 + TMP006_CCONST_B1 * tdiff drivers/tmp006/tmp006.c: double f_obj = (sens_v - v_os) + TMP006_CCONST_C2 * pow((sens_v - v_os), 2); drivers/tmp006/tmp006.c: double t = pow(pow(tdie_k, 4) + (f_obj / s), 0.25); sys/crypto/modes/ocb.c: L_$ = double(L_*) sys/crypto/modes/ocb.c: L_0 = double(L_$) sys/crypto/modes/ocb.c: L_i = double(L_{i-1}) for every integer i > 0 sys/include/cb_mux.h: * @param[in] head double pointer to first list entry sys/include/cb_mux.h: * @param[in] head double pointer to first list entry sys/include/net/gnrc/rpl/structs.h: double link_metric; /**< metric of the link */ sys/include/random.h:double random_real(void); sys/include/random.h:double random_real_inclusive(void); sys/include/random.h:double random_real_exclusive(void); sys/include/random.h:double random_res53(void); sys/include/ubjson.h:static inline ssize_t ubjson_get_double(ubjson_cookie_t *__restrict cookie, ssize_t content, double *dest) sys/include/ubjson.h: double f; sys/include/ubjson.h:ssize_t ubjson_write_double(ubjson_cookie_t *__restrict cookie, double value); sys/net/routing/nhdp/iib_table.c:static const double const_dat = (((double)DAT_CONSTANT) / DAT_MAXIMUM_LOSS); sys/net/routing/nhdp/iib_table.c: double sum_total, sum_rcvd, loss; sys/net/routing/nhdp/iib_table.c: loss = (((double)ls_elt->hello_interval) * ((double)ls_elt->lost_hellos)) sys/quad_math/fixdfdi.c:quad_t __fixdfdi(double x) sys/quad_math/fixunsdfdi.c:u_quad_t __fixunsdfdi(double x) sys/quad_math/fixunssfdi.c: double x, toppart; sys/quad_math/fixunssfdi.c: x -= (double) t.uq; sys/quad_math/floatdidf.c:double __floatdidf(quad_t x) sys/quad_math/floatdidf.c: double d; sys/quad_math/floatdidf.c: d = (double) u.ul[H] * (((int) 1 << (INT_BITS - 2)) * 4.0); sys/quad_math/floatdisf.c: f = (double) u.ul[H] * (((int) 1 << (INT_BITS - 2)) * 4.0); sys/quad_math/floatunsdidf.c:double __floatunsdidf(u_quad_t x) sys/quad_math/floatunsdidf.c: double d; sys/quad_math/floatunsdidf.c: d = (double) u.ul[H] * (((int) 1 << (INT_BITS - 2)) * 4.0); sys/quad_math/quad.h:quad_t __fixdfdi(double); sys/quad_math/quad.h:u_quad_t __fixunsdfdi(double); sys/quad_math/quad.h:double __floatdidf(quad_t); sys/quad_math/quad.h:double __floatunsdidf(u_quad_t); sys/random/mersenne.c:double random_real(void) sys/random/mersenne.c:double random_real_inclusive(void) sys/random/mersenne.c:double random_real_exclusive(void) sys/random/mersenne.c: return ((double) random_uint32() + 0.5) * (1.0 / TWO_POW_32); sys/random/mersenne.c:double random_res53(void) sys/random/mersenne.c: double a = random_uint32() * TWO_POW_26; sys/random/mersenne.c: double b = random_uint32() * (1.0 / TWO_POW_6); sys/random/tinymt32/tinymt32.h:static inline double tinymt32_generate_32double(tinymt32_t *random) sys/ubjson/ubjson-write.c:ssize_t ubjson_write_double(ubjson_cookie_t *restrict cookie, double value) sys/ubjson/ubjson-write.c: double f; tests/bloom_bytes/main.c: double false_positive_rate = (double) in / (double) lenA; tests/float/main.c: double x = 1234567.0 / 1024.0; tests/float/main.c: double z = (x - floor(x)); tests/thread_float/main.c: printf("T(%" PRIkernel_pid "): %f\n", thread_getpid(), (double)f); tests/unittests/tests-bloom/tests-bloom.c: double false_positive_rate = 0; tests/unittests/tests-bloom/tests-bloom.c: false_positive_rate = (double) in / (double) lenA; tests/unittests/tests-printf_float/tests-printf_float.c:static const double in0 = 2016.0349; tests/unittests/tests-printf_float/tests-printf_float.c:static const double in1 = 123.4567; tests/unittests/tests-printf_float/tests-printf_float.c:static const double in2 = 0.0; tests/unittests/tests-sht1x/tests-sht1x.c: static const double d1_table[] = { -40.1, -39.8, -39.7, -39.6, -39.4 }; tests/unittests/tests-sht1x/tests-sht1x.c: double d1 = d1_table[dev->vdd]; tests/unittests/tests-sht1x/tests-sht1x.c: double d2 = (dev->conf & SHT1X_CONF_LOW_RESOLUTION) ? 0.04 : 0.01; tests/unittests/tests-sht1x/tests-sht1x.c: double raw = (double)_raw; tests/unittests/tests-sht1x/tests-sht1x.c: double temp = d1 + d2 * raw; tests/unittests/tests-sht1x/tests-sht1x.c: static const double c1 = -2.0468; tests/unittests/tests-sht1x/tests-sht1x.c: static const double t1 = 0.01; tests/unittests/tests-sht1x/tests-sht1x.c: double temp = ((double)_temp) / 100.0; tests/unittests/tests-sht1x/tests-sht1x.c: double raw = (double)_raw; tests/unittests/tests-sht1x/tests-sht1x.c: double c2, c3, t2, hum_linear, hum_real; ```Summary
``` cpu/esp32/vendor/esp-idf/include/esp32/esp_mesh.h: float percentage; /**< vote percentage threshold for approval of being a root */ cpu/esp32/vendor/esp-idf/include/esp32/esp_mesh.h:esp_err_t esp_mesh_set_vote_percentage(float percentage); cpu/esp32/vendor/esp-idf/include/esp32/esp_mesh.h:float esp_mesh_get_vote_percentage(void); cpu/esp8266/vendor/esp/phy_info.c: (float)info->spur_freq_primary / info->spur_freq_divisor, cpu/esp8266/vendor/esp/phy_info.c: (float)info->spur_freq_2_primary / info->spur_freq_2_divisor, cpu/esp8266/vendor/espressif/c_types.h:typedef float real32_t; cpu/esp8266/vendor/espressif/c_types.h:typedef float real32; drivers/adt7310/adt7310.c:#define ADT7310_TEMPERATURE_LSB_FLOAT (1.f/((float)((int)1 << ADT7310_VALUE_FRAC_BITS))) drivers/adt7310/adt7310.c:float adt7310_read_float(const adt7310_t *dev) drivers/adt7310/adt7310.c: return (((float) raw) * ADT7310_TEMPERATURE_LSB_FLOAT); drivers/at30tse75x/at30tse75x.c:static inline float temperature_to_float(uint16_t temp) drivers/at30tse75x/at30tse75x.c:int at30tse75x_get_temperature(const at30tse75x_t *dev, float *temperature) drivers/hih6130/hih6130.c: float *relative_humidity_percent, float *temperature_celsius) drivers/include/adt7310.h:float adt7310_read_float(const adt7310_t *dev); drivers/include/at30tse75x.h:int at30tse75x_get_temperature(const at30tse75x_t* dev, float* temperature); drivers/include/hih6130.h: float *relative_humidity_percent, float *temperature_celsius); drivers/include/isl29020.h: float lux_fac; /**< factor to calculate actual lux value */ drivers/include/isl29125.h: float red; /**< red lux value */ drivers/include/isl29125.h: float green; /**< green lux value */ drivers/include/isl29125.h: float blue; /**< blue lux value */ drivers/include/tmp006.h:void tmp006_convert(int16_t rawv, int16_t rawt, float *tamb, float *tobj); drivers/io1_xplained/io1_xplained_saul.c:static float temperature; drivers/isl29020/isl29020.c: dev->lux_fac = (float)((1 << (10 + (2 * DEV_RANGE))) - 1) / 0xffff; drivers/isl29125/isl29125.c: float luxfactor = (DEV_RANGE == ISL29125_RANGE_10K) ? 10000.0 / 65535.0 : 375.0 / 65535.0; drivers/lpsxxx/lpsxxx.c: float res = TEMP_BASE; /* reference value -> see datasheet */ drivers/lpsxxx/lpsxxx.c: res += ((float)val) / TEMP_DIVIDER; drivers/mpu9150/mpu9150.c: float fsr; drivers/mpu9150/mpu9150.c: float fsr; drivers/mpu9150/mpu9150.c: output->x_axis = (int16_t) (((float)output->x_axis) * drivers/mpu9150/mpu9150.c: output->y_axis = (int16_t) (((float)output->y_axis) * drivers/mpu9150/mpu9150.c: output->z_axis = (int16_t) (((float)output->z_axis) * drivers/mq3/mq3.c: float res = mq3_read_raw(dev); drivers/tmp006/tmp006.c:void tmp006_convert(int16_t rawv, int16_t rawt, float *tamb, float *tobj) drivers/tmp006/tmp006.c: float tamb, tobj; sys/analog_util/adc_util.c:float adc_util_mapf(int sample, adc_res_t res, float min, float max) sys/analog_util/dac_util.c:uint16_t dac_util_mapf(float value, float min, float max) sys/color/color.c: float rd, gd, bd, delta; sys/color/color.c: rd = (float)rgb->r / 255.0f; sys/color/color.c: gd = (float)rgb->g / 255.0f; sys/color/color.c: bd = (float)rgb->b / 255.0f; sys/color/color.c: float rc, gc, bc; sys/color/color.c: float aa, bb, cc, f, h; sys/fmt/fmt.c:size_t fmt_float(char *out, float f, unsigned precision) sys/fmt/fmt.c:void print_float(float f, unsigned precision) sys/include/analog_util.h:float adc_util_mapf(int sample, adc_res_t res, float min, float max); sys/include/analog_util.h:uint16_t dac_util_mapf(float value, float min, float max); sys/include/color.h: float h; /**< hue value [0.0 - 360.0] */ sys/include/color.h: float s; /**< saturation value [0.0 - 1.0] */ sys/include/color.h: float v; /**< value [0.0 - 1.0] */ sys/include/ubjson.h:static inline ssize_t ubjson_get_float(ubjson_cookie_t *__restrict cookie, ssize_t content, float *dest) sys/include/ubjson.h: float f; sys/include/ubjson.h:ssize_t ubjson_write_float(ubjson_cookie_t *__restrict cookie, float value); sys/quad_math/fixsfdi.c:quad_t __fixsfdi(float x) sys/quad_math/fixunssfdi.c:u_quad_t __fixunssfdi(float f) sys/quad_math/floatdisf.c:float __floatdisf(quad_t x) sys/quad_math/floatdisf.c: float f; sys/quad_math/quad.h:quad_t __fixsfdi(float); sys/quad_math/quad.h:u_quad_t __fixunssfdi(float); sys/quad_math/quad.h:float __floatdisf(quad_t); sys/random/tinymt32/tinymt32.h:static inline float tinymt32_temper_conv(tinymt32_t *random) sys/random/tinymt32/tinymt32.h: float f; sys/random/tinymt32/tinymt32.h:static inline float tinymt32_temper_conv_open(tinymt32_t *random) sys/random/tinymt32/tinymt32.h: float f; sys/random/tinymt32/tinymt32.h:static inline float tinymt32_generate_float(tinymt32_t *random) sys/random/tinymt32/tinymt32.h:static inline float tinymt32_generate_float12(tinymt32_t *random) sys/random/tinymt32/tinymt32.h:static inline float tinymt32_generate_float01(tinymt32_t *random) sys/random/tinymt32/tinymt32.h:static inline float tinymt32_generate_floatOC(tinymt32_t *random) sys/random/tinymt32/tinymt32.h:static inline float tinymt32_generate_floatOO(tinymt32_t *random) sys/shell/commands/sc_at30tse75x.c: float temperature; sys/ubjson/ubjson-write.c:ssize_t ubjson_write_float(ubjson_cookie_t *restrict cookie, float value) sys/ubjson/ubjson-write.c: float f; tests/driver_adt7310/main.c: float celsius_float; tests/driver_adt7310/main.c: float integral = 0; tests/driver_adt7310/main.c: float fractional; tests/driver_hih6130/main.c: float hum = 0.f; tests/driver_hih6130/main.c: float temp = 0.f; tests/driver_hih6130/main.c: float integral = 0.f; tests/driver_hih6130/main.c: float fractional; tests/driver_io1_xplained/main.c: float temperature; tests/driver_tmp006/main.c: float tamb, tobj; tests/pkg_cmsis-dsp/main.c: float int_part; tests/pkg_cmsis-dsp/main.c: float frac_part = fabsf(modff(variance, &int_part)); tests/rng/test.c: float entropy = 0.0; tests/rng/test.c: float count = (float) buffer[i] / (float) length; tests/rng/test.h:#define log2f(x) (logf (x) / (float) 0.693147180559945309417) tests/thread_float/main.c: float f, init; tests/thread_float/main.c: float f, init; tests/unittests/tests-scanf_float/tests-scanf_float.c: float x = 0;\ tests/unittests/tests-scanf_float/tests-scanf_float.c: float x = 0; tests/unittests/tests-scanf_float/tests-scanf_float.c: float x = 0; ```What now?
The doubles should go away. I believe that's pretty clear.
There is float usage in sensor drivers. This is in my opinion wrong. The driver should have a layer which is responsible for retrieving data from the device. That layer has no business doing floating point computations. In any case the float is not needed even in cases of sensors like the mpu9150 where values can be perfecly expressed and manipulated using integer math exclusively.