fix(rtc): address code-review findings (5 fixes)
- Drop extern "C" from weak hooks (UB with C++ reference param) - syncTimeWithNTP returns bool; syncFromNTP uses it (robust success check) - Avoid duplicate NTP sync at boot (wificlass already syncs) - Clamp negative time deltas in 24h timer and JSON status - Cache rtc.now() in loopRTC to avoid I2C race with PN532
This commit is contained in:
@@ -53,8 +53,14 @@ void setup() {
|
||||
|
||||
setupRTC(); // RTC zuerst, damit Systemzeit vor WiFi plausibel ist
|
||||
setupWifi(); // WiFi initialisieren
|
||||
if (WiFi.status() == WL_CONNECTED) {
|
||||
syncFromNTP();
|
||||
// wificlass.h hat intern bereits syncTimeWithNTP() versucht.
|
||||
// Falls die Systemzeit jetzt plausibel ist, in RTC persistieren und Bookkeeping setzen —
|
||||
// ohne einen zweiten NTP-Roundtrip zu provozieren.
|
||||
if (WiFi.status() == WL_CONNECTED && time(NULL) >= 1735689600L) {
|
||||
time_t nowEpoch = time(NULL);
|
||||
persistSystemTimeToRTC(nowEpoch);
|
||||
ntpEverSynced = true;
|
||||
lastNtpSyncEpoch = nowEpoch;
|
||||
lastStaConnected = true; // Edge bereits "konsumiert"
|
||||
}
|
||||
setupOTA(&server);
|
||||
|
||||
@@ -12,6 +12,7 @@ bool rtcAvailable = false;
|
||||
bool ntpEverSynced = false;
|
||||
time_t lastNtpSyncEpoch = 0;
|
||||
bool lastStaConnected = false;
|
||||
time_t cachedRtcEpoch = 0; // letzter rtc.now()-Wert, gelesen aus loopRTC (I2C-Race-Guard)
|
||||
|
||||
// I2C-Init, PCF8523-Detektion, und Systemzeit-Fallback aus RTC.
|
||||
// Soft-Fail: bei nicht gefundener Hardware bleibt rtcAvailable=false.
|
||||
@@ -39,6 +40,7 @@ void setupRTC() {
|
||||
tv.tv_sec = (time_t)rtcEpoch;
|
||||
tv.tv_usec = 0;
|
||||
settimeofday(&tv, NULL);
|
||||
cachedRtcEpoch = (time_t)rtcEpoch;
|
||||
Serial.printf("[RTC] Systemzeit aus RTC gesetzt: %lu (UTC)\n",
|
||||
(unsigned long)rtcEpoch);
|
||||
} else {
|
||||
@@ -65,7 +67,7 @@ void persistSystemTimeToRTC(time_t t) {
|
||||
|
||||
// Weak-Hook-Override aus timesync.h — wird automatisch aufgerufen,
|
||||
// sobald irgendwo setSystemTime() Erfolg meldet.
|
||||
extern "C" void onSystemTimeSet(time_t t) {
|
||||
void onSystemTimeSet(time_t t) {
|
||||
persistSystemTimeToRTC(t);
|
||||
}
|
||||
|
||||
@@ -73,20 +75,15 @@ extern "C" void onSystemTimeSet(time_t t) {
|
||||
// setzt ntpEverSynced=true, aktualisiert lastNtpSyncEpoch.
|
||||
// Returns true bei Erfolg.
|
||||
bool syncFromNTP() {
|
||||
// Snapshot vor dem Sync — wenn die Zeit hinterher >= 2025 und neuer als vorher,
|
||||
// werten wir den Sync als erfolgreich.
|
||||
time_t before = time(NULL);
|
||||
syncTimeWithNTP(); // benutzt Defaults aus timesync.h: pool.ntp.org, +1h, kein DST
|
||||
time_t after = time(NULL);
|
||||
|
||||
bool ok = ((uint32_t)after >= RTC_MIN_EPOCH) && (after >= before);
|
||||
if (!ok) {
|
||||
if (!syncTimeWithNTP()) {
|
||||
Serial.println("[RTC] NTP-Sync fehlgeschlagen — RTC unverändert");
|
||||
return false;
|
||||
}
|
||||
|
||||
// setSystemTime() wird intern von syncTimeWithNTP NICHT aufgerufen,
|
||||
// also den Weak-Hook-Pfad hier umgehen und direkt schreiben.
|
||||
time_t after = time(NULL);
|
||||
if ((uint32_t)after < RTC_MIN_EPOCH) {
|
||||
Serial.println("[RTC] NTP-Sync lieferte unplausible Zeit — RTC unverändert");
|
||||
return false;
|
||||
}
|
||||
persistSystemTimeToRTC(after);
|
||||
ntpEverSynced = true;
|
||||
lastNtpSyncEpoch = after;
|
||||
@@ -98,6 +95,11 @@ bool syncFromNTP() {
|
||||
// - alle 24h, sobald STA verbunden ist
|
||||
// Reine Vergleichsoperationen, NTP-Roundtrip selbst ist blockierend (~ms..5s).
|
||||
void loopRTC() {
|
||||
// RTC-Lesen passiert NUR aus der main-loop (verhindert I2C-Race mit PN532).
|
||||
// appendTimeStatus() liest danach nur den gecachten Wert.
|
||||
if (rtcAvailable) {
|
||||
cachedRtcEpoch = (time_t)rtc.now().unixtime();
|
||||
}
|
||||
bool sta = (WiFi.status() == WL_CONNECTED);
|
||||
|
||||
// Edge: false -> true (frischer STA-Connect)
|
||||
@@ -110,6 +112,9 @@ void loopRTC() {
|
||||
// 24h-Periodik (nur wenn STA online)
|
||||
if (sta && ntpEverSynced) {
|
||||
time_t nowEpoch = time(NULL);
|
||||
// Defensive: Clock-Jump rückwärts (z.B. via Browser-Time auf altes Datum)
|
||||
// würde das Delta negativ machen. Auf "jetzt" zurücksetzen.
|
||||
if (nowEpoch < lastNtpSyncEpoch) lastNtpSyncEpoch = nowEpoch;
|
||||
if (nowEpoch - lastNtpSyncEpoch >= 86400) {
|
||||
Serial.println("[RTC] 24h-Periodik — NTP-Sync");
|
||||
syncFromNTP();
|
||||
@@ -118,14 +123,11 @@ void loopRTC() {
|
||||
}
|
||||
|
||||
// Weak-Hook-Override aus timesync.h — erweitert /api/time um RTC-Status.
|
||||
extern "C" void appendTimeStatus(JsonDocument &doc) {
|
||||
void appendTimeStatus(JsonDocument &doc) {
|
||||
doc["rtc_available"] = rtcAvailable;
|
||||
doc["rtc_synced_from_ntp"] = ntpEverSynced;
|
||||
doc["last_ntp_sync_ago_s"] =
|
||||
ntpEverSynced ? (long)(time(NULL) - lastNtpSyncEpoch) : (long)-1;
|
||||
if (rtcAvailable) {
|
||||
doc["rtc_time_utc"] = (long)rtc.now().unixtime();
|
||||
} else {
|
||||
doc["rtc_time_utc"] = (long)0;
|
||||
}
|
||||
long ago = ntpEverSynced ? (long)(time(NULL) - lastNtpSyncEpoch) : (long)-1;
|
||||
if (ago < 0 && ntpEverSynced) ago = 0; // Clock-Jump rückwärts → 0 statt negativ
|
||||
doc["last_ntp_sync_ago_s"] = ago;
|
||||
doc["rtc_time_utc"] = rtcAvailable ? (long)cachedRtcEpoch : (long)0;
|
||||
}
|
||||
|
||||
@@ -8,8 +8,8 @@
|
||||
|
||||
// Weak hooks — falls rtcsync.h kompiliert/gelinkt wird, überschreibt es diese.
|
||||
// Ohne rtcsync.h sind beide Symbole nullptr und werden nicht aufgerufen.
|
||||
extern "C" void onSystemTimeSet(time_t t) __attribute__((weak));
|
||||
extern "C" void appendTimeStatus(JsonDocument &doc) __attribute__((weak));
|
||||
void onSystemTimeSet(time_t t) __attribute__((weak));
|
||||
void appendTimeStatus(JsonDocument &doc) __attribute__((weak));
|
||||
|
||||
// Globale Zeitvariablen
|
||||
struct timeval tv;
|
||||
@@ -55,7 +55,7 @@ String getCurrentTimeJSON() {
|
||||
return response;
|
||||
}
|
||||
|
||||
void syncTimeWithNTP(const char *ntpServer = "pool.ntp.org",
|
||||
bool syncTimeWithNTP(const char *ntpServer = "pool.ntp.org",
|
||||
long gmtOffset_sec = 3600, int daylightOffset_sec = 0) {
|
||||
configTime(gmtOffset_sec, daylightOffset_sec, ntpServer);
|
||||
Serial.println("Warte auf NTP-Zeit (max 5s)...");
|
||||
@@ -76,6 +76,7 @@ void syncTimeWithNTP(const char *ntpServer = "pool.ntp.org",
|
||||
} else {
|
||||
Serial.println("\nNTP-Sync fehlgeschlagen (Timeout nach 5s)");
|
||||
}
|
||||
return synced;
|
||||
}
|
||||
|
||||
// Hilfsfunktion: Setzt die Systemzeit auf den angegebenen Zeitstempel.
|
||||
|
||||
Reference in New Issue
Block a user