Separate table axis I/O conversion from iteration (#767)

* Missing include

* 1. Add int16_byte class
2. Add axis factory function for int16_byte instances
(flyweight pattern)

* Separate out axis iteration and I/O conversions

* Optimize for size and performance.

* Fix unit test
This commit is contained in:
tx_haggis 2022-02-23 23:11:54 -06:00 committed by GitHub
parent 3a4470642c
commit 6c12bcc32d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 166 additions and 109 deletions

View File

@ -18,6 +18,7 @@ A full copy of the license may be found in the projects root directory
#include "pages.h"
#include "page_crc.h"
#include "logger.h"
#include "table3d_axis_io.h"
#ifdef RTC_ENABLED
#include "rtc_common.h"
#endif
@ -793,9 +794,10 @@ namespace {
inline void send_table_axis(table_axis_iterator it)
{
const int16_byte *pConverter = table3d_axis_io::get_converter(it.domain());
while (!it.at_end())
{
Serial.write((byte)*it);
Serial.write(pConverter->to_byte(*it));
++it;
}
}
@ -901,7 +903,7 @@ namespace {
void print_row(const table_axis_iterator &y_it, table_row_iterator row)
{
serial_print_prepadded_value((byte)*y_it);
serial_print_prepadded_value(table3d_axis_io::to_byte(y_it.domain(), *y_it));
while (!row.at_end())
{
@ -916,9 +918,11 @@ namespace {
Serial.print(F(" "));
auto x_it = x_begin(pTable, key);
const int16_byte *pConverter = table3d_axis_io::get_converter(x_it.domain());
while(!x_it.at_end())
{
serial_print_prepadded_value((byte)*x_it);
serial_print_prepadded_value(pConverter->to_byte(*x_it));
++x_it;
}
}

48
speeduino/int16_byte.h Normal file
View File

@ -0,0 +1,48 @@
/**
* @addtogroup table_3d
* @{
*/
#pragma once
#include <stdint.h>
#include "src/libdivide/libdivide.h"
/** @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 tiem.
*/
constexpr int16_byte(uint8_t factor, const libdivide::libdivide_s16_t &divider)
: _factor(factor), _divider(divider)
{
}
/** @brief Convert to a \c byte */
inline byte to_byte(int16_t value) const { return _factor==1 ? value : _factor==2 ? value>>1 : (byte)libdivide::libdivide_s16_do(value, &_divider); }
/** @brief Convert from a \c byte */
inline int16_t from_byte( byte in ) const { return (int16_t)in * _factor; }
private:
uint8_t _factor;
libdivide::libdivide_s16_t _divider;
};
/** @} */

View File

@ -1,55 +0,0 @@
/**
* @addtogroup table_3d
* @{
*/
#pragma once
#include <stdint.h>
#include "src/libdivide/libdivide.h"
/** @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_ref
{
public:
#pragma pack(push, 1)
struct scalar {
uint8_t _factor;
libdivide::libdivide_s16_t divider;
};
#pragma pack(pop)
/**
* @brief Construct
* @param value The \c int16_t to encapsulate.
* @param pScalar The factor to scale the \c int16_t value by when converting to/from a \c byte
*/
int16_ref(int16_t &value, const scalar *pScalar)
: _value(value), _pScalar(pScalar)
{
}
/** @brief Convert to a \c byte */
inline byte operator*() const { return (byte)libdivide::libdivide_s16_do(_value, &_pScalar->divider); }
/** @brief Convert to a \c byte */
inline explicit operator byte() const { return **this; }
/** @brief Convert from a \c byte */
inline int16_ref &operator=( byte in ) { _value = (int16_t)in * (int16_t)(_pScalar->_factor); return *this; }
private:
int16_t &_value;
const scalar *_pScalar;
};
/** @} */

View File

@ -20,6 +20,7 @@ A full copy of the license may be found in the projects root directory
#include "logger.h"
#include "comms.h"
#include "src/FastCRC/FastCRC.h"
#include "table3d_axis_io.h"
#ifdef RTC_ENABLED
#include "rtc_common.h"
#endif
@ -920,9 +921,10 @@ namespace
inline void send_table_axis(table_axis_iterator it)
{
const int16_byte *pConverter = table3d_axis_io::get_converter(it.domain());
while (!it.at_end())
{
Serial.write((byte)*it);
Serial.write(pConverter->to_byte(*it));
++it;
}
}

View File

@ -1,7 +1,7 @@
#include "globals.h"
#include "page_crc.h"
#include "pages.h"
//#include "src/FastCRC/FastCRC.h"
#include "table3d_axis_io.h"
typedef uint32_t (FastCRC32::*pCrcCalc)(const uint8_t *, const uint16_t, bool);
@ -30,11 +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)
{
const int16_byte *pConverter = table3d_axis_io::get_converter(it.domain());
byte values[32]; // Fingers crossed we don't have a table bigger than 32x32
byte *pValue = values;
while (!it.at_end())
{
*pValue++ = (byte)*it;
*pValue++ = pConverter->to_byte(*it);
++it;
}
return pValue-values==0 ? crc : CRC32.crc32_upd(values, pValue-values, false);

View File

@ -1,6 +1,7 @@
#include "pages.h"
#include "globals.h"
#include "utilities.h"
#include "table3d_axis_io.h"
// Maps from virtual page "addresses" to addresses/bytes of real in memory entities
//
@ -79,10 +80,10 @@ public:
case table_location_values:
return get_value_value();
case table_location_xaxis:
return *get_xaxis_value();
return table3d_axis_io::to_byte(table_t::xaxis_t::domain, get_xaxis_value());
case table_location_yaxis:
default:
return *get_yaxis_value();
return table3d_axis_io::to_byte(table_t::yaxis_t::domain, get_yaxis_value());
}
}
@ -96,12 +97,12 @@ public:
break;
case table_location_xaxis:
get_xaxis_value() = new_value;
get_xaxis_value() = table3d_axis_io::from_byte(table_t::xaxis_t::domain, new_value);
break;
case table_location_yaxis:
default:
get_yaxis_value() = new_value;
get_yaxis_value() = table3d_axis_io::from_byte(table_t::yaxis_t::domain, new_value);
}
invalidate_cache(&_pTable->get_value_cache);
return *this;
@ -114,14 +115,14 @@ private:
return _pTable->values.value_at((uint8_t)_table_offset);
}
inline int16_ref get_xaxis_value() const
inline table3d_axis_t& get_xaxis_value() const
{
return *_pTable->axisX.begin().advance(_table_offset - get_table_value_end<table_t>());
return *(_pTable->axisX.begin().advance(_table_offset - get_table_value_end<table_t>()));
}
inline int16_ref get_yaxis_value() const
inline table3d_axis_t& get_yaxis_value() const
{
return *_pTable->axisY.begin().advance(_table_offset - get_table_axisx_end<table_t>());
return *(_pTable->axisY.begin().advance(_table_offset - get_table_axisx_end<table_t>()));
}
enum table_location {

View File

@ -16,9 +16,9 @@
#define LIBDIVIDE_VERSION_MINOR 0
#include <stdint.h>
#include <stdlib.h>
#if !defined(__AVR__)
#include <stdio.h>
#include <stdlib.h>
#endif
#if defined(LIBDIVIDE_SSE2)

View File

@ -11,6 +11,7 @@ A full copy of the license may be found in the projects root directory
#include EEPROM_LIB_H //This is defined in the board .h files
#include "storage.h"
#include "pages.h"
#include "table3d_axis_io.h"
//The maximum number of write operations that will be performed in one go. If we try to write to the EEPROM too fast (Each write takes ~3ms) then the rest of the system can hang)
#if defined(CORE_STM32) || defined(CORE_TEENSY) & !defined(USE_SPI_EEPROM)
@ -120,9 +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.domain());
while ((location.can_write() || forceBurn) && !it.at_end())
{
location.update((byte)*it);
location.update(pConverter->to_byte(*it));
++location;
++it;
}
@ -346,9 +348,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.domain());
while (!it.at_end())
{
*it = EEPROM.read(address);
*it = pConverter->from_byte(EEPROM.read(address));
++address;
++it;
}

