From 471af9eecf78c7a87898fc8d8945cf2bc686c4ed Mon Sep 17 00:00:00 2001 From: GioCC <25667790+GioCC@users.noreply.github.com> Date: Sat, 23 Apr 2022 19:35:45 +0200 Subject: [PATCH 1/2] Improvement of pointer handling in EEPROM functions for config parsing --- src/Config.cpp | 343 +++++++++++++++++++----------------- src/MF_Modules/MFEEPROM.cpp | 42 +++-- src/MF_Modules/MFEEPROM.h | 15 +- 3 files changed, 218 insertions(+), 182 deletions(-) diff --git a/src/Config.cpp b/src/Config.cpp index 75ece139..8d16bb0c 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -46,7 +46,7 @@ MFEEPROM MFeeprom; #if MF_MUX_SUPPORT == 1 -extern MFMuxDriver MUX; +extern MFMuxDriver MUX; #endif const uint8_t MEM_OFFSET_NAME = 0; @@ -54,54 +54,62 @@ const uint8_t MEM_LEN_NAME = 48; const uint8_t MEM_OFFSET_SERIAL = MEM_OFFSET_NAME + MEM_LEN_NAME; const uint8_t MEM_LEN_SERIAL = 11; const uint8_t MEM_OFFSET_CONFIG = MEM_OFFSET_NAME + MEM_LEN_NAME + MEM_LEN_SERIAL; - -const char type[sizeof(MOBIFLIGHT_TYPE)] = MOBIFLIGHT_TYPE; -char serial[MEM_LEN_SERIAL] = MOBIFLIGHT_SERIAL; -char name[MEM_LEN_NAME] = MOBIFLIGHT_NAME; -const int MEM_LEN_CONFIG = MEMLEN_CONFIG; -char nameBuffer[MEM_LEN_CONFIG] = ""; -uint16_t configLength = 0; -boolean configActivated = false; - -void resetConfig(); -void readConfig(); -void _activateConfig(); +const uint16_t MEM_LEN_CONFIG = MEMLEN_CONFIG; + +struct { + bool activated; + uint16_t length; + const char type[sizeof(MOBIFLIGHT_TYPE)]; + char name[MEM_LEN_NAME]; + char serial[MEM_LEN_SERIAL]; + char nameBuffer[MEMLEN_NAMES_BUFFER]; +} config = { + false, + 0, + MOBIFLIGHT_TYPE, + MOBIFLIGHT_NAME, + MOBIFLIGHT_SERIAL, + ""}; + +static void resetConfig(void); +static void readConfig(void); +static void activateConfig(void); // ************************************************************ // configBuffer handling // ************************************************************ // reads the EEPROM until NUL terminator and returns the number of characters incl. terminator, starting from given address -bool readConfigLength() +bool readConfigLength(void) { char temp = 0; uint16_t addreeprom = MEM_OFFSET_CONFIG; uint16_t length = MFeeprom.get_length(); - configLength = 0; + config.length = 0; do { temp = MFeeprom.read_char(addreeprom++); - configLength++; + config.length++; if (addreeprom > length) // abort if EEPROM size will be exceeded { cmdMessenger.sendCmd(kStatus, F("Loading config failed")); // text or "-1" like config upload? return false; } } while (temp != 0x00); // reads until NULL - configLength--; + config.length--; return true; } -void loadConfig() +void loadConfig(void) { #ifdef DEBUG2CMDMESSENGER cmdMessenger.sendCmd(kStatus, F("Load config")); #endif if (readConfigLength()) { readConfig(); - _activateConfig(); + activateConfig(); } } -void OnSetConfig() +void OnSetConfig(void) { #ifdef DEBUG2CMDMESSENGER cmdMessenger.sendCmd(kStatus, F("Setting config start")); @@ -110,18 +118,19 @@ void OnSetConfig() char *cfg = cmdMessenger.readStringArg(); uint8_t cfgLen = strlen(cfg); - if (configLength + cfgLen + 1 < MEM_LEN_CONFIG) { - MFeeprom.write_block(MEM_OFFSET_CONFIG + configLength, cfg, cfgLen + 1); // save the received config string including the terminatung NULL (+1) to EEPROM - configLength += cfgLen; - cmdMessenger.sendCmd(kStatus, configLength); - } else + if (config.length + cfgLen + 1 < MEM_LEN_CONFIG) { + MFeeprom.write_block(MEM_OFFSET_CONFIG + config.length, cfg, cfgLen + 1); // save the received config string including the terminatung NULL (+1) to EEPROM + config.length += cfgLen; + cmdMessenger.sendCmd(kStatus, config.length); + } else { cmdMessenger.sendCmd(kStatus, -1); // last successfull saving block is already NULL terminated, nothing more todo + } #ifdef DEBUG2CMDMESSENGER cmdMessenger.sendCmd(kStatus, F("Setting config end")); #endif } -void resetConfig() +void resetConfig(void) { Button::Clear(); Encoder::Clear(); @@ -146,9 +155,13 @@ void resetConfig() #endif #if MF_INPUT_SHIFTER_SUPPORT == 1 InputShifter::Clear(); - configLength = 0; - configActivated = false; #endif +#if MF_DIGIN_MUX_SUPPORT == 1 + DigInMux::Clear(); +#endif + + config.length = 0; + config.activated = false; } void OnResetConfig() @@ -157,195 +170,198 @@ void OnResetConfig() cmdMessenger.sendCmd(kStatus, F("OK")); } -void OnSaveConfig() +void OnSaveConfig(void) { cmdMessenger.sendCmd(kConfigSaved, F("OK")); } -void OnActivateConfig() +void OnActivateConfig(void) { readConfig(); - _activateConfig(); + activateConfig(); } -void _activateConfig() +void activateConfig() { - configActivated = true; + config.activated = true; cmdMessenger.sendCmd(kConfigActivated, F("OK")); } -// reads an ascii value which is '.' terminated from EEPROM and returns it's value -uint8_t readUintFromEEPROM(volatile uint16_t *addreeprom) +void resetEEPROMpointer(uint16_t pos = 0) { - char params[4] = {0}; // max 3 (255) digits NULL terminated + MFeeprom.setPosition(pos); +} + +// Read from EEPROM a '.' terminated ASCII number and return its value +uint8_t readUintFromEEPROM(void) +{ + char params[4] = {0}; // max 3 (255) digits, NUL terminated uint8_t counter = 0; do { - params[counter++] = MFeeprom.read_char((*addreeprom)++); // read character from eeprom and locate next buffer and eeprom location + params[counter++] = MFeeprom.read_char(); // read character from eeprom and locate next buffer and eeprom location } while (params[counter - 1] != '.' && counter < sizeof(params)); // reads until limiter '.' and for safety reason not more then size of params[] params[counter - 1] = 0x00; // replace '.' by NULL to terminate the string return atoi(params); } -// reads a string from EEPROM at given address which is ':' terminated and saves it in the nameBuffer -// once the nameBuffer is not needed anymore, just read until the ":" termination -> see function below -bool readNameFromEEPROM(uint16_t *addreeprom, char *buffer, uint16_t *addrbuffer) +// Reads the ':' terminated command tail from EEPROM at given address +// If a buffer is specified, the caracters read are stored there and terminated by NUL. +// Note: +// when the nameBuffer will be no longer needed, just remove the parts +// under the "if(dest)" clauses (and return "true") + +bool readRecordTailFromEEPROM(char **dest = NULL, char *cap = (char *)0xFFFF) { char temp = 0; do { - temp = MFeeprom.read_char((*addreeprom)++); // read the first character - buffer[(*addrbuffer)++] = temp; // save character and locate next buffer position - if (*addrbuffer >= MEMLEN_NAMES_BUFFER) { // nameBuffer will be exceeded - return false; // abort copying from EEPROM to nameBuffer + temp = MFeeprom.read_char(); // read the first character + if (dest) { + *((*dest)++) = temp; + // TODO: this function should be buffer-agnostic - + // the specific constant for buffer size should not be hardcoded! + if (*dest >= cap) break; // nameBuffer full: stop copying } - } while (temp != ':'); // reads until limiter ':' and locates the next free buffer position - buffer[(*addrbuffer) - 1] = 0x00; // replace ':' by NULL, terminates the string - return true; -} - -// reads the EEPRROM until end of command which ':' terminated -bool readEndCommandFromEEPROM(uint16_t *addreeprom) -{ - char temp = 0; - uint16_t length = MFeeprom.get_length(); - do { - temp = MFeeprom.read_char((*addreeprom)++); - if (*addreeprom > length) // abort if EEPROM size will be exceeded - return false; - } while (temp != ':'); // reads until limiter ':' - return true; + } while (temp != ':'); // reads until limiter ':' and locates the next free buffer position + if (dest) { + *((*dest)--) = 0x00; // replace ':' by NULL, terminates the string + } + return (*dest >= cap); } -void readConfig() +void readConfig(void) { - if (configLength == 0) // do nothing if no config is available + if (config.length == 0) // do nothing if no config is available return; - uint16_t addreeprom = MEM_OFFSET_CONFIG; // define first memory location where config is saved in EEPROM - uint16_t addrbuffer = 0; // and start with first memory location from nameBuffer - char params[6] = ""; - char command = readUintFromEEPROM(&addreeprom); // read the first value from EEPROM, it's a device definition - bool copy_success = true; // will be set to false if copying input names to nameBuffer exceeds array dimensions - // not required anymore when pins instead of names are transferred to the UI - - if (command == 0) // just to be sure, configLength should also be 0 + char *nameBufPtr = config.nameBuffer; // Current position in name buffer + char *topOfBuf = config.nameBuffer + MEMLEN_NAMES_BUFFER; // Max available position (+ 1) in name buffer + char params[6] = ""; + char devCode = 0; // Code for device type + bool copySuccess = true; // will be set to false if copying input names to nameBuffer exceeds array dimensions + // (no longer required when pins will be transferred to the UI instead of names) + + resetEEPROMpointer(MEM_OFFSET_CONFIG); // Start from first memory location where config is saved in EEPROM + + devCode = readUintFromEEPROM(); // read the first value from EEPROM, it's a device definition + if (devCode == 0) // just to be sure, config.length should also be 0 return; do // go through the EEPROM until it is NULL terminated { - switch (command) { + switch (devCode) { case kTypeButton: - params[0] = readUintFromEEPROM(&addreeprom); // Pin number - Button::Add(params[0], &nameBuffer[addrbuffer]); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name - copy_success = readNameFromEEPROM(&addreeprom, nameBuffer, &addrbuffer); // copy the NULL terminated name to nameBuffer and set to next free memory location + params[0] = readUintFromEEPROM(); // Pin + Button::Add(params[0], nameBufPtr); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name + copySuccess = readRecordTailFromEEPROM(&nameBufPtr, topOfBuf); // copy the NUL terminated name to to nameBuffer and set to next free memory location break; case kTypeOutput: - params[0] = readUintFromEEPROM(&addreeprom); // Pin number + params[0] = readUintFromEEPROM(); // Pin Output::Add(params[0]); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name break; #if MF_SEGMENT_SUPPORT == 1 case kTypeLedSegment: - params[0] = readUintFromEEPROM(&addreeprom); // Pin Data number - params[1] = readUintFromEEPROM(&addreeprom); // Pin CS number - params[2] = readUintFromEEPROM(&addreeprom); // Pin CLK number - params[3] = readUintFromEEPROM(&addreeprom); // brightness - params[4] = readUintFromEEPROM(&addreeprom); // number of modules + params[0] = readUintFromEEPROM(); // Pin Data + params[1] = readUintFromEEPROM(); // Pin CS + params[2] = readUintFromEEPROM(); // Pin CLK + params[3] = readUintFromEEPROM(); // brightness + params[4] = readUintFromEEPROM(); // numModules LedSegment::Add(params[0], params[1], params[2], params[4], params[3]); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name break; #endif #if MF_STEPPER_SUPPORT == 1 case kTypeStepperDeprecated: // this is for backwards compatibility - params[0] = readUintFromEEPROM(&addreeprom); // Pin1 number - params[1] = readUintFromEEPROM(&addreeprom); // Pin2 number - params[2] = readUintFromEEPROM(&addreeprom); // Pin3 number - params[3] = readUintFromEEPROM(&addreeprom); // Pin4 number - params[4] = readUintFromEEPROM(&addreeprom); // Button number + params[0] = readUintFromEEPROM(); // Pin1 + params[1] = readUintFromEEPROM(); // Pin2 + params[2] = readUintFromEEPROM(); // Pin3 + params[3] = readUintFromEEPROM(); // Pin4 + params[4] = readUintFromEEPROM(); // Button Stepper::Add(params[0], params[1], params[2], params[3], 0); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name break; #endif #if MF_STEPPER_SUPPORT == 1 case kTypeStepper: - params[0] = readUintFromEEPROM(&addreeprom); // Pin1 number - params[1] = readUintFromEEPROM(&addreeprom); // Pin2 number - params[2] = readUintFromEEPROM(&addreeprom); // Pin3 number - params[3] = readUintFromEEPROM(&addreeprom); // Pin4 number - params[4] = readUintFromEEPROM(&addreeprom); // Button number + params[0] = readUintFromEEPROM(); // Pin1 + params[1] = readUintFromEEPROM(); // Pin2 + params[2] = readUintFromEEPROM(); // Pin3 + params[3] = readUintFromEEPROM(); // Pin4 + params[4] = readUintFromEEPROM(); // Button Stepper::Add(params[0], params[1], params[2], params[3], params[4]); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name break; #endif #if MF_SERVO_SUPPORT == 1 case kTypeServo: - params[0] = readUintFromEEPROM(&addreeprom); // Pin number + params[0] = readUintFromEEPROM(); // Pin Servos::Add(params[0]); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name break; #endif case kTypeEncoderSingleDetent: - params[0] = readUintFromEEPROM(&addreeprom); // Pin1 number - params[1] = readUintFromEEPROM(&addreeprom); // Pin2 number - Encoder::Add(params[0], params[1], 0, &nameBuffer[addrbuffer]); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name - copy_success = readNameFromEEPROM(&addreeprom, nameBuffer, &addrbuffer); // copy the NULL terminated name to nameBuffer and set to next free memory location - // copy_success = readEndCommandFromEEPROM(&addreeprom); // once the nameBuffer is not required anymore uncomment this line and delete the line before + params[0] = readUintFromEEPROM(); // Pin1 + params[1] = readUintFromEEPROM(); // Pin2 + Encoder::Add(params[0], params[1], 0, nameBufPtr); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name + copySuccess = readRecordTailFromEEPROM(&nameBufPtr, topOfBuf); break; case kTypeEncoder: - params[0] = readUintFromEEPROM(&addreeprom); // Pin1 number - params[1] = readUintFromEEPROM(&addreeprom); // Pin2 number - params[2] = readUintFromEEPROM(&addreeprom); // type - Encoder::Add(params[0], params[1], params[2], &nameBuffer[addrbuffer]); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name - copy_success = readNameFromEEPROM(&addreeprom, nameBuffer, &addrbuffer); // copy the NULL terminated name to to nameBuffer and set to next free memory location - // copy_success = readEndCommandFromEEPROM(&addreeprom); // once the nameBuffer is not required anymore uncomment this line and delete the line before + params[0] = readUintFromEEPROM(); // Pin1 + params[1] = readUintFromEEPROM(); // Pin2 + params[2] = readUintFromEEPROM(); // type + Encoder::Add(params[0], params[1], params[2], nameBufPtr); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name + copySuccess = readRecordTailFromEEPROM(&nameBufPtr, topOfBuf); // copy the NULL terminated name to to nameBuffer and set to next free memory location + // copy the NULL terminated name to to nameBuffer and set to next free memory location + // copySuccess = readEndCommandFromEEPROM(); // once the nameBuffer is not required anymore uncomment this line and delete the line before break; #if MF_LCD_SUPPORT == 1 case kTypeLcdDisplayI2C: - params[0] = readUintFromEEPROM(&addreeprom); // address - params[1] = readUintFromEEPROM(&addreeprom); // columns - params[2] = readUintFromEEPROM(&addreeprom); // lines + params[0] = readUintFromEEPROM(); // address + params[1] = readUintFromEEPROM(); // columns + params[2] = readUintFromEEPROM(); // lines LCDDisplay::Add(params[0], params[1], params[2]); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); break; #endif #if MF_ANALOG_SUPPORT == 1 case kTypeAnalogInput: - params[0] = readUintFromEEPROM(&addreeprom); // pin number - params[1] = readUintFromEEPROM(&addreeprom); // sensitivity - Analog::Add(params[0], &nameBuffer[addrbuffer], params[1]); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name - copy_success = readNameFromEEPROM(&addreeprom, nameBuffer, &addrbuffer); // copy the NULL terminated name to to nameBuffer and set to next free memory location - // copy_success = readEndCommandFromEEPROM(&addreeprom); // once the nameBuffer is not required anymore uncomment this line and delete the line before + params[0] = readUintFromEEPROM(); // pin + params[1] = readUintFromEEPROM(); // sensitivity + Analog::Add(params[0], nameBufPtr, params[1]); // MUST be before readNameFromEEPROM because readNameFromEEPROM returns the pointer for the NEXT Name + copySuccess = readRecordTailFromEEPROM(&nameBufPtr, topOfBuf); // copy the NULL terminated name to to nameBuffer and set to next free memory location + // copySuccess = readEndCommandFromEEPROM(); // once the nameBuffer is not required anymore uncomment this line and delete the line before break; #endif #if MF_OUTPUT_SHIFTER_SUPPORT == 1 case kTypeOutputShifter: - params[0] = readUintFromEEPROM(&addreeprom); // latch Pin - params[1] = readUintFromEEPROM(&addreeprom); // clock Pin - params[2] = readUintFromEEPROM(&addreeprom); // data Pin - params[3] = readUintFromEEPROM(&addreeprom); // number of daisy chained modules + params[0] = readUintFromEEPROM(); // latch Pin + params[1] = readUintFromEEPROM(); // clock Pin + params[2] = readUintFromEEPROM(); // data Pin + params[3] = readUintFromEEPROM(); // number of daisy chained modules + // TODO check: OutputShifter::Add(params[0], params[1], params[2], params[3], nameBufPtr); OutputShifter::Add(params[0], params[1], params[2], params[3]); - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name break; #endif #if MF_INPUT_SHIFTER_SUPPORT == 1 case kTypeInputShifter: - params[0] = readUintFromEEPROM(&addreeprom); // latch Pin - params[1] = readUintFromEEPROM(&addreeprom); // clock Pin - params[2] = readUintFromEEPROM(&addreeprom); // data Pin - params[3] = readUintFromEEPROM(&addreeprom); // number of daisy chained modules - InputShifter::Add(params[0], params[1], params[2], params[3], &nameBuffer[addrbuffer]); - copy_success = readNameFromEEPROM(&addreeprom, nameBuffer, &addrbuffer); // copy the NULL terminated name to to nameBuffer and set to next free memory location - // copy_success = readEndCommandFromEEPROM(&addreeprom); // once the nameBuffer is not required anymore uncomment this line and delete the line before + params[0] = readUintFromEEPROM(); // latch Pin + params[1] = readUintFromEEPROM(); // clock Pin + params[2] = readUintFromEEPROM(); // data Pin + params[3] = readUintFromEEPROM(); // number of daisy chained modules + InputShifter::Add(params[0], params[1], params[2], params[3], nameBufPtr); + copySuccess = readRecordTailFromEEPROM(&nameBufPtr, topOfBuf); // copy the NULL terminated name to to nameBuffer and set to next free memory location break; #endif @@ -362,41 +378,44 @@ void readConfig() #if MF_DIGIN_MUX_SUPPORT == 1 case kTypeDigInMux: - params[0] = readUintFromEEPROM(&addreeprom); // data pin + params[0] = readUintFromEEPROM(); // data pin // Mux driver section // Repeated commands do not define more objects, but change the only existing one - // therefore beware that all DigInMux configuration commands are consistent! - params[1] = readUintFromEEPROM(&addreeprom); // Sel0 pin - params[2] = readUintFromEEPROM(&addreeprom); // Sel1 pin - params[3] = readUintFromEEPROM(&addreeprom); // Sel2 pin - params[4] = readUintFromEEPROM(&addreeprom); // Sel3 pin + params[1] = readUintFromEEPROM(); // Sel0 pin + params[2] = readUintFromEEPROM(); // Sel1 pin + params[3] = readUintFromEEPROM(); // Sel2 pin + params[4] = readUintFromEEPROM(); // Sel3 pin MUX.attach(params[1], params[2], params[3], params[4]); - params[5] = readUintFromEEPROM(&addreeprom); // 8-bit registers (1-2) - DigInMux::Add(params[0], params[5], &nameBuffer[addrbuffer]); - copy_success = readNameFromEEPROM(&addreeprom, nameBuffer, &addrbuffer); - - // cmdMessenger.sendCmd(kDebug, F("Mux loaded")); + params[5] = readUintFromEEPROM(); // 8-bit registers (1-2) + DigInMux::Add(params[0], params[5], nameBufPtr); + copySuccess = readRecordTailFromEEPROM(&nameBufPtr, topOfBuf); break; #endif default: - copy_success = readEndCommandFromEEPROM(&addreeprom); // check EEPROM until end of name + // read to the end of the current command which is apparently not understood + copySuccess = readRecordTailFromEEPROM(); // check EEPROM until end of name } - command = readUintFromEEPROM(&addreeprom); - } while (command && copy_success); - if (!copy_success) { // too much/long names for input devices - nameBuffer[MEMLEN_NAMES_BUFFER - 1] = 0x00; // terminate the last copied (part of) string with 0x00 - cmdMessenger.sendCmd(kStatus, F("Failure on reading config")); - } + + devCode = readUintFromEEPROM(); // Read dev code of next entry + + } while (devCode && copySuccess); + + // No longer required: now readRecordTailFromEEPROM() always terminates last string regardless if valid + // if (!copySuccess) { // too much/long names for input devices + // *(--nameBufPtr) = 0x00; + // // Was: + // // config.nameBuffer[MEMLEN_NAME_BUFFER - 1] = 0x00; // terminate the last copied (part of) string with 0x00 + // } } void OnGetConfig() { setLastCommandMillis(); cmdMessenger.sendCmdStart(kInfo); - if (configLength > 0) { + if (config.length > 0) { cmdMessenger.sendCmdArg(MFeeprom.read_char(MEM_OFFSET_CONFIG)); - for (uint16_t i = 1; i < configLength; i++) { + for (uint16_t i = 1; i < config.length; i++) { cmdMessenger.sendArg(MFeeprom.read_char(MEM_OFFSET_CONFIG + i)); } } @@ -407,16 +426,16 @@ void OnGetInfo() { setLastCommandMillis(); cmdMessenger.sendCmdStart(kInfo); - cmdMessenger.sendCmdArg(type); - cmdMessenger.sendCmdArg(name); - cmdMessenger.sendCmdArg(serial); + cmdMessenger.sendCmdArg(config.type); + cmdMessenger.sendCmdArg(config.name); + cmdMessenger.sendCmdArg(config.serial); cmdMessenger.sendCmdArg(VERSION); cmdMessenger.sendCmdEnd(); } bool getStatusConfig() { - return configActivated; + return config.activated; } // ************************************************************ @@ -424,13 +443,13 @@ bool getStatusConfig() // ************************************************************ void generateSerial(bool force) { - MFeeprom.read_block(MEM_OFFSET_SERIAL, serial, MEM_LEN_SERIAL); - if (!force && serial[0] == 'S' && serial[1] == 'N') + MFeeprom.read_block(MEM_OFFSET_SERIAL, config.serial, MEM_LEN_SERIAL); + if (!force && config.serial[0] == 'S' && config.serial[1] == 'N') return; randomSeed(analogRead(RANDOM_SEED_INPUT)); - sprintf(serial, "SN-%03x-", (unsigned int)random(4095)); - sprintf(&serial[7], "%03x", (unsigned int)random(4095)); - MFeeprom.write_block(MEM_OFFSET_SERIAL, serial, MEM_LEN_SERIAL); + sprintf(config.serial, "SN-%03x-", (unsigned int)random(4095)); + sprintf(&config.serial[7], "%03x", (unsigned int)random(4095)); + MFeeprom.write_block(MEM_OFFSET_SERIAL, config.serial, MEM_LEN_SERIAL); if (!force) { MFeeprom.write_byte(MEM_OFFSET_CONFIG, 0x00); // First byte of config to 0x00 to ensure to start 1st time with empty config, but not if forced from the connector to generate a new one } @@ -439,7 +458,7 @@ void generateSerial(bool force) void OnGenNewSerial() { generateSerial(true); - cmdMessenger.sendCmd(kInfo, serial); + cmdMessenger.sendCmd(kInfo, config.serial); } // ************************************************************ @@ -449,7 +468,7 @@ void storeName() { char prefix[] = "#"; MFeeprom.write_block(MEM_OFFSET_NAME, prefix, 1); - MFeeprom.write_block(MEM_OFFSET_NAME + 1, name, MEM_LEN_NAME - 1); + MFeeprom.write_block(MEM_OFFSET_NAME + 1, config.name, MEM_LEN_NAME - 1); } void restoreName() @@ -459,15 +478,15 @@ void restoreName() if (testHasName[0] != '#') return; - MFeeprom.read_block(MEM_OFFSET_NAME + 1, name, MEM_LEN_NAME - 1); + MFeeprom.read_block(MEM_OFFSET_NAME + 1, config.name, MEM_LEN_NAME - 1); } void OnSetName() { char *cfg = cmdMessenger.readStringArg(); - memcpy(name, cfg, MEM_LEN_NAME); + memcpy(config.name, cfg, MEM_LEN_NAME); storeName(); - cmdMessenger.sendCmd(kStatus, name); + cmdMessenger.sendCmd(kStatus, config.name); } // config.cpp diff --git a/src/MF_Modules/MFEEPROM.cpp b/src/MF_Modules/MFEEPROM.cpp index 120b1f03..e56e2f3e 100644 --- a/src/MF_Modules/MFEEPROM.cpp +++ b/src/MF_Modules/MFEEPROM.cpp @@ -4,45 +4,55 @@ // (C) MobiFlight Project 2022 // +// WARNING: This code is based on Arduino / Atmel AVR libraries (although itself not AVR-specific). +// When compiling for other platforms, check that the available libraries are compatible. + #include #include "MFEEPROM.h" #include -MFEEPROM::MFEEPROM() -{ - eepromLength = EEPROM.length(); -} +MFEEPROM::MFEEPROM() {} uint16_t MFEEPROM::get_length(void) { - return eepromLength; + return EEPROM.length(); } -void MFEEPROM::read_block(uint16_t adr, char data[], uint16_t len) +bool MFEEPROM::read_block(uint16_t adr, char data[], uint16_t len) { - for (uint16_t i = 0; i < len; i++) { - data[i] = read_char(adr + i); + // If length is exceeded, return only the legitimate part + for (uint16_t i = 0; i < len && adr < EEPROM.length(); i++, adr++) { + data[i] = read_char(adr); } + return (adr < EEPROM.length()); } -void MFEEPROM::write_block(uint16_t adr, char data[], uint16_t len) +bool MFEEPROM::write_block(uint16_t adr, char data[], uint16_t len) { - if (adr + len >= eepromLength) return; - for (uint16_t i = 0; i < len; i++) { - EEPROM.put(adr + i, data[i]); + // If length is exceeded, do not write anything + if (adr + len >= EEPROM.length()) return false; + for (uint16_t i = 0; i < len; i++, adr++) { + EEPROM.put(adr, data[i]); } + return true; } char MFEEPROM::read_char(uint16_t adr) { - if (adr >= eepromLength) return 0; + if (adr >= EEPROM.length()) return 0; return EEPROM.read(adr); } -void MFEEPROM::write_byte(uint16_t adr, char data) +bool MFEEPROM::write_byte(uint16_t adr, char data) { - if (adr >= eepromLength) return; + if (adr >= EEPROM.length()) return false; EEPROM.put(adr, data); + return true; } -// MFEEPROM.cpp \ No newline at end of file +bool MFEEPROM::setPosition(uint16_t pos) +{ + return ((pos < EEPROM.length()) ? ((_pos = pos), true) : false); +} + +// MFEEPROM.cpp diff --git a/src/MF_Modules/MFEEPROM.h b/src/MF_Modules/MFEEPROM.h index b3d8193b..068731c0 100644 --- a/src/MF_Modules/MFEEPROM.h +++ b/src/MF_Modules/MFEEPROM.h @@ -14,13 +14,20 @@ class MFEEPROM public: MFEEPROM(); uint16_t get_length(void); - void read_block(uint16_t addr, char data[], uint16_t len); - void write_block(uint16_t addr, char data[], uint16_t len); + bool read_block(uint16_t addr, char data[], uint16_t len); + bool write_block(uint16_t addr, char data[], uint16_t len); char read_char(uint16_t adr); - void write_byte(uint16_t adr, char data); + bool write_byte(uint16_t adr, char data); + + bool read_block(char data[], uint16_t len) { return (read_block(_pos, data, len) ? (_pos += len, true) : false); } + bool write_block(char data[], uint16_t len) { return (write_block(_pos, data, len) ? (_pos += len, true) : false); } + char read_char(void) { return (read_char(_pos) ? (_pos++, true) : false); } + bool write_byte(char data) { return (write_byte(_pos, data) ? (_pos++, true) : false); } + + bool setPosition(uint16_t pos = 0); private: - uint16_t eepromLength = 0; + uint16_t _pos; }; // MFEEPROM.h \ No newline at end of file From 623173ef1ba448eff554dbfcfed8aab6adc5d399 Mon Sep 17 00:00:00 2001 From: GioCC <25667790+GioCC@users.noreply.github.com> Date: Mon, 25 Apr 2022 21:22:17 +0200 Subject: [PATCH 2/2] Fix: FW did not read back board name from EEPROM at startup --- src/config.h | 1 + src/mobiflight.cpp | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/config.h b/src/config.h index cb70dd20..3db642b9 100644 --- a/src/config.h +++ b/src/config.h @@ -25,6 +25,7 @@ enum { }; void loadConfig(void); +void restoreName(void); bool getStatusConfig(void); void generateSerial(bool); void OnSetConfig(void); diff --git a/src/mobiflight.cpp b/src/mobiflight.cpp index 45352545..9516a527 100644 --- a/src/mobiflight.cpp +++ b/src/mobiflight.cpp @@ -69,7 +69,7 @@ typedef struct { lastUpdate_t lastUpdate; -void initPollIntervals(void) +void initPollIntervals(void) { // Init Time Gap between Inputs, do not read at the same loop lastUpdate.Buttons = millis(); @@ -135,6 +135,7 @@ void ResetBoard() { generateSerial(false); setLastCommandMillis(); + restoreName(); loadConfig(); }