From 1beac007b764bcea1f7365b0c8cf1a8c4838853e Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:23:18 +0200 Subject: [PATCH] documenting and error handling --- releasenotes/RELEASE_NOTES_1.17.4-zephcore.md | 36 ++++++++++ .../adapters/datastore/ZephyrDataStore.cpp | 67 ++++++++++++++++++- zephcore/app/RepeaterDataStore.cpp | 20 +++++- zephcore/app/observer_creds.cpp | 11 ++- 4 files changed, 128 insertions(+), 6 deletions(-) diff --git a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md index 9a141ce..179ed3e 100644 --- a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md +++ b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md @@ -148,6 +148,36 @@ being invisible. > busy channel the node still waits — transmitting over traffic it can hear would cause exactly the > collision the check exists to avoid. +### A node set too sensitive can now find its own way back + +The safeguard above rescues a transmission that is already queued. It does not help the case where the +listening threshold itself is set so sensitive that the node can never clear the channel at all — which +`set cad.auto off` makes possible, and which nothing used to undo. A node in that state still receives, +and still accepts a command telling it to fix itself, but cannot transmit the reply. In practice no +ordinary app can complete the login it is waiting on, so a repeater on a mast is simply unreachable. + +Two layers now recover from it. + +The first watches for a node that has traffic to send and has been refused for a full minute. It relaxes +the threshold one step regardless of whether automatic tuning is switched on, and repeats until something +gets out. On the bench, a node parked at the most sensitive setting with every other recovery path +deliberately disabled went from completely silent to transmitting in five minutes. + +The second does not wait for traffic, because a node with nothing queued would otherwise wait up to two +days for its next scheduled advert to reveal the problem. It reads the measurements the node already +collects and steps the threshold back when the current setting reports the channel busy on essentially +every check — the signature of a detector that cannot clear, rather than a channel that is genuinely +occupied. On the bench it walked a node from the most sensitive setting back to a working one in about +half an hour, with nothing queued to send at any point. + +Both write the new threshold to flash, so a node that heals itself stays healed across a reboot. Both +stop as soon as the evidence stops — neither runs to the end of the range. + +> [!NOTE] +> **This means `get cad.offset` can read something you did not set.** That is the point: it records a +> node that recovered itself. If you find a node away from where you left it, its threshold was refusing +> every transmission at the old value. + --- ## The built-in `v` contact had an unusable key on half of all nodes @@ -310,3 +340,9 @@ picks its own moments to reboot. Companions and observers were never affected. times over on one repeater. It is now the packet's own airtime, the same figure received airtime has always used, and the "packets sent" total can no longer disagree with the flood and direct counts beneath it. +- **Nodes logged filesystem errors that were not errors.** Routine checks for files that simply do not + exist yet — a leftover temporary file, a contacts list on a node that has never stored a contact — + were reported as failures by the layer underneath, one on every settings save on a repeater and four + on every boot on a companion. Nothing was wrong and nothing was lost, but a genuine storage fault had + to be spotted among them. The checks now look before they act, and a healthy node boots with a clean + log. diff --git a/zephcore/adapters/datastore/ZephyrDataStore.cpp b/zephcore/adapters/datastore/ZephyrDataStore.cpp index cafb140..f87330f 100644 --- a/zephcore/adapters/datastore/ZephyrDataStore.cpp +++ b/zephcore/adapters/datastore/ZephyrDataStore.cpp @@ -88,11 +88,32 @@ static bool atomicWriteTempFile(const char *path, AtomicWriteFn write_fn, void * static bool lfs_mounted; static bool ext_lfs_mounted; -/* Check if a filesystem is mounted using fs_statvfs */ +/* Check if a filesystem is mounted (mount-list lookup, never logs) */ static bool is_mounted(const char *mount_point) { - struct fs_statvfs stat; - return fs_statvfs(mount_point, &stat) == 0; + /* Walk the registered mount list rather than calling fs_statvfs(). + * + * fs_statvfs() on a path that is not mounted makes Zephyr's FS layer log + * "mount point not found!!" at ERR, which put an line on the happy + * path of every boot: on boards with no external flash (the probe can + * never succeed) and, on boards that do have it, on every boot before the + * deferred-init retry below mounts it. + * + * Deliberately NOT solved by gating the probe on + * DT_NODE_EXISTS(DT_NODELABEL(qspi_lfs)): that would still log on the + * deferred-init path, and it would silently stop detecting /ext on any + * future board that mounts external flash under a different node label. + * fs_readmount() is a pure lookup, logs nothing, and stays correct in + * every one of those permutations. */ + int index = 0; + const char *name = NULL; + + while (fs_readmount(&index, &name) == 0) { + if (name != NULL && strcmp(name, mount_point) == 0) { + return true; + } + } + return false; } bool ZephyrDataStore::mount() @@ -194,6 +215,19 @@ bool ZephyrDataStore::exists(const char *path) const bool ZephyrDataStore::removeFile(const char *path) { + /* Idempotent: an absent file is a successful removal. + * + * Guarded by fs_stat rather than unlinking blind because fs_unlink() + * returns -ENOENT for a missing path and Zephyr's FS layer logs that at + * ERR level regardless of us ignoring the return. Every caller here is + * best-effort cleanup of a file that usually is NOT there, so unguarded + * this puts an line on the happy path of every boot — exactly the + * noise that makes a real filesystem error invisible. */ + struct fs_dirent entry; + + if (fs_stat(path, &entry) != 0) { + return true; + } return fs_unlink(path) == 0; } @@ -535,6 +569,16 @@ uint8_t ZephyrDataStore::takeShutdownReason() uint8_t code = 0; size_t len = 0; + /* No marker is the NORMAL case — it exists only after a software + * power-off. Probe before opening: fs_open() on a missing path is + * logged at ERR level by Zephyr's FS layer, so an unguarded read here + * put a second line on every clean boot. */ + struct fs_dirent marker; + + if (fs_stat(SHUTDOWN_FILE, &marker) != 0) { + return 0; + } + if (openRead(SHUTDOWN_FILE, &code, sizeof(code), len) && len >= 1) { removeFile(SHUTDOWN_FILE); return code; @@ -1034,6 +1078,15 @@ void ZephyrDataStore::loadContacts(DataStoreHost *host) { const char *path = contactsFile(); + /* Probe first: this file does not exist until a contact is stored, and + * fs_open() on a missing path is logged at ERR by Zephyr's FS layer no + * matter how gracefully we handle the return. The open below is kept as + * the real error path (a file that exists but cannot be opened). */ + if (!exists(path)) { + LOG_DBG("loadContacts: no contacts file found"); + return; + } + struct fs_file_t file; fs_file_t_init(&file); int rc = fs_open(&file, path, FS_O_READ); @@ -1182,6 +1235,14 @@ void ZephyrDataStore::saveContacts(DataStoreHost *host) void ZephyrDataStore::loadChannels(DataStoreHost *host) { const char *path = channelsFile(); + + /* Probe first — same reason as loadContacts(): absent until a channel is + * configured, and a missing-path fs_open() is logged at ERR by the FS + * layer regardless of us handling it. */ + if (!exists(path)) { + return; + } + struct fs_file_t file; fs_file_t_init(&file); if (fs_open(&file, path, FS_O_READ) < 0) { diff --git a/zephcore/app/RepeaterDataStore.cpp b/zephcore/app/RepeaterDataStore.cpp index a627457..6dce753 100644 --- a/zephcore/app/RepeaterDataStore.cpp +++ b/zephcore/app/RepeaterDataStore.cpp @@ -137,7 +137,15 @@ bool RepeaterDataStore::saveIdentity(const mesh::LocalIdentity& id) { return false; } - fs_unlink(tmp_path); + /* Guarded by fileExists() rather than unlinking blind: on the normal path + * the temp is absent, fs_unlink() returns -ENOENT, and Zephyr's FS layer + * logs that at ERR level regardless of us ignoring the return -- putting an + * line on the happy path of every save, which is exactly the noise + * that makes a real filesystem error invisible. Same guard as + * ZephyrDataStore::atomicWrite(). */ + if (fileExists(tmp_path)) { + fs_unlink(tmp_path); + } struct fs_file_t file; fs_file_t_init(&file); @@ -363,7 +371,15 @@ bool RepeaterDataStore::savePrefs(const NodePrefs& prefs) { return false; } - fs_unlink(tmp_path); + /* Guarded by fileExists() rather than unlinking blind: on the normal path + * the temp is absent, fs_unlink() returns -ENOENT, and Zephyr's FS layer + * logs that at ERR level regardless of us ignoring the return -- putting an + * line on the happy path of every save, which is exactly the noise + * that makes a real filesystem error invisible. Same guard as + * ZephyrDataStore::atomicWrite(). */ + if (fileExists(tmp_path)) { + fs_unlink(tmp_path); + } struct fs_file_t file; fs_file_t_init(&file); diff --git a/zephcore/app/observer_creds.cpp b/zephcore/app/observer_creds.cpp index af5ff30..8af19ee 100644 --- a/zephcore/app/observer_creds.cpp +++ b/zephcore/app/observer_creds.cpp @@ -55,7 +55,16 @@ extern "C" bool observer_creds_save(const struct ObserverCreds *creds, return false; } - fs_unlink(tmp_path); + /* Guarded by fs_stat() rather than unlinking blind: on the normal path the + * temp is absent, fs_unlink() returns -ENOENT, and Zephyr's FS layer logs + * that at ERR level regardless of us ignoring the return -- putting an + * line on the happy path of every save. Same guard as + * ZephyrDataStore::atomicWrite(). */ + struct fs_dirent tmp_ent; + + if (fs_stat(tmp_path, &tmp_ent) == 0) { + fs_unlink(tmp_path); + } struct fs_file_t f; fs_file_t_init(&f);