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

@@ -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);

speeduino/int16_byte.h

@@ -1,65 +0,0 @@
-/** 
- * @addtogroup table_3d 
- * @{
- */
-
-#pragma once
-
-
-#include <stdint.h>
-
-#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 &divider
-#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
-};
-
-/** @} */

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);

speeduino/pages.cpp

@@ -31,17 +31,17 @@ constexpr const uint16_t PROGMEM ini_page_sizes[] = { 0, 128, 288, 288, 128, 288
 // calling context.
 
 template <class table_t>
-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 <class table_t>
-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>()+table_t::xaxis_t::length;
 }
 template <class table_t>
-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>()+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<table_t>()));
   }
 
-  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<table_t>()));
   }
@@ -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<get_table_value_end<table_t>())
     {
@@ -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)>((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)>((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)

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), 

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);
 }

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);
 /** @} */

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); \
         } \
     };
 

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;
+static byte to_byte_100(int16_t value) { return (byte)div100(value); }
-constexpr int16_byte table3d_axis_io::converter_2;
+static int16_t from_byte_100(byte in) { return (int16_t)in * 100; }
-constexpr int16_byte table3d_axis_io::converter_1;
+
+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 };
+}

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 
+/** @brief Byte type. This is not defined in any C or C++ standard header. */
+typedef uint8_t byte;
+
+/** @brief Models int16->byte conversion 
+ * @see table3d_axis_io_converter 
+ */
+typedef byte(*pToByteConverter)(int16_t value);
+
+/** @brief Models byte->int16 conversion
+ * @see table3d_axis_io_converter 
+ */
+typedef int16_t(*pFromByteConverter)(byte in);
+
+/** @brief Convert a 16-bit value to/from a byte. Useful for I/O. 
  *
- * @attention Using \c constexpr class static variables seems to be the best
+ * Often we need to deal internally with values that fit in 16-bits but do
- * combination of size and speed - \c constexpr implies \c inline, so we
+ * not require much accuracy. E.g. table axes in RPM. For these values we can 
- * can't use it on traditional \c extern global variables.
+ * save storage space (EEPROM) by scaling to/from 8-bits.
-*/
+ */
-class table3d_axis_io {
+struct table3d_axis_io_converter {
-public:
+    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 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 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 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 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);
+
 /** @} */

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;
+  pMax = pMax + 1U;
-  if (lastBinMax!=minBinIndex && is_in_bin(value, *(pMax - stride), *pMax))
+  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;
+  pMax -= 2U;
-  if (lastBinMax!=maxElement && is_in_bin(value, *(pMax - stride), *pMax))
+  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;
+    ++lastBinMax;
-    pMax -= stride;
+    ++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_INTEGER_SHIFT = 8;
-static constexpr QU1X8_t QU1X8_ONE = 1U << QU1X8_INTEGER_SHIFT;
+static constexpr QU1X8_t QU1X8_ONE = (QU1X8_t)1U << QU1X8_INTEGER_SHIFT;
-static constexpr QU1X8_t QU1X8_HALF = 1U << (QU1X8_INTEGER_SHIFT-1);
+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 colMax = axisSize - pValueCache->lastXBinMax - 1U;
-    table3d_dim_t colMin = colMax - 1;
+    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 p = compute_bin_position(X_in, pValueCache->lastXBinMax, pXAxis);
-      const QU1X8_t q = compute_bin_position(Y_in, pValueCache->lastYBinMax, -1, pYAxis);
+      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);

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())

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 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(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 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