Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

floats and doubles being used all over the place. #12045

Closed
jcarrano opened this issue Aug 20, 2019 · 6 comments
Closed

floats and doubles being used all over the place. #12045

jcarrano opened this issue Aug 20, 2019 · 6 comments
Labels
Area: drivers Area: Device drivers Area: sys Area: System Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@jcarrano
Copy link
Contributor

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:

  • They increase code size if soft-float support has to be linked-in.
  • Soft float is slow.
  • Even if hard float is available, it should normally not be used in interrupts or kernel code, as this would involve saving/restoring fp registers.
    • With drivers it is hard to know what runs in which context.
  • The user may not even suspect that, for example the sx127x driver 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:

$ git grep  "^\([^/]\|\(/[^*/]\)\)*\bdouble\b"  -- "*.[ch]"
$ git grep  "^\([^/]\|\(/[^*/]\)\)*\bfloat\b"  -- "*.[ch]"

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.

@jcarrano jcarrano added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers Area: sys Area: System labels Aug 20, 2019
@garrettluu
Copy link

Interested! I would like to work on this issue.

@aabadie
Copy link
Contributor

aabadie commented Oct 8, 2019

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
Copy link

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
Copy link
Contributor

aabadie commented Oct 8, 2019

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
Copy link

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
Copy link
Member

maribu commented May 18, 2023

IMO this is more a rant than an issue. I was so free to create a tracking issue: #19614

I would like to close this in favor of that tracking issue. Please reopen, if anyone disagrees.

@maribu maribu closed this as completed May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: sys Area: System Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

6 participants