fix(companion): atomic save for contacts/channels/prefs

saveContacts/saveChannels/savePrefs truncated the live file up front and
then wrote in place, so a crash, reset or full flash mid-save wiped the
whole file — observed as all contacts disappearing after a UI hang.

Write to a temp file and only swap it in (via LittleFS atomic rename) once
every record has written cleanly; on any failure keep the previous good
file and drop the temp. savePrefs gates the swap on the tail-sentinel write
succeeding (writes return 0 once flash fills, so a good sentinel means the
whole record fit). Contacts/channels live on the multi-MB QSPIFlash, so the
transient second copy fits easily.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
MarekZegare4
2026-06-24 21:39:46 +02:00
parent 3113dabd2c
commit c917bf59fd

View File

@@ -44,6 +44,21 @@ static File openWrite(FILESYSTEM* fs, const char* filename) {
#endif
}
// Atomically swap a fully-written temp file over its final path. LittleFS
// (nRF52/STM32) rename replaces an existing destination atomically, so a crash
// leaves either the old file or the new one intact — never a truncated mix.
// Other Arduino filesystems can't rename onto an existing file, so the
// destination is dropped first (a metadata-only window, vs. the record-by-record
// write window of a direct overwrite). Returns false if the swap fails.
static bool commitTempFile(FILESYSTEM* fs, const char* tmp, const char* final_path) {
#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
return fs->rename(tmp, final_path);
#else
fs->remove(final_path);
return fs->rename(tmp, final_path);
#endif
}
#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
static uint32_t _ContactsChannelsTotalBlocks = 0;
#endif
@@ -474,7 +489,9 @@ void DataStore::loadPrefsInt(const char *filename, NodePrefs& _prefs, double& no
}
void DataStore::savePrefs(const NodePrefs& _prefs, double node_lat, double node_lon) {
File file = ::openWrite(_fs, "/new_prefs");
// Atomic temp-then-rename (see commitTempFile) so an interrupted save can't
// wipe settings; loadPrefs() still validates the tail sentinel on read.
File file = ::openWrite(_fs, "/new_prefs.tmp");
if (file) {
uint8_t pad[8];
memset(pad, 0, sizeof(pad));
@@ -598,11 +615,18 @@ void DataStore::savePrefs(const NodePrefs& _prefs, double node_lat, double node_
file.write((uint8_t *)&_prefs.locator_target_kind, sizeof(_prefs.locator_target_kind));
file.write((uint8_t *)_prefs.locator_key, sizeof(_prefs.locator_key));
// Tail sentinel — must be last. See NodePrefs::SCHEMA_SENTINEL.
// Tail sentinel — must be last. See NodePrefs::SCHEMA_SENTINEL. Its write is
// the one we check: once the flash fills, writes return 0, so a good
// sentinel write means the whole record fit. Only then swap it in.
uint32_t sentinel = NodePrefs::SCHEMA_SENTINEL;
file.write((uint8_t *)&sentinel, sizeof(sentinel));
bool ok = (file.write((uint8_t *)&sentinel, sizeof(sentinel)) == sizeof(sentinel));
file.close();
if (ok) {
commitTempFile(_fs, "/new_prefs.tmp", "/new_prefs");
} else {
_fs->remove("/new_prefs.tmp"); // keep the previous good /new_prefs
}
}
}
@@ -658,35 +682,48 @@ File file = openRead(_getContactsChannelsFS(), "/contacts3");
}
void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactInfo& c)) {
File file = ::openWrite(_getContactsChannelsFS(), "/contacts3");
if (file) {
uint32_t idx = 0;
ContactInfo c;
uint8_t unused = 0;
FILESYSTEM* fs = _getContactsChannelsFS();
// Write to a temp file, then atomically rename it over /contacts3 only once
// every record has written cleanly. The old code truncated /contacts3 up
// front and wrote in place, so a crash, reset or full flash mid-save wiped
// the entire contact list. Now an interrupted save leaves the previous good
// file untouched.
File file = ::openWrite(fs, "/contacts3.tmp");
if (!file) return;
while (host->getContactForSave(idx, c)) {
if (filter && !filter(c)) {
idx++; // advance to next contact
continue;
}
bool success = (file.write(c.id.pub_key, 32) == 32);
success = success && (file.write((uint8_t *)&c.name, 32) == 32);
success = success && (file.write(&c.type, 1) == 1);
success = success && (file.write(&c.flags, 1) == 1);
success = success && (file.write(&unused, 1) == 1);
success = success && (file.write((uint8_t *)&c.sync_since, 4) == 4);
success = success && (file.write((uint8_t *)&c.out_path_len, 1) == 1);
success = success && (file.write((uint8_t *)&c.last_advert_timestamp, 4) == 4);
success = success && (file.write(c.out_path, 64) == 64);
success = success && (file.write((uint8_t *)&c.lastmod, 4) == 4);
success = success && (file.write((uint8_t *)&c.gps_lat, 4) == 4);
success = success && (file.write((uint8_t *)&c.gps_lon, 4) == 4);
if (!success) break; // write failed
bool ok = true;
uint32_t idx = 0;
ContactInfo c;
uint8_t unused = 0;
while (host->getContactForSave(idx, c)) {
if (filter && !filter(c)) {
idx++; // advance to next contact
continue;
}
file.close();
bool success = (file.write(c.id.pub_key, 32) == 32);
success = success && (file.write((uint8_t *)&c.name, 32) == 32);
success = success && (file.write(&c.type, 1) == 1);
success = success && (file.write(&c.flags, 1) == 1);
success = success && (file.write(&unused, 1) == 1);
success = success && (file.write((uint8_t *)&c.sync_since, 4) == 4);
success = success && (file.write((uint8_t *)&c.out_path_len, 1) == 1);
success = success && (file.write((uint8_t *)&c.last_advert_timestamp, 4) == 4);
success = success && (file.write(c.out_path, 64) == 64);
success = success && (file.write((uint8_t *)&c.lastmod, 4) == 4);
success = success && (file.write((uint8_t *)&c.gps_lat, 4) == 4);
success = success && (file.write((uint8_t *)&c.gps_lon, 4) == 4);
if (!success) { ok = false; break; } // write failed (e.g. flash full)
idx++; // advance to next contact
}
file.close();
if (ok) {
commitTempFile(fs, "/contacts3.tmp", "/contacts3");
} else {
fs->remove("/contacts3.tmp"); // keep the previous good /contacts3
}
}
@@ -738,29 +775,39 @@ void DataStore::loadChannels(DataStoreHost* host) {
}
void DataStore::saveChannels(DataStoreHost* host) {
File file = ::openWrite(_getContactsChannelsFS(), "/channels2");
if (file) {
uint8_t channel_idx = 0;
ChannelDetails ch;
uint8_t unused[4];
memset(unused, 0, 4);
FILESYSTEM* fs = _getContactsChannelsFS();
// Same atomic temp-then-rename pattern as saveContacts() — never truncate the
// live /channels2 before the new copy is fully written.
File file = ::openWrite(fs, "/channels2.tmp");
if (!file) return;
while (host->getChannelForSave(channel_idx, ch)) {
channel_idx++;
// getChannelForSave() returns every slot up to MAX_GROUP_CHANNELS, so skip
// the unused ones (all-zero secret) rather than writing all 40 — otherwise
// the file is always ~2.7 KB and wears the flash needlessly. loadChannels()
// already compacts empty entries on read, so the loaded result is identical.
bool empty = true;
for (int b = 0; b < 32; b++) if (ch.channel.secret[b]) { empty = false; break; }
if (empty) continue;
bool ok = true;
uint8_t channel_idx = 0;
ChannelDetails ch;
uint8_t unused[4];
memset(unused, 0, 4);
bool success = (file.write(unused, 4) == 4);
success = success && (file.write((uint8_t *)ch.name, 32) == 32);
success = success && (file.write((uint8_t *)ch.channel.secret, 32) == 32);
if (!success) break; // write failed
}
file.close();
while (host->getChannelForSave(channel_idx, ch)) {
channel_idx++;
// getChannelForSave() returns every slot up to MAX_GROUP_CHANNELS, so skip
// the unused ones (all-zero secret) rather than writing all 40 — otherwise
// the file is always ~2.7 KB and wears the flash needlessly. loadChannels()
// already compacts empty entries on read, so the loaded result is identical.
bool empty = true;
for (int b = 0; b < 32; b++) if (ch.channel.secret[b]) { empty = false; break; }
if (empty) continue;
bool success = (file.write(unused, 4) == 4);
success = success && (file.write((uint8_t *)ch.name, 32) == 32);
success = success && (file.write((uint8_t *)ch.channel.secret, 32) == 32);
if (!success) { ok = false; break; } // write failed
}
file.close();
if (ok) {
commitTempFile(fs, "/channels2.tmp", "/channels2");
} else {
fs->remove("/channels2.tmp"); // keep the previous good /channels2
}
}