View File

@ -1,6 +0,0 @@
#include <stdlib.h>
#include "table3d_axes.h"
constexpr int16_ref::scalar table3d_axis_base::scalar_100;
constexpr int16_ref::scalar table3d_axis_base::scalar_2;
constexpr int16_ref::scalar table3d_axis_base::scalar_1;

View File

@ -10,8 +10,6 @@
#pragma once
#include "table3d_typedefs.h"
#include "int16_ref.h"
#include "src/libdivide/constant_fast_div.h"
/**\enum axis_domain
* @brief Encodes the real world measurement that a table axis captures
@ -31,11 +29,13 @@ class table_axis_iterator
public:
/** @brief Construct */
table_axis_iterator(const table3d_axis_t *pStart, const table3d_axis_t *pEnd, const int16_ref::scalar *pScalar, int8_t stride)
: _pAxis(pStart), _pAxisEnd(pEnd), _stride(stride), _pScalar(pScalar)
table_axis_iterator(const table3d_axis_t *pStart, const table3d_axis_t *pEnd, int8_t stride, axis_domain domain)
: _pAxis(pStart), _pAxisEnd(pEnd), _stride(stride), _domain(domain)
{
}
axis_domain domain(void) const { return _domain; }
/** @brief Advance the iterator
* @param steps The number of elements to move the iterator
*/
@ -58,14 +58,14 @@ public:
}
/** @brief Dereference the iterator */
inline int16_ref operator*()
inline table3d_axis_t& operator*()
{
return int16_ref(*const_cast<table3d_axis_t*>(_pAxis), _pScalar);
return *const_cast<table3d_axis_t *>(_pAxis);
}
/** @copydoc table_axis_iterator::operator*() */
inline const int16_ref operator*() const
inline const table3d_axis_t& operator*() const
{
return int16_ref(*const_cast<table3d_axis_t*>(_pAxis), _pScalar);
return *_pAxis;
}
/** @brief Reverse the iterator direction
@ -85,28 +85,14 @@ private:
const table3d_axis_t *_pAxis;
const table3d_axis_t *_pAxisEnd;
int8_t _stride;
const int16_ref::scalar *_pScalar;
};
/** @brief Shared code for the axis types */
class table3d_axis_base {
protected:
static constexpr const int16_ref::scalar* domain_to_scalar(axis_domain domain) {
// This really, really needs to be done at compile time, hence the contexpr
return domain==axis_domain_Rpm ? &scalar_100 :
domain==axis_domain_Load ? &scalar_2 : &scalar_1;
}
private:
static constexpr const int16_ref::scalar scalar_100 = { 100, { S16_MAGIC(100), S16_MORE(100) } };
static constexpr const int16_ref::scalar scalar_2 = { 2, { S16_MAGIC(2), S16_MORE(2) } };
static constexpr const int16_ref::scalar scalar_1 = { 1, { S16_MAGIC(1), S16_MORE(1) } };
axis_domain _domain;
};
#define TABLE3D_TYPENAME_XAXIS(size, xDom, yDom) CONCAT(TABLE3D_TYPENAME_BASE(size, xDom, yDom), _xaxis)
#define TABLE3D_GEN_XAXIS(size, xDom, yDom) \
/** @brief The x-axis for a 3D table with size x size dimensions, xDom x-axis and yDom y-axis */ \
struct TABLE3D_TYPENAME_XAXIS(size, xDom, yDom) : public table3d_axis_base { \
struct TABLE3D_TYPENAME_XAXIS(size, xDom, yDom) { \
/** @brief The length of the axis in elements */ \
static constexpr table3d_dim_t length = size; \
/** @brief The domain the axis represents */ \
@ -120,7 +106,7 @@ private:
/** @brief Iterate over the axis elements */ \
inline table_axis_iterator begin() \
{ \
return table_axis_iterator(axis, axis+size, domain_to_scalar(domain), 1); \
return table_axis_iterator(axis, axis+size, 1, domain); \
} \
};
TABLE3D_GENERATOR(TABLE3D_GEN_XAXIS)
@ -129,7 +115,7 @@ TABLE3D_GENERATOR(TABLE3D_GEN_XAXIS)
#define TABLE3D_GEN_YAXIS(size, xDom, yDom) \
/** @brief The y-axis for a 3D table with size x size dimensions, xDom x-axis and yDom y-axis */ \
struct CONCAT(TABLE3D_TYPENAME_BASE(size, xDom, yDom), _yaxis) : public table3d_axis_base { \
struct CONCAT(TABLE3D_TYPENAME_BASE(size, xDom, yDom), _yaxis) { \
/** @brief The length of the axis in elements */ \
static constexpr table3d_dim_t length = size; \
/** @brief The domain the axis represents */ \
@ -143,7 +129,7 @@ TABLE3D_GENERATOR(TABLE3D_GEN_XAXIS)
/** @brief Iterate over the axis elements */ \
inline table_axis_iterator begin() \
{ \
return table_axis_iterator(axis+(size-1), axis-1, domain_to_scalar(domain), -1); \
return table_axis_iterator(axis+(size-1), axis-1, -1, domain); \
} \
};
TABLE3D_GENERATOR(TABLE3D_GEN_YAXIS)

View File

@ -0,0 +1,5 @@
#include "table3d_axis_io.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;

View File

@ -0,0 +1,67 @@
/**
* @addtogroup table_3d
* @{
*/
/** \file
* @brief 3D table axis I/O support
*/
#pragma once
#include "int16_byte.h"
#include "table3d_axes.h"
#include "src/libdivide/constant_fast_div.h"
/** @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 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:
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) } };
};
/** @} */

View File

@ -622,7 +622,7 @@ void multiplyTableLoad(const void *pTable, table_type_t key, uint8_t multiplier)
auto y_it = y_begin(pTable, key);
while(!y_it.at_end())
{
*y_it = (byte)*y_it * multiplier;
*y_it = *y_it * multiplier;
++y_it;
}
}
@ -632,7 +632,7 @@ void divideTableLoad(const void *pTable, table_type_t key, uint8_t divisor)
auto y_it = y_begin(pTable, key);
while(!y_it.at_end())
{
*y_it = (byte)*y_it / divisor; //Previous TS scale was 2.0, now is 0.5, 4x increase
*y_it = *y_it / divisor; //Previous TS scale was 2.0, now is 0.5, 4x increase
++y_it;
}
}

View File

@ -8,9 +8,9 @@
#define _countof(x) (sizeof(x) / sizeof (x[0]))
#if defined(PROGMEM)
const PROGMEM byte values[] = {
const PROGMEM table3d_value_t values[] = {
#else
const byte values[] = {
const table3d_value_t values[] = {
#endif
//0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
109, 111, 112, 113, 114, 114, 114, 115, 115, 115, 114, 114, 114, 114, 114, 114,