Skip to content

Commit b829e51

Browse files
authored
Page.* SonarQube fixes (speeduino#1422)
* Replace #define with constexpr * Change entity_type to an enum class. * Apply 'hidden friend' idiom to entity_page_location_t operators. * "static inline constexpr" -> "static constexpr" * Remove getPageCount(), replace with MAX_PAGE_NUM * byte -> uint8_t
1 parent f74c616 commit b829e51

File tree

7 files changed

+75
-84
lines changed

7 files changed

+75
-84
lines changed

speeduino/comms_legacy.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -930,15 +930,15 @@ namespace {
930930
{
931931
switch (entity.type)
932932
{
933-
case Raw:
933+
case EntityType::Raw:
934934
return send_raw_entity(entity);
935935
break;
936936

937-
case Table:
937+
case EntityType::Table:
938938
return send_table_entity(entity);
939939
break;
940940

941-
case NoEntity:
941+
case EntityType::NoEntity:
942942
// No-op
943943
break;
944944

@@ -960,7 +960,7 @@ void sendPage(void)
960960
{
961961
page_iterator_t entity = page_begin(currentPage);
962962

963-
while (entity.type!=End)
963+
while (entity.type!=EntityType::End)
964964
{
965965
send_entity(entity);
966966
entity = advance(entity);

speeduino/page_crc.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#include "page_crc.h"
33
#include "pages.h"
44

5-
uint32_t __attribute__((optimize("Os"))) calculatePageCRC32(byte pageNum)
5+
uint32_t __attribute__((optimize("Os"))) calculatePageCRC32(uint8_t pageNum)
66
{
77
FastCRC32 crcCalc;
88

speeduino/page_crc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
/*
55
* Calculates and returns the CRC32 value of a given page of memory
66
*/
7-
uint32_t calculatePageCRC32(byte pageNum /**< [in] The page number to compute CRC for. */);
7+
uint32_t calculatePageCRC32(uint8_t pageNum /**< [in] The page number to compute CRC for. */);

speeduino/pages.cpp

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,17 @@
3333
// calling context.
3434

3535
template <class table_t>
36-
static inline constexpr uint16_t get_table_value_end(void)
36+
static constexpr uint16_t get_table_value_end(void)
3737
{
3838
return table_t::xaxis_t::length*table_t::yaxis_t::length;
3939
}
4040
template <class table_t>
41-
static inline constexpr uint16_t get_table_axisx_end(void)
41+
static constexpr uint16_t get_table_axisx_end(void)
4242
{
4343
return get_table_value_end<table_t>()+table_t::xaxis_t::length;
4444
}
4545
template <class table_t>
46-
static inline constexpr uint16_t get_table_axisy_end(const table_t *table)
46+
static constexpr uint16_t get_table_axisy_end(const table_t *table)
4747
{
4848
UNUSED(table);
4949
return get_table_axisx_end<table_t>()+table_t::yaxis_t::length;
@@ -194,11 +194,11 @@ static inline byte get_table_value(const page_iterator_t &entity, uint16_t offse
194194

195195
byte getEntityValue(const page_iterator_t &entity, uint16_t offset)
196196
{
197-
if (Raw==entity.type)
197+
if (EntityType::Raw==entity.type)
198198
{
199199
return get_raw_location(entity, offset);
200200
}
201-
if (Table==entity.type)
201+
if (EntityType::Table==entity.type)
202202
{
203203
return get_table_value(entity, offset);
204204
}
@@ -224,11 +224,11 @@ static inline bool set_table_value(page_iterator_t &entity, uint16_t offset, byt
224224

225225
bool setEntityValue(page_iterator_t &entity, uint16_t offset, byte value)
226226
{
227-
if (Raw==entity.type)
227+
if (EntityType::Raw==entity.type)
228228
{
229229
return set_raw_location(entity, offset, value);
230230
}
231-
else if (Table==entity.type)
231+
else if (EntityType::Table==entity.type)
232232
{
233233
return set_table_value(entity, offset, value);
234234
}
@@ -252,7 +252,7 @@ void nextEntity(page_iterator_t &entity, uint16_t nextBlockSize)
252252
template <class table_t>
253253
static void checkIsInTable(page_iterator_t &result, table_t *pTable, uint16_t offset)
254254
{
255-
if (result.type==End)
255+
if (result.type==EntityType::End)
256256
{
257257
nextEntity(result, get_table_axisy_end(pTable));
258258
if (result.address.isOffsetInEntity(offset))
@@ -266,7 +266,7 @@ static void checkIsInTable(page_iterator_t &result, table_t *pTable, uint16_t of
266266

267267
static void checkIsInRaw(page_iterator_t &result, void *pEntity, uint16_t entitySize, uint16_t offset)
268268
{
269-
if (result.type==End)
269+
if (result.type==EntityType::End)
270270
{
271271
nextEntity(result, entitySize);
272272
if (result.address.isOffsetInEntity(offset))
@@ -280,7 +280,7 @@ static void checkIsInRaw(page_iterator_t &result, void *pEntity, uint16_t entity
280280

281281
static void checkIsInEmpty(page_iterator_t &result, uint16_t entitySize, uint16_t offset)
282282
{
283-
if (result.type==End)
283+
if (result.type==EntityType::End)
284284
{
285285
nextEntity(result, entitySize);
286286
if (result.address.isOffsetInEntity(offset))
@@ -299,7 +299,7 @@ static void checkIsInEmpty(page_iterator_t &result, uint16_t entitySize, uint16_
299299
static page_iterator_t map_page_offset_to_entity(uint8_t pageNumber, uint16_t offset)
300300
{
301301
// This is mutated by the checkIsIn* functions to return the entity that matches the offset
302-
page_iterator_t result( End, // Signal that no entity has been found yet
302+
page_iterator_t result( EntityType::End, // Signal that no entity has been found yet
303303
entity_page_location_t(pageNumber, (uint8_t)-1 /* Deliberate, so we can increment index AND address as one operation */),
304304
entity_page_address_t(0U, 0U));
305305

@@ -384,7 +384,7 @@ static page_iterator_t map_page_offset_to_entity(uint8_t pageNumber, uint16_t of
384384
}
385385

386386
// Nothing matched, so we are at the end of the known entities for the page.
387-
if (result.type==End)
387+
if (result.type==EntityType::End)
388388
{
389389
nextEntity(result, 0U);
390390
}
@@ -395,12 +395,7 @@ static page_iterator_t map_page_offset_to_entity(uint8_t pageNumber, uint16_t of
395395

396396
// ====================================== External functions ====================================
397397

398-
uint8_t getPageCount(void)
399-
{
400-
return 16U;
401-
}
402-
403-
uint16_t getPageSize(byte pageNum)
398+
uint16_t getPageSize(uint8_t pageNum)
404399
{
405400
page_iterator_t entity = map_page_offset_to_entity(pageNum, UINT16_MAX);
406401
return entity.address.start + entity.address.size;
@@ -411,14 +406,14 @@ static inline uint16_t pageOffsetToEntityOffset(const page_iterator_t &entity, u
411406
return pageOffset-entity.address.start;
412407
}
413408

414-
bool setPageValue(byte pageNum, uint16_t offset, byte value)
409+
bool setPageValue(uint8_t pageNum, uint16_t offset, byte value)
415410
{
416411
page_iterator_t entity = map_page_offset_to_entity(pageNum, offset);
417412

418413
return setEntityValue(entity, pageOffsetToEntityOffset(entity, offset), value);
419414
}
420415

421-
byte getPageValue(byte pageNum, uint16_t offset)
416+
byte getPageValue(uint8_t pageNum, uint16_t offset)
422417
{
423418
page_iterator_t entity = map_page_offset_to_entity(pageNum, offset);
424419

@@ -429,7 +424,7 @@ byte getPageValue(byte pageNum, uint16_t offset)
429424
// No need to have coverage on simple wrappers
430425

431426
// Support iteration over a pages entities.
432-
page_iterator_t page_begin(byte pageNum)
427+
page_iterator_t page_begin(uint8_t pageNum)
433428
{
434429
return map_page_offset_to_entity(pageNum, 0U);
435430
}

speeduino/pages.h

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,38 +2,34 @@
22
#include <Arduino.h>
33
#include "table3d.h"
44

5-
/**
6-
* Page count, as defined in the INI file
7-
*/
8-
uint8_t getPageCount(void);
9-
105
/**
116
* Page size in bytes
127
*/
13-
uint16_t getPageSize(byte pageNum /**< [in] The page number */ );
8+
uint16_t getPageSize(uint8_t pageNum /**< [in] The page number */ );
149

1510
// These are the page numbers that the Tuner Studio serial protocol uses to transverse the different map and config pages.
16-
#define veMapPage 2
17-
#define veSetPage 1 //Note that this and the veMapPage were swapped in Feb 2019 as the 'algorithm' field must be declared in the ini before it's used in the fuel table
18-
#define ignMapPage 3
19-
#define ignSetPage 4//Config Page 2
20-
#define afrMapPage 5
21-
#define afrSetPage 6//Config Page 3
22-
#define boostvvtPage 7
23-
#define seqFuelPage 8
24-
#define canbusPage 9//Config Page 9
25-
#define warmupPage 10 //Config Page 10
26-
#define fuelMap2Page 11
27-
#define wmiMapPage 12
28-
#define progOutsPage 13
29-
#define ignMap2Page 14
30-
#define boostvvtPage2 15
31-
#define MAX_PAGE_NUM (boostvvtPage2+1U)
11+
constexpr uint8_t veMapPage = 2;
12+
constexpr uint8_t veSetPage = 1; //Note that this and the veMapPage were swapped in Feb 2019 as the 'algorithm' field must be declared in the ini before it's used in the fuel table
13+
constexpr uint8_t ignMapPage = 3;
14+
constexpr uint8_t ignSetPage = 4;
15+
constexpr uint8_t afrMapPage = 5;
16+
constexpr uint8_t afrSetPage = 6;
17+
constexpr uint8_t boostvvtPage = 7;
18+
constexpr uint8_t seqFuelPage = 8;
19+
constexpr uint8_t canbusPage = 9;
20+
constexpr uint8_t warmupPage = 10;
21+
constexpr uint8_t fuelMap2Page = 11;
22+
constexpr uint8_t wmiMapPage = 12;
23+
constexpr uint8_t progOutsPage = 13;
24+
constexpr uint8_t ignMap2Page = 14;
25+
constexpr uint8_t boostvvtPage2 = 15;
26+
constexpr uint8_t MIN_PAGE_NUM = veSetPage;
27+
constexpr uint8_t MAX_PAGE_NUM = (boostvvtPage2+1U);
3228

3329
// ============================== Per-byte page access ==========================
3430

3531
/** @brief Gets a single value from a page, with data aligned as per the ini file */
36-
byte getPageValue( byte pageNum, /**< [in] The page number to retrieve data from. */
32+
byte getPageValue( uint8_t pageNum, /**< [in] The page number to retrieve data from. */
3733
uint16_t offset /**< [in] The address in the page that should be returned. This is as per the page definition in the ini. */
3834
);
3935

@@ -42,7 +38,7 @@ byte getPageValue( byte pageNum, /**< [in] The page number to retrieve da
4238
*
4339
* @returns true if value set, false otherwise
4440
*/
45-
bool setPageValue( byte pageNum, /**< [in] The page number to update. */
41+
bool setPageValue( uint8_t pageNum, /**< [in] The page number to update. */
4642
uint16_t offset, /**< [in] The offset within the page. */
4743
byte value /**< [in] The new value */
4844
);
@@ -54,7 +50,7 @@ bool setPageValue( byte pageNum, /**< [in] The page number to update. */
5450
// over those entities.
5551

5652
// Type of entity
57-
enum entity_type {
53+
enum class EntityType : uint8_t {
5854
Raw, // A block of memory
5955
Table, // A 3D table
6056
NoEntity, // No entity, but a valid offset
@@ -72,15 +68,16 @@ struct entity_page_location_t {
7268
{
7369
}
7470

75-
bool operator==(const entity_page_location_t &other) const
71+
friend bool operator==(const entity_page_location_t &lhs, const entity_page_location_t &rhs)
7672
{
77-
return page==other.page
78-
&& index==other.index;
73+
return lhs.page==rhs.page
74+
&& lhs.index==rhs.index;
7975
}
80-
bool operator!=(const entity_page_location_t &other) const
76+
77+
friend bool operator!=(const entity_page_location_t &lhs, const entity_page_location_t &rhs)
8178
{
82-
return !operator==(other);
83-
}
79+
return !(lhs==rhs);
80+
}
8481
};
8582

8683
/** @brief Position and size of an entity within a page */
@@ -114,12 +111,12 @@ struct entity_page_address_t {
114111
// A entity on a logical page.
115112
struct page_iterator_t {
116113
void *pData = nullptr;
117-
entity_type type;
114+
EntityType type;
118115
table_type_t table_key = table_type_None;
119116
entity_page_location_t location;
120117
entity_page_address_t address;
121118

122-
page_iterator_t(entity_type theType, const entity_page_location_t &entityLocation, const entity_page_address_t &entityAddress)
119+
page_iterator_t(EntityType theType, const entity_page_location_t &entityLocation, const entity_page_address_t &entityAddress)
123120
: type(theType)
124121
, location(entityLocation)
125122
, address(entityAddress)
@@ -129,21 +126,21 @@ struct page_iterator_t {
129126
void setNoEntity(void)
130127
{
131128
pData = nullptr;
132-
type = NoEntity;
129+
type = EntityType::NoEntity;
133130
table_key = table_type_None;
134131
}
135132

136133
void setTable(void *pTable, table_type_t key)
137134
{
138135
pData = pTable;
139-
type = Table;
136+
type = EntityType::Table;
140137
table_key = key;
141138
}
142139

143140
void setRaw(void *pBuffer)
144141
{
145142
pData = pBuffer;
146-
type = Raw;
143+
type = EntityType::Raw;
147144
table_key = table_type_None;
148145
}
149146
};
@@ -168,7 +165,7 @@ bool setEntityValue(page_iterator_t &entity, /**< [in] The entity to update */
168165
* Initiates iteration over a pages entities.
169166
* Test `entity.type==End` to determine the end of the page.
170167
*/
171-
page_iterator_t page_begin(byte pageNum /**< [in] The page number to iterate over. */);
168+
page_iterator_t page_begin(uint8_t pageNum /**< [in] The page number to iterate over. */);
172169

173170
/**
174171
* Moves the iterator to the next sub-entity on the page

speeduino/storage.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,13 @@ bool isEepromWritePending(void)
4444
*/
4545
void writeAllConfig(void)
4646
{
47-
uint8_t pageCount = getPageCount();
48-
uint8_t page = 1U;
47+
uint8_t page = MIN_PAGE_NUM;
4948
writeConfig(page);
50-
page = page + 1;
51-
while (page<pageCount && !isEepromWritePending())
49+
++page;
50+
while (page<MAX_PAGE_NUM && !isEepromWritePending())
5251
{
5352
writeConfig(page);
54-
page = page + 1;
53+
++page;
5554
}
5655
}
5756

@@ -303,12 +302,12 @@ void writeConfig(uint8_t pageNum)
303302
*/
304303
void resetConfigPages(void)
305304
{
306-
for (uint8_t page=1; page<getPageCount(); ++page)
305+
for (uint8_t page=MIN_PAGE_NUM; page<MAX_PAGE_NUM; ++page)
307306
{
308307
page_iterator_t entity = page_begin(page);
309-
while (entity.type!=End)
308+
while (entity.type!=EntityType::End)
310309
{
311-
if (entity.type==Raw)
310+
if (entity.type==EntityType::Raw)
312311
{
313312
memset(entity.pData, 0, entity.address.size);
314313
}
@@ -501,7 +500,7 @@ void writeCalibrationPage(uint8_t pageNum)
501500

502501
static eeprom_address_t compute_crc_address(uint8_t pageNum)
503502
{
504-
return EEPROM_LAST_BARO-((getPageCount() - pageNum)*sizeof(uint32_t));
503+
return EEPROM_LAST_BARO-((MAX_PAGE_NUM - pageNum)*sizeof(uint32_t));
505504
}
506505

507506
/** Write CRC32 checksum to EEPROM.

0 commit comments

Comments
 (0)