Skip to content

Commit 9bca9ff

Browse files
davefiddesdavefiddes
authored
Fix Arduino build and warnings on PlatformIO and Arduino (#5)
* Enable warnings for PlatformIO builds Enable a basic set of warnings to find problems with PlatformIO builds. Stop on error so they don't build up in future. Note: There are many warnings currently so this breaks the build. * Remove unused variables and redundant code * Fix serious warnings in OICan This addresses the following problems in OICan functions: - handleSdoResponse() and handleUpdate() explicitly handle the end states of the state machines to fix a warnings with unhandled switch cases. - SendJson() function was not correctly advancing the update state. - SendCanMapping() is more careful about the initial state of the data for each CAN mapping to avoid an uninitialised data read warning. * Fix CppCheck linting problems CppCheck was picking up a number of issues: - Missing prototype for deleteOldest() - Not setting the path correctly in handleFileList() - Confusing type and variable names for OICan::state and OICan::updstate * Fix ambigous overload of OICan::setValueSdo() The newer gcc 12.2 compiler in the Arduino platform for the ESP32 objects to the overload of OICan::setValueSdo(). As the floating point conversion is used in only one place it is probably easiest to move the conversion to the call site. * Portable printf/snprintf formatting of uint32_t values The PlatformIO esp32 platform uses an older ESP-IDF compiler based on gcc 8.4 whereas the official Espressif Arduino IDE platform uses a much newer gcc 12.2 compiler. The definition of uint32_t has changed from unsigned int to unsigned long int. This causes problems for printf() and snprintf() format strings. The format strings have been updated to use the portable PRI_xxx macros. This reduces readability somewhat but stops undefined behaviour. * Fix structure initialisation in ESP-IDF structures The uart_config_t structure used in the main sketch setup() was not fully initialised. The fix uses the correct constants from the ESP-IDF to represent the zero value the original code was implicitly assuming. The twai_general_config_t structure in OICan::Init() was missing an interrupt flag initialiser. Fix the warning by explicitly setting to zero to match the previous implict assumption. * Fix odd stringop-overread warning in binaryLoggingStart() The newer gcc 12.2 in the Arduino platform reports -Wstringop-overread in binaryLoggingStart(). The length check doesn't make a lot of sense so change to using a plain strlen(). * Allow boards with a pre-defined LED_BUILTIN A warning about masking the LED_BUILTIN define is reported when using a WEMOS LOLIN32 Lite board. The board should be compatible with the official hardware so only define LED_BUILTIN if required. * Upgrade to ArduinoJson 7 ArduinoJson 7 uses the heap (intelligently) for allocations rather than requiring users to choose between static and dynamic allocation. This updates the project to use the new API. In doing this it fixes an intermittent allocation failure in SendJson() when concurrent requests are received from the browser.
1 parent fad7802 commit 9bca9ff

File tree

3 files changed

+63
-51
lines changed

3 files changed

+63
-51
lines changed

‎esp32-web-interface.ino

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@
6464
#define INVERTER_TX 17
6565
#define UART_TIMEOUT (100 / portTICK_PERIOD_MS)
6666
#define UART_MESSBUF_SIZE 100
67+
#ifndef LED_BUILTIN
6768
#define LED_BUILTIN 5
69+
#endif
6870

6971
#define RESERVED_SD_SPACE 2000000000
7072
#define SDIO_BUFFER_SIZE 16384
@@ -101,6 +103,8 @@ uint16_t blockCountSD = 0;
101103
File dataFile;
102104
int startLogAttempt = 0;
103105

106+
uint32_t deleteOldest(uint64_t spaceRequired);
107+
104108
bool createNextSDFile()
105109
{
106110
char filename[50];
@@ -113,9 +117,9 @@ bool createNextSDFile()
113117
do
114118
{
115119
if(haveRTC)
116-
snprintf(filename, 50, "/%d-%02d-%02d-%02d-%02d-%02d_%d.bin", int_rtc.getYear(), int_rtc.getMonth(), int_rtc.getDay(), int_rtc.getHour(), int_rtc.getMinute(), int_rtc.getSecond(), nextFileIndex++);
120+
snprintf(filename, 50, "/%d-%02d-%02d-%02d-%02d-%02d_%" PRIu32 ".bin", int_rtc.getYear(), int_rtc.getMonth(), int_rtc.getDay(), int_rtc.getHour(), int_rtc.getMinute(), int_rtc.getSecond(), nextFileIndex++);
117121
else
118-
snprintf(filename, 50, "/%010d.bin", nextFileIndex++);
122+
snprintf(filename, 50, "/%010" PRIu32 ".bin", nextFileIndex++);
119123
}
120124
while(SD_MMC.exists(filename));
121125

@@ -132,7 +136,6 @@ bool createNextSDFile()
132136

133137
uint32_t deleteOldest(uint64_t spaceRequired)
134138
{
135-
time_t oldestTime = 0;
136139
File root, file;
137140
String oldestFileName;
138141
uint64_t spaceRem;
@@ -149,7 +152,7 @@ uint32_t deleteOldest(uint64_t spaceRequired)
149152
{
150153
root = SD_MMC.open("/");
151154

152-
oldestTime = 0;
155+
time_t oldestTime = 0;
153156
fileCount = 0;
154157
while(file = root.openNextFile())
155158
{
@@ -242,7 +245,7 @@ bool handleFileRead(String path){
242245
path += ".gz";
243246
File file = SPIFFS.open(path, "r");
244247
server.sendHeader("Cache-Control", "max-age=86400");
245-
size_t sent = server.streamFile(file, contentType);
248+
server.streamFile(file, contentType);
246249
file.close();
247250
return true;
248251
}
@@ -255,7 +258,7 @@ bool handleFileRead(String path){
255258

256259
if (SD_MMC.exists(path)) {
257260
File file = SD_MMC.open(path, "r");
258-
size_t sent = server.streamFile(file, contentType);
261+
server.streamFile(file, contentType);
259262
file.close();
260263
return true;
261264
}
@@ -343,16 +346,13 @@ void handleRTCSet() {
343346
void handleSdCardDeleteAll() {
344347
if (haveSDCard) {
345348
File root, file;
346-
if (haveSDCard) {
347-
root = SD_MMC.open("/");
348-
while(file = root.openNextFile())
349-
{
350-
String filename = file.name();
351-
if(SD_MMC.remove("/" + filename))
352-
DBG_OUTPUT_PORT.println("Deleted file: " + filename);
353-
else
354-
DBG_OUTPUT_PORT.println("Couldn't delete: " + filename);
355-
}
349+
root = SD_MMC.open("/");
350+
while(file = root.openNextFile()) {
351+
String filename = file.name();
352+
if(SD_MMC.remove("/" + filename))
353+
DBG_OUTPUT_PORT.println("Deleted file: " + filename);
354+
else
355+
DBG_OUTPUT_PORT.println("Couldn't delete: " + filename);
356356
}
357357
}
358358

@@ -394,7 +394,7 @@ void handleSdCardList() {
394394
void handleFileList() {
395395
String path = "/";
396396
if(server.hasArg("dir"))
397-
String path = server.arg("dir");
397+
path = server.arg("dir");
398398
//DBG_OUTPUT_PORT.println("handleFileList: " + path);
399399
File root = SPIFFS.open(path);
400400
String output = "[";
@@ -407,7 +407,6 @@ void handleFileList() {
407407
File file = root.openNextFile();
408408
while(file){
409409
if (output != "[") output += ',';
410-
bool isDir = false;
411410
output += "{\"type\":\"";
412411
output += file.isDirectory()?"dir":"file";
413412
output += "\",\"name\":\"";
@@ -464,7 +463,6 @@ static void sendCommand(String cmd)
464463
}
465464

466465
static void handleCommand() {
467-
const int cmdBufSize = 128;
468466
if(!server.hasArg("cmd")) {server.send(500, "text/plain", "BAD ARGS"); return;}
469467

470468
String cmd = server.arg("cmd");
@@ -609,7 +607,7 @@ static void handleWifi()
609607
if (updated)
610608
{
611609
File file = SPIFFS.open("/wifi-updated.html", "r");
612-
size_t sent = server.streamFile(file, getContentType("wifi-updated.html"));
610+
server.streamFile(file, getContentType("wifi-updated.html"));
613611
file.close();
614612
}
615613
}
@@ -640,7 +638,9 @@ void setup(void){
640638
.data_bits = UART_DATA_8_BITS,
641639
.parity = UART_PARITY_DISABLE,
642640
.stop_bits = UART_STOP_BITS_1,
643-
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE};
641+
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
642+
.rx_flow_ctrl_thresh = UART_HW_FLOWCTRL_DISABLE,
643+
.source_clk = UART_SCLK_APB};
644644

645645
uart_param_config(INVERTER_PORT, &uart_config);
646646
uart_set_pin(INVERTER_PORT, INVERTER_TX, INVERTER_RX, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
@@ -763,7 +763,7 @@ void binaryLoggingStart()
763763
uart_set_baudrate(INVERTER_PORT, 2250000);
764764
uart_write_bytes(INVERTER_PORT, "\n", 1);
765765
delay(1);
766-
uart_write_bytes(INVERTER_PORT, "binarylogging 0", strnlen("binarylogging 0", UART_MESSBUF_SIZE));
766+
uart_write_bytes(INVERTER_PORT, "binarylogging 0", strlen("binarylogging 0"));
767767
uart_write_bytes(INVERTER_PORT, "\n", 1);
768768
uart_wait_tx_done(INVERTER_PORT, UART_TIMEOUT);
769769
uart_set_baudrate(INVERTER_PORT, 115200);
@@ -777,7 +777,7 @@ void binaryLoggingStop()
777777
{
778778
uart_write_bytes(INVERTER_PORT, "\n", 1);
779779
delay(1);
780-
uart_write_bytes(INVERTER_PORT, "binarylogging 0", strnlen("binarylogging 0", UART_MESSBUF_SIZE));
780+
uart_write_bytes(INVERTER_PORT, "binarylogging 0", strlen("binarylogging 0"));
781781
uart_write_bytes(INVERTER_PORT, "\n", 1);
782782
uart_wait_tx_done(INVERTER_PORT, UART_TIMEOUT);
783783
uart_set_baudrate(INVERTER_PORT, 115200);
@@ -803,8 +803,6 @@ void binaryLoggingStop()
803803
}
804804

805805
void loop(void){
806-
static int subIndex = 0;
807-
static uint32_t serial[4];
808806
// note: ArduinoOTA.handle() calls MDNS.update();
809807
server.handleClient();
810808
ArduinoOTA.handle();

‎platformio.ini

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@ upload_flags =
3636
build_flags =
3737
${env.build_flags}
3838
-D RELEASE
39+
-Wall -Werror
3940
build_type = release
4041
lib_deps =
4142
fbiego/ESP32Time@^2.0.0
4243
adafruit/RTClib@^2.1.1
43-
bblanchon/ArduinoJson@^6.21.2
44+
bblanchon/ArduinoJson@^7.1.0
4445
bblanchon/StreamUtils@^1.7.3
4546
SPI
4647

@@ -51,10 +52,11 @@ build_flags =
5152
-DDEBUG_ESP_PORT=Serial
5253
-DDEBUG_ESP_CORE
5354
-DDEBUG_ESP_WIFI
55+
-Wall -Werror
5456
build_type = debug
5557
lib_deps =
5658
fbiego/ESP32Time@^2.0.0
5759
adafruit/RTClib@^2.1.1
58-
bblanchon/ArduinoJson@^6.21.2
60+
bblanchon/ArduinoJson@^7.1.0
5961
bblanchon/StreamUtils@^1.7.3
6062
SPI

‎src/oi_can.cpp

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,15 @@
5858

5959
namespace OICan {
6060

61-
enum state { IDLE, ERROR, OBTAINSERIAL, OBTAIN_JSON };
62-
enum updstate { UPD_IDLE, SEND_MAGIC, SEND_SIZE, SEND_PAGE, CHECK_CRC, REQUEST_JSON };
61+
enum State { IDLE, ERROR, OBTAINSERIAL, OBTAIN_JSON };
62+
enum UpdState { UPD_IDLE, SEND_MAGIC, SEND_SIZE, SEND_PAGE, CHECK_CRC, REQUEST_JSON };
6363

6464
static uint8_t _nodeId;
6565
static BaudRate baudRate;
66-
static state state;
67-
static updstate updstate;
66+
static State state;
67+
static UpdState updstate;
6868
static uint32_t serial[4]; //contains id sum as well
6969
static char jsonFileName[20];
70-
static char canFileName[20];
7170
static twai_message_t tx_frame;
7271
static File updateFile;
7372
static int currentPage = 0;
@@ -103,13 +102,9 @@ static void setValueSdo(uint16_t index, uint8_t subIndex, uint32_t value) {
103102
twai_transmit(&tx_frame, pdMS_TO_TICKS(10));
104103
}
105104

106-
static void setValueSdo(uint16_t index, uint8_t subIndex, double value) {
107-
setValueSdo(index, subIndex, (uint32_t)(value * 32));
108-
}
109-
110105
static int getId(String name) {
111-
DynamicJsonDocument doc(300);
112-
StaticJsonDocument<200> filter;
106+
JsonDocument doc;
107+
JsonDocument filter;
113108

114109
File file = SPIFFS.open(jsonFileName, "r");
115110
filter[name]["id"] = true;
@@ -154,8 +149,8 @@ static void handleSdoResponse(twai_message_t *rxframe) {
154149
requestSdoElement(SDO_INDEX_SERIAL, rxframe->data[3] + 1);
155150
}
156151
else {
157-
sprintf(jsonFileName, "/%x.json", serial[3]);
158-
DBG_OUTPUT_PORT.printf("Got Serial Number %X:%X:%X:%X\r\n", serial[0], serial[1], serial[2], serial[3]);
152+
sprintf(jsonFileName, "/%" PRIx32 ".json", serial[3]);
153+
DBG_OUTPUT_PORT.printf("Got Serial Number %" PRIX32 ":%" PRIX32 ":%" PRIX32 ":%" PRIX32 "\r\n", serial[0], serial[1], serial[2], serial[3]);
159154

160155
if (SPIFFS.exists(jsonFileName)) {
161156
state = IDLE;
@@ -190,6 +185,12 @@ static void handleSdoResponse(twai_message_t *rxframe) {
190185
requestNextSegment(toggleBit);
191186
}
192187

188+
break;
189+
case ERROR:
190+
// Do not exit this state
191+
break;
192+
case IDLE:
193+
// Do not exit this state
193194
break;
194195
}
195196
}
@@ -225,7 +226,7 @@ static void handleUpdate(twai_message_t *rxframe) {
225226
tx_frame.data[2] = rxframe->data[6];
226227
tx_frame.data[3] = rxframe->data[7];
227228
updstate = SEND_SIZE;
228-
DBG_OUTPUT_PORT.printf("Sending ID %u\r\n", *(uint32_t*)tx_frame.data);
229+
DBG_OUTPUT_PORT.printf("Sending ID %" PRIu32 "\r\n", *(uint32_t*)tx_frame.data);
229230
twai_transmit(&tx_frame, pdMS_TO_TICKS(10));
230231
}
231232
break;
@@ -309,6 +310,12 @@ static void handleUpdate(twai_message_t *rxframe) {
309310
DBG_OUTPUT_PORT.printf("Done!\r\n");
310311
}
311312
break;
313+
case REQUEST_JSON:
314+
// Do not exit this state
315+
break;
316+
case UPD_IDLE:
317+
// Do not exit this state
318+
break;
312319
}
313320

314321
}
@@ -329,7 +336,7 @@ int GetCurrentUpdatePage() {
329336
bool SendJson(WiFiClient client) {
330337
if (state != IDLE) return false;
331338

332-
DynamicJsonDocument doc(50000);
339+
JsonDocument doc;
333340
twai_message_t rxframe;
334341

335342
File file = SPIFFS.open(jsonFileName, "r");
@@ -338,7 +345,7 @@ bool SendJson(WiFiClient client) {
338345

339346
if (result != DeserializationError::Ok) {
340347
SPIFFS.remove(jsonFileName); //if json file is invalid, remove it and trigger re-download
341-
updstate == REQUEST_JSON;
348+
updstate = REQUEST_JSON;
342349
retries = 50;
343350
DBG_OUTPUT_PORT.println("JSON file invalid, re-downloading");
344351
return false;
@@ -372,18 +379,22 @@ void SendCanMapping(WiFiClient client) {
372379

373380
twai_message_t rxframe;
374381
int index = SDO_INDEX_MAP_RD, subIndex = 0;
375-
int cobid, pos, len, paramid;
382+
int cobid = 0, pos = 0, len = 0, paramid = 0;
376383
bool rx = false;
377384
String result;
378385
ReqMapStt reqMapStt = START;
379386

380-
DynamicJsonDocument doc(16384);
387+
JsonDocument doc;
381388

382389
while (DONE != reqMapStt) {
383390
switch (reqMapStt) {
384391
case START:
385392
requestSdoElement(index, 0); //request COB ID
386393
reqMapStt = COBID;
394+
cobid = 0;
395+
pos = 0;
396+
len = 0;
397+
paramid = 0;
387398
break;
388399
case COBID:
389400
if (twai_receive(&rxframe, pdMS_TO_TICKS(10)) == ESP_OK) {
@@ -432,7 +443,7 @@ void SendCanMapping(WiFiClient client) {
432443
gain /= 1000;
433444
int offset = (int8_t)rxframe.data[7];
434445
DBG_OUTPUT_PORT.printf("can %s %d %d %d %d %f %d\r\n", rx ? "rx" : "tx", paramid, cobid, pos, len, gain, offset);
435-
StaticJsonDocument<200> subdoc;
446+
JsonDocument subdoc;
436447
JsonObject object = subdoc.to<JsonObject>();
437448
object["isrx"] = rx;
438449
object["id"] = cobid;
@@ -466,7 +477,7 @@ void SendCanMapping(WiFiClient client) {
466477
SetResult AddCanMapping(String json) {
467478
if (state != IDLE) return CommError;
468479

469-
StaticJsonDocument<256> doc;
480+
JsonDocument doc;
470481
twai_message_t rxframe;
471482

472483
deserializeJson(doc, json);
@@ -505,7 +516,7 @@ SetResult AddCanMapping(String json) {
505516
SetResult RemoveCanMapping(String json){
506517
if (state != IDLE) return CommError;
507518

508-
StaticJsonDocument<256> doc;
519+
JsonDocument doc;
509520
twai_message_t rxframe;
510521

511522
deserializeJson(doc, json);
@@ -539,7 +550,7 @@ SetResult SetValue(String name, double value) {
539550

540551
int id = getId(name);
541552

542-
setValueSdo(SDO_INDEX_PARAM_UID | (id >> 8), id & 0xFF, value);
553+
setValueSdo(SDO_INDEX_PARAM_UID | (id >> 8), id & 0xFF, (uint32_t)(value * 32));
543554

544555
if (twai_receive(&rxframe, pdMS_TO_TICKS(10)) == ESP_OK) {
545556
if (rxframe.data[0] == SDO_RESPONSE_DOWNLOAD)
@@ -646,7 +657,8 @@ void Init(uint8_t nodeId, BaudRate baud) {
646657
.tx_queue_len = 30,
647658
.rx_queue_len = 30,
648659
.alerts_enabled = TWAI_ALERT_NONE,
649-
.clkout_divider = 0
660+
.clkout_divider = 0,
661+
.intr_flags = 0
650662
};
651663

652664
uint16_t id = 0x580 + nodeId;
@@ -707,7 +719,7 @@ void Loop() {
707719
else if (rxframe.identifier == 0x7de)
708720
handleUpdate(&rxframe);
709721
else
710-
DBG_OUTPUT_PORT.printf("Received unwanted frame %u\r\n", rxframe.identifier);
722+
DBG_OUTPUT_PORT.printf("Received unwanted frame %" PRIu32 "\r\n", rxframe.identifier);
711723
}
712724

713725
if (updstate == REQUEST_JSON) {

0 commit comments

Comments
 (0)