Extract out entity location and address structs from page_iterator_t (speeduino#1411)

* Refactor: extract entity_page_location_t and entity_page_address_t from page_iterator_t

* Remove unecessary #include

Author: adbancroft

speeduino/comms_legacy.cpp

@@ -897,7 +897,7 @@ namespace {
 
   void send_raw_entity(const page_iterator_t &entity)
   {
-    primarySerial.write((byte *)entity.pData, entity.size);
+    primarySerial.write((byte *)entity.pData, entity.address.size);
   }
 
   inline void send_table_values(table_value_iterator it)

speeduino/page_crc.cpp

@@ -1,7 +1,6 @@
-#include "globals.h"
+#include <FastCRC.h>
 #include "page_crc.h"
 #include "pages.h"
-#include <FastCRC.h>
 
 // Abstract the FastCrC32 functions 
 // - they have have very slight differences in signatures, which causes the Arduino
@@ -17,7 +16,7 @@ using pCrcCalc = uint32_t (*)(FastCRC32 &, const uint8_t *, uint16_t);
 
 static inline uint32_t compute_raw_crc(const page_iterator_t &entity, pCrcCalc calcFunc, FastCRC32 &crcCalc)
 {
-    return calcFunc(crcCalc, (uint8_t*)entity.pData, entity.size);
+    return calcFunc(crcCalc, (uint8_t*)entity.pData, entity.address.size);
 }
 
 static inline uint32_t compute_row_crc(const table_row_iterator &row, pCrcCalc calcFunc, FastCRC32 &crcCalc)
@@ -83,7 +82,7 @@ static inline uint32_t compute_crc(const page_iterator_t &entity, pCrcCalc calcF
         break;
 
     case NoEntity:
-        return pad_crc(entity.size, 0U, crcCalc);
+        return pad_crc(entity.address.size, 0U, crcCalc);
         break;
 
     default:
@@ -105,5 +104,5 @@ uint32_t calculatePageCRC32(byte pageNum)
     crc = compute_crc(entity, &updateCrc /* Note that we are *updating* */, crcCalc);
     entity = advance(entity);
   }
-  return pad_crc(getPageSize(pageNum) - entity.size, crc, crcCalc);
+  return pad_crc(getPageSize(pageNum) - entity.address.size, crc, crcCalc);
 }

speeduino/pages.cpp

@@ -147,20 +147,20 @@ class offset_to_table
 
 // ========================= Offset to entity byte mapping =========================
 
-inline byte& get_raw_location(page_iterator_t &entity, uint16_t offset)
+static inline byte& get_raw_location(const page_iterator_t &entity, uint16_t offset)
 {
-  return *((byte*)entity.pData + (offset-entity.start));
+  return *((byte*)entity.pData + (offset-entity.address.start));
 }
 
-inline byte get_table_value(page_iterator_t &entity, uint16_t offset)
+static inline byte get_table_value(const 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);
   #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));  
+  CONCRETE_TABLE_ACTION(entity.table_key, CTA_GET_TABLE_VALUE, CTA_GET_TABLE_VALUE_DEFAULT, entity.pData, (offset-entity.address.start));  
 }
 
