From cb64725b42d9f9bdb2e529eb95dc3a50a2a582be Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sat, 24 Jul 2021 21:11:18 +0200 Subject: [PATCH 1/9] make output ID two bytes --- DCCEXParser.cpp | 6 ++++++ Outputs.cpp | 6 +++--- Outputs.h | 16 +++++++++------- version.h | 3 ++- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index cfb7219..29e7074 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -579,6 +579,12 @@ bool DCCEXParser::parseZ(Print *stream, int16_t params, int16_t p[]) return true; case 3: // + if (p[0] < 0) + return false; + if (p[1] > 255 || p[1] < 0) + return false; + if (!(p[2] == 0 || p[2] == 1)) + return false; if (!Output::create(p[0], p[1], p[2], 1)) return false; StringFormatter::send(stream, F("\n")); diff --git a/Outputs.cpp b/Outputs.cpp index b7aa7bc..1e13807 100644 --- a/Outputs.cpp +++ b/Outputs.cpp @@ -100,14 +100,14 @@ void Output::activate(int s){ /////////////////////////////////////////////////////////////////////////////// -Output* Output::get(int n){ +Output* Output::get(uint16_t n){ Output *tt; for(tt=firstOutput;tt!=NULL && tt->data.id!=n;tt=tt->nextOutput); return(tt); } /////////////////////////////////////////////////////////////////////////////// -bool Output::remove(int n){ +bool Output::remove(uint16_t n){ Output *tt,*pp=NULL; for(tt=firstOutput;tt!=NULL && tt->data.id!=n;pp=tt,tt=tt->nextOutput); @@ -160,7 +160,7 @@ void Output::store(){ } /////////////////////////////////////////////////////////////////////////////// -Output *Output::create(int id, int pin, int iFlag, int v){ +Output *Output::create(uint16_t id, uint8_t pin, uint8_t iFlag, uint8_t v){ Output *tt; if(firstOutput==NULL){ diff --git a/Outputs.h b/Outputs.h index 319f704..a0431e0 100644 --- a/Outputs.h +++ b/Outputs.h @@ -23,25 +23,27 @@ struct OutputData { uint8_t oStatus; - uint8_t id; + uint16_t id; uint8_t pin; uint8_t iFlag; }; class Output{ - public: + +public: void activate(int s); - static Output* get(int); - static bool remove(int); + static Output* get(uint16_t); + static bool remove(uint16_t); static void load(); static void store(); - static Output *create(int, int, int, int=0); + static Output *create(uint16_t, uint8_t, uint8_t, uint8_t=0); static Output *firstOutput; struct OutputData data; Output *nextOutput; static void printAll(Print *); - private: - int num; // Chris has no idea what this is all about! + +private: + int num; // EEPROM pointer (Chris has no idea what this is all about!) }; // Output diff --git a/version.h b/version.h index 9b926cd..97e34c7 100644 --- a/version.h +++ b/version.h @@ -3,7 +3,8 @@ #include "StringFormatter.h" -#define VERSION "3.1.4" +#define VERSION "3.1.5" +// 3.1.5 Make output ID two bytes // 3.1.4 Refactor OLED and LCD drivers and remove unused code // 3.1.3 Add a loop delay to give more time for sensing an Ethernet cable connection // 3.1.2 Eliminate wait after write when prog is joined or prog power is off From f24bcd6819146633385c25ca18433cf40220aebe Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sat, 24 Jul 2021 21:14:19 +0200 Subject: [PATCH 2/9] step version --- version.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/version.h b/version.h index 97e34c7..d6ac9a6 100644 --- a/version.h +++ b/version.h @@ -3,8 +3,9 @@ #include "StringFormatter.h" -#define VERSION "3.1.5" -// 3.1.5 Make output ID two bytes +#define VERSION "3.1.6" +// 3.1.6 Make output ID two bytes +// 3.1.5 Fix LCD corruption on power-up // 3.1.4 Refactor OLED and LCD drivers and remove unused code // 3.1.3 Add a loop delay to give more time for sensing an Ethernet cable connection // 3.1.2 Eliminate wait after write when prog is joined or prog power is off From ec2295219d5aa69c6f21f4752d21508e3313e5a0 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sat, 24 Jul 2021 23:44:24 +0200 Subject: [PATCH 3/9] 3rd arg of Z is bitfield --- DCCEXParser.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index 29e7074..a2086c9 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -578,12 +578,10 @@ bool DCCEXParser::parseZ(Print *stream, int16_t params, int16_t p[]) } return true; - case 3: // - if (p[0] < 0) - return false; - if (p[1] > 255 || p[1] < 0) - return false; - if (!(p[2] == 0 || p[2] == 1)) + case 3: // + if (p[0] < 0 || + p[1] > 255 || p[1] < 0 || + p[2] < 0 || p[2] > 7 ) return false; if (!Output::create(p[0], p[1], p[2], 1)) return false; From c15d5048b5842f8821eb34b0295588c7593f36ee Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sun, 25 Jul 2021 22:53:20 +0200 Subject: [PATCH 4/9] EEPROM format heuristics --- Outputs.cpp | 45 +++++++++++++++++++++++++++++++++++++-------- Outputs.h | 7 +++++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/Outputs.cpp b/Outputs.cpp index 1e13807..38e4282 100644 --- a/Outputs.cpp +++ b/Outputs.cpp @@ -127,17 +127,46 @@ bool Output::remove(uint16_t n){ /////////////////////////////////////////////////////////////////////////////// void Output::load(){ - struct OutputData data; + struct BrokenOutputData bdata; Output *tt; + bool isBroken=1; + + // This is a scary kluge. As we have two formats in EEPROM due to an + // earlier bug, we don't know which we encounter now. So we guess + // that if in all entries this byte has value of 7 or lower this is + // an iFlag and thus the broken format. Otherwise it would be a pin + // id. If someone uses only pins 0 to 7 of their arduino, they + // loose. This is (if you look at an arduino) however unlikely. for(int i=0;idata.nOutputs;i++){ - EEPROM.get(EEStore::pointer(),data); - tt=create(data.id,data.pin,data.iFlag); - tt->data.oStatus=bitRead(tt->data.iFlag,1)?bitRead(tt->data.iFlag,2):data.oStatus; // restore status to EEPROM value is bit 1 of iFlag=0, otherwise set to value of bit 2 of iFlag - digitalWrite(tt->data.pin,tt->data.oStatus ^ bitRead(tt->data.iFlag,0)); - pinMode(tt->data.pin,OUTPUT); - tt->num=EEStore::pointer(); - EEStore::advance(sizeof(tt->data)); + EEPROM.get(EEStore::pointer()+ i*sizeof(struct BrokenOutputData),bdata); + if (bdata.iFlag > 7) { // it's a pin and not an iFlag! + isBroken=0; + break; + } + } + if ( isBroken ) { + for(int i=0;idata.nOutputs;i++){ + EEPROM.get(EEStore::pointer(),bdata); + tt=create(bdata.id,bdata.pin,bdata.iFlag); + tt->data.oStatus=bitRead(tt->data.iFlag,1)?bitRead(tt->data.iFlag,2):bdata.oStatus; // restore status to EEPROM value is bit 1 of iFlag=0, otherwise set to value of bit 2 of iFlag + digitalWrite(tt->data.pin,tt->data.oStatus ^ bitRead(tt->data.iFlag,0)); + pinMode(tt->data.pin,OUTPUT); + tt->num=EEStore::pointer(); + EEStore::advance(sizeof(struct OutputData)); + } + } else { + struct OutputData data; + + for(int i=0;idata.nOutputs;i++){ + EEPROM.get(EEStore::pointer(),data); + tt=create(data.id,data.pin,data.iFlag); + tt->data.oStatus=bitRead(tt->data.iFlag,1)?bitRead(tt->data.iFlag,2):data.oStatus; // restore status to EEPROM value is bit 1 of iFlag=0, otherwise set to value of bit 2 of iFlag + digitalWrite(tt->data.pin,tt->data.oStatus ^ bitRead(tt->data.iFlag,0)); + pinMode(tt->data.pin,OUTPUT); + tt->num=EEStore::pointer(); + EEStore::advance(sizeof(struct OutputData)); + } } } diff --git a/Outputs.h b/Outputs.h index a0431e0..6132102 100644 --- a/Outputs.h +++ b/Outputs.h @@ -28,6 +28,13 @@ struct OutputData { uint8_t iFlag; }; +struct BrokenOutputData { + uint8_t oStatus; + uint8_t id; + uint8_t pin; + uint8_t iFlag; +}; + class Output{ public: From cc4de0ad14e21ea3f583a4157430d648700d5be9 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sun, 25 Jul 2021 23:07:20 +0200 Subject: [PATCH 5/9] fix size of struct at right place --- Outputs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Outputs.cpp b/Outputs.cpp index 38e4282..c03a0b1 100644 --- a/Outputs.cpp +++ b/Outputs.cpp @@ -153,7 +153,7 @@ void Output::load(){ digitalWrite(tt->data.pin,tt->data.oStatus ^ bitRead(tt->data.iFlag,0)); pinMode(tt->data.pin,OUTPUT); tt->num=EEStore::pointer(); - EEStore::advance(sizeof(struct OutputData)); + EEStore::advance(sizeof(struct BrokenOutputData)); } } else { struct OutputData data; From c292f210a4e61ed3075c30a3615dcbd54c7817fb Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Sun, 25 Jul 2021 23:12:12 +0200 Subject: [PATCH 6/9] datatypes used in eeprom should be a data type that has a given size --- EEStore.h | 6 +++--- Outputs.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/EEStore.h b/EEStore.h index b1d6d31..f7af1e7 100644 --- a/EEStore.h +++ b/EEStore.h @@ -14,9 +14,9 @@ extern ExternalEEPROM EEPROM; struct EEStoreData{ char id[sizeof(EESTORE_ID)]; - int nTurnouts; - int nSensors; - int nOutputs; + uint16_t nTurnouts; + uint16_t nSensors; + uint16_t nOutputs; }; struct EEStore{ diff --git a/Outputs.cpp b/Outputs.cpp index c03a0b1..52cafb8 100644 --- a/Outputs.cpp +++ b/Outputs.cpp @@ -138,7 +138,7 @@ void Output::load(){ // id. If someone uses only pins 0 to 7 of their arduino, they // loose. This is (if you look at an arduino) however unlikely. - for(int i=0;idata.nOutputs;i++){ + for(uint16_t i=0;idata.nOutputs;i++){ EEPROM.get(EEStore::pointer()+ i*sizeof(struct BrokenOutputData),bdata); if (bdata.iFlag > 7) { // it's a pin and not an iFlag! isBroken=0; @@ -146,7 +146,7 @@ void Output::load(){ } } if ( isBroken ) { - for(int i=0;idata.nOutputs;i++){ + for(uint16_t i=0;idata.nOutputs;i++){ EEPROM.get(EEStore::pointer(),bdata); tt=create(bdata.id,bdata.pin,bdata.iFlag); tt->data.oStatus=bitRead(tt->data.iFlag,1)?bitRead(tt->data.iFlag,2):bdata.oStatus; // restore status to EEPROM value is bit 1 of iFlag=0, otherwise set to value of bit 2 of iFlag @@ -158,7 +158,7 @@ void Output::load(){ } else { struct OutputData data; - for(int i=0;idata.nOutputs;i++){ + for(uint16_t i=0;idata.nOutputs;i++){ EEPROM.get(EEStore::pointer(),data); tt=create(data.id,data.pin,data.iFlag); tt->data.oStatus=bitRead(tt->data.iFlag,1)?bitRead(tt->data.iFlag,2):data.oStatus; // restore status to EEPROM value is bit 1 of iFlag=0, otherwise set to value of bit 2 of iFlag From a88454dded71d1b6d287174d4486946cc30baa8e Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Tue, 27 Jul 2021 18:35:22 +0200 Subject: [PATCH 7/9] Disallow pins <= 7 --- DCCEXParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index a2086c9..cbe6519 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -580,7 +580,7 @@ bool DCCEXParser::parseZ(Print *stream, int16_t params, int16_t p[]) case 3: // if (p[0] < 0 || - p[1] > 255 || p[1] < 0 || + p[1] > 255 || p[1] <= 7 || p[2] < 0 || p[2] > 7 ) return false; if (!Output::create(p[0], p[1], p[2], 1)) From 2443e5903c8288c841f23bc421c4ed849e464624 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Tue, 27 Jul 2021 18:39:54 +0200 Subject: [PATCH 8/9] Fix type warnings --- Sensors.cpp | 2 +- Turnouts.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sensors.cpp b/Sensors.cpp index d04f99e..33c3fd6 100644 --- a/Sensors.cpp +++ b/Sensors.cpp @@ -184,7 +184,7 @@ void Sensor::load(){ struct SensorData data; Sensor *tt; - for(int i=0;idata.nSensors;i++){ + for(uint16_t i=0;idata.nSensors;i++){ EEPROM.get(EEStore::pointer(),data); tt=create(data.snum,data.pin,data.pullUp); EEStore::advance(sizeof(tt->data)); diff --git a/Turnouts.cpp b/Turnouts.cpp index 677e746..4c46a68 100644 --- a/Turnouts.cpp +++ b/Turnouts.cpp @@ -103,7 +103,7 @@ void Turnout::load(){ struct TurnoutData data; Turnout *tt; - for(int i=0;idata.nTurnouts;i++){ + for(uint16_t i=0;idata.nTurnouts;i++){ EEPROM.get(EEStore::pointer(),data); if (data.tStatus & STATUS_PWM) tt=create(data.id,data.tStatus & STATUS_PWMPIN, data.inactiveAngle,data.moveAngle); else tt=create(data.id,data.address,data.subAddress); From 8f1ed21aa3a98167a8f573a695f0fc4429aae728 Mon Sep 17 00:00:00 2001 From: Harald Barth Date: Tue, 27 Jul 2021 19:53:21 +0200 Subject: [PATCH 9/9] Allow some pins that might be useful --- DCCEXParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DCCEXParser.cpp b/DCCEXParser.cpp index cbe6519..2c3885a 100644 --- a/DCCEXParser.cpp +++ b/DCCEXParser.cpp @@ -580,7 +580,7 @@ bool DCCEXParser::parseZ(Print *stream, int16_t params, int16_t p[]) case 3: // if (p[0] < 0 || - p[1] > 255 || p[1] <= 7 || + p[1] > 255 || p[1] <= 1 || // Pins 0 and 1 are Serial to USB p[2] < 0 || p[2] > 7 ) return false; if (!Output::create(p[0], p[1], p[2], 1))