fix: post-review issues + close M2/M3/M4 from internal backlog (v1.9.9)
From Sonnet pre-release code review (3 real issues): - F1 (high): factory reset atomicity — save cleared creds BEFORE zeroing reset counter, so a power loss between commits still results in WiFiManager AP boot instead of inconsistent "counter=0 + stale creds" state - F2: isValidSSID now allows single-char SSIDs (per 802.11 spec) - F3: ntp_interval changes now trigger needsRestart (NTPClient constructed once in setup() with this value, doesn't pick up runtime changes) From internal backlog: - M2: setupWiFi Try-2 now supports open networks (no password) — fixed the &&-condition that required both ssid and password - M3: removed dead NTP_WAITING/NTP_SUCCESS/NTP_FAILED enum values - M4: randomSeed() with ESP.getChipId() ^ micros() — dissolve pattern varies Tested on hardware: 72/73 tests pass (single failure is expected — ntp_interval fuzz cases now trigger restart due to F3, second case lands in reboot window). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
f42ede5fa4
commit
259eda6663
@@ -5,6 +5,30 @@ All notable changes to this project will be documented in this file.
|
|||||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
|
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
|
||||||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||||
|
|
||||||
|
## [1.9.9] - 2026-05-19
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- **Factory reset atomicity**: cleared WiFi credentials are now committed BEFORE
|
||||||
|
the reset counter is zeroed. If power fails between the two writes, the device
|
||||||
|
still boots into WiFiManager AP on next start (cleared creds win over stale counter)
|
||||||
|
instead of being stuck in an inconsistent state
|
||||||
|
- **`isValidSSID` rejected single-char SSIDs**: length-1 networks like "A" were
|
||||||
|
incorrectly flagged as all-same garbage. Now allowed (per 802.11 spec)
|
||||||
|
- **`ntp_interval` config changes ignored until reboot**: `NTPClient` is constructed
|
||||||
|
in `setup()` with this value and never re-initialized. Now triggers `needsRestart`
|
||||||
|
when the interval actually changes
|
||||||
|
- **Open WiFi (no password) failed to connect at setup** (M2): Try-2 block in
|
||||||
|
`setupWiFi()` required both ssid and password; now falls back to `WiFi.begin(ssid)`
|
||||||
|
when password is empty
|
||||||
|
- **Unseeded PRNG, identical dissolve pattern every boot** (M4): `randomSeed()`
|
||||||
|
now called with `ESP.getChipId() ^ micros()` — varies between devices and boots
|
||||||
|
|
||||||
|
### Removed
|
||||||
|
|
||||||
|
- **Dead `NTPState` enum values** (M3): `NTP_WAITING`, `NTP_SUCCESS`, `NTP_FAILED`
|
||||||
|
were defined but never assigned. Removed to reduce code surface area
|
||||||
|
|
||||||
## [1.9.8] - 2026-05-19
|
## [1.9.8] - 2026-05-19
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
@@ -9,7 +9,7 @@
|
|||||||
#include <Arduino.h>
|
#include <Arduino.h>
|
||||||
|
|
||||||
// Firmware version
|
// Firmware version
|
||||||
#define FIRMWARE_VERSION "1.9.8"
|
#define FIRMWARE_VERSION "1.9.9"
|
||||||
|
|
||||||
// OLED I2C Configuration
|
// OLED I2C Configuration
|
||||||
#define I2C_SDA 0 // GPIO0 (I2C Data) - SWAPPED!
|
#define I2C_SDA 0 // GPIO0 (I2C Data) - SWAPPED!
|
||||||
@@ -120,13 +120,11 @@ enum WeatherState {
|
|||||||
WEATHER_FAILED
|
WEATHER_FAILED
|
||||||
};
|
};
|
||||||
|
|
||||||
// Async NTP state machine
|
// Async NTP state machine — only IDLE and REQUEST_SENT are used
|
||||||
|
// (response handler transitions back to IDLE directly on success or timeout)
|
||||||
enum NTPState {
|
enum NTPState {
|
||||||
NTP_IDLE,
|
NTP_IDLE,
|
||||||
NTP_REQUEST_SENT,
|
NTP_REQUEST_SENT
|
||||||
NTP_WAITING,
|
|
||||||
NTP_SUCCESS,
|
|
||||||
NTP_FAILED
|
|
||||||
};
|
};
|
||||||
|
|
||||||
// Async WiFi state machine
|
// Async WiFi state machine
|
||||||
|
|||||||
@@ -133,16 +133,22 @@ void ICACHE_FLASH_ATTR checkFactoryReset() {
|
|||||||
|
|
||||||
if (rc.count >= RESET_COUNTER_TRIPS) {
|
if (rc.count >= RESET_COUNTER_TRIPS) {
|
||||||
Serial.println("!!! FACTORY RESET triggered !!!");
|
Serial.println("!!! FACTORY RESET triggered !!!");
|
||||||
|
|
||||||
|
// Atomicity: clear credentials FIRST (saveConfig commits), then zero counter.
|
||||||
|
// If power fails between the two commits, the device boots into AP mode next
|
||||||
|
// time (creds are already cleared) — instead of being in a "counter zeroed but
|
||||||
|
// creds still valid" inconsistent state.
|
||||||
|
memset(config.ssid, 0, sizeof(config.ssid));
|
||||||
|
memset(config.password, 0, sizeof(config.password));
|
||||||
|
EEPROM.end(); // close current handle before saveConfig opens its own
|
||||||
|
saveConfig(); // commits cleared credentials to flash
|
||||||
|
|
||||||
|
EEPROM.begin(512);
|
||||||
rc.count = 0;
|
rc.count = 0;
|
||||||
EEPROM.put(RESET_COUNTER_ADDR, rc);
|
EEPROM.put(RESET_COUNTER_ADDR, rc);
|
||||||
EEPROM.commit();
|
EEPROM.commit();
|
||||||
EEPROM.end();
|
EEPROM.end();
|
||||||
|
|
||||||
// Clear WiFi credentials only — keep other settings
|
|
||||||
memset(config.ssid, 0, sizeof(config.ssid));
|
|
||||||
memset(config.password, 0, sizeof(config.password));
|
|
||||||
saveConfig();
|
|
||||||
|
|
||||||
// Show reset screen
|
// Show reset screen
|
||||||
display.clearDisplay();
|
display.clearDisplay();
|
||||||
display.setTextColor(SSD1306_WHITE);
|
display.setTextColor(SSD1306_WHITE);
|
||||||
@@ -244,6 +250,11 @@ void ICACHE_FLASH_ATTR setupOTA() {
|
|||||||
void setup() {
|
void setup() {
|
||||||
Serial.begin(115200);
|
Serial.begin(115200);
|
||||||
delay(100);
|
delay(100);
|
||||||
|
|
||||||
|
// Seed PRNG with hardware entropy: chip ID is unique per device,
|
||||||
|
// micros() varies on each boot due to power-on timing jitter
|
||||||
|
randomSeed(ESP.getChipId() ^ micros());
|
||||||
|
|
||||||
Serial.println("\n\nTJ-56-654 NTP Clock with OTA v" FIRMWARE_VERSION);
|
Serial.println("\n\nTJ-56-654 NTP Clock with OTA v" FIRMWARE_VERSION);
|
||||||
Serial.println("==========================================");
|
Serial.println("==========================================");
|
||||||
Serial.println("Display: GM009605v4.3 OLED 128x64 (SSD1306 I2C)");
|
Serial.println("Display: GM009605v4.3 OLED 128x64 (SSD1306 I2C)");
|
||||||
|
|||||||
@@ -294,7 +294,8 @@ static bool isValidSSID(const String& s) {
|
|||||||
if (c < 0x20 || c > 0x7E) return false; // non-printable
|
if (c < 0x20 || c > 0x7E) return false; // non-printable
|
||||||
if (c != first) allSame = false;
|
if (c != first) allSame = false;
|
||||||
}
|
}
|
||||||
return !allSame; // reject "AAAAA...", "BBBBB...", etc.
|
// Single-char SSIDs ("A") are valid per 802.11. Only reject all-same for length>1.
|
||||||
|
return n == 1 || !allSame;
|
||||||
}
|
}
|
||||||
|
|
||||||
void ICACHE_FLASH_ATTR handleConfigSave() {
|
void ICACHE_FLASH_ATTR handleConfigSave() {
|
||||||
@@ -358,7 +359,9 @@ void ICACHE_FLASH_ATTR handleConfigSave() {
|
|||||||
}
|
}
|
||||||
if (server.hasArg("ntp_interval")) {
|
if (server.hasArg("ntp_interval")) {
|
||||||
long ni = server.arg("ntp_interval").toInt();
|
long ni = server.arg("ntp_interval").toInt();
|
||||||
config.ntp_interval = constrain(ni, 60L, 86400L);
|
unsigned long newInterval = constrain(ni, 60L, 86400L);
|
||||||
|
if (newInterval != config.ntp_interval) needsRestart = true; // NTPClient constructed at boot with this
|
||||||
|
config.ntp_interval = newInterval;
|
||||||
}
|
}
|
||||||
if (server.hasArg("ntp_server")) {
|
if (server.hasArg("ntp_server")) {
|
||||||
String n = server.arg("ntp_server");
|
String n = server.arg("ntp_server");
|
||||||
|
|||||||
@@ -105,10 +105,14 @@ void ICACHE_FLASH_ATTR setupWiFi() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Try 2: If we have EEPROM credentials, try those
|
// Try 2: If we have EEPROM credentials, try those (M2: support open networks)
|
||||||
if (strlen(config.ssid) > 0 && strlen(config.password) > 0) {
|
if (strlen(config.ssid) > 0) {
|
||||||
Serial.println("\nTrying EEPROM credentials...");
|
Serial.println("\nTrying EEPROM credentials...");
|
||||||
WiFi.begin(config.ssid, config.password);
|
if (strlen(config.password) > 0) {
|
||||||
|
WiFi.begin(config.ssid, config.password);
|
||||||
|
} else {
|
||||||
|
WiFi.begin(config.ssid); // open network — no password
|
||||||
|
}
|
||||||
|
|
||||||
int attempts = 0;
|
int attempts = 0;
|
||||||
while (WiFi.status() != WL_CONNECTED && attempts < 20) {
|
while (WiFi.status() != WL_CONNECTED && attempts < 20) {
|
||||||
|
|||||||
Reference in New Issue
Block a user