-inline byte get_value(page_iterator_t &entity, uint16_t offset)
+static inline byte get_value(const page_iterator_t &entity, uint16_t offset)
 {
   if (Raw==entity.type)
   {
@@ -173,15 +173,15 @@ inline byte get_value(page_iterator_t &entity, uint16_t offset)
   return 0U;
 }
 
-inline void set_table_value(page_iterator_t &entity, uint16_t offset, byte new_value)
+inline void set_table_value(const page_iterator_t &entity, uint16_t offset, byte new_value)
 {
   #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;
   #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);  
+  CONCRETE_TABLE_ACTION(entity.table_key, CTA_SET_TABLE_VALUE, CTA_SET_TABLE_VALUE_DEFAULT, entity.pData, (offset-entity.address.start), new_value);  
 }
 
-inline void set_value(page_iterator_t &entity, byte value, uint16_t offset)
+inline void set_value(const page_iterator_t &entity, byte value, uint16_t offset)
 {    
   if (Raw==entity.type)
   {
@@ -224,16 +224,11 @@ static inline void check_size() {
 //
 // Instead we use this (and other) intermediate factory function(s) - it provides a barrier that
 // forces GCC to construct the page_iterator_t instance at runtime.
-inline const page_iterator_t create_end_iterator(uint8_t pageNum, uint16_t start)
+static inline page_iterator_t create_end_iterator(uint8_t pageNum, uint16_t start)
 {
-  return page_iterator_t {
-    .pData = nullptr,
-    .table_key = table_type_None,
-    .page = pageNum,
-    .start = start,
-    .size = start,
-    .type = End,
-  };
+  return page_iterator_t( End, 
+                          entity_page_location_t(pageNum, UINT8_MAX),
+                          entity_page_address_t(start, 0U));
 }
 
 // Signal the end of a page
@@ -243,47 +238,37 @@ inline const page_iterator_t create_end_iterator(uint8_t pageNum, uint16_t start
 
 // ========================= Table processing  ===================
 
-inline const page_iterator_t create_table_iterator(void *pTable, table_type_t key, uint8_t pageNum, uint16_t start, uint16_t size)
+static inline page_iterator_t create_table_iterator(void *pTable, table_type_t key, uint8_t pageNum, uint8_t index, uint16_t start, uint16_t size)
 {
-  return page_iterator_t {
-    .pData = pTable,
-    .table_key = key,
-    .page = pageNum,
-    .start = start,
-    .size = size,
-    .type = Table,
-  };
+  return page_iterator_t( pTable, key,
+                          entity_page_location_t(pageNum, index),
+                          entity_page_address_t(start, size));
 }
 
 // If the offset is in range, create a Table entity_t
 #define CHECK_TABLE(pageNum, offset, pTable, entityNum) \
   if (offset < ENTITY_START_VAR(entityNum)+get_table_axisy_end(pTable)) \
   { \
     return create_table_iterator(pTable, (pTable)->type_key, \
-                                  pageNum, \
-                                  ENTITY_START_VAR(entityNum), get_table_axisy_end(pTable)); \
+                                  (pageNum), (entityNum), \
+                                  ENTITY_START_VAR(entityNum), get_table_axisy_end((pTable))); \
   } \
   DECLARE_NEXT_ENTITY_START(entityNum, get_table_axisy_end(pTable))
 
 // ========================= Raw memory block processing  ===================
 
-inline const page_iterator_t create_raw_iterator(void *pBuffer, uint8_t pageNum, uint16_t start, uint16_t size)
+static inline page_iterator_t create_raw_iterator(void *pBuffer, uint8_t pageNum, uint8_t index, uint16_t start, uint16_t size)
 {
-  return page_iterator_t {
-    .pData = pBuffer,
-    .table_key = table_type_None,
-    .page = pageNum,
-    .start = start,
-    .size = size,
-    .type = Raw,
-  };
+  return page_iterator_t( pBuffer,
+                          entity_page_location_t(pageNum, index),    
+                          entity_page_address_t(start, size));
 }
 
 // If the offset is in range, create a Raw entity_t
 #define CHECK_RAW(pageNum, offset, pDataBlock, blockSize, entityNum) \
   if (offset < ENTITY_START_VAR(entityNum)+blockSize) \
   { \
-    return create_raw_iterator(pDataBlock, pageNum, ENTITY_START_VAR(entityNum), blockSize);\
+    return create_raw_iterator((pDataBlock), (pageNum), (entityNum), ENTITY_START_VAR(entityNum), (blockSize));\
   } \
   DECLARE_NEXT_ENTITY_START(entityNum, blockSize)
 
@@ -448,7 +433,7 @@ page_iterator_t page_begin(byte pageNum)
 
 page_iterator_t advance(const page_iterator_t &it)
 {
-    return map_page_offset_to_entity(it.page, it.start+it.size);
+    return map_page_offset_to_entity(it.location.page, it.address.start+it.address.size);
 }
 
 /**

speeduino/pages.h

@@ -59,14 +59,73 @@ enum entity_type {
     End         // The offset was past any known entity for the page
 };
 
+/** @brief The *unique* location of an entity within all pages */
+struct entity_page_location_t {
+    uint8_t page;   // The index of the page the entity belongs to
+    uint8_t index;  // The sub-index of the item within the page
+
+    constexpr entity_page_location_t(uint8_t pageNum, uint8_t pageSubIndex)
+    : page(pageNum)
+    , index(pageSubIndex)
+    {        
+    }
+
+    bool operator==(const entity_page_location_t &other) const
+    {
+        return page==other.page
+            && index==other.index;
+    }
+    bool operator!=(const entity_page_location_t &other) const
+    {
+        return !operator==(other);
+    }
+};
+
+/** @brief  Position and size of an entity within a page */
+struct entity_page_address_t {
+    uint16_t start; // The start position of the entity, in bytes, from the start of the page
+    uint16_t size;  // Size of the entity in bytes
+
+    entity_page_address_t(uint16_t base, uint16_t length)
+    : start(base)
+    , size(length)
+    {        
+    }
+};
+
 // A entity on a logical page.
 struct page_iterator_t {
     void *pData;
-    table_type_t table_key;
-    uint8_t page;   // The page the entity belongs to
-    uint16_t start; // The start position of the entity, in bytes, from the start of the page
-    uint16_t size;  // Size of the entity in bytes
     entity_type type;
+    table_type_t table_key;
+    entity_page_location_t location;
+    entity_page_address_t address;
+
+    page_iterator_t(entity_type theType, const entity_page_location_t &entityLocation, const entity_page_address_t &entityAddress)
+    : pData(nullptr)
+    , type(theType)
+    , table_key(table_type_None)
+    , location(entityLocation)    
+    , address(entityAddress)
+    {
+    }
+
+    page_iterator_t(void *pBuffer, const entity_page_location_t &entityLocation, const entity_page_address_t &entityAddress)
+    : pData(pBuffer)
+    , type(Raw)
+    , table_key(table_type_None)
+    , location(entityLocation)    
+    , address(entityAddress)
+    {
+    }
+    page_iterator_t(void *pTable, table_type_t key, const entity_page_location_t &entityLocation, const entity_page_address_t &entityAddress)
+    : pData(pTable)
+    , type(Table)
+    , table_key(key)
+    , location(entityLocation)    
+    , address(entityAddress)
+    {
+    }
 };
 
 /**

speeduino/storage.cpp

@@ -341,7 +341,7 @@ void resetConfigPages(void)
     {
       if (entity.type==Raw)
       {
-        memset(entity.pData, 0, entity.size);
+        memset(entity.pData, 0, entity.address.size);
       }
       entity = advance(entity);
     }