From cd3b4dfeaaf24754d35b7b8f89a0bc223a62df2c Mon Sep 17 00:00:00 2001 From: tx_haggis <13982343+adbancroft@users.noreply.github.com> Date: Wed, 1 Nov 2023 17:08:31 -0500 Subject: [PATCH] Simplify 3D table I/O conversions - save 12 bytes RAM (#1107) * Simplify 3d table I/O conversions Saves 10 bytes RAM * MISRA fix: remove abort() from CONCRETE_TABLE_ACTION * CppCheck fix (pointer past end of array warning) * MISRA fixes --- speeduino/comms_legacy.cpp | 14 ++--- speeduino/int16_byte.h | 65 ---------------------- speeduino/page_crc.cpp | 4 +- speeduino/pages.cpp | 30 ++++++----- speeduino/storage.cpp | 12 ++--- speeduino/table3d.cpp | 25 +++++---- speeduino/table3d.h | 14 ++--- speeduino/table3d_axes.h | 21 ++++---- speeduino/table3d_axis_io.cpp | 18 +++++-- speeduino/table3d_axis_io.h | 90 ++++++++++++------------------- speeduino/table3d_interpolate.cpp | 57 ++++++++++---------- speeduino/updates.cpp | 4 +- speeduino/updates.h | 4 +- 13 files changed, 146 insertions(+), 212 deletions(-) delete mode 100644 speeduino/int16_byte.h diff --git a/speeduino/comms_legacy.cpp b/speeduino/comms_legacy.cpp index 2a5ada2b..ea8ef577 100644 --- a/speeduino/comms_legacy.cpp +++ b/speeduino/comms_legacy.cpp @@ -887,10 +887,10 @@ namespace { inline void send_table_axis(table_axis_iterator it) { - const int16_byte *pConverter = table3d_axis_io::get_converter(it.get_domain()); + const table3d_axis_io_converter converter = get_table3d_axis_converter(it.get_domain()); while (!it.at_end()) { - Serial.write(pConverter->to_byte(*it)); + Serial.write(converter.to_byte(*it)); ++it; } } @@ -996,7 +996,7 @@ namespace { void print_row(const table_axis_iterator &y_it, table_row_iterator row) { - serial_print_prepadded_value(table3d_axis_io::to_byte(y_it.get_domain(), *y_it)); + serial_print_prepadded_value(get_table3d_axis_converter(y_it.get_domain()).to_byte(*y_it)); while (!row.at_end()) { @@ -1006,21 +1006,21 @@ namespace { Serial.println(); } - void print_x_axis(const void *pTable, table_type_t key) + void print_x_axis(void *pTable, table_type_t key) { Serial.print(F(" ")); auto x_it = x_begin(pTable, key); - const int16_byte *pConverter = table3d_axis_io::get_converter(x_it.get_domain()); + const table3d_axis_io_converter converter = get_table3d_axis_converter(x_it.get_domain()); while(!x_it.at_end()) { - serial_print_prepadded_value(pConverter->to_byte(*x_it)); + serial_print_prepadded_value(converter.to_byte(*x_it)); ++x_it; } } - void serial_print_3dtable(const void *pTable, table_type_t key) + void serial_print_3dtable(void *pTable, table_type_t key) { auto y_it = y_begin(pTable, key); auto row_it = rows_begin(pTable, key); diff --git a/speeduino/int16_byte.h b/speeduino/int16_byte.h deleted file mode 100644 index 42ea1ab7..00000000 --- a/speeduino/int16_byte.h +++ /dev/null @@ -1,65 +0,0 @@ -/** - * @addtogroup table_3d - * @{ - */ - -#pragma once - - -#include - -#ifdef USE_LIBDIVIDE -#include "src/libdivide/libdivide.h" -#endif - -/** @brief Byte type. This is not defined in any C or C++ standard header. */ -typedef uint8_t byte; - -/** @brief Represents a 16-bit value as a byte. Useful for I/O. - * - * Often we need to deal internally with values that fit in 16-bits but do - * not require much accuracy. E.g. table axes in RPM. For these values we can - * save storage space (EEPROM) by scaling to/from 8-bits using a fixed divisor. - */ -class int16_byte -{ -public: - - /** - * @brief Construct - * - * @param factor The factor to multiply when converting \c byte to \c int16_t - * @param divider The factor to divide by when converting \c int16_t to \c byte - * - * \c divider could be computed from \c factor, but including it as a parameter - * allows callers to create \c factor instances at compile time. - */ - constexpr int16_byte(uint8_t factor -#ifdef USE_LIBDIVIDE - , const libdivide::libdivide_s16_t ÷r -#endif - ) - : _factor(factor) -#ifdef USE_LIBDIVIDE - , _divider(divider) -#endif - { - } - - /** @brief Convert to a \c byte */ -#ifdef USE_LIBDIVIDE - inline byte to_byte(int16_t value) const { return _factor==1 ? value : _factor==2 ? value>>1 : (byte)libdivide::libdivide_s16_do(value, &_divider); } -#else - inline byte to_byte(int16_t value) const { return (byte)(value/_factor); } -#endif - /** @brief Convert from a \c byte */ - inline int16_t from_byte( byte in ) const { return (int16_t)in * _factor; } - -private: - uint8_t _factor; -#ifdef USE_LIBDIVIDE - libdivide::libdivide_s16_t _divider; -#endif -}; - -/** @} */ diff --git a/speeduino/page_crc.cpp b/speeduino/page_crc.cpp index da464a75..b23962b3 100644 --- a/speeduino/page_crc.cpp +++ b/speeduino/page_crc.cpp @@ -30,13 +30,13 @@ static inline uint32_t compute_tablevalues_crc(table_value_iterator it, pCrcCalc static inline uint32_t compute_tableaxis_crc(table_axis_iterator it, uint32_t crc, FastCRC32 &crcCalc) { - const int16_byte *pConverter = table3d_axis_io::get_converter(it.get_domain()); + const table3d_axis_io_converter converter = get_table3d_axis_converter(it.get_domain()); byte values[32]; // Fingers crossed we don't have a table bigger than 32x32 byte *pValue = values; while (!it.at_end()) { - *pValue++ = pConverter->to_byte(*it); + *pValue++ = converter.to_byte(*it); ++it; } return pValue-values==0 ? crc : crcCalc.crc32_upd(values, pValue-values, false); diff --git a/speeduino/pages.cpp b/speeduino/pages.cpp index 4b511053..28c9e001 100644 --- a/speeduino/pages.cpp +++ b/speeduino/pages.cpp @@ -31,17 +31,17 @@ constexpr const uint16_t PROGMEM ini_page_sizes[] = { 0, 128, 288, 288, 128, 288 // calling context. template -inline constexpr uint16_t get_table_value_end() +static inline constexpr uint16_t get_table_value_end(void) { return table_t::xaxis_t::length*table_t::yaxis_t::length; } template -inline constexpr uint16_t get_table_axisx_end() +static inline constexpr uint16_t get_table_axisx_end(void) { return get_table_value_end()+table_t::xaxis_t::length; } template -inline constexpr uint16_t get_table_axisy_end(const table_t *) +static inline constexpr uint16_t get_table_axisy_end(const table_t *) { return get_table_axisx_end()+table_t::yaxis_t::length; } @@ -73,17 +73,17 @@ public: } // Getter - inline byte operator*() const + inline byte operator*(void) const { switch (get_table_location()) { case table_location_values: return get_value_value(); case table_location_xaxis: - return table3d_axis_io::to_byte(table_t::xaxis_t::domain, get_xaxis_value()); + return get_table3d_axis_converter(table_t::xaxis_t::domain).to_byte(get_xaxis_value()); case table_location_yaxis: default: - return table3d_axis_io::to_byte(table_t::yaxis_t::domain, get_yaxis_value()); + return get_table3d_axis_converter(table_t::yaxis_t::domain).to_byte(get_yaxis_value()); } } @@ -97,12 +97,12 @@ public: break; case table_location_xaxis: - get_xaxis_value() = table3d_axis_io::from_byte(table_t::xaxis_t::domain, new_value); + get_xaxis_value() = get_table3d_axis_converter(table_t::xaxis_t::domain).from_byte(new_value); break; case table_location_yaxis: default: - get_yaxis_value() = table3d_axis_io::from_byte(table_t::yaxis_t::domain, new_value); + get_yaxis_value() = get_table3d_axis_converter(table_t::yaxis_t::domain).from_byte(new_value); } invalidate_cache(&_pTable->get_value_cache); return *this; @@ -110,17 +110,17 @@ public: private: - inline byte& get_value_value() const + inline byte& get_value_value(void) const { return _pTable->values.value_at((uint8_t)_table_offset); } - inline table3d_axis_t& get_xaxis_value() const + inline table3d_axis_t& get_xaxis_value(void) const { return *(_pTable->axisX.begin().advance(_table_offset - get_table_value_end())); } - inline table3d_axis_t& get_yaxis_value() const + inline table3d_axis_t& get_yaxis_value(void) const { return *(_pTable->axisY.begin().advance(_table_offset - get_table_axisx_end())); } @@ -129,7 +129,7 @@ private: table_location_values, table_location_xaxis, table_location_yaxis }; - inline table_location get_table_location() const + inline table_location get_table_location(void) const { if (_table_offset()) { @@ -157,7 +157,8 @@ inline byte get_table_value(page_iterator_t &entity, uint16_t offset) { #define CTA_GET_TABLE_VALUE(size, xDomain, yDomain, pTable, offset) \ return *offset_to_table((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable, offset); - CONCRETE_TABLE_ACTION(entity.table_key, CTA_GET_TABLE_VALUE, entity.pData, (offset-entity.start)); + #define CTA_GET_TABLE_VALUE_DEFAULT ({ return 0U; }) + CONCRETE_TABLE_ACTION(entity.table_key, CTA_GET_TABLE_VALUE, CTA_GET_TABLE_VALUE_DEFAULT, entity.pData, (offset-entity.start)); } inline byte get_value(page_iterator_t &entity, uint16_t offset) @@ -177,7 +178,8 @@ inline void set_table_value(page_iterator_t &entity, uint16_t offset, byte new_v { #define CTA_SET_TABLE_VALUE(size, xDomain, yDomain, pTable, offset, new_value) \ offset_to_table((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable, offset) = new_value; break; - CONCRETE_TABLE_ACTION(entity.table_key, CTA_SET_TABLE_VALUE, entity.pData, (offset-entity.start), new_value); + #define CTA_SET_TABLE_VALUE_DEFAULT ({ }) + CONCRETE_TABLE_ACTION(entity.table_key, CTA_SET_TABLE_VALUE, CTA_SET_TABLE_VALUE_DEFAULT, entity.pData, (offset-entity.start), new_value); } inline void set_value(page_iterator_t &entity, byte value, uint16_t offset) diff --git a/speeduino/storage.cpp b/speeduino/storage.cpp index 273bac1b..058620fb 100644 --- a/speeduino/storage.cpp +++ b/speeduino/storage.cpp @@ -121,10 +121,10 @@ static inline write_location write(table_value_iterator it, write_location locat static inline write_location write(table_axis_iterator it, write_location location) { - const int16_byte *pConverter = table3d_axis_io::get_converter(it.get_domain()); + const table3d_axis_io_converter converter = get_table3d_axis_converter(it.get_domain()); while (location.can_write() && !it.at_end()) { - location.update(pConverter->to_byte(*it)); + location.update(converter.to_byte(*it)); ++location; ++it; } @@ -132,7 +132,7 @@ static inline write_location write(table_axis_iterator it, write_location locati } -static inline write_location writeTable(const void *pTable, table_type_t key, write_location location) +static inline write_location writeTable(void *pTable, table_type_t key, write_location location) { return write(y_rbegin(pTable, key), write(x_begin(pTable, key), @@ -384,10 +384,10 @@ static inline eeprom_address_t load(table_value_iterator it, eeprom_address_t ad static inline eeprom_address_t load(table_axis_iterator it, eeprom_address_t address) { - const int16_byte *pConverter = table3d_axis_io::get_converter(it.get_domain()); + const table3d_axis_io_converter converter = get_table3d_axis_converter(it.get_domain()); while (!it.at_end()) { - *it = pConverter->from_byte(EEPROM.read(address)); + *it = converter.from_byte(EEPROM.read(address)); ++address; ++it; } @@ -395,7 +395,7 @@ static inline eeprom_address_t load(table_axis_iterator it, eeprom_address_t add } -static inline eeprom_address_t loadTable(const void *pTable, table_type_t key, eeprom_address_t address) +static inline eeprom_address_t loadTable(void *pTable, table_type_t key, eeprom_address_t address) { return load(y_rbegin(pTable, key), load(x_begin(pTable, key), diff --git a/speeduino/table3d.cpp b/speeduino/table3d.cpp index f8b33e1b..29880aef 100644 --- a/speeduino/table3d.cpp +++ b/speeduino/table3d.cpp @@ -3,44 +3,49 @@ // =============================== Iterators ========================= -table_value_iterator rows_begin(const void *pTable, table_type_t key) +table_value_iterator rows_begin(void *pTable, table_type_t key) { #define CTA_GET_ROW_ITERATOR(size, xDomain, yDomain, pTable) \ return ((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable)->values.begin(); - CONCRETE_TABLE_ACTION(key, CTA_GET_ROW_ITERATOR, pTable); + #define CTA_GET_ROW_ITERATOR_DEFAULT ({ return table_value_iterator(NULL, 0U); }) + CONCRETE_TABLE_ACTION(key, CTA_GET_ROW_ITERATOR, CTA_GET_ROW_ITERATOR_DEFAULT, pTable); } /** * Convert page iterator to table x axis iterator. */ -table_axis_iterator x_begin(const void *pTable, table_type_t key) +table_axis_iterator x_begin(void *pTable, table_type_t key) { #define CTA_GET_X_ITERATOR(size, xDomain, yDomain, pTable) \ return ((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable)->axisX.begin(); - CONCRETE_TABLE_ACTION(key, CTA_GET_X_ITERATOR, pTable); + #define CTA_GET_X_ITERATOR_DEFAULT ({ return table_axis_iterator(NULL, NULL, axis_domain_Tps); }) + CONCRETE_TABLE_ACTION(key, CTA_GET_X_ITERATOR, CTA_GET_X_ITERATOR_DEFAULT, pTable); } -table_axis_iterator x_rbegin(const void *pTable, table_type_t key) +table_axis_iterator x_rbegin(void *pTable, table_type_t key) { #define CTA_GET_X_RITERATOR(size, xDomain, yDomain, pTable) \ return ((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable)->axisX.rbegin(); - CONCRETE_TABLE_ACTION(key, CTA_GET_X_RITERATOR, pTable); + #define CTA_GET_X_ITERATOR_DEFAULT ({ return table_axis_iterator(NULL, NULL, axis_domain_Tps); }) + CONCRETE_TABLE_ACTION(key, CTA_GET_X_RITERATOR, CTA_GET_X_ITERATOR_DEFAULT, pTable); } /** * Convert page iterator to table y axis iterator. */ -table_axis_iterator y_begin(const void *pTable, table_type_t key) +table_axis_iterator y_begin(void *pTable, table_type_t key) { #define CTA_GET_Y_ITERATOR(size, xDomain, yDomain, pTable) \ return ((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable)->axisY.begin(); - CONCRETE_TABLE_ACTION(key, CTA_GET_Y_ITERATOR, pTable); + #define CTA_GET_Y_ITERATOR_DEFAULT ({ return table_axis_iterator(NULL, NULL, axis_domain_Tps); }) + CONCRETE_TABLE_ACTION(key, CTA_GET_Y_ITERATOR, CTA_GET_Y_ITERATOR_DEFAULT, pTable); } -table_axis_iterator y_rbegin(const void *pTable, table_type_t key) +table_axis_iterator y_rbegin(void *pTable, table_type_t key) { #define CTA_GET_Y_RITERATOR(size, xDomain, yDomain, pTable) \ return ((TABLE3D_TYPENAME_BASE(size, xDomain, yDomain)*)pTable)->axisY.rbegin(); - CONCRETE_TABLE_ACTION(key, CTA_GET_Y_RITERATOR, pTable); + #define CTA_GET_Y_ITERATOR_DEFAULT ({ return table_axis_iterator(NULL, NULL, axis_domain_Tps); }) + CONCRETE_TABLE_ACTION(key, CTA_GET_Y_RITERATOR, CTA_GET_Y_ITERATOR_DEFAULT, pTable); } \ No newline at end of file diff --git a/speeduino/table3d.h b/speeduino/table3d.h index 94513963..eafd6039 100644 --- a/speeduino/table3d.h +++ b/speeduino/table3d.h @@ -110,20 +110,20 @@ TABLE3D_GENERATOR(TABLE3D_GEN_GET_TABLE_VALUE) // to a caller defined function overloaded by the type of the table. #define CONCRETE_TABLE_ACTION_INNER(size, xDomain, yDomain, action, ...) \ case TO_TYPE_KEY(size, xDomain, yDomain): action(size, xDomain, yDomain, ##__VA_ARGS__); -#define CONCRETE_TABLE_ACTION(testKey, action, ...) \ +#define CONCRETE_TABLE_ACTION(testKey, action, defaultAction, ...) \ switch ((table_type_t)testKey) { \ TABLE3D_GENERATOR(CONCRETE_TABLE_ACTION_INNER, action, ##__VA_ARGS__ ) \ - default: abort(); } + default: defaultAction; } // =============================== Table function calls ========================= -table_value_iterator rows_begin(const void *pTable, table_type_t key); +table_value_iterator rows_begin(void *pTable, table_type_t key); -table_axis_iterator x_begin(const void *pTable, table_type_t key); +table_axis_iterator x_begin(void *pTable, table_type_t key); -table_axis_iterator x_rbegin(const void *pTable, table_type_t key); +table_axis_iterator x_rbegin(void *pTable, table_type_t key); -table_axis_iterator y_begin(const void *pTable, table_type_t key); +table_axis_iterator y_begin(void *pTable, table_type_t key); -table_axis_iterator y_rbegin(const void *pTable, table_type_t key); +table_axis_iterator y_rbegin(void *pTable, table_type_t key); /** @} */ diff --git a/speeduino/table3d_axes.h b/speeduino/table3d_axes.h index 1a30f6b6..333c1b2b 100644 --- a/speeduino/table3d_axes.h +++ b/speeduino/table3d_axes.h @@ -30,7 +30,10 @@ public: /** @brief Construct */ table_axis_iterator(table3d_axis_t *pStart, const table3d_axis_t *pEnd, axis_domain domain) - : _pAxis(pStart), _pAxisEnd(pEnd), _stride(pEnd>pStart ? stride_inc : stride_dec), _domain(domain) //cppcheck-suppress misra-c2012-10.4 + : _stride(pEnd>pStart ? stride_inc : stride_dec) + , _pStart(pStart) + , _pEnd(pEnd + _stride) + , _domain(domain) //cppcheck-suppress misra-c2012-10.4 { } @@ -41,7 +44,7 @@ public: */ table_axis_iterator& advance(int8_t steps) { - _pAxis = _pAxis + ((int16_t)_stride * steps); + _pStart = _pStart + ((int16_t)_stride * steps); return *this; } @@ -54,27 +57,27 @@ public: /** @brief Test for end of iteration */ bool at_end(void) const { - return _pAxis == _pAxisEnd; + return _pStart == _pEnd; } /** @brief Dereference the iterator */ table3d_axis_t& operator*(void) { - return *_pAxis; + return *_pStart; } /** @copydoc table_axis_iterator::operator*() */ const table3d_axis_t& operator*(void) const { - return *_pAxis; + return *_pStart; } private: static constexpr int8_t stride_inc = 1; static constexpr int8_t stride_dec = -1; - table3d_axis_t *_pAxis; - const table3d_axis_t *_pAxisEnd; int8_t _stride; + table3d_axis_t *_pStart; + const table3d_axis_t *_pEnd; const axis_domain _domain; }; @@ -95,12 +98,12 @@ private: /** @brief Iterate over the axis elements */ \ table_axis_iterator begin(void) \ { \ - return table_axis_iterator(axis+(size)-1, axis-1, domain); \ + return table_axis_iterator(axis+(size)-1, axis, domain); \ } \ /** @brief Iterate over the axis elements, from largest to smallest */ \ table_axis_iterator rbegin(void) \ { \ - return table_axis_iterator(axis, axis+(size), domain); \ + return table_axis_iterator(axis, axis+(size)-1, domain); \ } \ }; diff --git a/speeduino/table3d_axis_io.cpp b/speeduino/table3d_axis_io.cpp index 4ba6502e..0b30c67c 100644 --- a/speeduino/table3d_axis_io.cpp +++ b/speeduino/table3d_axis_io.cpp @@ -1,5 +1,17 @@ #include "table3d_axis_io.h" +#include "maths.h" -constexpr int16_byte table3d_axis_io::converter_100; -constexpr int16_byte table3d_axis_io::converter_2; -constexpr int16_byte table3d_axis_io::converter_1; \ No newline at end of file +static byte to_byte_100(int16_t value) { return (byte)div100(value); } +static int16_t from_byte_100(byte in) { return (int16_t)in * 100; } + +static byte to_byte_1(int16_t value) { return (byte)value; } +static int16_t from_byte_1(byte in) { return (int16_t)in; } + +static byte to_byte_2(int16_t value) { return (byte)(value/2); } //cppcheck-suppress misra-c2012-10.8 +static int16_t from_byte_2(byte in) { return (int16_t)in*2; } + +table3d_axis_io_converter get_table3d_axis_converter(axis_domain domain) { + return domain==axis_domain_Rpm ? table3d_axis_io_converter { &to_byte_100, &from_byte_100 } : + domain==axis_domain_Load ? table3d_axis_io_converter { &to_byte_2, &from_byte_2 } : + table3d_axis_io_converter { &to_byte_1, &from_byte_1 }; +} \ No newline at end of file diff --git a/speeduino/table3d_axis_io.h b/speeduino/table3d_axis_io.h index d40c57e8..2899f212 100644 --- a/speeduino/table3d_axis_io.h +++ b/speeduino/table3d_axis_io.h @@ -9,67 +9,45 @@ #pragma once -#include "int16_byte.h" #include "table3d_axes.h" -#ifdef USE_LIBDIVIDE -#include "src/libdivide/constant_fast_div.h" -#endif -/** @brief table axis I/O support - * - * @attention Using \c constexpr class static variables seems to be the best - * combination of size and speed - \c constexpr implies \c inline, so we - * can't use it on traditional \c extern global variables. -*/ -class table3d_axis_io { -public: +/** @brief Byte type. This is not defined in any C or C++ standard header. */ +typedef uint8_t byte; - /** - * @brief Obtain a converter instance for a given axis domain. - * - * Often we need to deal internally with values that fit in 16-bits but do - * not require much accuracy. E.g. RPM. For these values we can - * save storage space (EEPROM) by scaling to/from 8-bits using a fixed divisor. - * - * The divisor is dependent on the domain. I.e all axes with the same domain use - * the same divisor - * - * Conversion during I/O is orthogonal to other axis concerns, so is separated and - * encapsulated here. - */ - static constexpr const int16_byte* get_converter(axis_domain domain) { - return domain==axis_domain_Rpm ? &converter_100 : - domain==axis_domain_Load ? &converter_2 : &converter_1; - } +/** @brief Models int16->byte conversion + * @see table3d_axis_io_converter + */ +typedef byte(*pToByteConverter)(int16_t value); - /** - * @brief Convert to a \c byte - * - * Useful for converting a single value. - * If converting multiple, probably faster to cache the converter rather than - * repeatedly calling this function. - */ - static inline byte to_byte(axis_domain domain, int16_t value) { return get_converter(domain)->to_byte(value); } +/** @brief Models byte->int16 conversion + * @see table3d_axis_io_converter + */ +typedef int16_t(*pFromByteConverter)(byte in); - /** - * @brief Convert from a \c byte - * - * Useful for converting a single value. - * If converting multiple, probably faster to cache the converter rather than - * repeatedly calling this function. - */ - static inline int16_t from_byte(axis_domain domain, byte in ) { return get_converter(domain)->from_byte(in); } - -private: -#ifdef USE_LIBDIVIDE - static constexpr int16_byte converter_100 = { 100, { S16_MAGIC(100), S16_MORE(100) } }; - static constexpr int16_byte converter_2 = { 2, { S16_MAGIC(2), S16_MORE(2) } }; - static constexpr int16_byte converter_1 = { 1, { S16_MAGIC(1), S16_MORE(1) } }; -#else - static constexpr int16_byte converter_100 = { 100 }; - static constexpr int16_byte converter_2 = { 2 }; - static constexpr int16_byte converter_1 = { 1 }; -#endif +/** @brief Convert a 16-bit value to/from a byte. Useful for I/O. + * + * Often we need to deal internally with values that fit in 16-bits but do + * not require much accuracy. E.g. table axes in RPM. For these values we can + * save storage space (EEPROM) by scaling to/from 8-bits. + */ +struct table3d_axis_io_converter { + pToByteConverter to_byte; + pFromByteConverter from_byte; }; +/** + * @brief Obtain a converter instance for a given axis domain. + * + * Often we need to deal internally with values that fit in 16-bits but do + * not require much accuracy. E.g. RPM. For these values we can + * save storage space (EEPROM) by scaling to/from 8-bits. + * + * The converter is dependent on the domain. I.e all axes with the same domain use + * the same converter + * + * Conversion during I/O is orthogonal to other axis concerns, so is separated and + * encapsulated here. + */ +table3d_axis_io_converter get_table3d_axis_converter(axis_domain domain); + /** @} */ \ No newline at end of file diff --git a/speeduino/table3d_interpolate.cpp b/speeduino/table3d_interpolate.cpp index f77eab11..b6ba4074 100644 --- a/speeduino/table3d_interpolate.cpp +++ b/speeduino/table3d_interpolate.cpp @@ -18,36 +18,34 @@ static inline table3d_dim_t find_bin_max( table3d_dim_t maxElement, // Axis index of the element with the highest value (at the other end of the array) table3d_dim_t lastBinMax) // The last result from this call - used to speed up searches { - // Direction to search (1 conventional, -1 to go backwards from pAxis) - int8_t stride = maxElement>minElement ? 1 : -1; // It's quicker to increment/adjust this pointer than to repeatedly // index the array - minimum 2%, often >5% const table3d_axis_t *pMax = nullptr; // minElement is at one end of the array, so the "lowest" bin // is [minElement, minElement+stride]. Since we're working with the upper // index of the bin pair, we can't go below minElement + stride. - table3d_dim_t minBinIndex = minElement + stride; + table3d_dim_t minBinIndex = minElement - 1U; // Check the cached last bin and either side first - it's likely that this will give a hit under // real world conditions // Check if we're still in the same bin as last time pMax = pAxis + lastBinMax; - if (is_in_bin(value, *(pMax - stride), *pMax)) + if (is_in_bin(value, *(pMax + 1U), *pMax)) { return lastBinMax; } // Check the bin above the last one - pMax = pMax - stride; - if (lastBinMax!=minBinIndex && is_in_bin(value, *(pMax - stride), *pMax)) + pMax = pMax + 1U; + if (lastBinMax!=minBinIndex && is_in_bin(value, *(pMax + 1U), *pMax)) { - return lastBinMax-stride; + return lastBinMax + 1U; } // Check the bin below the last one - pMax += stride*2; - if (lastBinMax!=maxElement && is_in_bin(value, *(pMax - stride), *pMax)) + pMax -= 2U; + if (lastBinMax!=maxElement && is_in_bin(value, *(pMax + 1U), *pMax)) { - return lastBinMax+stride; + return lastBinMax - 1U; } // Check if outside array limits - won't happen often in the real world @@ -62,7 +60,7 @@ static inline table3d_dim_t find_bin_max( if (value<=pAxis[minElement]) { value = pAxis[minElement]; - return minElement+stride; + return minElement - 1U; } // No hits above, so run a linear search. @@ -72,24 +70,24 @@ static inline table3d_dim_t find_bin_max( // when the RPM is highest (and hence the CPU is needed most) lastBinMax = maxElement; pMax = pAxis + lastBinMax; - while (lastBinMax!=minBinIndex && !is_in_bin(value, *(pMax - stride), *pMax)) + while (lastBinMax!=minBinIndex && !is_in_bin(value, *(pMax + 1U), *pMax)) { - lastBinMax -= stride; - pMax -= stride; + ++lastBinMax; + ++pMax; } return lastBinMax; } table3d_dim_t find_xbin(table3d_axis_t &value, const table3d_axis_t *pAxis, table3d_dim_t size, table3d_dim_t lastBin) { - return find_bin_max(value, pAxis, size-1, 0, lastBin); + return find_bin_max(value, pAxis, size-1U, 0U, lastBin); } table3d_dim_t find_ybin(table3d_axis_t &value, const table3d_axis_t *pAxis, table3d_dim_t size, table3d_dim_t lastBin) { // Y axis is stored in reverse for performance purposes (not sure that's still valid). // The minimum value is at the end & max at the start. So need to adjust for that. - return find_bin_max(value, pAxis, size-1, 0, lastBin); + return find_bin_max(value, pAxis, size-1U, 0U, lastBin); } // ========================= Fixed point math ========================= @@ -100,11 +98,11 @@ table3d_dim_t find_ybin(table3d_axis_t &value, const table3d_axis_t *pAxis, tabl // class would miss some important optimisations. Specifically, we can avoid // type promotion during multiplication. typedef uint16_t QU1X8_t; -static constexpr uint8_t QU1X8_INTEGER_SHIFT = 8; -static constexpr QU1X8_t QU1X8_ONE = 1U << QU1X8_INTEGER_SHIFT; -static constexpr QU1X8_t QU1X8_HALF = 1U << (QU1X8_INTEGER_SHIFT-1); +static constexpr QU1X8_t QU1X8_INTEGER_SHIFT = 8; +static constexpr QU1X8_t QU1X8_ONE = (QU1X8_t)1U << QU1X8_INTEGER_SHIFT; +static constexpr QU1X8_t QU1X8_HALF = (QU1X8_t)1U << (QU1X8_INTEGER_SHIFT-1U); -inline QU1X8_t mulQU1X8(QU1X8_t a, QU1X8_t b) +static inline QU1X8_t mulQU1X8(QU1X8_t a, QU1X8_t b) { // 1x1 == 1....but the real reason for this is to avoid 16-bit multiplication overflow. // @@ -127,9 +125,9 @@ inline QU1X8_t mulQU1X8(QU1X8_t a, QU1X8_t b) // ============================= Axis value to bin % ========================= -static inline QU1X8_t compute_bin_position(table3d_axis_t value, const table3d_dim_t &bin, int8_t stride, const table3d_axis_t *pAxis) +static inline QU1X8_t compute_bin_position(table3d_axis_t value, const table3d_dim_t &bin, const table3d_axis_t *pAxis) { - table3d_axis_t binMinValue = pAxis[bin-stride]; + table3d_axis_t binMinValue = pAxis[bin+1U]; if (value==binMinValue) { return 0; } table3d_axis_t binMaxValue = pAxis[bin]; if (value==binMaxValue) { return QU1X8_ONE; } @@ -137,11 +135,12 @@ static inline QU1X8_t compute_bin_position(table3d_axis_t value, const table3d_d // Since we can have bins of any width, we need to use // 24.8 fixed point to avoid overflow - uint32_t p = (uint32_t)(value - binMinValue) << QU1X8_INTEGER_SHIFT; + table3d_axis_t binPosition = value - binMinValue; + uint32_t p = (uint32_t)binPosition << QU1X8_INTEGER_SHIFT; // But since we are computing the ratio (0 to 1), p is guaranteed to be // less than binWidth and thus the division below will result in a value // <=1. So we can reduce the data type from 24.8 (uint32_t) to 1.8 (uint16_t) - return p / binWidth; + return p / (uint32_t)binWidth; } @@ -149,7 +148,7 @@ static inline QU1X8_t compute_bin_position(table3d_axis_t value, const table3d_d //This function pulls a value from a 3D table given a target for X and Y coordinates. //It performs a 2D linear interpolation as described in: www.megamanual.com/v22manual/ve_tuner.pdf -table3d_value_t get3DTableValue(struct table3DGetValueCache *pValueCache, +table3d_value_t __attribute__((noclone)) get3DTableValue(struct table3DGetValueCache *pValueCache, table3d_dim_t axisSize, const table3d_value_t *pValues, const table3d_axis_t *pXAxis, @@ -186,8 +185,8 @@ table3d_value_t get3DTableValue(struct table3DGetValueCache *pValueCache, */ table3d_dim_t rowMax = pValueCache->lastYBinMax * axisSize; table3d_dim_t rowMin = rowMax + axisSize; - table3d_dim_t colMax = axisSize - pValueCache->lastXBinMax - 1; - table3d_dim_t colMin = colMax - 1; + table3d_dim_t colMax = axisSize - pValueCache->lastXBinMax - 1U; + table3d_dim_t colMin = colMax - 1U; table3d_value_t A = pValues[rowMax + colMin]; table3d_value_t B = pValues[rowMax + colMax]; table3d_value_t C = pValues[rowMin + colMin]; @@ -199,8 +198,8 @@ table3d_value_t get3DTableValue(struct table3DGetValueCache *pValueCache, { //Create some normalised position values //These are essentially percentages (between 0 and 1) of where the desired value falls between the nearest bins on each axis - const QU1X8_t p = compute_bin_position(X_in, pValueCache->lastXBinMax, -1, pXAxis); - const QU1X8_t q = compute_bin_position(Y_in, pValueCache->lastYBinMax, -1, pYAxis); + const QU1X8_t p = compute_bin_position(X_in, pValueCache->lastXBinMax, pXAxis); + const QU1X8_t q = compute_bin_position(Y_in, pValueCache->lastYBinMax, pYAxis); const QU1X8_t m = mulQU1X8(QU1X8_ONE-p, q); const QU1X8_t n = mulQU1X8(p, q); diff --git a/speeduino/updates.cpp b/speeduino/updates.cpp index 16f04776..92aea37b 100644 --- a/speeduino/updates.cpp +++ b/speeduino/updates.cpp @@ -770,7 +770,7 @@ void doUpdates(void) if( readEEPROMVersion() > CURRENT_DATA_VERSION ) { storeEEPROMVersion(CURRENT_DATA_VERSION); } } -void multiplyTableLoad(const void *pTable, table_type_t key, uint8_t multiplier) +void multiplyTableLoad(void *pTable, table_type_t key, uint8_t multiplier) { auto y_it = y_begin(pTable, key); while(!y_it.at_end()) @@ -780,7 +780,7 @@ void multiplyTableLoad(const void *pTable, table_type_t key, uint8_t multiplier) } } -void divideTableLoad(const void *pTable, table_type_t key, uint8_t divisor) +void divideTableLoad(void *pTable, table_type_t key, uint8_t divisor) { auto y_it = y_begin(pTable, key); while(!y_it.at_end()) diff --git a/speeduino/updates.h b/speeduino/updates.h index f149f88d..53aaf13c 100644 --- a/speeduino/updates.h +++ b/speeduino/updates.h @@ -4,7 +4,7 @@ #include "table3d.h" void doUpdates(void); -void multiplyTableLoad(const void *pTable, table_type_t key, uint8_t multiplier); //Added 202201 - to update the table Y axis as TPS now works at 0.5% increments. Multiplies the load axis values by 4 (most tables) or by 2 (VVT table) -void divideTableLoad(const void *pTable, table_type_t key, uint8_t divisor); //Added 202201 - to update the table Y axis as TPS now works at 0.5% increments. This should only be needed by the VVT tables when using MAP as load. +void multiplyTableLoad(void *pTable, table_type_t key, uint8_t multiplier); //Added 202201 - to update the table Y axis as TPS now works at 0.5% increments. Multiplies the load axis values by 4 (most tables) or by 2 (VVT table) +void divideTableLoad(void *pTable, table_type_t key, uint8_t divisor); //Added 202201 - to update the table Y axis as TPS now works at 0.5% increments. This should only be needed by the VVT tables when using MAP as load. #endif \ No newline at end of file