Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credentials not saved or loaded correctly #388

Closed
bramhut opened this issue Aug 5, 2021 · 5 comments
Closed

Credentials not saved or loaded correctly #388

bramhut opened this issue Aug 5, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@bramhut
Copy link

bramhut commented Aug 5, 2021

Hello,
There seem to be issues with saving, deleting or loading the credentials. It appears that the data is loaded or saved at the wrong location in the EEPROM or the data is corrupted. I've expanded your code in issue #307 to also save and delete credentials via serial.

#include <Arduino.h>
#include <ctype.h>
#include <ESP8266WiFi.h>
#include <ESP8266WebServer.h>
#include <AutoConnect.h>

#define DUMP_WIDTH_COLUMNS    8
#define DUMP_WIDTH_REMAIN(x)  (x & (DUMP_WIDTH_COLUMNS - 1))

AutoConnectCredential credit;

void printCredits(){
  Serial.printf("Credential entries:%d\n", credit.entries());
  for (uint8_t e = 0 ; e < credit.entries(); e++) {
    Serial.printf("\nentry #%d:", e);
    station_config_t  entry;
    if (credit.load(e, &entry)) {
      size_t  p = 0;
      while (p < sizeof(station_config_t)) {
        if (!DUMP_WIDTH_REMAIN(p))
          Serial.printf("\n%04x  ", p);
        Serial.printf("%02x ", (uint8_t)*(reinterpret_cast<uint8_t*>(&entry) + p++));
        if (!DUMP_WIDTH_REMAIN(p) || (p >= sizeof(station_config_t))) {
          int sp = DUMP_WIDTH_REMAIN(p) ? 3 * DUMP_WIDTH_REMAIN(p) + 1 : 1;
          Serial.printf("%*c", sp, ' ');
          int w = DUMP_WIDTH_COLUMNS - DUMP_WIDTH_REMAIN(p);
          for (uint8_t cp = p - w; w; cp++, w--)
            Serial.printf("%c", isprint((int)*(reinterpret_cast<uint8_t*>(&entry) + cp)) ? (char)*(reinterpret_cast<uint8_t*>(&entry) + cp) : '.');
        }
      }
      Serial.println();
    }
    else {
      Serial.printf(" loading failed\n");
    }
  }
}

void saveCredit(){
  station_config_t config;
  strcpy( (char*) config.ssid, "TestSSID " );
  char randomChar = (char) random(97,122);
  strncat( (char*) config.ssid, &randomChar, 1);
  strcpy( (char*) config.password , "Password" );
  config.bssid[0] = 0x00;
  config.bssid[1] = 0x01;
  config.bssid[2] = 0x02;
  config.bssid[3] = 0x03;
  config.bssid[4] = 0x04;
  config.bssid[5] = random(0xff);
  config.dhcp = 0;

  credit.save(&config);
}

void delCredit(uint16_t entry = 0){
  if (credit.entries() > 0){
    station_config_t config;
    credit.load(entry, &config);
    credit.del((const char*)&config.ssid[0]);
  }
}

void setup() {
  randomSeed(analogRead(A0));
  delay(1000);
  Serial.begin(115200);
  Serial.println();
  printCredits();
}

void loop() {
  if (Serial.available()){
    String command = Serial.readStringUntil('\n');
    String argument;
    int8_t argumentIndex = command.lastIndexOf(' ');
    if (argumentIndex > 0){
      argument = command.substring(argumentIndex);
      command.remove(argumentIndex);
    }
    if (command.equalsIgnoreCase("Print")){
      printCredits();
    }
    else if (command.equalsIgnoreCase("Save")){
      saveCredit();
      printCredits();
    }
    else if (command.equalsIgnoreCase("Delete") || command.equalsIgnoreCase("Del")){
      if (argument.toInt() >= 0){
        delCredit(argument.toInt());
      }else{
        delCredit();
      }
      printCredits();
    }
  }
}

When saving, or deleting a credential with partly random SSID and BSSID, sometimes the data is corrupted (as seen in the pictures).

image

Another example, which also shows a shift in some data (SSID).
image

I'm using an ESP8266 with ESP8266 Arduino framework 3.1.0 using PlatformIO. I've tried using multiple ESP8266's and different versions of AutoConnect (v1.2.2, v1.2.0, v130) but the issue persists.
When using EEPROM.put() and EEPROM.get() in my own sketch, no issues were found.

Thanks in advance.

@bramhut
Copy link
Author

bramhut commented Aug 10, 2021

After some more debugging I found that AutoConnectCredential::del() doesn't delete the flag for DHCP or Static IP. When using DHCP the byte is set to 0x00. After the first entry is deleted, the SSID of the second entry is preluded with 0x00. Because the del() function looks for the first 0x00 when deleting the SSID or password, the second entry SSID does not get deleted as it immediately encounters 0x00. This means that the while loop for deleting the password actually deletes the SSID. This bug results in the entry not being fully deleted. I solved the issue by replacing

// Erase ip configuration extention
if (_eeprom->read(_dp) == STA_STATIC) {
  for (uint8_t i = 0; i < sizeof(station_config_t::_config); i++)
    _eeprom->write(_dp++, 0xff);
}

with

if (_eeprom->read(_dp) == STA_STATIC) {
  for (uint8_t i = 0; i < sizeof(station_config_t::_config); i++)
    _eeprom->write(_dp++, 0xff);
}
_eeprom->write(_dp++, 0xff);

In AutoConnectCredential.cpp

@Hieromon
Copy link
Owner

Hieromon commented Aug 24, 2021

It is incorrect instantiation of AutoConnectCredentail. I'm sure, I have mentioned this note somewhere in the past, but I forgot where I wrote it.
I understand your desire to declare AutoConnectCredential globally. But, AutoConnctCredential for ESP8266 starts EEPROM according to the area size currently used in the constructor. Therefore, the area that has increased since then cannot be determined by AutoConnectCredential. In order for the credential to be accessed correctly, it must be instantiated within the function locally that accesses the AutoConnectCredential.

This topic is a known limitation and will be closed.

@bramhut
Copy link
Author

bramhut commented Aug 29, 2021

Sorry to insist but the issue is still present when declaring the AutoConnectCredential object locally in each function. I have reperformed the test and the results are the same as before. Although declaring the object globally can certainly cause issues, I still think the issue in this case is in AutoConnectCredential::del(). Could you have a closer look at it? Thanks!

Hieromon added a commit that referenced this issue Aug 30, 2021
@Hieromon
Copy link
Owner

@bramhut I was able to reproduce the issue.
You are right. AutoConnectCredential::del has incompletely deleted entry.
I have staged the patch to the brach enhance/v130. Can you evaluate it?
If the issue does not occur in your environment, it will be merged to release v130.
Thank you for your contribution.

@Hieromon Hieromon reopened this Aug 30, 2021
@Hieromon Hieromon added the bug Something isn't working label Aug 30, 2021
@bramhut
Copy link
Author

bramhut commented Aug 30, 2021

Thank you for the solution, it is working as expected now.

@bramhut bramhut closed this as completed Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants