7110 Commits

Include renames

Author	SHA1	Message	Date
Ben Meadors	a41ddec1a7	fix(nrf54l15): don't write past String buffer when a grow fails (#11454) ... * fix(nrf54l15): don't write past String buffer when a grow fails reserve() correctly keeps the old buffer when realloc returns NULL, but returned void, and assign()/concat() proceeded to memcpy with n >= _cap anyway - a heap overflow of up to n+1-_cap bytes into adjacent allocations. On this Zephyr target allocation failure is a realistic condition, and the result was heap corruption instead of a clean no-op. reserve() now reports success and the callers leave the string unchanged when the grow fails. * fix(nrf54l15): guard String length arithmetic against wraparound Per review: reject size requests whose n+1 / _len+n arithmetic would wrap before they reach the capacity check, and make reserve(0) fail without calling realloc (realloc(p, 0) would free the buffer and return NULL, leaving _buf dangling).	2026-08-12 18:17:33 -05:00
Ben Meadors	5172e58526	fix(nimble): stop leaking BLE2904 descriptor on BLE re-setup (#11453) ... setupService() re-runs on every Bluetooth re-enable cycle, and every other callback object in it is deliberately function-local static for exactly that reason - the battery level descriptor was the one heap allocation that was missed. The framework never frees descriptors (~BLECharacteristic has an empty body and BLEDevice::deinit only deletes server/advertising/scan), so each cycle leaked one BLE2904. Make it static like its neighbors.	2026-08-12 18:15:18 -05:00
Jason P	f06f808391	Add Transmit Enabled menu to LoRa frame (#11442) ... * Add Transmit Enabled menu to LoRa frame * Fix outer variable shadows	2026-08-12 21:59:26 +00:00
6 people	fdb644e0b7	Fix millis() rollover in deadline, interval, and timestamp handling (#11291) ... * Add native test coverage for the UptimeClock monotonic seam src/UptimeClock.{h,cpp} shipped without a dedicated test suite. Port the six tests from the monotonic-time branch (test/test_time), retargeted to the renamed header. The wrap test crosses 0xFFFFFFFF via advanceTestMillis() rather than a second setTestMillis(): setTestMillis() sets clockSourceChanged, which makes getMillis64() rebase its accumulator and swallow the wrap. * NextHopRouter: fix 49.7-day millis() rollover in retransmission timing Resolves the "FIXME, handle 51 day rolloever here!!!" in NextHopRouter::doRetransmissions() by switching the retransmission-due comparison from plain unsigned <= to a signed-difference cast. The previous p.nextTxMsec <= now comparison silently breaks across the ~49.7 day millis() wraparound: pending retransmissions either stall for the remainder of the wrap window, or all fire simultaneously at the rollover boundary. Long-running router/infrastructure nodes do hit this in practice. The replacement (int32_t)(p.nextTxMsec - now) <= 0 is the standard Arduino/embedded idiom for rollover-safe deadline checks and behaves identically to the original for any non-wrap timing. * Address Copilot review: use unsigned half-range for rollover-safe retransmit check Review feedback from @Copilot on PR #10227: casting a uint32_t subtraction to int32_t is implementation-defined in C++ when the unsigned value exceeds INT32_MAX (even though it works on typical two's-complement targets). Switch to the fully well-defined unsigned half-range form: nextTxMsec is in the past-or-equal iff (now - nextTxMsec) has not wrapped past 2^31 ms. Future offsets < 2^31 ms wrap into the top half and read as 'not yet'. Same semantics as the signed-cast version on every two's-complement platform we care about, but portable to any conforming C++ impl. * Use monotonic time for airtime windows * Document monotonic airtime windows * Fix test_packet_signing sentinel that #10227's rollover fix inverts test_C3_invalid_repeated_packet_cannot_ack_or_change_retry_state parked a pending packet at nextTxMsec = UINT32_MAX to mean "never retransmit", then asserted that a rejected repeated packet leaves the retry state untouched. NextHopRouter::doRetransmissions() now tests whether a retransmit is due with an unsigned half-range compare, (uint32_t)(now - nextTxMsec) < 0x80000000u, so that retransmission timing survives the ~49.7 day millis() wrap. Under it now - 0xFFFFFFFF == now + 1, a small positive delta, so UINT32_MAX reads as ~1ms in the past: the retransmit fires and rewrites nextTxMsec, and the test failed with "Expected 4294967295 Was 6247". Use a representable future time instead. Production is unaffected either way - nextTxMsec is only ever written as millis() + d, and UINT32_MAX came from the test harness alone - so the sentinel is what needs to go, not the comparison. Special-casing UINT32_MAX in the retransmit path would keep a value that reads as "expired" under any wrap-correct compare. The value is held in a local because millis() advances across runPipelineIngress(), so recomputing it at the assertion would compare against a different number. Reported upstream on meshtastic/firmware#10227, whose branch predates this test. * Make Throttle time-injectable and add hasElapsed() Throttle backs ~94 call sites, which makes it the highest-leverage place in the tree to put the clock seam: reading Time::getMillis() instead of millis() in its three call sites turns all of them into time-injectable code at once, without touching any of them. The 32-bit millis() wrap is not otherwise reachable from a native test. The read is behaviour-preserving - Time::getMillis() returns millis() unless a test injects a clock - and the full native suite passes with it live. Also add hasElapsed(), the complement of isWithinTimespanMs(), because 51 of the 94 call sites are spelled !isWithinTimespanMs and read poorly. Its boundary is inclusive (>=) since isWithinTimespanMs uses <; both are documented. It deliberately does not treat lastExecutionMs == 0 as "never run": call sites pair that test with the interval check themselves, and absorbing a sentinel into the one helper every module depends on is exactly the value-overloading hazard being removed elsewhere. Migrating the existing !isWithinTimespanMs sites is cosmetic and deliberately left out of this commit. test/test_throttle/ covers window semantics, both boundaries, the complement identity, execute()'s first-run and throttled paths, and - the point of the exercise - a window opened before the wrap closing correctly after it, including at the 24h interval that is the longest in the tree. * Stop disarmed deadline sentinels reaching the comparison Two deadline variables encoded "inactive" as a magic value that only reads as "never" because the comparison against it is a naive millis() compare. Under any rollover-correct comparison both invert to "expired ~49 days ago", so they have to be untangled before those comparisons can be fixed. Power::reboot() set rebootAtMsec = -1 on platforms with no reboot implementation, intending "never fire". Every reader already treats 0 as the disarm value - powerCommandsCheck() tests `if (rebootAtMsec && ...)`, and AdminModule writes 0 to cancel - so -1 was both wrong and unnecessary. Use 0. Left as UINT32_MAX it would reboot-loop the moment the comparison is corrected. ExternalNotificationModule's nag window compared against nagCycleCutoff, which holds UINT32_MAX once stopped and 1 at boot. isNagging is the real armed flag, so test it first and short-circuit: a disarmed cutoff can no longer reach the arithmetic, while an idle module still takes the same sleep path that the boot-time value of 1 was relying on. Note this fixes the sentinel only. The comparison itself is still a naive `nagCycleCutoff < millis()` and remains on the list to convert. * Fix millis() rollover in every deadline and interval comparison Roughly 20 sites compared against millis() directly - `millis() > deadline`, `deadline < millis()`, `last + interval < millis()`. All of them break for about 24 days after the 32-bit millis() wrap: depending on which side of the wrap each value sits, the action either stalls for weeks or fires immediately and repeatedly. The longest affected interval is the 12 hour NTP renewal, a ~50x margin against the wrap, so none of these needed the range - only the correct comparison. Add Throttle::deadlinePassed(deadlineMs) for sites that store an absolute deadline they cannot re-express as "interval since an event". It uses the same unsigned half-range test as NextHopRouter::doRetransmissions() rather than introducing a competing signed-cast idiom, and unlike the signed cast it is defined for every input. Sites that do store an event use the existing isWithinTimespanMs / hasElapsed. Nothing gained new state. Because both helpers read Time::getMillis(), every converted site is now reachable from a native test that drives the clock across the wrap; the comparison itself is covered directly in test/test_throttle/. Sentinel handling is the reason this could not be a mechanical rewrite. The disarm convention is not uniform: 0 means "inactive" for rebootAtMsec, shutdownAtMsec, alertBannerUntil, fixHoldEnds, suppressUntilMs and touchResumeBlockUntilMs; 0 means "due now" for ntp_renew, which is forced to 0 at link-up; UINT32_MAX means "inactive" for nagCycleCutoff; and alertBannerUntil == 0 in isOverlayBannerShowing() means "show indefinitely". Every inactive marker is arithmetically far in the past, so a correct comparison fires on it - each site tests its sentinel before the arithmetic, and keeps the meaning it had. Two sites carried a second bug found on the way: BME680Sensor tested (stateUpdateCounter * STATE_SAVE_PERIOD) < millis(). With a 6 hour period and a uint16_t counter that product overflows uint32_t after about 198 saves, independently of the millis() wrap. It now measures the interval since the last save. EInkDynamicDisplay had `if (previousRunMs > millis()) return;` as a millis() overflow guard, which skipped rate limiting entirely for the whole post-wrap period - the bug it meant to prevent. Every check below it already goes through Throttle, so the guard is removed rather than fixed. MotionSensor's calibration countdown is converted to a signed delta rather than deadlinePassed, because it needs the remaining magnitude and not a boolean; that matches the already-correct check in the same file. * Remove getMillis64() and use Throttle for the NodeInfo reply window getMillis64() had exactly one caller and no callers in tests. It also carried obligations that made it the wrong shape for this firmware: a wrap accumulator in mutable statics, which is not ISR-safe, and which must be polled at least once every ~49.7 days or it silently misses a wrap and returns a time ~49 days short. Its one caller only wanted to know whether a 12 hour suppression window had elapsed - which Throttle answers correctly across the wrap without any accumulator. NodeInfoModule now stores Time::getMillis() in lastNodeInfoSeen and tests the window with Throttle::isWithinTimespanMs, so the map holds milliseconds rather than seconds derived from a 64-bit read. USERPREFS_NODEINFO_REPLY_SUPPRESS_SECS is user-overridable and now feeds a multiply by 1000, so a static_assert rejects any value too large to express in milliseconds instead of letting it wrap. clockSourceChanged goes too. It existed solely to rebase getMillis64()'s accumulator when a test swapped clock sources, and it made the wrap untestable through the injection API: setTestMillis() set the flag, so a wrap crossed by two setTestMillis() calls was swallowed. With the accumulator gone the flag has nothing to rebase, and the injection API is a plain settable clock. The three getMillis64 tests are dropped as they no longer describe anything. One test replaces them, pinning that advanceTestMillis() wraps past 0xFFFFFFFF rather than saturating, since the Throttle wrap tests rely on it. Also fix eviction in pruneLastNodeInfoCache(): it picked the entry with the smallest stored stamp, which is the wrong victim once some stamps sit on the far side of the wrap. It now evicts the largest elapsed time. * Add CI guard and docs rule against naive millis() comparisons Fixing the existing sites does not stop the next one being added. The millis-deadline-check job rejects millis() placed directly next to a comparison operator, in either order, anywhere in src/. It lives in test_native.yml alongside suite-count-check, which sets the precedent for a repo-hygiene guard that CI enforces and bin/run-tests.sh does not. The correct idioms all subtract before comparing, so none of them match the pattern. Line comments are stripped first, so documentation is free to name the broken form - as the guard's own comment and the coding conventions both do. Writing the check before finishing the sweep turned out to be worth it: it found roughly 14 sites that a by-hand audit of deadline variables had missed, including two extra nagCycleCutoff compares, both boot-screen timeouts, and a 6 hour sensor save interval that was also overflowing a uint32_t multiply. .github/millis-deadline-allowlist.txt covers the cases that are genuinely not deadline tests. Both current entries are uptime thresholds - "has the device been up N ms" - with no stored deadline and no event to measure from: a 30s button holdoff against phantom shutdown from floating pins, and a 10s window for the OEM boot logo. Each re-crosses its threshold once per wrap, which is harmless for boot-holdoff logic and not worth new state to avoid. Entries are keyed on file plus exact source text, without line numbers, so an edit above an entry does not silently invalidate it. Locally the guard reports 19 matches before the sweep and 2 after, both allowlisted. The Throttle bullet in the coding conventions is rewritten from "prefer Throttle for rate limiting" to "never compare against millis() directly", lists all four helpers with when to use which, names the CI guard, and documents the sentinel hazard with the rebootAtMsec = -1 case that would have become a reboot loop. Mirrored into AGENTS.md; CLAUDE.md gets a pointer row. * Trim rollover comments to what the code needs The comments added with the millis() rollover fixes carried too much of the investigation that produced them: how many sites were found, which document recorded them, what the old code used to do. That belongs in the commit history, not in the source, and some of it was already stale - Power::reboot() still described the check it disarms as "a naive millis() > deadline" when that comparison had been fixed in the same series. What stays is the non-obvious part at each site: which sentinel value the variable overloads and what it means there, since that differs between call sites and is what a correct comparison gets wrong. 0 means "not scheduled" for rebootAtMsec, "renew now" for ntp_renew, and "show indefinitely" in isOverlayBannerShowing(). Exposition is kept where it earns its place: the Throttle helpers, the uptime clock's note on why there is no 64-bit variant, and the tests. The Throttle docs lose only the site count and the "longest interval in the firmware" statistic, both of which would age badly; the range trade-off between the two forms is what a caller actually needs. Comments only - no code changed, verified by diff. * possible fixes * Address review feedback on the rollover fixes - BME680Sensor: checkpoint lastStateSaveMs after a successful write instead of at the interval test. The first save (IAQ accuracy >= 2) left it at 0, timing the next save from boot, and stamping before the write deferred the retry a full period when the write failed. Reads Time::getMillis(), the same clock Throttle compares against. - Throttle: add deadlinePassedAt(now, deadline) for loops that snapshot the clock once and test many deadlines; deadlinePassed() now delegates to it. NextHopRouter::doRetransmissions() uses it, replacing the inline half-range compare adopted from #10227 (nightjoker7) - same arithmetic, credited at the call site - and takes its snapshot from Time::getMillis() so setNextTx() deadlines and the due test cannot diverge under an injected test clock. - test_native.yml: set -euo pipefail in the millis-deadline guard, matching the sibling suite-count job. Without -e a partially failed scan could report "no violations" from truncated output. - test_packet_signing: build the not-due deadline from Time::getMillis() rather than millis(), so the test and the router read one clock. - test_throttle: cover deadlinePassedAt(), and correct a wrapped-value comment (0xFFFFFF00 + 400 is 0x00000090, not 0x00000094). Two review comments were declined: the AirTime mutex (every airTime-> caller runs in the single cooperative loop, WebServerThread included) and the MotionSensor 0-sentinel countdown (the calibration frame is only installed while a window is open). clod helped out here * Correct the described failure window of a naive millis() compare The comments and agent docs said a bare `millis() > deadline` "breaks for ~24 days after the wrap". That figure belongs to the fix, not the bug: it is the half-range limit of deadlinePassed(), which reads deadlines more than 2^31 ms ahead as already passed, and the range over which a UINT32_MAX sentinel reads as passed. The naive compare's actual failure is an inversion lasting only while the deadline sits on the far side of the wrap, so it is bounded by the interval: the action fires immediately and loses its wait, or blocks for about the wait it should have performed - days for the nRF52 flash-corruption backoff, one skipped cycle for a seconds-long retransmit timer. Comments and docs only; the ~24.8 day statements that correctly describe deadlinePassed()'s own range are left as they were. clod helped out here * Restore a monotonic uptime clock and consolidate the wrap counters Time::getMillisMonotonic() is the getMillis64() shape - a 32-bit wrap counter carried across reads - promoted to the shared timebase, with Time::getUptimeSecs() as the derived whole-seconds view. This deliberately reverses the earlier removal of getMillis64(), and the distinction matters: removal was right for a lazily-read accumulator with one rare caller, where a 49.7-day gap between reads silently swallowed a wrap. Here every read is the poll and AirTime::runOnce() guarantees one per second; the missed-wrap contract is pinned by a test rather than left as a footnote. Three private wrap counters collapse into it: - AirTime::syncNow() takes its seconds from Time::getUptimeSecs() and drops its lastSyncMsec checkpoint; window rotation is unchanged. - DeviceTelemetryModule loses refreshUptime()/uptimeWrapCount/uptimeLastMs; uptime_seconds comes from Time::getUptimeSecs(), which also removes the 0.296s-per-wrap truncation of (0xFFFFFFFF / 1000) * wraps. Its two interval checks move to Throttle::hasElapsed(). - HostMetricsModule's copies of those members were never read (its uptime comes from /proc/uptime) - deleted. Not ISR-safe (unguarded mutable carry): ISRs keep using getMillis(), which stays a pure read. Audited: no interrupt-context file reads getTime(), getValidTime(), or the new accessors. test/native-suite-count 44 -> 45: the bump for test_uptime_clock was lost in a branch history rewrite, leaving every later value off by one - run-tests.sh reports AMBER and CI's suite-count-check fails on the current push until this correction. * Anchor the wall clock in monotonic milliseconds getTime() computed elapsed-since-time-set as a 32-bit millis() delta, so a node that took time once and stayed up past 49.7 days reported a wall clock one full cycle in the past - and last_heard, rx_time, message and position stamps all inherited it. The anchor is now the 64-bit monotonic count (timeStartMsec -> timeStartMs64) and the elapsed term is computed in 64-bit, so the wall clock is exact at any uptime. All six anchor writers follow: the five hardware-RTC read branches and perhapsSetRTC(), which keeps a truncated 32-bit copy of the same instant for its Throttle-checked rate-limit stamps. The test seams anchor the same way. Two native regression tests drive getTime() across the wrap through the Time seam - one anchored before the wrap and read after it, one anchored after a counted wrap - with the test epoch derived from BUILD_EPOCH so the plausibility window cannot rot as the build date advances. * Stamp the rx_time placeholder in monotonic uptime seconds computeRxTimeStamp() stamped Time::getMillis() when the clock was untrusted, and reconcilePendingRxTimes() back-calculated with a 32-bit millis() delta - correct within one wrap, but a placeholder older than 49.7 days aliased to a small elapsed value and reconciled to a plausible-but-wrong recent epoch: the exact failure has_rx_time exists to prevent, reachable by an ordinary unattended router whose phone connects two months in. The placeholder is now Time::getUptimeSecs(). Both stamps come off the monotonic counter, so the elapsed term is exact at any age and the aliasing window is gone outright rather than widened. If elapsed somehow exceeds the epoch itself, the packet stays un-dated (absent, never wrong) instead of clamping to a pre-1970 value. Defence in depth: a placeholder that leaks needs ~50 years of uptime to cross MIN_PLAUSIBLE_EPOCH, where milliseconds took 18.3 days. The stream-API reconciliation tests keep their scenarios with the placeholder unit switched, and ScopedTimeFixture resets the monotonic carry so uptime seconds are deterministic per case. * Date nodes heard before the clock arrives, without polluting last_heard A node first heard while the wall clock was untrusted got no last_heard at all, and nothing backfilled it once time arrived - the phone showed "Last heard: unknown" for a node it had just announced. The arrival instant now waits in a RAM-only sidecar (NodeNum -> uptime seconds, 32 slots, reuse-oldest - the RouteHealth shape) and is converted to a real epoch on the clock-becoming-trusted transition, beside the existing rx_time reconciliation. last_heard itself never holds anything but a real epoch or 0: it persists to flash and the warm tier, where an uptime-relative value would be meaningless after reboot. The sidecar's write sites are updateFrom()'s no-trusted-clock path (the rx_time placeholder already carries the arrival instant, so this is a store, not a second clock read) and addFromContact's anti-eviction stamps, which previously wrote a bare getTime() - boot-relative seconds on a clockless node, the exact value lastHeardIsWallClock() exists to catch. Eviction ranking honours the stamps: heard-this-boot outranks every stored epoch, ordered among themselves, so a stamped contact is not the first victim. PhoneAPI re-reads last_heard at nodeinfo send time: a record prefetched before the clock became trusted can carry 0 while the store has since been backfilled, and re-reading at the pop makes handshake ordering (time-set vs node-list download) irrelevant. Backfill never moves last_heard backwards and skips the pathological elapsed-exceeds-epoch case. A node evicted to the warm tier before time arrives is still absorbed with last_heard 0 - same as before, bounded to the untrusted window. * Update the agent docs for the monotonic timebase The conventions bullet asserted there is deliberately no 64-bit millis; the monotonic uptime clock restored for timestamps changes that contract. State the split explicitly: Throttle for deadlines and intervals (no carry state), Time::getMillisMonotonic()/getUptimeSecs() for timestamps, polled by construction and not ISR-safe. * Publish the monotonic wrap carry from a single writer getMillisMonotonic() was a read-modify-write on two unguarded statics, and it is reached off the main loop: the nRF52 Bluefruit task via onFromRadioAuthorize() -> PhoneAPI::getFromRadio -> getValidTime(), and the portduino civetweb workers via the same path. Two readers interleaving inside the wrap window could each increment the carry, putting every uptime and wall-clock reading 2^32 ms ahead for the rest of the boot - a permanent ~49.7 day jump in rx_time, last_heard and ClientNotification.time. Readers no longer write. serviceMonotonic() publishes a snapshot behind a seqlock and is the only writer; a reader adds its own unsigned elapsed time to that snapshot, which is exact across the wrap, so it never inspects the boundary and cannot miscount it. The main loop publishes every iteration, so the once-per-49.7-days obligation now has the whole window of margin instead of resting on an instruction-wide race. AirTime was the guaranteed poller and is now a pure reader, so the two airtime wrap tests step the clock the way loop() does. The test clock itself is atomic so a suite can drive it from one thread while others read. * Re-arm the GPS ephemeris hold when none is in force The rollover sweep guarded the hold re-arm with `fixHoldEnds != 0 &&`, which reads like the sentinel rule but inverts this site. The comparison it replaced, `(fixHoldEnds + GPS_THREAD_INTERVAL) < millis()`, was always true when nothing was armed - that was the point, since 0 means "not holding" and so is a reason to arm. With the guard, a publish that cleared the hold without sleeping (the `shouldPublish && !tooLong && !holdExpired` path, which does not call down()) left hasValidLocation set and prev_fixQual non-zero, so no disjunct held: nothing re-armed, nothing published, and the receiver stayed powered at the 200ms poll until searchedTooLong() fired. State the question positively instead. fixHoldInForce() is the only place the sentinel is interpreted, and both of runOnce()'s decisions derive from it - the asymmetry is now visible rather than implied, since arming does not require a prior hold but expiring does. Its `!= 0` test is not redundant with the arithmetic: deadlinePassed() is an unsigned half-range test, so past 2^31 ms of uptime the sentinel reads as a deadline ~24.9 days in the future. Kept beside its caller rather than in a header; the native test build compiles GPS.cpp, so the suite declares the prototypes. Also converts the getACK() wait to isWithinTimespanMs(start, interval): it has both the start instant and the interval in hand, which gives the full 49.7-day range instead of 24.8 days ahead, and takes its anchor from Time::getMillis() so the wait is injectable. * Date the NodeInfo reply window in uptime seconds The 12h reply-suppression stamp regressed from wrap-immune 64-bit seconds to raw 32-bit milliseconds, and pruneLastNodeInfoCache() evicts only by node count and DB membership - never by age. A stable mesh under the node cap therefore keeps every stamp indefinitely, and once uptime passes 49.7 days an old one aliases back into the window: `now - stamp` computes as ~0 and a legitimate NodeInfo request goes unanswered for up to 12h. It self-heals and repeats once per wrap cycle. Store Time::getUptimeSecs() instead, which does not wrap for 136 years, and drop the millisecond conversion the previous shape needed. Entries past the window are now evicted too: they can only ever decide "don't suppress". N8-N11 cover the window from both sides, and N10 pins the regression - it needs a full 2^32 ms of uptime to elapse, not merely a crossing of the boundary, because that is when a millisecond stamp reads as "answered this instant". tearDown() now restores the injected clock and C14's region and TX bucket. A failing assertion aborts the test body, so restoring at the end of it leaked that state into every later case. * Update the agent docs for the single-writer clock and sentinel direction Two rules the preceding three commits changed. The monotonic clock is no longer maintained by whoever happens to read it: serviceMonotonic() is the only writer, readers are pure, and calling it from anywhere but the main loop reintroduces the double-count. The sentinel guidance gained the half it was missing. It named UINT32_MAX as a sentinel while prescribing an idiom that only covers 0, and it assumed the sentinel always means "suppress" - at the GPS fix-hold site it meant "fire", which is how that regression passed review looking like the rule. * Name the fix-hold expiry predicate and arm it from the injected clock holdJustExpired() gives the second reading of the fixHoldEnds sentinel a name beside the first, so both are pinned by test/test_gps_fix_hold/ and neither can be respelled at the call site. The old inline form could not be tested: written as a literal, its guard folds at compile time and the assertion asserts nothing. The arm site used bare millis() while the evaluation reads the Throttle clock; same value in production, but it kept that write out of reach of Time::setTestMillis(). Remap a deadline that lands on 0, which would otherwise read as no hold at all. * Share the extend formula between the clock's reader and writer getMillisMonotonic() and serviceMonotonic() carried byte-identical wrap arithmetic. A one-sided edit to either would drift the published carry from what readers report, so keep one copy. * Trim the NodeInfo dedup comment to the house limit * todo note for potential future imrpovments * fix some simple deadlines * Trim the hold-expiry test comment to the house limit * Fix non-blocking uptime publication and pre-clock recency edges (#29) * fix(time): avoid blocking monotonic readers * test(time): make paused-publisher check deterministic * fix(time): address review portability gaps * Init the eviction sentinel to the newest possible recency EvictionRecency{} is {0, false}, which evictionRecencyOlder() ranks as older than every candidate: without the oldestIndex/oldestBoringIndex guards nothing would ever be selected and a full node DB would stop evicting entirely. Init to the genuine maximum instead, so the sentinel is correct on its own. The index guards stay: two independent reasons the scan is right beats one. * Keep the deadline-guard check name branch protection matches The guard was widened to cover Time::getMillis() and unqualified getMillis(), and renamed to suit. Upstream branch protection matches required checks by name, so a rename means the old name never reports and merges block on a check that will never arrive. Widen the guard, keep the name; the descriptive text carries the broader scope. * Correct native-suite-count to 47 after the develop merge Upstream #11293 added test_nmea_wpl and took develop's count to 43; this branch had independently reached 46. Merging develop resolved the counter textually, keeping 46, while the directory set became the union of both sides at 47. The suite-count CI gate fails on the mismatch, and it gates the native test jobs, so the tests themselves were being skipped. * test(uptime): make the wrap fall where the comment says it does The concurrent-reader case started at 0xFFFFF000, leaving 0x1000 to the wrap, so the 0x800 advance annotated "cross the wrap" fell short and the wrap actually happened during the following 60s advance. Start at 0xFFFFF800 instead, so the first advance lands exactly on the wrap while the readers are running and the second is the ordinary time after it - the shape both comments already described. Total elapsed is unchanged, so the closing assertion still holds. * Respond to human comments * Did I ever tell you about the time I went to Shelbyville? I wore an onion on my belt, which was the style at the time. * Convert the I2S nag deadline develop dragged in The HAS_I2S_SPEAKER_NRF52 RTTTL block arrived from develop with a raw nagCycleCutoff >= millis(), which the deadline guard rejects. Use the same Throttle::deadlinePassed() form as the two sibling paths in this function. * Arm the LittleFS format guard with a flag, not a zero timestamp preFSBegin() runs in the first millisecond of boot, so millis() can legitimately return 0 there. Both readers of last_format_ms treated 0 as "nothing formatted this boot", which would skip the repeat-corruption escalation and let a dead flash reformat-loop instead of reporting FLASH_CORRUPTION_UNRECOVERABLE. * Note the single-thread contract on AirTime * Note the AirTime locking TODO, and tighten the thread note The two constant getters are not constrained, and getSilentMinutes() reads the buckets without rotating them, so "the accessors mutate" was not accurate. * trunk: ignore trufflehog false positives on millis-wrap test constants test_throttle and test_uptime_clock pin dense clusters of hex boundary constants (0xFFFFFF00u and neighbors) to exercise 32-bit millis() rollover. trufflehog's Lob detector stitches nearby hex literals into one candidate string, and the result happens to match a Lob API key shape - not a secret, just test fixtures. Same pattern already used for the gitleaks/nodedb-fixture false positive in this file. --------- Co-authored-by: nightjoker7 <mattdeering7@gmail.com> Co-authored-by: Clive Blackledge <clive@ansible.org> Co-authored-by: Benjamin Faershtein <119711889+RCGV1@users.noreply.github.com> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-08-12 16:49:17 -05:00
Clive BlackledgeandClaude Opus 5	db3eb91015	fix(security): never log the X25519 identity private key (#11435) ... installDefaultConfig() restores a preserved identity key when the config is reset with preserveKey=true. On that path it called: printBytes("Restored key", config.security.private_key.bytes, config.security.private_key.size); printBytes() hex-dumps the buffer to LOG_DEBUG, so this emitted all 32 bytes of the raw X25519 identity private key to the serial/BLE debug log. Debug logs are not a private channel. They are routinely captured over serial or BLE and pasted verbatim into GitHub issues, Discord threads and support requests. Anyone who reads such a log recovers the node's identity private key, and can then impersonate the node and decrypt every PKI direct message addressed to it -- past messages included, since the key is long-lived and the DH shared secret is static per node pair. There is no revocation story short of generating a new identity. Replaced with a LOG_DEBUG that records that a key was restored and contains no key-derived bytes. The restore/no-restore signal is the genuinely useful diagnostic here ("did my key survive the reset?"), and it costs nothing to keep; the bytes were never what made the line useful. Log level is unchanged -- printBytes() already logged at LOG_DEBUG. Deliberately NOT a truncated prefix or a hash. A prefix is still key material: it hands an attacker free bytes and shrinks the search space. A hash is a confirmation oracle -- it lets anyone holding a candidate key verify it against the log, which is exactly the check an attacker needs. Neither is a compromise; both leak. If a log line survives at all it must carry zero key-derived bytes. Sites changed: - src/mesh/NodeDB.cpp:988 -- the only full private-key dump in src/. Audited and deliberately left alone: - NodeDB.cpp:3552,3604 ("Incoming Pubkey", "Saved Pubkey") -- public keys, published to the mesh by design; not secret. - CryptoEngine.cpp:245,285 -- nonces, not key material. - CryptoEngine.cpp:246,286 -- first 8 bytes of the derived shared_key, and AdminModule.cpp:2006,2013,2014 -- the 8-byte admin session passkey. Both are secrets rather than public values, but neither is the identity private key and both are out of scope for this fix; noted for follow-up. No unused-variable fallout: private_key_temp is still read by the memcpy above, and printBytes() is still used by the two pubkey sites, so the meshUtils.h include is still required. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-12 13:05:20 +00:00
3 people	54d6ce833e	gps: avoid pow() in GPS_HARDSLEEP threshold heuristic (#11179) ... * gps: avoid pow() in GPS_HARDSLEEP threshold heuristic GPS::down() used pow(seconds, 1.22) to pick between GPS_SOFTSLEEP and GPS_HARDSLEEP - a curve fit the surrounding comment already describes as "not particularly accurate". On flash-constrained builds where this was the only pow() call site (e.g. wio-e5), it single-handedly pulled in the full double-precision libm pow/rem_pio2 chain for a heuristic threshold decision. Replaces it with gpsHardsleepThresholdMs(), a piecewise-linear lookup over the same curve, sampled at 16 points and verified to track the original formula within ~0.5% for inputs >=10s and ~1.6% for 5-10s (worse only in relative terms below 5s, where the absolute difference is at most a couple of seconds - negligible against update intervals measured in tens of seconds to hours). Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * gps: trim comments to repo convention (1-2 lines) Addresses a CodeRabbit nitpick: the explanatory comments in GPSUpdateScheduling.cpp and test_gps_update_scheduling/test_main.cpp had grown into multi-line blocks with provenance detail that belongs in the commit message, not inline. Trims each to 1-2 lines, keeping only the essential rationale/bounds. Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * Refactor main function to setup and loop for tests Signed-off-by: Thomas Göttgens <tgoettgens@gmail.com> * gps: extend the hardsleep threshold table below 5s and tighten its tests The 0s-to-5s chord read 42% high at 1s, 22% at 2s and 12% at 3s, against the ~1.6% the comment claimed. Adding 1s, 2s and 3s sample points brings the worst error below 10s to 1.60% at 7s. Above 10s it is 0.55% at 728s, unchanged. Tests: sample off-breakpoint values only, including both worst-error inputs (7s and 728s). Replace the 3000ms absolute floor, which made the 1s assertion unfalsifiable given a true value of 2750ms, with 2% and 0.75% bounds. Add breakpoint-exactness and clamp-boundary coverage. --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Signed-off-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: Austin <vidplace7@gmail.com> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-12 12:42:19 +00:00
Clive BlackledgeandClaude Opus 5	579d26e1b2	fix(NodeDB): don't let an empty contact key erase a stored public key (#11432) ... Clients send `add_contact` before every text-message DM, because a phone often holds a larger contact database (with public keys) than the radio can keep. That makes `addFromContact()` the highest-volume key-write path on the device - and it had no protection against key erasure. Its only key guard covered the manually-verified case: if the local entry was marked manually verified and the incoming contact was not, a key mismatch aborted the update. Every ordinary entry fell straight through to `CopyUserToNodeInfoLite()`, which assigns `public_key` unconditionally. So a SharedContact with `has_user` set and an empty `public_key` overwrote a peer's stored, XEdDSA-proven key with zeros - and `addFromContact()` calls `saveNodeDatabaseToDisk()`, so the erasure survived a reboot. Subsequent DMs to that peer then failed with PKI_SEND_FAIL_PUBLIC_KEY, with no way to recover until the peer's NodeInfo was re-exchanged. `public_key` is a singular (non-optional) bytes field, so "absent" and "empty" both decode to size 0; a client that simply has no key for a contact is indistinguishable on the wire from one asking to clear it. The fix is deliberately narrow: keep the stored key when the entry already holds a full 32-byte key and the incoming contact does not. A well-formed 32-byte contact key still updates the entry exactly as before. Deliberately NOT changed here: - `updateUser()`'s first-key-wins pin is not applied to this path. Clients legitimately use add_contact to supply keys the radio never had and to update them (QR-code contact sharing); a blanket pin would break that documented flow. Only erasure is refused. - `CopyUserToNodeInfoLite()` itself is untouched - it has many other callers (self-record refresh, updateUser, warm-tier rehydration), so the guard lives at this call site. - The manually-verified branch is unchanged. - Node-number validation (reserved/broadcast/self) on this path remains open and is tracked separately. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-12 12:22:53 +00:00
Andrew Yong	2f6906974e	gps: replace GeoCoord::latLongToMeter's spherical trig with equirectangular approximation (#11184)	2026-08-12 12:27:36 +02:00
Tadayoshi MIURA	9199e6b663	fix(stm32wl): smaller MAX_RX_TOPHONE and PACKETHISTORY_MAX on stm32wl (#11400) ... * fixes for stm32: memory optimization and constraints tuning * fold stm32wl elif to existing define * revert changes for packet pool	2026-08-12 07:02:56 +00:00
4 people	d5d5bad97c	SEN5X: fix version parsing, VOC index reporting, and read-buffer handling (#11114) ... * SEN5X: validate read lengths and initialize read buffers readBuffer() returns the number of data bytes written (0 on error). Check the return value against the number of bytes each caller consumes before parsing, and zero-initialize the destination buffers: findModel (5), getMeasurements (2), readValues (16), readPNValues (20), and vocStateFromSensor (SEN5X_VOC_STATE_BUFFER_SIZE). This also resolves maybe-uninitialized compiler warnings. Small simplifications in the same area: - Assign the converted measurement values directly; the isnan() checks on integer intermediates always took the conversion branch, so this preserves behavior. - Fold a redundant state comparison in wakeUp() that immediately followed the assignment of the same value. - Add an explicit 'return false' to the non-FSCom branches of loadState() and saveState(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * SEN5X: correct version parsing, VOC index gating, and cleaning wait - getVersion() requested 3 raw I2C bytes (2 data bytes) but parsed versionBuffer[0..6], so the hardware and protocol versions came from the buffer's initialized-but-unwritten tail. Request the full 12-byte reply (8 data bytes, the layout used by Sensirion's embedded-i2c-sen5x driver) and validate the received length before parsing. Also make the error message specific to the version read. - Use floating-point division when deriving major.minor version numbers so minor versions below 10 are preserved (integer division reported e.g. firmware 2.2 as 2.00). - Gate pm_voc_idx on vocIndex rather than noxIndex, so SEN54 devices (VOC but no NOx) report their VOC index. - Widen the millis() snapshot in startCleaning() to uint32_t so the 10-second fan-cleaning wait always measures elapsed time correctly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * SEN5X: add size checks to I2C helpers, stage VOC state, handle unavailable readings - readBuffer(): accept only request sizes that are a multiple of 3 (2 data bytes + 1 CRC per group), keeping the read loop's size arithmetic in bounds for any future caller. Current callers all comply. - sendCommand(): likewise accept only even payload sizes on the write side. - vocStateFromSensor(): read into a staging buffer and copy to vocState only after the full transfer verifies, so the stored state stays consistent if a read fails partway through. - readValues()/readPNValues(): the sensor reports unavailable values as 0xFFFF (unsigned) / 0x7FFF (signed); map these to the UINT16_MAX / UINT32_MAX / FLT_MAX sentinels that getMetrics() checks, so unavailable channels are omitted from telemetry rather than scaled into numeric readings. Guard the cumulative-to-binned PN subtraction so the sentinels are preserved. - readPNValues(): convert #/cm3 to #/0.1l as raw * 10, retaining the 0.1-resolution digit that dividing before multiplying discarded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * SEN5X: size read buffers in data bytes and document I2C helper conventions readBuffer()'s size parameter is the raw I2C transfer size including CRC bytes, while only the verified data bytes (2/3 of the request) are written to the destination. Two call sites sized their buffers in raw units (findModel: 48 for 32 data bytes; getMeasurements: 3 for 2); both were safe over-allocations. Size them in data bytes so every call site reflects the same convention, and document the raw-vs-data contracts on the readBuffer() and sendCommand() declarations. No functional change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * SEN5X: use named defines for I2C reply buffer sizes Follow the SEN5X_VOC_STATE_BUFFER_SIZE pattern for all reply reads, per review feedback: define each reply's payload size in data bytes, size the destination buffer with it, request <size> + <size> / 2 raw bytes, and compare the received count against the same define. The version and product-name guards now compare against the full reply size rather than the bytes parsed (previously 7 and 5); readBuffer() returns either 0 or the full data count, so the conditions accept and reject the same transfers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * SEN5X: document I2C helper size requirements instead of checking at runtime Per review feedback: drop the runtime even-size and multiple-of-3 checks from sendCommand()/readBuffer() and state the requirements in @brief/@param documentation on the declarations. All callers pass sizes derived from the SEN5X_*_BUFFER_SIZE defines, which satisfy both requirements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * SEN5X: name the sensor's invalid-value constants Per review feedback, define SEN5X_UINT_INVALID (0xFFFF) and SEN5X_INT_INVALID (0x7FFF) for the values the sensor reports when a reading is unavailable, and use them in the readValues()/readPNValues() conversions in place of the numeric literals. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: oscgonfer <oscgonfer@users.noreply.github.com> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-12 06:56:46 +00:00
Andrew Yong	f960c84f5b	fix(stm32wl): reset instead of hanging on faults (#11420) ... * fix(stm32wl): reset instead of hanging on faults Reset instead of hanging forever on three unrecoverable faults, each of which previously required a manual power cycle to recover: - HardFault_Handler_C: blinked SOS forever with no debugger attached; now resets once the fault registers are printed. - __wrap___assert_func: silently hung on an assert failure; now prints file/line/func/expr via debug_printf, then resets. - earlyBootCheck: silently hung if the jump into the bootloader ROM failed to take; now calls the bare NVIC_SystemReset(), not the HAL wrapper, since it runs pre-HAL_Init() and MSP/VTOR are already repointed at the bootloader by this point, so a return would unwind through a corrupted stack frame. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * refactor(stm32wl): group fault-handling code Group the fault-handling code together and drop incidental cruft: - Move __wrap___assert_func next to HardFault_Handler_C and the other fault-reporting helpers. - Add banner comments separating linker-hack wrappers from fault-handling/recovery code, matching the existing Bootloader redirect banner. - Drop the forward declaration for debug_printf, no longer needed now that __wrap___assert_func sits below its definition. - Trim the Bootloader redirect banner comment to 1-3 lines. No behavior change. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> --------- Signed-off-by: Andrew Yong <me@ndoo.sg>	2026-08-12 06:29:52 +00:00
Jonathan BennettandClaude Fable 5	5baad2e2a8	logging: compile out LOG_TRACE by default, demote chatty DEBUG lines, drop redundant logs (#11391) ... * logging: gate LOG_TRACE behind MESHTASTIC_TRACE_LOGGING, drop redundant reclock logs LOG_TRACE now compiles out by default so trace-level diagnostics cost no flash; enable with -DMESHTASTIC_TRACE_LOGGING. Portduino keeps it on for the traceFilename packet-trace feature. Remove the 66 caller-side I2C reclock/restore log lines in the telemetry sensors: ReClockI2C::setClock/restoreClock already log both frequencies internally (now at trace level, since they fire every sensor read). Also unify near-duplicate literals (colon/case/punctuation variants) so linker string dedup applies, and drop an information-free bare 'done'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: demote chatty per-packet/per-poll DEBUG lines to trace level With LOG_TRACE compiled out by default, per-iteration chatter (packet bookkeeping, sensor poll values, e-ink refresh reasons, GPS pin states, UI runState traces) now costs no flash on device builds while remaining one -DMESHTASTIC_TRACE_LOGGING away. 108 lines demoted, 4 information- free lines removed; failure paths, drop reasons, and one-time init logs all stay at debug level. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: address CodeRabbit review on trace-gate PR - GPS: pass serial-derived buffers as %s args, never as format strings (untrusted bytes could contain % directives) - 0x%08x for packet id / NodeNum per convention (Router, CannedMessage, NeighborInfo); unsigned casts for size_t args; %u for uint32_t delta - EInk: async full-refresh begin/complete back to DEBUG (rare state transitions); per-frame SKIPPED lines stay trace Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: gate trace on the flag's value, not its presence -DMESHTASTIC_TRACE_LOGGING=0 previously *enabled* trace logging because the gate tested definedness. The flag now defaults per-platform (portduino 1, else 0) and both backends test the value, so =0 disables, =1 or a bare -D enables. Also cast tx_after-millis() to uint32_t for %u (millis() is unsigned long on native). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: clang-format rewrap after specifier widening Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * Even fewer bytes! * logging: keep compile-gated debug lines at debug level; fix native-suite-count Lines already inside default-off #ifdef blocks (GPS_DEBUG, DEBUG_LOOP_TIMING) cost no flash and should stay visible at debug level when their gate is enabled, rather than also requiring MESHTASTIC_TRACE_LOGGING. test/native-suite-count lags the two test_event_channel_* suites added by #11045 (develop's Native Suite Count check has the same mismatch); bump 46 -> 47. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * gps: route GPS_DEBUG diagnostics through a LOG_DEBUG_GPS() macro (#11414) Replaces 27 log-only #ifdef GPS_DEBUG blocks across GPS.cpp, PositionModule, MeshService, and GPSStatus.h with a single-line LOG_DEBUG_GPS() call (src/gps/GPSLog.h, modeled on LOG_MIGRATION: value-gated, ((void)0) when off). Blocks containing declarations, control flow, hexDump, or nested conditionals keep an explicit '#if GPS_DEBUG' guard. RTC.cpp's per-reading raw time dumps and per-candidate rejection chatter fold under the same gate; quality transitions and boot-time seeding stay at debug. Also fixes the '// define GPS_DEBUG' missing-# typo in two variant headers and updates all seven commented examples to the value form ('#define GPS_DEBUG 1') required by the value-based gate. Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 Co-authored-by: Claude <noreply@anthropic.com> * gps: declare RTC gmtime result as pointer to const (cppcheck) With the setTime debug dump gated behind GPS_DEBUG, all remaining uses of t are reads; cppcheck (constVariablePointer) now flags it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-12 00:05:51 +00:00
Ben MeadorsandJonathan Bennett	87a009da63	fix(nrf52): LTO was dropping the board variant's weak hook overrides (#11415) ... * fix(nrf52): LTO was dropping the board variant's weak hook overrides Whole-image LTO (enabled arch-wide for nrf52840 in #10655) inlines the empty weak body of earlyInitVariant()/lateInitVariant()/variant_shutdown()/ variant_nrf52LoopHook()/variantDefault*Config() at the call site, because the weak default and the call site live in the SAME translation unit. The strong override in variants/<arch>/<board>/variant.cpp is then never linked, and the board's hardware setup silently does not run. nrf52_lto.py's -fno-lto variant recompile does not help here: the caller is the problem, not the variant object. Needs both ingredients, so this only affects 2.8: the earlyInitVariant() indirection landed in #9438 and is present in v2.7.26 too, but v2.7.26 has no -flto, so the override linked normally. Found on the muzi R1 Neo, whose earlyInitVariant() drives DCDC_EN_HOLD (P0.13, the DC-DC hold after the user button) and NRF_ON (P0.29, "tells IO controller device is on"). Both were dropped from the image, so the companion MCU never saw the nRF application come up and stayed in its DFU indication (purple LED). Verified in the ELF: pre-fix setup() runs straight from waitUntilPowerLevelSafe() to the LED_NOTIFICATION block with no earlyInitVariant symbol in the binary and no pinMode/digitalWrite on P0.13 or P0.29 anywhere; post-fix it calls the real override. HW-confirmed on an R1 Neo. Also affected on nrf52840: earlyInitVariant() on 10 variants (incl. t-echo-card, which sequences its RT9080 3V3 rail there), variant_shutdown() on 18 variants (t114, t-echo, ThinkNode M1-M8, meshlink, wio-tracker-L1 ... - sleep pin parking, so deep-sleep leakage), variant_nrf52LoopHook() on 3 RAK variants. Confirmed dropped on heltec-mesh-node-t114 by build, not just by inspection. Fix is __attribute__((noinline)) on both the weak declaration and definition - the same guard already carried by loopCanSleep(), preFSBegin(), PowerHAL and variant_enableBatteryLpcompWake(), whose comment in main-nrf52.cpp already documents this exact failure mode. Also extend _VARIANT_OVERRIDES in extra_scripts/nrf52_lto.py from just _Z11initVariantv to all eight hooks. That post-link guard already had the right logic and would have caught this on every PR - it simply was not listing Meshtastic's own weak variant hooks, only the core's. With the list extended it goes red on both r1-neo and heltec-mesh-node-t114 when the noinline is reverted, and green with it. Its failure message now names both possible causes. * review: trim the noinline rationale comments to two lines Per AGENTS.md ("keep code comments minimal - one or two lines, max"), the incident detail and extended background belong in the PR description, not the source. Keeps the LTO/noinline rationale and the pointer to the guard. --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>	2026-08-11 22:57:45 +00:00
Carlos ValdesandJonathan Bennett	204f88ddfe	fix(nrf54l15): restore the nrf54l15dk build (#11410) ... * fix(nrf54l15): restore the nrf54l15dk build Three unrelated faults stacked up, so the env has not built from a clean cache for some time. All three were diagnosed in July but never committed. Pin framework-zephyr to 3.40201.251021 (Zephyr 4.2.1). Seeed's platform script only maps their own seeed-xiao-* board ids to a package; any other board -- ours included -- falls back to whatever platform.json declares as the default, which is now Zephyr 4.4.0. Its west manifest pulls a CMSIS_6 whose cmsis_gcc.h calls the ACLE builtins __sxtb16/__sxtab16, and none of the GCC ARM toolchains PlatformIO ships (8.2.1/9.2.1/9.3.1) declare them in arm_acle.h. In C that is only an implicit-declaration warning; in C++ it is a hard error. So a fresh cache silently breaks the build even though nothing in the tree changed. Guard the MMC5983MA case in MagnetometerThread with __has_include. The switch arm constructs MMC5983MASensor unconditionally, so any env whose libdeps lack SparkFun_MMC5983MA_Arduino_Library fails with "expected type-specifier before 'MMC5983MASensor'". Add Print::availableForWrite() to the nrf54l15 Arduino shim. The shim declares flush() but not availableForWrite(), which StreamFrameWriter calls -- so it went unnoticed until that code landed. Verified: clean build of nrf54l15dk from an empty package cache, SUCCESS in 16:01, FLASH 39.04% (570804 B of 1428 KB), RAM 65.65%. The three had never been exercised together -- a previous run with only the pin applied got 17:30 in before hitting the other two. * review: collapse the pin rationale to one repo-local comment The block was pasted twice, and both copies pointed at a note that does not exist in this repository. Kept one, and only the part a reader here can act on: why the fallback happens, and why it is a C++ error rather than the warning the pure-C Zephyr core gets away with. --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>	2026-08-11 20:15:05 +00:00
Jonathan Bennett	d765bd99ca	Fix the all-zero MAC address for own node (#11409) ... * Fix the all-zero MAC address for own node * Increment native suite count from 46 to 47 * Fix condition for copying MAC address in PhoneAPI	2026-08-11 19:28:25 +00:00
Jonathan BennettandBen Meadors	9cc1ff99f5	The position block is for a single event, so disable by default (#11411) ... Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-08-11 18:16:15 +00:00
oscgonferandcoderabbitai[bot]	97f8361867	Correct for awake time in AQ telemetry (#11404) ... * Correct for awake time in AQ telemetry * Minor typo on debug log * Avoid updating start of cycle twice Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fix log type * Only update ahead of time if successful transmit --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>	2026-08-11 16:28:47 +00:00
oscgonferandThomas Göttgens	cb557d89a6	Multi one wire measurements (#10192) ... * First version of DS248X bridge * Add first iteration of DS248X sensor * Supports single readings on DS2484 * Supports readings on ch0 for DS2484_800 * Detection of variant for DS248X * Minor fix on retries for sensor init * Allow multiple channel detect passes on 8-ch version * Always read temperature via ROM matching * Small comment to show how to send all channels * Minor logging changes * Prevent one-wire double definitions * Detect ROMs per round * Fix comment * Prevent skipping on DS2482 ALT3 check * Fix comment (again) * Fix style checks * Remove comment for multiple measurements * Add multi-sensor measurements for one-wire sensors using wildcard message * Move to unpacked measurements in one-wire * Add admin command to set main temperature in 8-channel one-wire bridge. * Fix merge ref * Trunk fmt * Remove unused variable --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-11 16:28:40 +00:00
Thomas Göttgens	5d04f86af3	fix(Radio): reject bogus coding rate and length readbacks on RX (#11408)	2026-08-11 18:28:04 +02:00
4 people	546b9d9e40	Block coordinate traffic on configured event channels (#11045) ... * Block coordinate traffic on configured event channels Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Suppress event coordinates in reliable relay paths Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Reject blocked phone coordinates before rate limiting Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Prevent event coordinates from reaching MQTT Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Add event coordinate policy preference Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Test event coordinate policy in native CI Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Make event policy test tolerate a full NodeDB Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Test Router event coordinate enforcement Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Test PhoneAPI event coordinate retry handling Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Test reliable event coordinate suppression Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Test MQTT event coordinate suppression Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Run event policy behavioral suites in native CI Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * tests: address CodeRabbit review feedback - test_event_channel_phone_api: complete the setUp/tearDown save-restore pair. GlobalState now carries cryptLock and myNodeInfo; setUp() nulls cryptLock before constructing MockRouter (Router's ctor asserts it is unset), and tearDown() restores both so the suite leaves no global mutated. Not reachable today - the globals start null in this binary - but the pair was asymmetric. - Replace the strcpy calls this branch added on Channel.settings.name (char[12]) with the bounded form the rest of the test tree already uses, strncpy(dst, src, sizeof(dst) - 1). Covers the flagged site in test_nexthop_routing plus the six equivalents in test_event_channel_phone_api, test_mqtt and test_position_precision, which trip the same ast-grep dangerous-buffer-functions-cpp rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-11 12:34:08 +00:00
Quency-D	fb17f6ddbe	Add LoRa FEM LNA toggle menu support (#11341)	2026-08-11 12:05:30 +00:00
github-actions[bot]andcaveman99	c6a20811b2	Update protobufs (#11406) ... Co-authored-by: caveman99 <25002+caveman99@users.noreply.github.com>	2026-08-11 14:56:11 +02:00
Thomas Göttgens	bcf486fa8b	fix(Power): survive a BQ27220 fuel gauge that fails to init (#11401) ... * fix(Power): survive a BQ27220 fuel gauge that fails to init Keep the BQ25896 as the battery source when only the gauge fails, so Power stays enabled instead of falling through to an ADC that these variants do not have. Null-guard the gauge in getBattVoltage() and isCharging(). Retry the gauge from the power thread (3 attempts, 60s apart, address probe first) since it is soldered on, and reset the I2C master after a failed init so the bus scan does not run against a stale transaction. Fixes #11372 * fix(Power): address review feedback on the BQ27220 retry Derive "no attempt yet" from gaugeAttemptsLeft instead of a millis() zero sentinel, drop the zero-padding on the logged I2C address, and condense the new comment blocks.	2026-08-11 07:00:01 +00:00
Jonathan BennettandClaude	b8dee13d0b	fix(BaseUI): never render banner font tags ([S]/[M]/[L]) as literal text (#11392) ... The nRF52 BLE pairing banner sends "Bluetooth\nPIN\n[M]<pin>" where [M] is a medium-font marker for the PIN line. The renderer only honored font tags for lines covered by the parsed-line cache, so any path that draws a banner line from the raw message - a stored-without-reparse banner, or a draw racing the parse from the BLE task - showed a literal "[M]" ahead of the pairing PIN. Extract the per-line text/font decision into resolveBannerLine(): parsed (tag-stripped) line when the cache covers it, otherwise strip a leading font tag from the raw line on the fly and honor the font it names. Picker content (e.g. node names) is deliberately exempt - it can be user data and must never be tag-interpreted. Also measure line widths on the text actually rendered (they were measured on the raw, un-stripped string, skewing box width and centering), and check p[1] for NUL before reading p[2] in the tag probe, which could read one byte past the end of a line that ends in '['. Add a native test suite (test_banner_font_tags) covering tag parsing, the BLE pairing message, the cache-miss fallback, and the picker exemption. Claude-Session: https://claude.ai/code/session_01CFMdkE6d8fY28Ax3hFVDMU Co-authored-by: Claude <noreply@anthropic.com>	2026-08-11 00:06:00 +00:00
Giacomo Di Ciocco	fa87730e7a	fix(rak4631_eth_gw): handle multicast socket exhaustion (#11132)	2026-08-10 22:49:40 +02:00
Thomas Göttgens	b1132cfec9	fix static IP on wired ETH (#11385) ... * fix https://github.com/meshtastic/firmware/issues/11370 * address coderabbit review	2026-08-10 12:39:01 +00:00
Thomas Göttgens	80d16234e5	fix(lora): skip DIO detach when no ISR is attached (#11386) ... * fix(lora): skip DIO detach when no ISR is attached Fixes #11371 * fix(lora): latch the ISR-armed flag instead of tracking attach state The flag is now written once from task context and only read from ISR context.	2026-08-10 12:39:25 +00:00
Jonathan BennettandClaude	512154ff97	feat(BaseUI): show 'GPS Time Only' when GNSS has time but no position fix (#11361) ... The position frame's drawGpsCoordinates() only distinguished 'No GPS present' / 'No GPS Lock' / coordinates, so a GNSS that had decoded valid time but no fix displayed identically to a cold chip. - GPSStatus: add per-acquisition hasTime flag (5th ctor param, accessor, matches() term, updateStatus() copy) - GPS::runOnce(): publish immediately on the gotTime rising edge so the flag reaches observers on the time-only path, which previously never published; done directly rather than via the end-of-loop block so fixHoldEnds is preserved and hold/power behavior is unchanged. Safe without a location: PositionModule ignores invalid positions. gotTime is already cleared on each GPS_ACTIVE entry, so the state is not sticky across acquisitions. - UIRenderer::drawGpsCoordinates(): the 'No GPS Lock' line becomes 'GPS Time Only' when time is valid. The drawGps() header renderer is intentionally untouched (its branches need separate de-clobbering work). Claude-Session: https://claude.ai/code/session_01CcrasD4QsatunDreANDgCx Co-authored-by: Claude <noreply@anthropic.com>	2026-08-10 02:39:40 +00:00
3 people	2e958821f9	fix(BaseUI): suppress new-message pop-up while a menu is on screen (#11373) ... Menus share the single global banner slot with notification pop-ups (Screen::showOverlayBanner overwrites message, options, and callback unconditionally), so a "New Message" banner arriving mid-menu destroyed the open menu and stole its input. Add NotificationRenderer::isMenuShowing() — true when the active overlay is interactive (a menu with options, or any picker/keyboard/pairing-PIN type) rather than a plain text banner — and skip the new-message banner in handleNewMessage() while such an overlay is up. Screen wake and hasUnreadMessage behavior are unchanged, and a new message can still replace an earlier plain banner. Claude-Session: https://claude.ai/code/session_01PSAouemtAihV5P87AgCadu Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jason P <applewiz@mac.com>	2026-08-10 02:30:37 +00:00
4 people	cd716fe384	logging: audit log strings for terseness, reclaiming ~6.8 KB of string data (#11374) ... * logging: strip redundant punctuation, level prefixes, and 'successfully' from log strings The logger already appends a newline and prints the level tag, so trailing '.', '!', '...', literal \n, and 'Error:'/'Warning:' prefixes inside format strings are wasted flash bytes. Same for 'successfully' (the affirmative form already implies it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: tighten verbose log strings in modules, radio, platform, and system code Rewrite wordy log messages to terser equivalents - drop filler words (articles, 'attempting', 'due to', 'please'), use 'Can't X'/'X failed' phrasing, and abbreviate where the codebase already does (config, init, msg, BT). Format specifiers and argument lists are unchanged; distinctive greppable tokens are preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: tighten verbose log strings in telemetry sensors and GPS Same terseness pass: drop filler, 'Can't X'/'X failed' phrasing, common abbreviations (temp, msg). Specifiers, arguments, and sensor-name prefixes unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: tighten verbose log strings in mesh core Same terseness pass over NodeDB, Router, MeshService, PhoneAPI, RadioInterface, NextHopRouter, and PacketHistory: 'X failed'/'Can't X' phrasing, imperative verbs, dropped filler. Specifiers and arguments unchanged; duplicate literals kept identical to preserve linker string dedup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: 'Unable to/Could not/Cannot' -> "Can't" in log strings Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: clang-format rewrap after string shortening Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * logging: restore boot-logo trailing newline and progress-dot strings The terseness pass over-trimmed: the Meshtastic ASCII boot logo kept its blank line via a trailing \n, and three bare "." progress ticks were reduced to empty strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1 * Update src/mesh/wifi/WiFiAPClient.cpp Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * logging: address review feedback on the terseness audit - Node/packet IDs use the repo's 0x%08x convention in NextHopRouter, NodeDB, AdminModule and CannedMessageModule. The sibling log in each if/else pair is converted too, so a pair isn't split across two formats. next_hop stays 0x%x - it's the last-byte relay hint, not a NodeNum. - RTC: the read-path and set-path "not found" warnings were byte-identical, so the linker deduped them and the log couldn't say which one fired. Split into "RTC read:" / "RTC set:". (The four sites live in mutually exclusive #ifdef branches, so the RTC family was never ambiguous.) - SCD4X getAmbientPressure()/setAmbientPressure() logged "altitude", and SCD30 getASC() logged "Can't send command" for a read. Both now name the operation they actually perform. - LOG_ERROR already carries the level: ". Error: %u" -> ", rc=%u" (matching the existing rc=%d house style) and "Error executing X()" -> "X() failed". - Typos and wording: "OTA partiton. (Reason" -> "OTA partition (reason", "CST3530 not response ~" -> "CST3530 no response", "Packet received with to: of 0" -> "to=0", HostMetrics "Error decoding" -> "Can't decode", and the dangling ": " on the NextHopRouter retransmission line. Printf specifier sequences are byte-identical on all 37 touched lines apart from the 6 deliberate %x/%u -> %08x node-ID widenings, all on uint32_t args. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-08-10 01:02:19 +00:00
Tom	de6b23190a	Test suite rebuild (#11322) ... * docs(nodedb): make the native node cap unambiguous The native node cap was stated in four places that disagreed, and the disagreement already caused a wrong diagnosis: a saturated 200-node database looked arithmetically impossible because the cap had been read as 248, computed from a header that does not apply on this platform. The real value is 198. On portduino MAX_NUM_NODES is not a compile-time constant at all - the variant defines it as `portduino_config.MaxNodes`, resolved at runtime, default 200 and settable per host with `General: MaxNodes`. variant.h is reached before mesh-pb-constants.h, so that header's ARCH_PORTDUINO branch never fires and its plausible-looking 250 is dead code. - #error-guard the dead branch rather than leave a wrong number where people grep. The guard found a real defect: seven translation units reach mesh-pb-constants.h without configuration.h (SerialConsole.cpp, StreamAPI.cpp, PacketAPI.cpp, ServerAPI.cpp, PiWebServer.cpp, ServiceEnvelope.cpp, MeshtasticOTA.cpp, and test/TestUtil.cpp), so each was compiling with a different MAX_NUM_NODES - and therefore a different PACKETHISTORY_MAX - than the rest of the build. Each now includes configuration.h first. It cannot be included from mesh-pb-constants.h itself: that reaches SerialConsole.h through DebugConfiguration.h and closes a cycle. - Name the bare 250 in getMaxNodesAllocatedSize() NODEDB_MIGRATION_LOAD_CEILING. It is a decode allowance for files written by larger-cap firmware, not a cap, and it read like one. - Fix docs/node_info_stores.md, which named the wrong source and a "10-250" range that is wrong for native, and the copilot-instructions tunables line that said "portduino 250". * test(harness): give each suite its own scratch HOME and report leftovers Native suites shared one directory. Every suite that constructs a NodeDB loads and saves ~/.portduino/default/prefs/ - nodes.proto, config.proto, channels.proto, module.proto, device.proto, warm.dat, transmit_history.dat - and nothing cleared it, so state leaked suite -> suite within a run and run -> every run after it. A test run could also rewrite a real meshtasticd node database on the same machine. Per-run isolation does not fix this: the leak is generated inside a single run, so the boundary has to be per suite. bin/pio-test-isolate.sh runs each suite in its own scratch $HOME, registered as test_testing_command for env:native and env:coverage so a bare `pio test` and CI get the same boundary, not just bin/run-tests.sh. It runs the binary unchanged and exits with its exit code, so PlatformIO's pass/fail is untouched. Overriding HOME here rather than around `pio` also sidesteps the blocker that a bare HOME= breaks pio's own ~/.platformio/penv/bin/pio lookup. Leftovers are reported as a second axis, PASS/FAIL x CLEAN/DIRTY, because an unintended write has no matching assertion by definition - nobody writes TEST_ASSERT for a save they do not know is happening. The harness asserts it from outside, so it applies to every suite without the author opting in. - Only the *set of changed paths* is asserted, never contents. Hashes answer the boolean "did this change?" and nothing more; content baselines over protobuf bytes would churn on every NodeInfoLite field added, which is how snapshot suites become noise. - Deliberate writes are declared in test/state-manifest.tsv - one central file, suite / flags / mandatory reason. run-tests.sh prints the opt-out count on every run. - Granularity follows the state flag, so the two ship together: per-test by default (TestUtil redefines RUN_TEST to checkpoint after each test, naming the exact test that dirtied things), suite boundary for state=per-suite, where carrying state across test cases is the declared behaviour. - A declared write that does NOT happen is reported as MISSING, not folded into DIRTY. It catches silently broken persistence; a warning for now, since some are conditional. - Graded AMBER, not RED. With isolation in place DIRTY means "undeclared", not "dangerous", and a check that lands red on day one gets switched off. Guard the guard, both halves: state_assert_empty() refuses to run a suite against a sandbox that is not empty (otherwise the after-diff measures against the wrong baseline and reports CLEAN while meaning nothing), and bin/test-state-check.sh drives the real wrapper with fixtures asserting CLEAN / CLEAN / DIRTY / MISSING plus both directions of the empty assertion. A checker that silently matches everything would otherwise pass forever. --write-manifest proposes entries for a human to paste and justify; it never applies them, and neither does CI. * test(harness): stop reporting Unity's exit code as a signal A native suite ends in exit(UNITY_END()), and UNITY_END() returns the failure count. PlatformIO's native runner reads that non-zero exit code as a POSIX signal number, so four failures print "Program received signal SIGILL", five print "SIGTRAP", and the suite is classified [ERRORED] rather than [FAILED]. There is no crash. The signal name tracks the failure count and nothing else - it moved SIGILL -> SIGTRAP when a diagnostic probe added a fifth failure - and it cost hours of hunting a memory bug that did not exist, on an env (native) that carries no sanitizer at all. It also explains the phantom extra test case in the totals: the runner adds a synthetic entry for the signal it thinks it saw. run-tests.sh now says so inline whenever a signal line appears, and the three agent-facing docs say it too. * test(admin): isolate NodeDB and globals per test setUp() did `if (!nodeDB) nodeDB = new NodeDB();` and never deleted it, so 83 of the 85 tests shared one never-reset database and never restored config, owner, devicestate or channelFile. The fixture that does restore them was opt-in and armed by exactly two tests. The setUp comment claiming the rest "set their own config/region state and are unaffected" was not true - the admin handlers under test write all four globals. Route every test through the fixture instead: setUp saves the globals and installs a fresh NodeDB, tearDown restores and deletes it. The two tests that armed it themselves no longer need to. All 85 pass, so nothing was silently relying on the shared state. It costs about 7% of the suite's runtime (a NodeDB construction is a loadFromDisk plus, with a region set, key generation) - worth paying to write the phase 3 tests against a clean fixture rather than 83 tests' residue. Also cap the per-test attribution in the run summary at five entries; the full list stays in the suite's sandbox. * test(fs): cover the bounded file-manifest walk getFiles() runs on every phone sync via STATE_SEND_FILEMANIFEST, and nothing asserted any of its bounding behaviour. It does execute unasserted from test_stream_api's handshakes, but the cap, the depth limit, the wasLimited paths, overlong-path rejection and capacity release were all unguarded. Eight tests, all describing what the code does today: today's code is already correct here, since #10778 landed the by-reference collectFiles(), the 64-entry cap, the strlcpy bounds and the swap-idiom release. They pass on arrival, which is the point - this is the baseline a later change has to leave alone. Two things they do not cover, and cannot: - Moving reserve() outside the __cpp_exceptions guard. Exceptions are on natively, so the #else branch is not compiled. The suite's job there is to prove that change alters nothing observable. - The file.name() null guard. No in-tree backend returns null; the guard is defensive. The manifest-release test pins the swap idiom rather than calling PhoneAPI's releaseFilesManifest(), which is file-local. It asserts capacity() == 0, not just size() == 0 - a size-only check passes on clear(), which is the bug #7924 shipped. Suite count 43 -> 44, recounted against the directories rather than copied. * test(admin): assert node-DB metadata saves skip the radio reload set_favorite_node, set_ignored_node and toggle_muted_node each persist a NodeInfoLite bit and nothing else. MeshService::reloadConfig() gates its region re-derivation and configChanged notification on saveWhat & (SEGMENT_CONFIG | SEGMENT_CHANNELS), so a SEGMENT_NODEDATABASE-only save already skips the live radio reconfigure. Pure characterization - all three pass on develop. Worth pinning because that reconfigure is the path implicated in the WisMesh Tag favourite-node crash, and develop asserts nothing about it: widening the saveWhat mask or reordering the check would currently go unnoticed. Ported from the config-save series along with ConfigChangedCounter (an Observer<void *> counting configChanged notifications, the only externally visible signal that the reload branch was taken) and TEST_NODE_NUM. They join the existing suite, so no suite-count change. * refactor(menu): extract the mute toggle into a named function The node menu's mute action was inline in a banner-callback lambda, and that lambda only ever runs via screen->showOverlayBanner() - which is why nothing in MenuHandler.cpp was reachable from a test. Lift the `selected == Mute` branch into menuHandler::toggleNodeMuted(uint32_t) and call it from the lambda. Behaviour-neutral by construction: same statements, same order, same bare saveToDisk(). The null check moves into the function, so the call site no longer needs its own lookup. Verified by the native build and suite; the byte-identical-image check on a headroom-constrained nRF52 board was not run locally - CI's firmware-size comment covers it. Three tests come with it, all describing today's behaviour: - the bit flips both ways and no configChanged fires (develop never calls reloadConfig on this path); - an unknown node is a no-op rather than a write; - and the segment mask. Flipping one NodeInfoLite bit currently rewrites all five segments via bare saveToDisk(). That is asserted deliberately, with the comment naming it as characterization of a known defect: a pending fix narrows it to SEGMENT_NODEDATABASE, and when it lands this assertion is expected to change, which makes the improvement visible in the diff instead of silent. saveToDisk() is not virtual, so the mask is observed through its effect - remove the five prefs files, toggle, and see which reappear. * docs(test): make every suite count a pointer to the canonical one test/native-suite-count is the registered total and is machine-checked against test/test_* on every full run and by the suite-count-check CI job. Every other statement of the count is a copy that drifts: copilot-instructions said 12, AGENTS.md said 19, and the real number is 44. Replace both literals with a pointer to the file, say explicitly that no document should state the count as a literal, and reframe the two suite listings as descriptions rather than inventories - they carry per-suite information the count does not, so they stay, but nothing should infer completeness from their length. Register the new FS suite in both. * test(harness): randomise suite order, reproducibly Landed last, deliberately. Randomising an order-dependent suite set does not find bugs so much as convert a silent pass into intermittent red, and the first instinct is to revert the randomisation rather than fix the coupling. Phases 1-2 removed the coupling; this keeps it removed. Both runners previously hid order dependence behind a fixed order that happened to differ between them, and neither order was chosen: CI's area rules put admin first, PlatformIO's local discovery is reverse alphabetical and put it last. CI was green by accident. - bin/run-tests.sh --shuffle / --seed <n>. The seed defaults to HEAD's short SHA: one order per commit, so a red is replayable and attributable to the diff instead of flaky, while the project keeps exploring orders. Printed at the start and carried into the RESULT line, so a verdict is replayable from that line alone; the full order is printed on failure, because for an order-dependent failure the order is the diagnostic. - The shuffle is a Fisher-Yates over a MINSTD generator rather than awk's rand(), whose sequence differs between gawk and mawk. A seed that does not reproduce the same order on another machine is not a seed. - Shuffling needs one `pio test -f <suite>` invocation per suite - PlatformIO orders by its own os.walk() over test/ and filters only select - which measures at about 4.7s per suite of extra startup. - CI shuffles its area order, seeded from GITHUB_SHA and printed with the command to replay it locally. Intra-area order stays PlatformIO's; controlling it there would mean per-suite invocations, which is a cost worth deciding separately. Also records the 16 measured entries in test/state-manifest.tsv, each with its reason, taken from a full run's --write-manifest output rather than guessed. * test(default): cover the region-throttle interval overload getConfiguredOrDefaultMsScaled(configured, default, nodes, TrafficType) is the overload every telemetry and position module actually calls, and nothing referenced TrafficType anywhere under test/. All four of its behaviours were unguarded: the no-region guard, the throttle <= 1 short-circuit, the multiply, and the 64-bit overflow clamp. The throttles are real, not hypothetical - EU_866 carries PROFILE_LITE, which sets both positionThrottle and telemetryThrottle to 10, so a change here moves broadcast spacing in that region by an order of magnitude. Each test pins numOnlineNodes at the congestion threshold and uses ROUTER, which never congestion-scales, so the coefficient is 1 and the throttle is the only variable. The overflow case needs a base above INT32_MAX/10, hence three days rather than one. * ci(test): keep pull-request suite order fixed, seed the rest Shuffling the area order on every run - including pull_request - would turn a contributor's PR red for an ordering they did not choose, which is how a randomisation gets reverted instead of the coupling being fixed. That is the exact dynamic the ordering work was sequenced last to avoid, and the previous commit walked straight into it. - pull_request keeps the fixed declared area order. - push and schedule shuffle, seeded from the commit SHA: deterministic per commit, printed, attributable, and never blocking someone else's PR. - A suite_order_seed input on workflow_call and workflow_dispatch overrides both, so a specific failing order can be replayed anywhere, including on a PR. The run log prints which mode it took, the resulting order, and the local command to replay it. * ci(test): satisfy CKV_GHA_7 and yamllint on the seed input The seed is reachable through workflow_call, which callers can pass programmatically. The workflow_dispatch copy tripped checkov's "workflow_dispatch inputs MUST be empty" rule, and suppressing it was not worth it: replaying a specific order is a local operation, and the run log already prints the exact bin/run-tests.sh command to do it. * style(menu): apply the node-ID format convention RadioInterface.cpp documents the rule: 0x%08x in logs, !%08x in user-facing display. MenuHandler held every remaining exception - seven logs printing bare %08X, and two display labels doing the same. Repo-wide there are now no bare %08X node IDs left in log calls. * ci(test): pass workflow inputs through env, not shell interpolation suite_order_seed and github.event_name were spliced into the run: script as ${{ }} text, so a value carrying shell metacharacters would execute as code on the runner rather than being read as data. semgrep (run-shell-injection) and zizmor (template-injection) both flag it. Both now arrive as environment variables and are read as "$VAR". * refactor(test): share the seeded shuffle between the harness and CI bin/run-tests.sh and test_native.yml each carried a byte-identical copy of the MINSTD Fisher-Yates awk. The workflow prints "replay locally: ./bin/run-tests.sh --shuffle --seed $seed" after a shuffled CI run, and that instruction is only true while the two agree - drift would be announced by a replay quietly reproducing a different order than the one that failed. Extract shuffle_suites() to bin/lib/shuffle.sh and source it from both. Permutations verified identical across seeds before and after the move. * fix(test): correct the shared-state MISSING check and summary join Three defects in the new harness: state_classify() matched declarations two different ways - state_path_declared() for "undeclared", a hand-rolled regex for "missing". Interpolating an entry into an ERE also let a metacharacter in a manifest name match a file that is not the declared one. Both directions now go through the one helper. `paste -sd'; '` does not join with "; ": with -s, paste cycles through a multi-character delimiter one character per join, so paths rendered as "a;b c;d e". Replaced with an awk join. test-state-check.sh ran on after a failed cd instead of stopping (SC2164). ./bin/test-state-check.sh: 6/6 fixtures pass, MISSING included. * fix(portduino): bound General.MaxNodes MaxNodes was validated only for <= 0. Any positive value, including a typo'd or pasted-in one, propagates to MAX_NUM_NODES and scales both the node DB and the nodes.proto decode ceiling - failing at boot with no obvious cause. The ceiling is a sanity bound, not a capability limit; raise it if a host genuinely needs more. * docs(nodedb): reconcile the capacity tables The property matrix omitted the ESP32-S3 100-node flash tier that the platform table above it lists, and neither mentioned that the WASM build overrides MaxNodes to 80 in wasm_config_apply(). * fix(nodedb): make mesh-pb-constants.h self-sufficient on portduino The ARCH_PORTDUINO #error assumed it was unreachable in a normal build. It is not: the vendored device-ui sources include this header without configuration.h, which broke both native-tft docker builds. Include configuration.h here instead, ahead of every compile-time default - variant.h overrides MAX_RX_TOPHONE as well as MAX_NUM_NODES, so placing it lower in the file just moves the divergence to a redefinition. The #error stays as a backstop for the case where that include genuinely stops providing the cap. Verified with the native env's own flags: a TU including only this header now compiles, normal-order use of both macros compiles, and NodeDB.cpp compiles. * fix(portduino): raise the MaxNodes ceiling to 16000 Marked artificial: nothing in the node DB fails at 16001. 16000 sits just under the 16384 (128 x 128) population where HopScalingModule saturates its sampling denominator and starts dropping nodes, so a host inside the bound still gets meaningful hop recommendations. * lint(trunk): advise on node IDs logged as bare %08x RadioInterface.cpp documents the convention - 0x%08x in logs, !%08x in display - but nothing enforced it, which is how the MenuHandler cluster drifted. 22 call sites in PacketHistory, NodeInfoModule and PositionModule are still off it. A trunk linter rather than a CI grep job, because trunk checks changed files: new violations get flagged without a 22-site cleanup landing in an unrelated PR. Modelled on the existing too-many-defined definition. Scoped to values it can tell are IDs - an ID-shaped argument (->num, .from, getNodeNum) or message text naming one. A 32-bit hex that is not an ID is out of scope, so the CRC32 logs in ethOTA.cpp are correctly ignored. Emits "note", trunk's only non-blocking level: "warning" and "info" both exit non-zero and would gate CI, which is not what a log-format nit deserves. The pre-existing sites are line-scoped in the allowlist, so a new bad call in those same files is still caught. * lint(trunk): stop exempting the known node-id-format sites The seeded allowlist made the rule green by declaring the backlog acceptable. Empty it instead, so the 22 pre-existing sites are reported and get cleaned up by whoever next edits those files. Costs nothing to do: the rule emits "note", so these are non-blocking either way. The allowlist stays for its real purpose - a value the linter misreads as an ID. * style: log node and packet IDs as 0x%08x Clears the 22 sites the node-id-format linter reports, so the rule starts from zero rather than from a backlog nobody can see - trunk suppresses pre-existing findings by default, so left alone these would not have surfaced on edit the way an empty allowlist implies. Format strings only; no argument or control flow changes. The !%08x user-facing display forms are deliberately untouched - that is the other half of the same convention. * test(harness): build once up front, so suite timings mean something run-tests.sh fused build and run in a single pio invocation, so whichever suite PlatformIO's directory walk reached first absorbed the entire src compile and reported it as its own duration. On a real run that made a 0.03s suite report 13m21s, and hid the build cost from every other number in the summary. Do what .github/workflows/test_native.yml already does: one --without-testing build pass, then run with --without-building. Measured on a full 44-suite run - the build is now a single reported figure and 968 test cases execute in 1.9s, with no suite above 0.084s. Build output goes to its own log rather than $LOG: the outcome regexes match "error:" and "[ERRORED]", so a compiler diagnostic sharing that file would read as a test failure. Both red paths now keep the log they quote from. $LOG and the build log are mktemps the EXIT trap removes, so the three grepped lines were previously all anyone ever saw - and the cause is usually further up than the first [FAILED]. * test(harness): keep the run log on every red path bin/pio-test-isolate.sh already keeps a failing or DIRTY suite's sandbox and log under .pio/test-state/<suite>/. What was missing is the cross-suite view: $LOG is a mktemp the EXIT trap deletes, so run-tests.sh quoted three grepped lines from a file that no longer existed by the time anyone looked. Preserve it as .pio/build/<env>/test-failure.log from both red paths - including "no success summary found", which said "see log" while preserving nothing, and which is exactly the case where the build died before any suite ran and so left no per-suite sandbox either. Cleared at the start of every run, so a green run cannot leave a red one's log lying around looking current. * fix(test): report the real failure count on a shuffled red A shuffled run is one `pio test` invocation per suite, all appending to the same log, so the log carries one PlatformIO "N test cases:" summary per suite. verdict_red() took `tail -1`, which reports whatever the LAST suite did: a failure in suite 3 printed a "0 failed" summary from suite 44 directly under "RED - failures detected:". Sum the summaries instead. A single summary line - every unshuffled run - is passed through verbatim, so the familiar output is byte-identical. The patterns are passed to the awk helper as strings rather than /regex/ literals: awk evaluates a regex literal in argument position as `$0 ~ /re/`, so the callee would receive 0 or 1 and silently sum garbage. * fix(test): do not emit an empty suite name for an empty shuffle `printf '%s\n' "$@"` with no arguments still writes one empty line, and both callers read shuffle_suites through mapfile, so an empty suite list arrived as a single suite named "". Return before the printf when there is nothing to shuffle. * test(harness): state and enforce the Linux host requirement The native harness is a Linux tool: bash 4+ (mapfile), GNU coreutils and GNU find (-printf, md5sum, -executable). Most of that predates this branch - mapfile and both find predicates are already on develop - but none of it was written down, so the requirement was there to be discovered rather than read. Refuse to start on a non-Linux uname instead of degrading. On a BSD userland this would not fail cleanly: it would mis-hash the sandbox and mis-read the suite list, and still print a verdict. A state check that silently measures the wrong thing is worse than one that declines to run. Carrying a per-host fallback was the alternative, and it buys a second code path that nothing in CI exercises. bin/test-native-docker.sh already exists for macOS and non-Linux hosts, and the native-macos PlatformIO env is a build target for meshtasticd, not a test host - the isolation wrapper is registered for env:native and env:coverage only. Documented in the script header, test/README.md, and both agent docs. * fix(test): terminate every suite with exit(UNITY_END()) Two sites across two suites ended on a bare UNITY_END(). That ends the reporting, not the suite: setup() returns, the runtime goes on calling loop(), and the process runs forever. PlatformIO does not notice - it reports a suite from its Unity output, not from process exit - so the suite passes, the run goes green, and the binary stays resident. Thirteen of them had accumulated on one dev box, the oldest 19 hours old. The costs are quiet by construction: - the per-suite sandbox is deleted underneath a live process, so its CLEAN/DIRTY verdict describes what the suite had written when the harness stopped looking, not what it left behind; - .gcda coverage and LeakSanitizer's report both flush from atexit handlers, so a suite that never exits contributes no coverage and gets no leak check; - each survivor pins its own deleted 94 MB binary, which du cannot see. One of the two is the #else of an architecture guard, which is the easiest one to get wrong - it looks like there is nothing to clean up. test_mqtt has a correct exit(UNITY_END()) in its live branch, so a "does this file call exit() anywhere" check passes the file whole. test_serial had two more. develop's serial-config validation rework restructured that suite - the architecture guard is gone and both remaining branches now exit correctly - so this commit no longer has anything to change there; bin/lint-unity-exit.sh, added later on this branch, is what keeps it that way. test/README.md gets a section on it, since the skeleton showing the right shape had not stopped this happening. * test(harness): detect and reap suites that outlive their run A suite that never exits was invisible: PlatformIO reports a suite from its Unity output, so the run stayed green while the binary kept running. Two checks, because they fail differently. Runtime, in bin/pio-test-isolate.sh: the sandbox $HOME is mktemp-unique per suite, so any process still holding it is a survivor of that suite. Matching on the environment rather than a remembered PID identifies one whatever its parentage - a fork, a grandchild, a process already reparented to init - none of which a $! comparison catches. Reaped before the after-fingerprint is taken, so that fingerprint measures a tree nobody is still writing to, and so a run cannot leave processes accumulating on the host. Recorded as a sixth summary column and graded AMBER: the tests did pass, but the CLEAN verdict and the coverage were measured under a false assumption. Author-time, as bin/lint-unity-exit.sh, wired into trunk at "note" like node-id-format: every UNITY_END() must be wrapped in exit(). The rule is per occurrence, and that is the point - a file-level "calls exit() somewhere" check passes test_serial and test_mqtt, which have a correct one in their live branch and a bare one in the #else. Running it over the tree turned up test_mqtt, which the file-level pass had missed. It allows `int rc = UNITY_END(); ...; exit(rc)`, used by test_packet_signing to restore globals between the summary and the exit. That is where the rule gives ground: capturing and never exiting would leak and is not flagged. Flagging a correct idiom would push someone to "fix" working code. bin/test-state-check.sh gains a survivor fixture, asserting the wrapper both reports and reaps - a detector that only reports leaves the host accumulating processes, which is half the harm. 8/8. * fix(lint): make the unity-exit scanner statement-aware The rule judged one physical line at a time, which reports two kinds of correct code as bare: /* a comment that happens to mention UNITY_END() */ <- interior lines were never stripped exit( UNITY_END()); <- exit( and the macro never met On a probe of both, two of three findings were wrong. This is a note-level rule whose whole job is advice, and bin/lint-node-id-format.sh already says why that matters: a false positive costs more than a miss. One that cries wolf gets ignored, and the real finding goes with it. Carry /* ... */ state across lines and accumulate logical statements before testing, with a 12-line cap so one unclosed call cannot swallow the rest of the file - the same structure lint-node-id-format.sh uses, so the two custom linters in bin/ work alike rather than each having its own idea. Verified both directions: the develop-era sources still produce the same four findings, the fixed tree produces none, and a probe covering block-comment interiors, wrapped exit(), line comments, return UNITY_END() and capture-then- exit reports only the genuinely bare calls - including a complete block comment followed by real bare code on the same line, which the state machine has to keep live. Reported by CodeRabbit on #11322. * fix(lint): tokenise instead of pattern-matching, and self-test it Second round of review findings on the same scanner, all confirmed by direct test before changing anything. Six defects, one root cause: layered regexes cannot tokenise C++. False positives (correct code reported): - UNITY_END() inside a string literal read as code False negatives (real leaks missed): - a string containing "/*" opened comment state and swallowed later lines - greedy .* removed everything between two block comments on one line, taking a bare call with it - myexit(UNITY_END()) matched the exit() exemption as a substring - x == UNITY_END() and total += UNITY_END() matched the assignment exemption Replaced with a character-level scan carrying comment state, and token-bounded exemptions: exit must be a whole identifier, and the capture form must be a plain `=`. Raw string literals are still not modelled - there are none under test/, and delimiter tracking for a case that does not occur would be untested code guarding untested code, so it is documented rather than guessed at. Also drops the `return UNITY_END()` exemption. It only terminates from main(), there is no main() under test/, and from a helper it just returns a count. bin/test-lint-unity-exit.sh pins all fifteen cases, every false positive and false negative found in review among them. The rule has been wrong twice in a way that looked fine by inspection; it needed a self-test more than it needed another careful reading. Two further findings in the same review: - bin/run-tests.sh dropped PASSTHRU in shuffled mode, so `--shuffle -vvv` built verbosely and then ran quietly. The shuffled loop now forwards EXTRA_ARGS, which is PASSTHRU minus the -f pair it supplies per suite. - bin/run-tests.sh did not guard `cd "$ROOT_DIR"`. And one that did not reproduce: the survivor fixture's glob does find the pid file (verified with the lookup instrumented - the earlier failure was an artifact of running the script from /tmp, where SCRIPT_DIR cannot resolve). The assertion was still weak, because an empty pid took the "not running" branch and passed vacuously. It now fails if the pid was never recorded, and finds the file by search rather than assuming a directory depth. Reported by CodeRabbit on #11322. * fix(lint): report each UNITY_END occurrence at its own location The self-test only asked "did the linter say anything", so it could not have caught a wrong line, a wrong column, or a missing second finding. Fixtures now assert the exact diagnostics as line:col, and the first run of that assertion found two real problems. The caret pointed at the wrong occurrence. For `exit(UNITY_END()); UNITY_END();` the verdict was right but the column was 17 - the wrapped call - because the scanner stripped terminating forms out of the whole statement and then reported the first occurrence it had seen. Two bare calls on one line reported once. Judged per occurrence now, by looking back through whitespace at what wraps it, so both the count and the caret are right. That also needed a position map from strip_noncode(): removing a comment or collapsing a literal shifts every later column, and counting occurrences in the raw line does not recover it either - TEST_MESSAGE("... UNITY_END() ..."); UNITY_END(); has two occurrences in the raw text and one in the code. Four of the expected columns I wrote by hand were also wrong, off by one. The linter was right in every case; the assertions were not. They are computed from the fixture text now rather than pasted from output, because a baseline accepted from the tool it is testing asserts nothing. 17 fixtures, including the two-on-one-line case from review and its mirror. Reported by CodeRabbit on #11322.	2026-08-06 14:05:07 +00:00
Benjamin Faershtein	7d54c13ec0	fix(crypto): hash full size_t inputs (#11359)	2026-08-06 11:21:56 +00:00
Ixitxachitl	32c3b62ef3	fix(BaseUI): restore node list menu swallowed by module-frame SELECT dispatch (#11358) ... * Fix input event handling to correctly identify real module frames in the UI * Refactor input event handling comments for clarity on module frame validation	2026-08-05 18:44:58 +00:00
Ethac.chenandThomas Göttgens	08722da3d9	fix(lr2021): live LF↔HF reconfigure via full begin() (#11279) ... * fix(lr2021): full begin() on live LF/HF reconfigure.Avoid RadioLib -706 / assert when live-switching Sub-GHz ↔ LORA_24. * fix(lr2021): align band-hop begin() with init() robustnessMirror. RF-switch GPIOs, SPI/TCXO retries, and log CRC/RX-gain errors. * chore: trunk fmt LR20x0Interface bandHop line wrap * fix(lr2021): harden live band reconfigure (companion #1) * fix(lr2021): reject invalid freq in band-hop path select Require requestedMHz > 0 in isLr20x0BandHop, expose lr20x0ReconfigurePathfor FullBegin vs incremental selection, and extend native radio tests. --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-05 10:07:14 +00:00
HarukiToredaandThomas Göttgens	74b9f6ff02	BaseUI: Select Environmental Data Source (#11209) ... * Select Source * Fixes * remove per sensor name --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-05 09:55:18 +00:00
oscgonferandThomas Göttgens	b59cd58d75	Add HM330x PM Sensor (#11069) ... * Add HM330X PM Sensor * Update HM330X library * Bring reclock I2C to HM330x sensor * Fix probeHM330x for variants without AQ telemetry or telemetry in general * Remove old import * Bring back SHT2X from develop * Remove test comment and unused method in HM330X class * Reorder detection method. Add pending TODO for INA219 detection * Rework detection order and add INA219 register check --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-05 08:35:59 +00:00
3 people	e78b121d9f	Lr1121 tcxo optional tries xtal first, and get all my yamls in a row (#11215) ... * LR11x0: try XTAL before TCXO when oscillator type is uncertain On boards with TCXO_OPTIONAL, a TCXO-first attempt either hangs RadioLib's calibration wait forever on a bare/non-TCXO module (unpatched upstream), or costs a slow failed attempt before falling back even once that's fixed with a timeout. Measured on hardware: XTAL succeeds immediately on a bare module (~350ms) and fails fast and cleanly on a genuine TCXO module (~300ms, RADIOLIB_ERR_SPI_CMD_FAILED), so trying XTAL first is a strict improvement for hang-avoidance regardless of which oscillator is actually present. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * compacted * fix review comment * fix femtofox switches * correct the correction * 13 * 3s timeout * Treat SPI_CMD_TIMEOUT as an LR11x0 init failure The BUSY watchdog breaks RadioLib's wait, so the next bounded transfer returns SPI_CMD_TIMEOUT rather than SPI_CMD_FAILED. Only the latter was checked, so a watchdog-triggered failure fell through to getVersionInfo(), setRfSwitchTable() and startReceive() against an unresponsive chip. Also use Throttle::isWithinTimespanMs() for the watchdog's elapsed-time check instead of raw millis() arithmetic. * Drop the BUSY watchdog and probe XTAL before TCXO The watchdog bounded RadioLib's unbounded BUSY wait in LR11x0::config() by having LockingArduinoHal::digitalRead() report a stuck pin low exactly once. That let a TCXO-first attempt fail cleanly rather than hang, but it meant lying to RadioLib about a GPIO from a HAL shared by every radio driver. Ordering the attempts XTAL-first avoids the hang outright instead: attempt 1 configures no DIO3 Vref, so there is no calibration wait to get stuck in, and the TCXO fallback is only reached on a module that answered and refused XTAL. Attempts are now XTAL, then TCXO, then a settling retry on whichever oscillator was settled on - after a fallback that is a second TCXO attempt. Only TCXO_OPTIONAL builds probe XTAL; a variant that declares a Vref unconditionally still goes straight to it and never probes XTAL at all. SPI_CMD_TIMEOUT stays a failure alongside SPI_CMD_FAILED: a bounded per-command BUSY wait in Module::SPItransferStream() reports it in its own right, independently of the removed watchdog. * Drop a stray tab from the promicro TCXO readme trunk fmt: prettier flags the whitespace-only line inside the <summary> block, which was the only failing check on the PR. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-05 08:29:23 +00:00
3 people	03e6b80989	Serial config validation (#11339) ... * fix(serial): validate serial module config on every platform AdminModule guarded the serial config validation by architecture but not the assignment beneath it: #if ARCH_ESP32 || ARCH_NRF52 || ARCH_RP2040 if (!SerialModule::isValidConfig(...)) return false; disableBluetooth(); #endif moduleConfig.serial = c.payload_variant.serial; So on every other platform an admin "set module config: serial" stored a config the firmware rejects on ESP32. override_console_serial_port combined with DEFAULT, SIMPLE, TEXTMSG or PROTO is accepted and persisted today. Two families are affected, for different reasons: - portduino/meshtasticd, where the validation did not exist at all: isValidConfig was a static member of SerialModule, and that class is inside the same architecture guard, so `nm` finds no such symbol in the native object. - STM32WL (rak3172, wio-e5, CDEBYTE_E77-MBL, russell), where it existed and was never called: the class guard includes ARCH_STM32WL and the AdminModule call site did not. Validation is pure config logic with no serial hardware behind it, so it moves out of the class and out of the guard as a free serialConfigIsValid(). Its only external references - clientNotificationPool, service, getValidTime - are already unguarded elsewhere, so it links on every target. AdminModule's include of SerialModule.h is unguarded for the same reason; the class itself stays guarded inside the header. Only disableBluetooth() remains architecture-specific. This changes what meshtasticd and the STM32WL targets accept: a host relying on the unvalidated path (override_console_serial_port with a mode other than NMEA, CalTopo or MS_CONFIG) is now rejected, as it already is on ESP32. test/test_serial has asserted nothing since it was added in 28aeb0f09e (2025-07-26): its body is behind the same guard, so on portduino it logged a warning and ran zero assertions while counting as one of the canonical suites. Enabling it showed the code did not even compile - its designated initializers list .override_console_serial_port before .mode, which is not declaration order, and C++ requires that. PlatformIO only builds test/ for the native env, so no build had ever compiled these lines. Reordered; all nine now run and pass. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * I'd say gimme 5 bees for a dollar. That's what we called a nickel, because they had bees on em. * style: wrap over-long warning string to the 120-col limit * Cover MS_CONFIG override and correct the validator comment serialConfigIsValid() accepts MS_CONFIG alongside NMEA and CALTOPO when override_console_serial_port is set, but only the first two had a valid-case test. Add the missing one. The declaration comment described the function as pure config logic; it also logs and, in non-test builds, sends a client notification on rejection. --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-08-05 06:55:31 +00:00
Matias Denda	1dde97f807	Fix stack buffer overflow in aes_ccm_encr for partial blocks (#11347) ... * Fix out-of-bounds write in aes_ccm_encr for partial blocks aes_ccm_encr() writes a full 16-byte AES block to the output before XOR-ing with the input, so a trailing partial block writes up to 15 bytes past the length the caller asked for. Every caller in the tree passes a buffer with enough slack, so nothing misbehaves today, but the decrypt path clears it by only a few bytes. Encrypt into a temporary block and XOR out of it, matching what aes_ccm_encr_auth() and aes_ccm_decr_auth() already do in this same file. The ciphertext is unchanged. * Add regression test for the CCM partial-block write and drop stale workarounds The guard bytes past the caller's buffer catch the overflow without relying on a sanitizer, so the test is meaningful in the native environment too. encryptCurve25519() no longer needs to write extraNonce before aes_ccm_ae(): the call stays inside numBytes now, so the copy after it is the only one required. The comment warning about the 15-byte overshoot no longer describes the code.	2026-08-04 22:31:38 +00:00
HarukiToredaandJason P	39e7aa6a6c	Full T-echo card support + Compact UI (#11342) ... * T-echo card * Update NRF52I2SOutput.cpp * Update NRF52I2SOutput.h * cleanup * Update buzz.cpp * use consistent runtime compact-panel check instead of mixing with compile-time macro * Update NodeDB.cpp * Update ExternalNotificationModule.cpp * switched to Throttle::isWithinTimespanMs * Update SharedUIDisplay.h * trunk fix * last cleanup * ClockRenderer.cpp for OLED_COMPACT_UI and setup Unit C6L for new UI. * Fixed regressions in standard OLED and TFT --------- Co-authored-by: Jason P <applewiz@mac.com>	2026-08-04 14:27:35 +00:00
Carlos Valdes	d89f3eb8fc	fix(api): bound the stream drain so a full config dump can't starve the watchdog (#11164) ... writeStream() drained the whole queue in one call - "send every packet we can". A client asking for the full config gets the node database, then the file manifest, then the packet backlog and the position replay, and none of that returns to loop(). On a full node database (120 entries) the dump runs past eight seconds, so on RP2350, where rp2040Loop() arms an 8s hardware watchdog and is the only thing that calls watchdog_update(), the board resets in the middle of the manifest. Reproducible on every connection; with a small node database the dump finished under the timeout and nothing looked wrong. Measured on a pico2_w5500_e22: last loop iteration at millis=27242, ServerAPI kept logging until uptime 35s, reset at 35.2s = 27.242 + 8.0. Take a slice instead. The PhoneAPI state machine is resumable, so writeStream() stops after STREAM_WRITE_BUDGET_MSEC and reports whether anything is left; runOncePart() then asks to be re-run immediately rather than sleeping out readStream's idle delay, so the dump keeps its throughput while loop() gets to feed the watchdog between slices. Backpressure on a retained frame still returns the normal delay - re-running at once would just spin on a full transport. Verified on hardware: 10/10 full config dumps against a node with 120 entries, no resets, dump still completes in ~8s.	2026-08-04 14:14:08 +00:00
Jonathan Bennett	7302db1672	Update LoRa firmware on Thinknode M7 (#11337)	2026-08-04 13:03:30 +00:00
Thomas Göttgens	98813fe79d	Shared e-ink hardware layer: foundation for target-by-target migration off GxEPD2 (#11142) ... * Add shared e-ink hardware layer (graphics/eink) alongside legacy drivers Foundation for a target-by-target migration off the GxEPD2-based EInkDisplay2/EInkDynamicDisplay/EInkParallelDisplay stack: - src/graphics/eink/: chipset drivers, panel profiles, backlight helper (promoted from the InkHUD driver set, shared by BaseUI and InkHUD) - src/graphics/BaseUIEInkDisplay: OLEDDisplay adapter driving the new layer, with EINK_* compat macros matching EInkDynamicDisplay - [niche] build helper in platformio.ini; graphics/eink/ excluded from arduino_base so unconverted targets are unaffected - Screen/CannedMessageModule dispatch between the two stacks per env - InkHUD-specific touch code in TouchScreenImpl1 guarded with MESHTASTIC_INCLUDE_INKHUD (no-op today, required once BaseUI variants define MESHTASTIC_INCLUDE_NICHE_GRAPHICS without InkHUD) No variant is converted and no legacy file is removed; every existing env builds identical firmware. * Fix clang-format comment alignment in Screen.cpp * Address review findings in the e-ink driver layer - Screen.cpp: exclude InkHUD builds from all NicheGraphics BaseUI guards - BaseUIEInkDisplay: size the OLEDDisplay buffer from its actual indexing - EInkParallel: defer update() while an async refresh is in flight, honor the selected clear mode in the async task, never delete a live task - ED047TC1: clean up on failed initPanel, fix inverted bbepI2CWrite checks - UC8175: drop bogus 0x12 soft reset (0x12 is display refresh on UC8175) - LCMEN2R13EFC1: guard absent reset pin, bound the busy wait - SSD16XX/SSD1682: build the RAM window from the instance, not statics - Doc corrections in driver banners and Drivers/README * SSD16XX/SSD1682: send inclusive Y-end address (height - 1) * LCMEN213EFC1: adopt the shared wait timeout / fail-through pattern wait() now bounds the busy poll via Throttle and sets the EInk failed flag on timeout; sendCommand/sendData fail through like the SSD16XX and UC8175 drivers. EInk::runOnce clears the flag after the failed cycle.	2026-08-04 12:34:45 +00:00
Jason P	9e60b23334	Targeted build fixes, especially Heltec T1 (#11345)	2026-08-03 14:13:52 +00:00
oscgonferandThomas Göttgens	8c0ef44b0b	Add one wire i2c bridge support (#10078) ... * First version of DS248X bridge * Add first iteration of DS248X sensor * Supports single readings on DS2484 * Supports readings on ch0 for DS2484_800 * Detection of variant for DS248X * Minor fix on retries for sensor init * Allow multiple channel detect passes on 8-ch version * Always read temperature via ROM matching * Small comment to show how to send all channels * Minor logging changes * Prevent one-wire double definitions * Detect ROMs per round * Fix comment * Prevent skipping on DS2482 ALT3 check * Fix comment (again) * Fix style checks * Remove comment for multiple measurements * Address CodeRabbit review findings on DS248X sensor Set _variant on every detectVariant path and branch on the member, so a failed variant probe retries instead of falling into single-channel init. Search DS2482-800 channels into a scratch buffer so a transient one-wire failure cannot erase a ROM found on an earlier pass, and count channels that already hold a ROM. Check every one-wire return value in readTemperatureROM, validate the scratchpad CRC, and return DS248X_INVALID_TEMPERATURE on failure so the existing sentinel checks reject failed reads instead of reporting stale or uninitialised data. * Probe IIS2MDCTR WHO_AM_I before the DS248X status check At HMC5883L_ADDR the DS248X probe reads 0xF0. On the IIS2MDCTR that sub-address sets auto-increment and targets 0x70, which is reserved and returns an unspecified value; any of bits 0x02, 0x04 or 0x10 makes the probe claim the magnetometer as a DS2482. Reading the WHO_AM_I at 0x4F first is deterministic for both parts. A DS2482 does not acknowledge 0x4F, an invalid command code, and leaves its read pointer untouched, so the subsequent read returns Status, Configuration, Channel Selection or Read Data. None of those can hold 0x40 at scan time, so the DS248X probe still runs and detects it. This also restores develop's detection order and matches the structure already used at BMA423_ADDR: specific ID match, then probe, then the generic fallback. * Read only the reported channel in getMetrics getMetrics walked all eight DS2482-800 channels, but only channel 0 is ever written into the measurement. Each populated channel costs a blocking 750ms conversion inside readTemperatureROM, so a fully wired bridge stalled telemetry for roughly 6s per cycle and discarded seven of the eight readings. Channels without a sensor were already cheap thanks to the isValidROM guard, so this only affects boards that actually use more than one channel, which is the reason to fit a DS2482-800 in the first place. Multi-channel reporting is handled separately in #10192; a note records that it should start the conversion on every channel before waiting, rather than reading each channel end to end. * Trim DS248X comments to one line each --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>	2026-08-03 13:52:02 +00:00
LILYGO_L	ddb7bbf090	fix(power): support SGM41562 family on T-Impulse Plus (#11298) ... * fix(power): support SGM41562 family on T-Impulse Plus * chore: format SGM41562 code	2026-08-03 10:56:45 +00:00
oscgonfer	5792e8751b	Fix PMSA003 Enables on RAK4631 (#11331) ... * Minor fix for PMSA003I * Remove class from State, make state verbose, sleep after init * Minor changes to gate some ifdefs and state class * Make decision tree explicit in AQ Telemetry. Also enable on phone * Minor change in comment * Fix issue staying on if failed enable. Minor change in log	2026-08-03 08:47:09 +00:00
Tom	41c5c92874	serial heap status messages (#11340)	2026-08-03 01:30:36 +00:00
Jason P	c0ddb209b5	Fix up the BiColor boot screen (#11334) ... * Fix up the BiColor boot screen Fits the color below the divide of the two OLED colors for better boot appearance. Calculates out the same math for any other screens. * Failed builds because of math, means you change the math	2026-08-03 01:03:08 +00:00
Austin	854eae456b	QMA6100PSingleton constructor fix pio check warnings (#11313) ... Fixes cppcheck issues src/motion/QMA6100PSensor.cpp:173: [medium:warning] Member variable 'QMA6100P::rawAccelData' is not initialized in the constructor. Maybe it should be initialized directly in the class QMA6100P? [uninitDerivedMemberVar] src/motion/QMA6100PSensor.cpp:173: [medium:warning] Member variable 'QMA6100P::_i2cPort' is not initialized in the constructor. Maybe it should be initialized directly in the class QMA6100P? [uninitDerivedMemberVar] src/motion/QMA6100PSensor.cpp:173: [medium:warning] Member variable 'QMA6100P::_deviceAddress' is not initialized in the constructor. Maybe it should be initialized directly in the class QMA6100P? [uninitDerivedMemberVar]	2026-08-01 23:13:22 +00:00