From fe46cae00dc66cd599da8fe57d43478452987e48 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 24 Oct 2018 13:04:09 +0200 Subject: [PATCH] sys/phydat: New phydat_fit API The current phydat_fit implementation the following limitations: - The API is way more complicated to use than needed - It doesn't perform any rounding - It uses `long` in a place where actual width (or better range) of the type is pretty important. This commit addresses these limitations and uses lookup-tables to reduce the number of divisions required. Before this commit code using it looked like this: ``` C long values[] = { 100000, 2000000, 30000000 }; phydat_t dat = { .scale = 42, .unit = UNIT_V }; phydat_fit(&dat, values[0], 0, phydat_fit(&dat, values[1], 1, phydat_fit(&dat, values[2], 2, 0))); ``` Now it can be used like this: ``` C int32_t values[] = { 100000, 2000000, 30000000 }; phydat_t dat = { .unit = UNIT_V, .scale = 42 }; phydat_fit(&dat, values, 3); ``` --- sys/include/phydat.h | 44 +++---- sys/phydat/phydat.c | 107 +++++++++++++--- tests/unittests/tests-phydat/tests-phydat.c | 134 ++++++++++++++------ 3 files changed, 206 insertions(+), 79 deletions(-) diff --git a/sys/include/phydat.h b/sys/include/phydat.h index 946a933703..77062314f3 100644 --- a/sys/include/phydat.h +++ b/sys/include/phydat.h @@ -181,38 +181,34 @@ const char *phydat_unit_to_str(uint8_t unit); char phydat_prefix_from_scale(int8_t scale); /** - * @brief Scale an integer value to fit into a @ref phydat_t + * @brief Scale integer value(s) to fit into a @ref phydat_t * - * Insert @p value at position @p index in the given @p dat while rescaling all - * numbers in @p dat->val so that @p value fits inside the limits of the data - * type, [@ref PHYDAT_MIN, @ref PHYDAT_MAX], and update the stored scale factor. - * The result will be rounded towards zero (the standard C99 integer division - * behaviour). - * The final parameter @p prescale can be used to chain multiple calls to - * this function in order to fit multidimensional values into the same phydat_t. - * - * The code example below shows how to chain multiple calls via the @p prescale parameter + * Inserts the @p values in the given @p dat so that all @p dim values in + * @p values fit inside the limits of the data type, + * [@ref PHYDAT_MIN, @ref PHYDAT_MAX], and updates the stored scale factor. + * The value is rounded to the nearest integer if possible, otherwise away from + * zero. E.g. `0.5` and `0.6` are rounded to `1`, `0.4` and `-0.4` are rounded + * to `0`, `-0.5` and `-0.6` are rounded to `-1`. * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ {.c} - * long val0 = 100000; - * long val1 = 2000000; - * long val2 = 30000000; - * phydat_t dat; - * dat.scale = 0; - * phydat_fit(&dat, val0, 0, phydat_fit(&dat, val1, 1, phydat_fit(&dat, val2, 2, 0))); + * int32_t values[] = { 100000, 2000000, 30000000 }; + * phydat_t dat = { .scale = 0 }; + * phydat_fit(&dat, values, 3); * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * - * The prescale scaling is only applied to @p value, the existing values in - * @p dat are only scaled if the prescaled @p value does not fit in phydat_t::val + * @note Unless compiled with `-DPHYDAT_FIT_TRADE_PRECISION_FOR_ROM=0`, this + * function will scale the value `-32768`, even though it would fit into a + * @ref phydat_t. Statistically, this precision loss happens in 0.00153% + * of the calls. This optimization saves a bit more than 20 bytes. + * + * @pre The @ref phydat_t::scale member in @p dat was initialized by the + caller prior to calling this function. * * @param[in, out] dat the value will be written into this data array - * @param[in] value value to rescale - * @param[in] index place the value at this position in the phydat_t::val array - * @param[in] prescale start by scaling the value by this exponent - * - * @return scaling offset that was applied + * @param[in] values value(s) to rescale + * @param[in] dim Number of elements in @p values */ -uint8_t phydat_fit(phydat_t *dat, long value, unsigned int index, uint8_t prescale); +void phydat_fit(phydat_t *dat, const int32_t *values, unsigned int dim); #ifdef __cplusplus } diff --git a/sys/phydat/phydat.c b/sys/phydat/phydat.c index 4b90e4006a..7c8dd82df0 100644 --- a/sys/phydat/phydat.c +++ b/sys/phydat/phydat.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2018 Eistec AB + * 2018 Otto-von-Guericke-Universität Magdeburg * * This file is subject to the terms and conditions of the GNU Lesser * General Public License v2.1. See the file LICENSE in the top level @@ -14,6 +15,7 @@ * @brief Generic sensor/actuator data handling * * @author Joakim Nohlgård + * @author Marian Buschsieweke * * @} */ @@ -24,24 +26,95 @@ #define ENABLE_DEBUG 0 #include "debug.h" -uint8_t phydat_fit(phydat_t *dat, long value, unsigned int index, uint8_t prescale) +#ifndef PHYDAT_FIT_TRADE_PRECISION_FOR_ROM +#define PHYDAT_FIT_TRADE_PRECISION_FOR_ROM 1 +#endif + +static const uint32_t lookup_table_positive[] = { + 327674999, + 32767499, + 3276749, + 327674, + 32767, +}; + +#if !(PHYDAT_FIT_TRADE_PRECISION_FOR_ROM) +static const uint32_t lookup_table_negative[] = { + 327684999, + 32768499, + 3276849, + 327684, + 32768, +}; +#endif + +static const uint32_t divisors[] = { + 100000, + 10000, + 1000, + 100, + 10, +}; + +#define LOOKUP_LEN (sizeof(lookup_table_positive) / sizeof(int32_t)) + +void phydat_fit(phydat_t *dat, const int32_t *values, unsigned int dim) { - assert(index < (sizeof(dat->val) / sizeof(dat->val[0]))); - uint8_t ret = prescale; - while (prescale > 0) { - value /= 10; - --prescale; - } - int8_t scale_offset = 0; - while ((value > PHYDAT_MAX) || (value < PHYDAT_MIN)) { - value /= 10; - for (unsigned int k = 0; k < (sizeof(dat->val) / sizeof(dat->val[0])); ++k) { - dat->val[k] /= 10; + assert(dim <= (sizeof(dat->val) / sizeof(dat->val[0]))); + uint32_t divisor = 0; + uint32_t max = 0; + const uint32_t *lookup = lookup_table_positive; + + /* Get the value with the highest magnitude and the correct lookup table. + * If PHYDAT_FIT_TRADE_PRECISION_FOR_ROM is true, the same lookup table will + * be used for both positive and negative values. As result, -32768 will be + * considered as out of range and scaled down. So statistically in 0.00153% + * of the cases an unneeded scaling is performed, when + * PHYDAT_FIT_TRADE_PRECISION_FOR_ROM is true. + */ + for (unsigned int i = 0; i < dim; i++) { + if (values[i] > (int32_t)max) { + max = values[i]; +#if !(PHYDAT_FIT_TRADE_PRECISION_FOR_ROM) + lookup = lookup_table_positive; +#endif + } + else if (-values[i] > (int32_t)max) { + max = -values[i]; +#if !(PHYDAT_FIT_TRADE_PRECISION_FOR_ROM) + lookup = lookup_table_negative; +#endif + } + } + + for (unsigned int i = 0; i < LOOKUP_LEN; i++) { + if (max > lookup[i]) { + divisor = divisors[i]; + dat->scale += 5 - i; + break; + } + } + + if (!divisor) { + /* No rescaling required */ + for (unsigned int i = 0; i < dim; i++) { + dat->val[i] = values[i]; + } + return; + } + + /* Applying scale and add half of the divisor for correct rounding */ + uint32_t divisor_half = divisor >> 1; + for (unsigned int i = 0; i < dim; i++) { + if (values[i] >= 0) { + dat->val[i] = (uint32_t)(values[i] + divisor_half) / divisor; + } + else { + /* For negative integers the C standards seems to lack information + * on whether to round down or towards zero. So using positive + * integer division as last resort here. + */ + dat->val[i] = -((uint32_t)((-values[i]) + divisor_half) / divisor); } - ++scale_offset; } - dat->val[index] = value; - dat->scale += scale_offset; - ret += scale_offset; - return ret; } diff --git a/tests/unittests/tests-phydat/tests-phydat.c b/tests/unittests/tests-phydat/tests-phydat.c index af7e7b99a1..8a947a90ef 100644 --- a/tests/unittests/tests-phydat/tests-phydat.c +++ b/tests/unittests/tests-phydat/tests-phydat.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2018 Eistec AB + * 2018 Otto-von-Guericke-Universität Magdeburg * * This file is subject to the terms and conditions of the GNU Lesser * General Public License v2.1. See the file LICENSE in the top level @@ -14,46 +15,103 @@ #define ENABLE_DEBUG (0) #include "debug.h" +/* Default is smaller implementation */ +#ifndef PHYDAT_FIT_TRADE_PRECISION_FOR_ROM +#define PHYDAT_FIT_TRADE_PRECISION_FOR_ROM 1 +#endif + static void test_phydat_fit(void) { - /* verify that these big numbers are scaled to fit in phydat_t::val which is int16_t */ - long val0 = 100445; - long val1 = 2000954; - long val2 = 30000455; - long val4 = 1234567; - phydat_t dat; - dat.scale = -6; - dat.unit = UNIT_V; - uint8_t res = phydat_fit(&dat, val0, 0, 0); - /* Check that the result was rescaled to 10044e-5 */ - /* The scaled number is rounded toward zero */ - TEST_ASSERT_EQUAL_INT(1, res); - TEST_ASSERT_EQUAL_INT(UNIT_V, dat.unit); - TEST_ASSERT_EQUAL_INT(-5, dat.scale); - TEST_ASSERT_EQUAL_INT( 10044, dat.val[0]); - /* Fit the next value in the phydat vector */ - res = phydat_fit(&dat, val1, 1, res); - TEST_ASSERT_EQUAL_INT(2, res); - TEST_ASSERT_EQUAL_INT(UNIT_V, dat.unit); - TEST_ASSERT_EQUAL_INT(-4, dat.scale); - TEST_ASSERT_EQUAL_INT( 1004, dat.val[0]); - TEST_ASSERT_EQUAL_INT( 20009, dat.val[1]); - /* Fit the third value in the phydat vector */ - res = phydat_fit(&dat, val2, 2, res); - TEST_ASSERT_EQUAL_INT(3, res); - TEST_ASSERT_EQUAL_INT(UNIT_V, dat.unit); - TEST_ASSERT_EQUAL_INT(-3, dat.scale); - TEST_ASSERT_EQUAL_INT( 100, dat.val[0]); - TEST_ASSERT_EQUAL_INT( 2000, dat.val[1]); - TEST_ASSERT_EQUAL_INT( 30000, dat.val[2]); - /* Overwrite the second value in the phydat vector */ - res = phydat_fit(&dat, val4, 1, res); - TEST_ASSERT_EQUAL_INT(3, res); - TEST_ASSERT_EQUAL_INT(UNIT_V, dat.unit); - TEST_ASSERT_EQUAL_INT(-3, dat.scale); - TEST_ASSERT_EQUAL_INT( 100, dat.val[0]); - TEST_ASSERT_EQUAL_INT( 1234, dat.val[1]); - TEST_ASSERT_EQUAL_INT( 30000, dat.val[2]); + /* Input values for each test: */ + static const int32_t values[][3] = { + { 100445, -1, -1 }, + { -5, 2000954, 3 }, + { 30000449, -30000450, 30000500 }, + { -30000449, -30000499, -30000500 }, + { 0, 0, 1234567 }, + { 32768, 32768, 32768 }, + { 32767, 32767, 32767 }, + { 32766, 32766, 32766 }, + { -32769, -32769, -32769 }, + { -32768, -32768, -32768 }, + { -32767, -32767, -32767 }, + { -32766, -32766, -32766 }, + }; + static const int8_t scales[] = { + -6, + 42, + 0, + -1, + 5, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + }; + static const unsigned int dims[] = { + 1, + 2, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + }; + static const uint8_t units[] = { + UNIT_V, + UNIT_A, + UNIT_NONE, + UNIT_LUX, + UNIT_M, + UNIT_NONE, + UNIT_NONE, + UNIT_NONE, + UNIT_NONE, + UNIT_NONE, + UNIT_NONE, + UNIT_NONE, + }; + /* Expected output values for each test: */ + static const phydat_t expected[] = { + { .val = { 10045, -1, -1 }, .unit = UNIT_V, .scale = -5 }, + { .val = { 0, 20010, -1 }, .unit = UNIT_A, .scale = 44 }, + { .val = { 30000, -30000, 30001 }, .unit = UNIT_NONE, .scale = 3 }, + { .val = { -30000, -30000, -30001 }, .unit = UNIT_LUX, .scale = 2 }, + { .val = { 0, 0, 12346 }, .unit = UNIT_M, .scale = 7 }, + { .val = { 3277, 3277, 3277 }, .unit = UNIT_NONE, .scale = 1 }, + { .val = { 32767, 32767, 32767 }, .unit = UNIT_NONE, .scale = 0 }, + { .val = { 32766, 32766, 32766 }, .unit = UNIT_NONE, .scale = 0 }, + { .val = { -3277, -3277, -3277 }, .unit = UNIT_NONE, .scale = 1 }, +#if PHYDAT_FIT_TRADE_PRECISION_FOR_ROM + { .val = { -3277, -3277, -3277 }, .unit = UNIT_NONE, .scale = 1 }, +#else + { .val = { -32768, -32768, -32768 }, .unit = UNIT_NONE, .scale = 0 }, +#endif + { .val = { -32767, -32767, -32767 }, .unit = UNIT_NONE, .scale = 0 }, + { .val = { -32766, -32766, -32766 }, .unit = UNIT_NONE, .scale = 0 }, + }; + + for (unsigned int i = 0; i < sizeof(dims) / sizeof(dims[0]); i++) { + phydat_t dat = { + .val = { -1, -1, -1 }, + .scale = scales[i], + .unit = units[i] + }; + phydat_fit(&dat, values[i], dims[i]); + TEST_ASSERT_EQUAL_INT(expected[i].val[0], dat.val[0]); + TEST_ASSERT_EQUAL_INT(expected[i].val[1], dat.val[1]); + TEST_ASSERT_EQUAL_INT(expected[i].val[2], dat.val[2]); + TEST_ASSERT_EQUAL_INT(expected[i].scale, dat.scale); + TEST_ASSERT_EQUAL_INT(expected[i].unit, dat.unit); + } } Test *tests_phydat_tests(void)