600 Commits

Include renames

Author	SHA1	Message	Date
Tom	f5314148c2	Serialise AirTime behind a lock, and stop handing out its buckets (#11362) ... * Copy airtime reports into a caller buffer instead of exposing the array airtimeReport() returned a pointer into the rotating bucket arrays, so the caller held a handle to state that logAirtime() and every accessor mutate underneath it. Copy into a caller-supplied buffer instead, and report failure for a null buffer, a count past the log depth, or an unknown report type. ContentHandler owns its buffer and hoists getPeriodsToLog() out of the three calls that repeated it. * Cover the AirTime report API and log-dispatch contract Half of AirTime's surface had no tests: which store each report type feeds, what airtimeReport() does when misused, how the first sync seeds itself, and whether calling several entry points in one interval compounds the rotation. Eighteen tests, asserted through the public API rather than the public bucket arrays - those arrays are meant to become private, and a test that reads them would have to be rewritten rather than pinning a contract. Two of them state a convention that was never written down: the report arrays are shift-ordered with slot 0 newest, and slot 0 covers only the time since the last rotation. channelUtilization and utilizationTX use the opposite convention - a modular ring indexed by uptime phase - and reading one as if it were the other is a defect that has already happened once. * Characterise AirTime window decay, TX gates, and sleep behaviour Thirty-three tests in three kinds. Invariants must hold forever; boundaries pin off-by-ones a refactor would move; five characterisations encode today's wrong numbers, each tagged with the phase that will flip it. Readings are asserted against an event-log oracle - airtime physically on air inside (now - window, now], computed from a list of completed packets - rather than against hand-worked constants, so a test states "this matches the definition" instead of "this looked right when I wrote it". The characterisations, all measured rather than assumed: - the window covers (N-1)p + phase but divides by Np, so a steady 10% load reads 8.33% right after a bucket boundary -> phase 5 - the same load sweeps across bucket phase instead of holding -> phase 5 - the hour window carries the same defect, 10x smaller -> phase 5 - a packet longer than its bucket is credited whole to the bucket it completed in, so a saturated LONG_SLOW channel reads >100% -> phase 4b - getSilentMinutes() reads a modular ring as if the index were an age, so identical airtime gives different answers by phase -> phase 6 Two tests needed correcting during the write, both my expectations rather than the code: a six-bucket ring sheds whole buckets, so a 30s gap drops three of five survivors and not "half"; and the oracle sees 59 completions in a 60s window, not 60, because the one on the lower edge is outside it. Not written: the planned RX_LOG/RX_ALL_LOG disjointness test. That is a property of the two radio drivers, which choose one or the other per packet - it is not observable from AirTime, which records what it is told. The AirTime-side half is already covered by the routing tests. * Drop write-only and undefined AirTime members None of this was reachable: air_period_tx / air_period_rx file-scope mirrors of airtimes.periodTX/RX, accumulated, rotated and memset in lockstep with them but never read out or serialised. Orphaned when #2552 re-pointed the writes at bare globals instead of deleting them. lastUtilPeriod, lastUtilPeriodTX written on every sync, read nowhere airtimes.lastPeriodIndex written on every rotation, read nowhere currentPeriodIndex() computes (secs / 3600) % 8 - a modular-ring index for the one array that is shift-ordered rather than a ring. Its only two uses were the dead field above and a log line. It is the fossil of the same confusion that makes getSilentMinutes() wrong. UtilizationPercentTX() declared, never defined free logAirtime()/airtimeReport() declared, never defined; the latter still carried the array-returning signature the previous commit removed, so it actively misled Also fixes the rotation log line, which read currentPeriodIndex() from inside the loop although the index is advanced before it - on a multi-hour wake it printed the same final value once per rotation. It now reports which of the crossed hours is being rotated. airtimeRotatePeriod() is kept: it has no caller in the tree either, but unlike the above it is a defined public method, so out-of-tree callers are plausible. Measured, not estimated: sizeof(AirTime) 464 -> 456 B, plus 64 B of globals, so -72 B of static RAM. Padding accounts for the difference from the 66 B the plan predicted by counting declared bytes. The whole point of writing the tests first: the suite is green here with zero test changes. * Document what the AirTime figures measure and how they are stored Comments only, but four of the things they replace were false. The header's example analytics claimed RX_ALL_LOG was "all received lora packets" and offered "RX_ALL_LOG - RX_LOG = other lora radios". Both radio drivers pick exactly one of the two per packet, so they are disjoint: RX_ALL_LOG is airtime we could not parse, the subtraction can go negative, and the total is TX + RX + RX_ALL. Replaced with the actual contract - four inputs, eight outputs, the window each spans, and the fact that the three thresholds are hard-coded members rather than the settings they look like. Names the two storage conventions on their declarations, because mixing them up is what makes getSilentMinutes() wrong: channelUtilization and utilizationTX are modular rings indexed by uptime phase, where the oldest bucket is (current + 1) % N; airtimes.period* is shift-ordered with slot 0 newest, where the index IS an age and slot 0 is a partial hour. Defines the measurement as wall time rather than awake time, and says why: a sleeping node still hears traffic, and per-node redefinition would make two broadcast readings incomparable. Records that the 60s figure is published to the mesh at >= 1h cadence, so what other nodes see is a snapshot - at LONG_FAST and 1% occupancy it reads exactly 0 in about 44% of reports - and that the contention window it feeds moves in 20-percentage-point steps, so small errors never reach the backoff. Finally, states that rotation happens on access rather than on the scheduler tick, names the test that enforces it, and leaves a TODO pointing at the plan phases that fix the characterised accuracy defects. * Serialise AirTime behind a lock proven by a private token Two mechanisms solving different halves. A lock-free inner core (Windows) holds all state and all logic; it has no lock and no way to reach one, so nesting is impossible by construction. A private Held token takes the lock in its own constructor and is the only thing that can be passed where a core method demands one, so the lock cannot be forgotten either. The rule is now uniform with no exceptions to remember: every public method takes the lock once and delegates. In particular isTxAllowed*() lock like everything else - before the split they could not, because they called the public accessors and the lock is not recursive. That asymmetry was the foot-gun the previous design documented in prose and hoped nobody would trip. getPeriodsToLog()/getSecondsPerPeriod() still take no lock; they return compile-time constants and touch no state. channelUtilization[] and utilizationTX[] were public, so the lock was bypassable at compile time. They move into the private core. Four test sites reached in; all four now use logAirtime() plus the virtual clock, and no new test seam was needed. Nothing in src/ was affected. The re-entry assert is guarded on PIO_UNIT_TESTING, so it exists in test builds only. The design sketched #ifdef DEBUG, but nothing in this tree defines DEBUG or NDEBUG, so either spelling ships the assert to every board - and nrf52_promicro_diy_tcxo has ~128 bytes of headroom under its 0xEA000 warm-store cap, which the assert's strings and abort path overrun. It would have worked on hardware, since the check runs in Held's owner initialiser and so precedes the blocking take; the objection is that abort()ing a live mesh node is a poor trade for a bug never seen in the field. Native tests are where it earns its keep anyway: Portduino compiles Lock::lock() to an empty body, so a nested take there succeeds silently and nothing else would notice. Also comments out ScopedBusyAirTime in test_traffic_management. It is inert twice over: the module holds no reference to airTime at all since hop exhaustion was shelved, and the fixture never worked anyway - writing the buckets on a fresh AirTime is undone by the first accessor call, which takes the firstTime branch and memsets them. It reported 0%, not the 100% it claimed. Left in place, commented, with both reasons recorded. Cost on the tightest board in the tree, nrf52_promicro_diy_tcxo: the six phases together add 96 bytes of flash, leaving it 32 bytes clear of the warm-store guard. RAM is 72 bytes lower from the dead-state removal. Suite green at 47/47, with test_airtime unedited apart from the added nesting test. * Count rotations with the loop variable, not a separate tally LOG_DEBUG compiles to nothing under DEBUG_MUTE, so the counter's only read disappeared with it and the tally became write-only. It does not warn today - this build has -Wunused-but-set-variable on, and it fires for other locals, but not for one that is only initialised and never read - so it was latent rather than broken: a stricter flag or -Werror would have failed muted builds only. Using the loop variable removes the class of problem, since the loop condition reads it, and drops the elapsedAirtimePeriods-- mutation as a side benefit. Same iteration count, same output. Found by compiling nrf52_promicro_diy_tcxo with -D DEBUG_MUTE, which is worth recording for its own sake: muting logs takes that image from 802 784 to 673 416 bytes, 98.5% to 82.6% of flash. Logging is 16% of the largest nrf52 image, and its 32 bytes of warm-store headroom are a logging-verbosity question rather than a code-size one. * Tighten the comments added by this branch Comment-only: with comments stripped, all five files are byte-identical to the previous commit. Removed the references to the planning notes. Those documents are working material and will go stale; the code should not depend on them. The five CHARACTERISATION tags now describe the defect they pin and stop there, and the accuracy TODO names the four defects and points at the tests instead of a plan file. Also removed, as noise rather than information: - comparisons against pre-#11291 behaviour, which nobody reading this needs - a comment describing the lock restructure as future work, written before it landed - speculation ("plausible", "worth pinning so a future...") - an aside arguing with an arithmetic slip made while writing the test Kept the mechanical facts that are slow to re-derive: the two storage orderings and which array uses which, RX_LOG/RX_ALL_LOG disjointness, the locking rule and the addSpanned() constraint that protects it, why the re-entry assert is test-only, and the concrete numbers - (N-1)p + phase, 14 164 ms, the 20 pp contention-window steps. Net 16 comment lines out of src/, 33 out of test/. * Gate the AirTime re-entry check on the host, not on testing PIO_UNIT_TESTING is injected by PlatformIO purely on BUILD_TYPE, with no platform check, so it is defined on an on-target `pio test` run too. The check arms before the lock is taken - a nested take blocks forever, so a later check would never run - which under preemption false-positives on legitimate contention and races on its own write. Derive AIRTIME_REENTRY_CHECK once from PIO_UNIT_TESTING && !HAS_FREE_RTOS and use it at all three sites. Had the three conditions ever diverged, an on-target test build would fail to compile on a member the header no longer declares. * Log AirTime outside the lock it serialises DEBUG_PORT.log() blocks on a UART write, and `lock` is a plain binary semaphore with no priority inheritance, so holding it across a log call lets the main thread stall the radio thread in getTxDelayMsec(). Move logAirtime()'s LOG_DEBUG into the shell, after the Held scope closes; the shell already has both arguments, so nothing has to be passed back out of the core. isTxAllowed{ChannelUtil,AirUtil} read into a local under the lock and warn after it. The log bodies are braced because LOG_DEBUG compiles away under DEBUG_MUTE and a bare `if (x) ;` trips -Wempty-body. Fold the two doubled index calls into `+=` while touching the lines. * Give each airtime report its own buffer handleReport() reused one array across the three airtimeReport() calls and ignored the bool. A failed report would have left the previous type's data in place and emitted it under the next type's key. Build each through a lambda whose buffer is zeroed per call, so a failure emits zeros. Unreachable today - the count is always PERIODS_TO_LOG and the type is always valid - but the old shape only read as correct by accident. * Drop a stray semicolon from the inert-guard comment * Address external review: name the race, tighten the claims and the tests The header sold the lock as mechanism without naming a second thread, which invites the reasonable objection that this is a cooperative OSThread codebase. There is a real race and it is nRF52-only: NRF52Bluetooth registers its ToRadio write callback with defer == false, so a phone's packet runs handleToRadio -> sendToMesh -> Router::send on the Bluefruit BLE task, reading utilizationTXPercent() and getSilentMinutes() while loopTask may be inside logAirtime(). ESP32 hands BLE work to the main task and does not have it. Three claims in the header were wrong or overstated: - "nesting is impossible by construction" - Windows is a nested class with an enclosing class's access rights, and `extern AirTime *airTime` is in the same header, so airTime->anyPublicMethod() from inside it is well-formed and would hang. Nothing does it; the assert is the backstop. Say that instead, because the comment below instructs contributors to add helpers to Windows on the strength of the guarantee. - "every public method takes the lock exactly once" - two constant accessors take none and isTxAllowedAirUtil() takes it zero or one times. State the exceptions where the invariant is stated, not only at the definitions. - "both radio drivers pick exactly one per packet" - five drop paths log neither. At most one. Recorded against plan4 rather than fixed here: it changes a telemetry value. getPeriodsToLog()/getSecondsPerPeriod() become static constexpr, which removes them from the locking claim structurally and lets ContentHandler size its buffer and its count from one constant. Tests: - C14's saturated AirTime is installed by a helper and restored in tearDown. Unity's TEST_ABORT() is longjmp and does not run destructors of automatic objects, so the scoped guard it replaces would leave airTime dangling into an abandoned frame on any assertion failure - and the same commit that added it removed the tearDown reset that did cover that. - test_getSilentMinutes_counts_minutes_until_enough_ages_out asserted only `mins <= 60`, which neither return path can violate. The answer is 59. - test_backwards_uptime_degrades_safely stepped 600s -> 60s, which leaves elapsedAirtimePeriods at 0, so it never reached the hourly-report branch its own comment describes. Step by the wrap instead and assert the exact figures. - test_airtime leaked EU_868 out of the duty-cycle case into every later one, and the reentry test's isTxAllowedAirUtil() coverage depended on it. Restore the region in tearDown and set it explicitly where it is wanted. - Rename that test to what it can actually check: no single method takes the lock twice. The calls are sequential, so it cannot catch two methods nesting. * trunk: suppress trufflehog/Lob false positives in test_airtime * Address CodeRabbit review: the rotate trace, the cap warn, the backoff Four findings from the CodeRabbit pass. Two were introduced by this branch, one is a real inconsistency it inherited, one is a naming slip. The rotate trace was the one that mattered. "Log AirTime outside the lock it serialises" moved the per-packet lines and the two TX-gate warnings out to the shell, but missed LOG_DEBUG("Rotate airtimes, crossed hour %u") because it does not sit in the shell at all: it is inside Windows::syncNow(), the lock-free core, which by construction only ever runs under Held. Nothing at that line looks like a lock, which is why it survived. The exposure is smaller than the review suggests - runOnce() syncs at 1 Hz, so in steady state this is one line an hour, and the PERIODS_TO_LOG - 1 burst needs an hour of light sleep with no intervening sync - but a UART write under a plain binary semaphore with no priority inheritance is exactly what the comment above logAirtime() says this code does not do. syncNow() now accumulates crossings in rotationsPendingLog and runOnce() drains it inside the Held scope, then logs after release. Any caller can cross an hour; only that thread reports it, so a crossing raised elsewhere is traced at most one tick late. The `if (rotations > 0)` guard keeps the drained value read under DEBUG_MUTE, where LOG_DEBUG expands to nothing - the write-only tally that "Count rotations with the loop variable" removed. addFromContact()'s favorite fallback stamped silently when the protected cap refused it. The stamp is new on this branch; the two sibling refusals (ignore, verify) both emit PROTECTED_CAP_WARN_FMT, so the operator lost the only signal that the cap was hit on the one path that has a fallback. lfs_assert() mixed clocks: Throttle read Time::getMillis(), the remainder was computed from a second, bare millis(). The review's stated failure mode - a native test overriding the clock - cannot happen, since the hook is behind PIO_UNIT_TESTING and this file is nRF52-only. The real defect is the second read: a tick landing on the 20-minute boundary between the check and the subtraction underflows the remainder into delay(~50 days), on a device that has just found its flash corrupt. One read, clamped, and preFSBegin() stores from the same clock. The eviction test is renamed to test_eviction_prefersCurrentBootStampOverPost2038Epoch. The finding is right that it was snake_case, but the suggested testEvictionPrefers... does not match this file either, which is test_<area>_<camelCase> throughout. Not taken, both pre-existing and out of scope for a rollover branch: - t5s3_epaper's touchResumeAtMs/suppressFromMs read an active suppression as inactive if the wake lands in the 1 ms where millis() is 0. Consequence is one skipped 150 ms touch-settle window per 49.7-day wrap. - NRF52Bluetooth::onPairingPasskey() busy-waits 30 s in a BLE callback. Worth saying plainly that this branch makes it more visible: the old `millis() < start_time + 30000` overflowed at the wrap and cut the wait short, so the correct Throttle form is what lets it run the full 30 s. Reworking it into an OSThread is its own change. Native suite GREEN, 48/48, 672 cases.	2026-08-13 13:12:14 -04:00
Ben Meadors	22079ca863	fix(platform): OOM null-write in stm32wl File and exception leak in portduino GPIO init (#11456) ... - STM32_LittleFS File::_open_dir: the _dir_path allocation was the only unchecked malloc in the file, followed immediately by strcpy - an OOM became a NULL write, and the half-initialized state (open dir, null path) would later feed strlen(NULL) in openNextFile(). Check it and unwind the already-opened dir, matching the sibling failure path. - PortduinoGlue initGPIOPin: if setSilent()/gpioBind() threw after the LinuxGPIOPin was constructed, the pointer was lost in the catch block. Hold it in a unique_ptr and release only after the gpio table takes ownership.	2026-08-12 19:13:07 -05:00
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
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
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
Giacomo Di Ciocco	fa87730e7a	fix(rak4631_eth_gw): handle multicast socket exhaustion (#11132)	2026-08-10 22:49:40 +02: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
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
Thomas GöttgensandAndrew Yong	6367132919	Fix the NMEA checksum offset and harden the buffer writes around it (#11293) ... * Checksum NMEA sentences from the $ delimiter The PositionLite printWPL() format begins with a CRLF, so the fixed start offset of 1 folded the newline and the $ into the checksum and every sentence went out with a wrong value. Locate the $ instead and stop at the terminator or a \*. * Clamp truncated writes and harden the remaining fixed buffers snprintf returns the length it would have written, so a truncated NMEA sentence made buf + len point past the buffer and bufsz - len underflow into a huge size for the checksum append. Clamp after each write. Also pulls in the rest of #11236: the two remaining Dropzone sprintf calls, the dead strcpy in mt_sprintf that wrote one byte past a zero-size allocation for an empty format, and the 10-byte errcode buffer that INT32_MIN overflows. Co-Authored-By: Andrew Yong <me@ndoo.sg> * Bail out on a zero-sized buffer and cast err for %ld snprintf writes nothing at all when bufsz is 0, not even a terminator, so the checksum helper would run strchr over whatever the buffer already held. Return before touching it. int32_t is not long on every target, so cast before formatting with %ld. Co-Authored-By: Andrew Yong <me@ndoo.sg> * Add NMEA sentence regression tests Covers checksum computation from the $ delimiter for both printWPL overloads and printGGA, zero-sized buffers, and truncated buffers down to one byte. Co-Authored-By: Andrew Yong <me@ndoo.sg> * Tighten checksum parsing and pin the WPL fixture checksum Require exactly two hex digits followed by the sentence terminator, and assert both WPL overloads against a known checksum instead of comparing them to each other. * Bump native suite count to 43 --------- Co-authored-by: Andrew Yong <me@ndoo.sg>	2026-07-31 08:20:56 +00:00
Thomas GöttgensandAustin	ecd59e3120	Package meshtasticd for Windows as an MSI (#11289) ... * Package meshtasticd for Windows as an MSI Adds a --service flag connecting meshtasticd to the Service Control Manager, a WiX MSI installing it as an auto-start LocalSystem service with config in %ProgramData%\Meshtastic, and a CI step attaching the MSI to releases. * Address review comments Bind workflow expressions to env vars in run: bodies, and build the service status per call with an atomic checkpoint. * Fix service stop state and CI lint Latch the stop under a mutex so a startup report cannot walk the state back. Ignore the new workflows in semgrep and checkov, as main_matrix already is. * Drop the checkov ignore for the winget workflow Resolve the newest release inside the job instead of taking workflow_dispatch inputs, so CKV_GHA_7 no longer fires and checkov stays active on the file. * Carry the MSI architecture into the winget manifest Parse it from the asset name instead of defaulting to x64, and fail on a multi-arch release rather than validating one at random. * Restore release/.gitignore * Leave the main matrix alone Release attachment moves to the matrix rework in #11151. The MSI is still built and uploaded as a CI artifact. --------- Co-authored-by: Austin <vidplace7@gmail.com>	2026-07-31 04:53:52 +00:00
3 people	597f6767b5	Add Elecrow ThinkNode M8 board support (thinknode_m8) (#11226) ... * Add Elecrow ThinkNode M8 variant scaffold (thinknode_m8) nRF52840 + SX1262 + 2.4" e-paper + ATGM336H-5NR32 GPS. All pins resolved from ThinkNode_M8_V0.3.sch; cross-checked against meshtastic/firmware#9181 (Elecrow V0.1 reference). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add Elecrow ThinkNode M8 board support (nRF52840/SX1262, 1.54in e-ink, ATGM336H GNSS, SC7A20, EC04 encoder) * Address review: keep the stored backlight level out of blanking, match only the SC7A20 WHO_AM_I byte, and transfer detents atomically * Use std::atomic for the press-and-turn detent counter so native builds compile * Drop the ThinkNode M8 LED_BUILTIN redefinition that warned on every translation unit --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-07-30 15:59:35 +00:00
TomandClaude Opus 5	21e3a583bd	Yaml check for Meshtasticd (#11224) ... * feat(portduino): add `meshtasticd --check` config validator Users hand-writing files in /etc/meshtasticd/config.d/ get no feedback when a key is misplaced, misspelled or duplicated: meshtasticd silently ignores what it does not read, so a broken config looks identical to a working one. Add a --check mode that loads the configuration exactly as startup does, then reports what it found and exits: - Duplicate keys, via the yaml-cpp Parser/EventHandler stream. The Node API cannot see them because the map is already collapsed by the time it exists, and yaml-cpp keeps the FIRST occurrence, so a later override is discarded. - Unknown or misnested keys, against a schema mirroring what loadConfig() reads, with a hint naming the section a stray key actually belongs to. - rfswitch_table validation: unrecognised pins, mode rows whose length does not match the pin list, values that are not HIGH/LOW, and unknown modes. - Cross-file overlap: every .yaml in the config directory merges into one portduino_config, so the file loaded LAST wins, the opposite of the within-file rule. Those files are read in filesystem order, not alphabetical. - A warning when more than one file defines a Lora section: spidev, spiSpeed, gpiochip, DIO2_AS_RF_SWITCH, DIO3_TCXO_VOLTAGE and USB_PID/VID/Serialnum are assigned unconditionally with a default every time one is seen, so any of them not repeated in the last file loaded is silently reset. - The resolved gpiochip/line for each pin, since a line that exists on the wrong chip is claimed successfully and then silently does nothing. Exits non-zero when errors were found so it can also gate CI over bin/config.d/**, keeping one implementation rather than a second schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(portduino): flag pins that resolve to -1 in --check A pin key whose value will not convert to a number falls back to RADIOLIB_NC (-1) while still being marked enabled, and initGPIOPin() then trips an assertion inside LinuxGPIOPin rather than failing cleanly. YAML indentation makes this easy to hit by accident: a stray line under "CS: 8" folds into the value as a multi-line scalar, so the file parses, the daemon crashes with a stack trace from a library file, and --check reported "Configuration looks good" while printing "pin -1" two lines above. Report it as an error naming the likely cause instead. Also correct a comment claiming unparseable config.d files are skipped silently. They are not: loadConfig() prints "*** Exception ..." with the line and column. It is the discarded return value, not the diagnostic, that makes the file's absence from the merged config easy to miss. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(portduino): cover `meshtasticd --check` with fixtures and a fuzz suite Adds the tests the config validator was missing, and the checks and fixes that writing them turned up. The theme throughout is configuration that the YAML parser accepts but that does not mean what it looks like it means. Tests ----- bin/test-config-check.sh - 57 assertions driving a built meshtasticd against test/fixtures/portduino-config (50 fixtures plus two config.d trees). A shell test rather than a Unity suite because both behaviours under test are properties of the process: --check is judged by its exit status and printed report, and the "a normal run rejects a bad config" path ends in exit() inside portduinoSetup(), neither of which is reachable from a suite that links one translation unit. Every fixture carries a comment header naming its planted fault and the expected finding, so it can be read on its own. Coverage: * a clean config for each of the ten radio module families (RF95, sx1262, sx1268, LLCC68, sx1280, lr1110, lr1120, lr1121, sim, auto), asserted both findings-free and resolving to that module, so a silent fallback to sim cannot pass * LR11xx rfswitch tables: unrecognised pins, rows longer and shorter than the pin count, levels that are not exactly HIGH, a missing pins list, more than five pins, a scalar table, unknown MODE_ keys, a MODE_ row stranded one level out, and a legal partial table * the PA gain table in both accepted shapes, entries outside the uint16 range it is stored in, and more than the 22 points that are kept * values of the wrong type, split by consequence: the two settings read with no fallback stop meshtasticd starting, everything else is silently replaced by its default * out-of-range and unit mistakes: TCXO voltage written in millivolts, ports outside their usable range, an over-long StatusMessage * MAC sources: both keys set at once, a malformed address, an interface that does not exist * structural faults: duplicate keys, non-mapping and unknown sections, a key left at the top level, a sequence at the document root, an empty file, unreadable pins, unparseable YAML * cross-file behaviour over a config.d directory, including the switch tables that do not override each other * five configs run WITHOUT --check, each of which must still be refused, so check mode cannot quietly make the normal path permissive test/test_fuzz_config - adversarial fuzzing of the checker itself, the "the tool meant to diagnose your config crashes on it" failure mode. Scope is deliberately narrow: yaml-cpp does the parsing and is fuzzed upstream, so what is exercised here is our code above the parse, above all the duplicate-key detector, which is the one hand-rolled piece and walks the raw parser event stream with its own stack. Groups: the checked-in fixtures as a seed corpus, 3000 byte mutations of them (flips, truncation, insertion, splicing, deletion), and structural torture (nesting to 4096 in flow and block style, duplicate keys at depth, anchors, aliases and merge keys, 64KB keys, 256KB scalars, multi-document files). A fourth group of random bytes is present but disabled behind FUZZ_CONFIG_RANDOM_BYTES: it was half the runtime for the least return, since uniform noise is rejected on the first token. The contract is crash-freedom and termination under AddressSanitizer, not any particular finding. CI runs the shell test in the existing native simulator job; the fuzz suite is picked up by the existing ^test_fuzz_ area rule. native-suite-count 40 -> 41. The fixtures are exempt from trunk in .trunk/trunk.yaml, since prettier rejects the duplicate keys and bad indentation that are the point of them. Checker fixes found while writing the tests ------------------------------------------- --check reported a clean exit 0 on configs meshtasticd then refuses to boot, the worst failure a diagnostic tool can have. Four hard exits inside loadConfig() killed the report before it printed: an unparseable file, an unknown Lora.Module, MACAddress and MACAddressSource both set, and HUB75 on a build without it. All are now reported as findings, and all are still refused on a normal run. New validation: Lora.Module against the accepted spellings, which are matched exactly and inconsistently cased, with a suggestion when only case differs; a per-key value type table covering ~85 keys, tested by asking yaml-cpp to perform the same conversion loadConfig() will so it cannot drift; the PA gain table; DIO3_TCXO_VOLTAGE, which is in volts and multiplied by 1000, so the millivolt value everything else uses silently asks for 1800V; APIPort and Webserver.Port ranges; MaxNodes; StatusMessage truncation; MAC address and source; and an unreadable ConfigDirectory. Also fixes a crash: a ConfigDirectory that cannot be read threw an uncaught filesystem_error from directory_iterator and aborted meshtasticd with SIGABRT, taking --check down with it. It now fails cleanly. Two smaller ones: cppcheck's uselessCallsSubstr on the ancestor walk, which was failing every check job; and the duplicate-key detector's stack pop, which was unguarded and relied on yaml-cpp emitting balanced events. Switch tables are the one place "the file loaded last wins" is false. The loader only ever writes HIGH and never writes LOW back, so a HIGH from an earlier file survives a later file that clears it and the radio drives the OR of every table loaded. Confirmed with --output-yaml. Reported as an error for now; the loader itself is left alone, as that changes RF behaviour. * fix(portduino): report CH341 pins as adapter indexes, not gpiochip lines --check printed "Resolved GPIO lines (what meshtasticd will try to claim)" for every config, listing a gpiochip and line for each Lora pin and advising they be confirmed against gpiodetect and gpioinfo. For spidev: ch341 every part of that is false. portduinoSetup() skips initGPIOPin() for every Lora pin when spidev is ch341 and hands the raw numbers to Ch341Hal, so nothing is claimed from a gpiochip -- and on Windows and macOS, where a USB adapter is the only way to attach a radio, there is no gpiochip, gpiodetect or gpioinfo to check against in the first place. The checker had no ch341 coverage at all: not one fixture used it, so the whole USB-SPI path went unexercised. The summary now splits on the transport. A ch341 device gets its pins listed as adapter indexes with the gpiod advice dropped, and a gpiochip or line mapping written alongside it is reported: those are read, stored, and never used. Also: "RF switch table: not set" read as a gap on an SX126x, where there is nothing to set. setRfSwitchTable() is only ever called for an LR11xx, so absence is now "not needed for this module" everywhere else, and "not resolved yet" for auto, which has no module to judge against. Fixtures: usb-ch341.yaml (clean, the meshstick shape) and ch341-gpiochip.yaml. CI fix ------ test-native was RED on "config.d overrides are reported", which wanted 2 warnings and got 1. The fixture's two config.d files name different modules, so which one wins -- and whether the LR11xx-without-a-switch-table warning fires -- depends on the order the filesystem returns them in. That is the very thing the fixture exists to demonstrate, so the count is no longer asserted; the report's own order caveat is asserted instead. Review fixes ------------ The unreadable-ConfigDirectory diagnostic was the one new print in PortduinoGlue.cpp not gated behind !configCheck, so it landed ahead of the report header and broke the clean output the rest of the change is careful to keep. Docs: rfswitch-valid.yaml carries seven modes, not eight, and empty-file.yaml is comments-only rather than zero bytes. * style(portduino): trim --check comment blocks and reconcile suite count Condense the multi-paragraph comment blocks in the --check validator to the one-to-two-line convention, and bump test/native-suite-count to 42 for the test_fuzz_config suite added here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 11:06:24 +00:00
Thomas Göttgens	047c4e9feb	add LR 2021 to portduino, and allow Framebuffer devices to rotate the screen from config (#11252) ... * add LR 2021 to portduino, and allow Framebuffer devices to rotate the screen from config. Requires https://github.com/meshtastic/device-ui/pull/355 and supersedes https://github.com/meshtastic/firmware/pull/10567 and https://github.com/meshtastic/firmware/pull/11138 Many thanks to the original authors https://github.com/a-li3n and https://github.com/jessm33 * Build LR2021Interface.cpp in the wasm env initLoRa() constructs LR2021Interface for Lora.Module: lr2021, so excluding the file from the native-wasm source filter left the constructor undefined at link time. LR20x0Interface.cpp stays excluded; it is template-only and comes in via the InterfacesTemplates.cpp amalgamation. Also replace the non-UTF-8 degree signs in the framebuffer rotation comment and correct the rfswitch alias cleanup comment. * Trim comments * Report setenv failure for the framebuffer rotation	2026-07-30 10:44:52 +00:00
Thomas Göttgens	3e2c98420b	nRF52: don't truncate the AES-CTR block length to uint8_t (#11261) ... blockLen() rounds numBytes up to the AES block size, so 241..256 returns 256. Stored in a uint8_t that truncates to 0, giving a zero-length encBuf that Process() then writes numBytes bytes into, smashing the stack with attacker-controlled ciphertext. encrypted.size is a 256-byte protobuf field and encryptPacket admits numBytes up to MAX_BLOCKSIZE, so sizes above the 239-byte LoRa frame limit arrive via the MQTT and UDP multicast ingress paths, which pass the packet to the router without clamping. Only the hardware AES path for keys of 16 bytes or less is affected, which includes the default channel.	2026-07-27 15:51:39 -04:00
3 people	a640d44b5c	Add T-Beam BPF - 144-148 Mhz LoRa @ ~37 dBm (5 Watts) (#10558) ... * Add T-Beam BPF (144-148 Mhz LoRa) * minor correction to fix compiler warnings * Add T-Beam BPF (144-148 Mhz LoRa) * minor correction to fix compiler warnings * Add ITU regions for this device and make GPS work. * Switch pin after defining it as output Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Lora CS is indeed 1, SD Card CS is 10 * Include the back option. * Fix compilation with pioarduino (USB_MODE) * Default ham to narrow_fast * Default PROFILE_HAM to slot 17 This is an appropriate default in the USA but not the EU. The slot override really should follow the region itself, not the regionprofile. * Fix for ITU 2/3 split * Add ITU region options to MenuAction enum * Add HAS_HAM_2M definition to variant headers for 2M support * Re-add PROFILE_HAM regionprofile Accidentally removed in last merge * Trunk fmt * Initial default slots * Switch to TinyFast Still need to flesh out the default channels * Adjust slotOverrides for TinyFast * RadioLib doesn't accept 15.625 kHz Use 15.6 instead * Set RF95 pins for T-Beam Supreme May cause regressions!! * Remove other-variant changes (BPF-only) These have been moved to other PRs * Remove mismatch guarding (we need a more comprehensive approach) * Add comment back * Add template for PA curve * This is a 5-6W radio!! Add TX_GAIN_LORA * Trunk fmt because NomDeTom hates emdashes --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-07-27 13:54:53 +00:00
Andrew YongandBen Meadors	abf217c38d	fix(mesh): don't assert on malloc() failure in MemoryDynamic::alloc() (#11197) ... * fix(mesh): don't assert on malloc() failure in MemoryDynamic::alloc() MemoryDynamic<T>::alloc() called assert(p) right after malloc(), instead of returning nullptr like the static MemoryPool<T,N>::alloc() already does on exhaustion. All of packetPool's callers were already hardened to null-check allocCopy()/allocZeroed() (#10948, #10951), but that path is unreachable on real OOM here: assert() fires first, one level down, before the caller ever gets a chance to check anything. On STM32WL (packetPool is MemoryDynamic there - not enough static RAM for the fixed pool), assert() failures are wrapped to an infinite `while(true);` loop rather than aborting or resetting, so the thread just hangs forever instead of returning nullptr. Reproduced on wio-e5 hardware under a burst of incoming DMs, with free heap draining toward zero right before the hang. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * fix: null-check the two allocUnique*() call sites missed by prior audits #10948/#10951 null-checked every raw-pointer packetPool.allocCopy()/ allocZeroed() call site, but missed the two spots using the UniquePacketPoolPacket (unique_ptr) wrapper: UdpMulticastHandler::onReceive() and MQTT::onReceive() both dereferenced the allocation result unconditionally. Previously unreachable in practice: MemoryDynamic::alloc() asserted before ever returning null, so nothing downstream saw it. Fixed alongside that assert removal (this branch) since these two are now reachable with a genuine null - same fix as everywhere else, just an unchecked pointer that turns up under OOM. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * fix(SimRadio): don't leave isReceiving stuck true on allocation failure startReceive() set isReceiving = true before allocCopy(), so a failed allocation (now reachable with a real nullptr instead of hanging in assert()) left the simulated radio permanently marked as receiving: handleReceiveInterrupt() returns immediately on a null receivingPacket, before ever reaching the code that would clear isReceiving. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-07-25 11:28:42 +00:00
Ben Meadors	3f4e7cc622	Fix nRF52 freeze + watchdog reset when saving config over BLE (#11202) ... * Fix nRF52 freeze + watchdog reset when saving config over BLE Since #10967 phone-originated admin messages are handled synchronously on Bluefruit's BLE event task. NRF52Bluetooth::disconnect() busy-waited for BLE_GAP_EVT_DISCONNECTED, which only that same task can process, so any config save that requires a reboot (e.g. position) deadlocked the device until the 90s watchdog fired. Bound the wait to 1s and sleep instead of spinning so lower-priority tasks (including the watchdog feed) keep running; the SoftDevice completes the link termination on its own. * Address review: use Throttle helper, tighten comment * Name the disconnect timeout constant * Log unconfirmed BLE disconnect at WARN with elapsed time	2026-07-25 11:20:04 +00:00
Copilot	67e2012136	fix: declare loop variable sa as const pointer in t5s3_epaper variant (#11111) ... * Initial plan * fix: declare sa as const pointer in t5s3_epaper variant.cpp --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>	2026-07-20 17:42:47 -05:00
Ben Meadors	829ff80a09	fix(t5s3-epaper): 40MHz octal PSRAM; guard EInk init; esp32s3 sw-atomic shim (#11062) ... - t5s3-epaper: the board's 3.3V octal (AP_3v3) PSRAM is unreliable at the qio_opi default of 80MHz (Total PSRAM reads 0, display dead / boot-loop); clock it at 40MHz. - EInkParallelDisplay: skip EInk init and no-op display ops when the PSRAM framebuffer is unavailable, so the node boots headless instead of hard-faulting on a NULL buffer. - src/platform/esp32: weak software __atomic_*_{1,2,4} so esp32s3 links on toolchains (e.g. macOS) that lack the sized libcalls GCC emits under -mdisable-hardware-atomics.	2026-07-20 12:13:50 +02:00
Jonathan Bennett	19ddadf3ff	Check for the SPI bufsize, and throw a useful error when set too small (#11072)	2026-07-20 09:58:58 +02:00
Thomas Göttgens	a808e992a1	Add native Windows build of meshtasticd (#11031)	2026-07-19 23:31:19 +02:00
Andrew Yong	a3c8778032	stm32wl(rak3172): implement cpuDeepSleep()/shutdown() via STM32LowPower (#10993) ... * stm32wl(rak3172): implement cpuDeepSleep()/shutdown() via STM32LowPower cpuDeepSleep() was an empty stub, so the SDS deep-sleep PowerFSM state and low-battery shutdown did nothing but leave the CPU running at full power. Power::shutdown() also didn't include STM32WL in its arch list, so an explicit shutdown command just logged a FIXME warning. Adds STM32LowPower as a lib_dep (rak3172 only, mirroring STM32RTC) and implements cpuDeepSleep() using it. Standby mode is used for both the finite-wake (SDS/low battery) and forever (shutdown) paths, via LowPower.shutdown(), with or without an RTC alarm. If the LSE-backed hardware RTC never came up (stm32wlRtcAvailable() false), this is a no-op - safer than sleeping without a confirmed wake source. LowPower.shutdown() resets the MCU on wake and should never return; if it somehow does, force a reset via HAL_NVIC_SystemReset() rather than hanging the device silently forever. Gated behind the existing HAS_LSE flag, so this is inert on every variant but rak3172. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * feat(stm32wl): Warn and reset when trying to deepSleep without RTC This is to prevent leaving the firmware catanonic when firmware has run its shutdown routine but doesn't actually shutdown. Signed-off-by: Andrew Yong <me@ndoo.sg> --------- Signed-off-by: Andrew Yong <me@ndoo.sg>	2026-07-17 05:26:12 -05:00
HarukiToredaandcopilot-swe-agent[bot]	88f20d349a	T5S3 ePaper build fix (#10791) ... * Build fix * update * Update platformio.ini * Update SerialConsole.cpp * Remove Network provisioning for this build * Update platformio.ini Per Vid, this is not needed once his PR is merged * fix: resolve merge conflict in SerialConsole.cpp - use setHostDraining(false) from develop --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>	2026-07-16 19:31:30 -05:00
Andrew YongandBen Meadors	b71c1adb26	stm32wl: add hardware RTC support (rak3172) (#10961) ... * stm32wl: add hardware RTC support infrastructure Wires the STM32WL chip's internal RTC (running off the LSE 32.768kHz crystal) into meshtastic's existing time-of-day framework (perhapsSetRTC()/readFromRTC()), following the same pattern already used for I2C RTC chips (RV3028, PCF8563/85063, RX8130CE). LSE is started and polled manually before ever calling into the STM32RTC library, with our own bounded timeout - the library's own internal LSE startup path has no bounded fallback and hangs forever via Error_Handler() if the crystal never locks, so this is required for a board with a missing/faulty crystal to boot normally rather than hang. Gated behind a new HAS_LSE variant flag (currently unset everywhere, so this is inert until a variant opts in - see follow-up commit). Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * gps: qualify RTC.h includes to avoid case-insensitive filesystem collision with STM32RTC The stm32duino STM32RTC library (added to lib_deps in a follow-up commit) ships its own src/rtc.h. On case-insensitive filesystems (the macOS default), an unqualified #include "RTC.h"/<RTC.h> from any file outside src/gps/ resolves to the library's rtc.h instead of src/gps/RTC.h, since PlatformIO's LDF puts lib_deps include paths ahead of the project's own -Isrc/gps. Qualify every include as gps/RTC.h so it can't collide with any same-named header a future dependency might ship, regardless of filesystem case sensitivity. Purely mechanical, no behavior change. Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * stm32wl(rak3172): enable hardware RTC support Opts rak3172 into the HAS_LSE infrastructure added previously: sets STM32WL_LSE_DRIVE to a conservative default and pulls in the STM32RTC library. rak3172 has ~63KB flash headroom going in; build-verified at 76.7% flash usage after this change (up from a 73.8% baseline), well within budget. wio-e5 is not opted in here despite sharing the same STM32WLE5 chip - it's already at 96.8% flash usage today (GPS + I2C sensor support compiled in, unlike rak3172), leaving too little headroom to safely add STM32RTC without first trimming something else. Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * stm32wl: add docstrings for LSE/RTC setup functions Addresses CodeRabbit's docstring coverage check on PR #10961. Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * stm32wl: address CodeRabbit nitpicks on PR #10961 - Brace the single-statement HAS_LSE branch in perhapsSetRTC() to match the sibling readFromRTC() branch's style. - Quote the RTC.h include in PhoneAPI.cpp for consistency with every other qualified include site. Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-07-16 19:00:45 -05:00
5 people	8d1fbbf55f	Snake! (#10936) ... * Snake! * Add spiLock to snake score saving * Check fixes * More careful locking * WIP: Big Display Node * Update src/graphics/HUB75Display.cpp Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add HUB75 Native * Add Tetris game module with core logic and UI integration - Implement TetrisGame class for game logic, including piece movement, rotation, and line clearing. - Create TetrisModule class to manage game state, input handling, and rendering on OLED display. - Introduce high score tracking with persistence and optional mesh broadcasting. - Define UI states for title, playing, paused, game over, and high scores. - Implement input handling for game controls and state transitions. - Add rendering functions for the game board, high scores, and title screen. * feat(snake): add snake graphics and update display logic in SnakeModule * Prompt for Initials for Tetris, too * refactor games module * Games refactor * hub75 native double buffer * Games tuning * Make joystick repeat events on held button * Add clouds and colors * Fix breakout and more color * difficulty tuning * trunk * refactor game announcements, etc * Scale chirpy gravity as game speed increases * Portduino, check for hub75 display before reading in hub75 options * Final(?) games tuning * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Properly ignore input when games screen not shown * Fail gracefully when HUB75 is selected but not supported. --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ixitxachitl <kramerfm@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-07-16 18:35:33 -05:00
Ben Meadors	43084873a6	nRF52 BLE: restore pairing passkey callback on re-enable (#11027) ... shutdown() installs onUnwantedPairing to actively refuse pairing (used on the factory-reset / BT-disable teardown path), but the correct callback (onPairingPasskey) is installed only in setup(). Re-enabling BLE on an already-constructed nrf52Bluetooth goes through resumeAdvertising(), not setup() (main-nrf52.cpp), which only re-armed advertising and never restored the pairing callback. So a shutdown()->resumeAdvertising() cycle without a reboot left the device silently refusing all pairing until the next reboot. Restore the correct pairing passkey callback in resumeAdvertising(), guarded by the same config.bluetooth.mode check setup() uses. Only PIN modes drive a passkey-display callback; NO_PIN (Just Works) never invokes it, so no restore is needed there. Reachable today only via the PowerStress module's BT_OFF/BT_ON opcodes (normal runtime BT-disable paths reboot), so low severity, but a real teardown-latches / re-enable-doesn't-restore asymmetry.	2026-07-16 13:02:43 -05:00
3 people	834357e94c	Add RAK WisMesh Repeater Mini V2 and HP variants (#10918) ... * fix(esp32): skip RTC timer wake on user shutdown Do not arm esp_sleep_enable_timer_wakeup when msecToWake is portMAX_DELAY (UI shutdown), matching nRF52 system_off semantics. fix(rak_wismesh_tap_v2): Tag OCV curve and 16MB partition Add OCV_ARRAY matching WisMesh Tag for accurate SOC. Use 16MB flash partition scheme for TAP V2 hardware. Co-authored-by: Cursor <cursoragent@cursor.com> * Trunkt * Add RAK WisMesh Repeater Mini V2 and HP variants Introduce rak_wismesh_repeater_mini (RAK4631) and rak_wismesh_repeater_mini_hp (RAK3401) with low-VDD System OFF. Skip LPCOMP battery wake on user-initiated shutdown to avoid immediate reboot near the LPCOMP threshold; keep LPCOMP for brownout-driven shutdown via variant_enableBatteryLpcompWake. * docs(variants): update RAK BOM numbers for Repeater Mini V2 & HP * chore: run trunk fmt --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-07-15 06:27:04 -05:00
James RichandClaude Opus 4.8	269b974e96	feat: populate MyNodeInfo.device_id on all platforms (#10995) ... * feat: populate MyNodeInfo.device_id on all platforms RP2040/RP2350 use the 64-bit pico unique board id, STM32WL the 96-bit silicon UID, ESP32-S2 joins the existing OPTIONAL_UNIQUE_ID efuse branch, and everything else (classic ESP32 in particular) falls back to a deterministic factory-MAC-derived id, resolving the long-standing FIXME. Portduino keeps the config-supplied id preferred and now uses the MAC fallback when the config omits one. No proto or persistence changes; the id is re-read from silicon each boot and PhoneAPI still zeroes it for unauthenticated clients. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: declare zero_mac const to satisfy cppcheck Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review feedback on device_id derivation Clear any disk-loaded device_id before the silicon derivation so a failed read leaves it unset rather than stale (Copilot), and size the portduino config copy with sizeof instead of a literal 16 (CodeRabbit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: extract device_id generation into per-arch getDeviceId() Per review feedback on #10995: move the platform-specific MyNodeInfo.device_id derivation out of the #if/#elif ladder in NodeDB.cpp into a getDeviceId() interface (target_specific.h) implemented per-architecture alongside each platform's getMacAddr(): - esp32: efuse OPTIONAL_UNIQUE_ID (C3/S2/S3/C6); classic ESP32 -> MAC - nrf52: FICR DEVICEID + DEVICEADDR - nrf54l15: FICR->INFO.DEVICEID + DEVICEADDR (NRF_FICR-guarded, MAC fallback) - rp2xx0: pico_get_unique_board_id() - stm32wl: HAL_GetUIDw0/1/2() - portduino: config-supplied id preferred, else MAC The shared MAC-derived fallback moves to meshUtils as getMacAddrDeviceId(). NodeDB.cpp now zero-inits the field and makes a single getDeviceId() call, dropping ~65 lines of platform boilerplate plus the esp_efuse/pico/stm32 includes that came with it. No behavior change: device_id is still re-read from silicon each boot and never persisted. Builds green: native-macos, tbeam, heltec-v3, rak4631, rak11310, rak3172. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: address review of device_id refactor - getMacAddrDeviceId(): zero-init the mac[6] buffer. getMacAddr() can return without writing (e.g. Portduino with no MAC source), so the old uninitialized buffer let stack garbage pass the all-zeros guard and become device_id. The pre-refactor code relied on the zero-initialized static ourMacAddr; restore that guarantee. - nrf54l15 getDeviceId(): drop the `#if defined(NRF_FICR)` guard and read FICR unconditionally (as the pre-refactor NodeDB code did). The guarded #else fell back to getMacAddr()'s hard-coded placeholder MAC, which would give every unit an identical device_id; a missing NRF_FICR should be a loud compile error. - NodeDB.cpp: `#include "target_specific.h"` instead of hand-copied externs for getMacAddr/getDeviceId; retire the stale FIXME. Same for meshUtils.cpp (whose extern comment wrongly claimed the TU was Arduino-free). - Delete the orphaned commented-out device_id hex-dump block in NodeDB.cpp. - Trim/de-duplicate the getDeviceId contract comments (single-sourced in target_specific.h). Builds green: native-macos, tbeam. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: trim device_id comments to the repo's 1-2 line guideline Addresses CodeRabbit review nitpick on #10995: shorten the getDeviceId() (target_specific.h), getMacAddrDeviceId() (meshUtils.h), device-id refresh (NodeDB.cpp), and nrf54l15 getDeviceId comments to two lines each, per the "one or two lines maximum" coding guideline. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-13 16:28:25 -05:00
Jonathan Bennett	9060ab4418	add linuxJoystick input module (#10970) ... * add linuxJoystick input module * close epollfd to avoid leaks	2026-07-10 00:29:23 -05:00
Andrew YongandBen Meadors	6768bd08df	fix(modules): null-check packetPool/clientNotificationPool/mqttClientProxyMessagePool allocations (#10951) ... * fix(modules): null-check packetPool/clientNotificationPool/mqttClientProxyMessagePool allocations Follow-up to fix/meshpacket-alloc-null-checks (PR meshtastic/firmware#10948), which fixed the core routing path (Router/NextHopRouter/ReliableRouter/ MeshService/RadioLibInterface) after reproducing a HardFault on STM32WL hardware under real mesh traffic. This covers the same allocCopy()/ allocZeroed() unchecked-return pattern in lower-frequency paths that were out of scope for that PR: SimRadio.cpp (portduino sim RX/TX), NodeInfoModule, the Telemetry modules' power-saving-sleep notifications, MQTT (map report, client-proxy messages, config-validation notifications), PositionModule, SerialModule, and KeyVerificationModule. Sites where the allocation result was already read back through an existing null check downstream (e.g. Telemetry's lastMeasurementPacket, SimRadio's receivingPacket, AdminModule::sendWarning) were left as-is - verified safe, not touched. Router::allocForSending() (and everything that funnels through it, e.g. allocDataProtobuf()) remains unguarded and out of scope here too - it's called from 30+ sites across the codebase and needs its own audit. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * fix(KeyVerificationModule): bound cn->message writes with snprintf CodeRabbit flagged unbounded sprintf() writes into the fixed-size ClientNotification.message buffer as a static-analysis nitpick while reviewing this PR's allocation null-checks. Align these four sites with the snprintf(dest, sizeof(dest), ...) pattern already used for the same field elsewhere in this PR (SerialModule.cpp, MQTT.cpp). Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-07-09 10:34:58 -05:00
Ben Meadors	b4b1ece8f1	nrf52: fix loop-task stack overflow causing reset on BLE pairing/first-sync (#10944) ... The Arduino loop task runs Meshtastic's entire cooperative OSThread scheduler on the Adafruit core's stock 4 KB (1024-word) stack. The 2.8 first-sync path - AdminModule::handleSetConfig -> saveChanges -> configChanged -> RadioInterface::reloadConfig -> LR11x0 reconfigure -> SPI Lock::lock, with vsnprintf/USB-CDC logging frames stacked on top - overflows it. SWD fault capture on a T1000-E (pyocd vector_catch=h) proved it: the loop task's SP was driven 40 bytes BELOW its own pxStack base, the stack paint was consumed to the floor, and the crash was a BusFault in xQueueSemaphoreTake dereferencing a semaphore handle that adjacent-heap corruption had overwritten with log text (0x3f3f207c = "| ??"). The core's HardFault handler is a bare NVIC_SystemReset, so in the field this presents as a silent reboot ~seconds after a phone pairs (or a 30 s zombie hang -> supervision timeout 0x8 when the corruption lands on task TCBs instead). Also explains the set_time_only -> immediate-NodeInfo crash variant reported on empty-DB nodes: same task, different deep chain. 2.7.26 is unaffected because the deep frames (XEdDSA, satellite map conversion, replay engine) did not exist. Fix: set -DLOOP_STACK_SZ=2048 (words = 8 KB) for all nrf52840 targets. Requires the #ifndef guard from meshtastic/Adafruit_nRF52_Arduino#7; until that merges the flag is a harmless redefinition warning. Validated on hardware: pairing + full sync completes, stack paint shows healthy margin under load. Also fix the boot-time reset-reason log: the core's init() caches RESETREAS and W1C-clears the register before setup() runs, so the raw read here has always printed 0. Use readResetReason() instead. (preFSBegin's RESETREAS==0 gate is intentionally left as-is - its degenerate GPREGRET-only behavior is what makes lfs recovery work.)	2026-07-08 17:11:32 -05:00
Ian McEwen	ade42830e0	Re-enable wantresponse and other fields in simradio when on the decoded path (#10934) ... * Re-enable wantresponse and other fields in simradio when on the decoded path * Make the comments less verbose	2026-07-08 08:59:07 -05:00
Andrew YongandBen Meadors	d16ae2b098	Rename power.h to Power.h for casing consistency with Power.cpp (#10919) ... Every other .cpp/.h pair in src/ (350 of 351) uses identical capitalization between the two files. Power.cpp/power.h was the sole outlier; this aligns it with the rest of the codebase. No functional change — all includes already resolved this file the same way on case-sensitive filesystems. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-07-07 10:15:01 -05:00
Ben Meadors	97302f1845	Add Meshnology W10 AIOT Dev Kit (SX1262 via MCP23017 I2C expander) (#10911) ... * Add software LoRa IRQ polling and an MCP23017 expander HAL for radios behind an I2C expander Some boards route the SX126x control lines (NRESET/DIO1/BUSY) through an I2C GPIO expander instead of native GPIOs, and don't wire the expander's /INT to the MCU, so RadioLib cannot take a hardware DIO1 interrupt. Two reusable pieces, both gated behind USE_MCP23017 / LORA_DIO1_SOFTWARE_POLL so builds that don't define them are unaffected: - ExtensionIOMCP23017 + MCP23017LockingArduinoHal: a RadioLib HAL that maps virtual pins (MCP23017_VPIN_BASE .. +15) onto an MCP23017's GPIO, so RESET/BUSY/DIO1 reads and writes become I2C transactions while the real SPI GPIOs (SCK/MOSI/MISO/CS) pass straight through. A failed I2C read skips the read-modify-write rather than clobbering the rest of the bank. - LORA_DIO1_SOFTWARE_POLL: with no DIO1 interrupt available, the SX126x interface polls the radio IRQ status register from the radio thread and synthesizes the ISR_TX/ISR_RX events, filtering noisy preamble/header IRQs so they can't starve TX. A 1 ms ISR_POLL_TICK drives the poll; TX timers may overwrite the pending tick (pollMissedIrqs() is the backup that bounds the latency of the busy-Rx contention window). Behaviour is unchanged on all boards that keep the hardware interrupt. * Add Meshnology W10 AIOT Dev Kit variant (SX1262 via MCP23017 expander) ESP32-S3R8 + EBYTE E22-900MM22S (SX1262) + AXP2101 PMIC + Quectel L76KB GPS + SPI TFT, with the radio's RESET/DIO1/BUSY and the LCD reset routed through an MCP23017 I2C expander at 0x20. The expander's /INT is not wired to the ESP32, so DIO1 uses the software poll. Pins come from the board schematic and the vendor firmware; the expander is an MCP23017 despite the V1.1 schematic's stale 'TCA9555' label (the V1.2 placement diagram and all vendor code use the MCP23017 register map). A local pins_arduino.h shadows the generic esp32s3 variant to drop RGB_BUILTIN, which otherwise pulls the Arduino RMT HAL into the link and fails against this build's trimmed FreeRTOS config. Reports HardwareModel 140 (MESHNOLOGY_W10, already present in the protobufs). Hardware-verified on a real W10 AIOT Dev Kit: AXP2101 PMU, MCP23017, SX1262 init + a full over-the-air TX/RX round trip through the expander, PCF85063 RTC, SHT41 and QMI8658 auto-detected, and an L76KB GPS fix. * meshnology-w10: enable ES8311 speaker for notification tones Bring up the ES8311 codec (I2C 0x18) -> NS4150 amp -> speaker so the board can play notification tones / ringtones over the I2S buzzer path (the use_i2s_as_buzzer external-notification option). Codec2 voice stays out of scope: it is SX1280-only and this is a sub-GHz board. - variant.h: HAS_I2S + DAC_I2S pins (MCLK=1, BCK=2, WS=4, DOUT=5, DIN=3) - platformio.ini: arduino-audio-driver + ESP8266Audio + ESP8266SAM - extra_variants/meshnology_w10/variant.cpp: lateInitVariant() configures the ES8311, mirroring the other Meshtastic ES8311 boards. Kept codec-only (no main.h) so the audio driver's 'using namespace audio_driver' does not pull a conflicting GpioPin into scope alongside Meshtastic's class GpioPin. - AudioThread.h: toggle the NS4150 amp (MCP23017 EXIO_PA_CTRL) around playback. Verified on hardware: a test tune played audibly through the speaker. * meshnology-w10: address review feedback on the MCP23017 driver - ExtensionIOMCP23017: serialize register access with a mutex, since the radio HAL (BUSY/DIO1/RESET) and AudioThread (amp enable) now reach the expander from different threads and the read-modify-write paths are not atomic. - digitalRead: return a fail-safe HIGH on a failed read instead of LOW, so a transient I2C error on the LoRa BUSY line can't look like 'ready' and let RadioLib start an SPI transaction early. (DIO1 is polled via the radio IRQ register, not this pin.) - enablePinChangeInterrupt: use the checked readReg() overload and skip on a failed read, matching the other read-modify-write helpers. - meshnology_w10 variant.cpp: log a warning if an ES8311 register write NACKs instead of silently leaving the codec half-configured. - RadioInterface.cpp: guard the MCP23017 HAL branch with ARCH_ESP32 to match the include and driver-file guards. * Replace board-model audio/sleep ifdefs with reusable variant capability macros Two shared-code spots keyed off specific hardware models; move the board-specific detail into opt-in macros the variants define, so the core code stays generic and new boards can opt into the behavior class without touching shared files. - AudioThread amp control: drop the T_LORA_PAGER / MESHNOLOGY_W10 ifdefs. A board with an I2S amp now defines AUDIO_AMP_ENABLE(on) in its variant.h (T-LoRa Pager and Meshnology W10 do), and AudioThread just calls it around playback. The expander includes are keyed on USE_XL9555 / USE_MCP23017 (capabilities) instead of the model name. - Light-sleep / DIO1 wakeup: drop the SENSECAP_INDICATOR check. Boards whose LORA_DIO1 is an expander pin (not an ESP32 GPIO) define LORA_DIO1_EXTENDED_IO (SenseCAP Indicator and Meshnology W10), and sleep.cpp keys off that for both the light-sleep skip and the DIO1 GPIO-wakeup skip. This also fixes a latent issue on the SenseCAP: it previously reached the GPIO-wakeup path and called gpio_pulldown_en() on a virtual expander pin; it now skips that cleanly. Builds verified on meshnology_w10, tlora-pager, and seeed-sensecap-indicator. * meshnology-w10: silence cppcheck constParameterPointer on the ISR-callback helper isIsrTxCallback() takes a function pointer it only compares; cppcheck's constParameterPointer wants it 'pointer to const', which is meaningless for a function pointer (the codebase already globally suppresses the sibling constParameterCallback). Inline-suppress it to keep the check green. * HopScaling: qualify the member assignment as this->count cppcheck (CI's version) flags 'count = newCount;' at the end of trimIfNeeded() as uselessAssignmentArg ('assignment of function parameter has no effect') - a false positive, since count is a member that outlives the call. Write it as this->count (matching the Step-1 assignment above) so the analyzer sees a member write. This finding comes in via the develop merge, not this board; fixing it here to unblock the PR's cppcheck check.	2026-07-07 09:58:54 -05:00
Ben Meadors	00685a4e61	nrf52840: right-size the SoftDevice RAM reservation (+8 KB heap arena) (#10903) ... The app RAM ORIGIN in both nrf52840 linker scripts has been a hard-coded 0x20006000 (24 KB reserved for S140) since they were introduced, but sd_ble_enable() with our fixed Bluefruit configuration (one peripheral link, BANDWIDTH_MAX / ATT MTU 247, 0x1000 attribute table) requires substantially less. The SoftDevice cannot use the gap and the app image is linked above it, so every byte between the true requirement and the ORIGIN is simply unusable RAM - on a part where 2.8.0 field reports showed the heap arena at 99% use. Lower the ORIGIN to 0x20004000 in both s140 v6 and v7 scripts. The heap arena is the linker gap on this platform, so the change is worth exactly +8,192 B of free heap on every nRF52840 board (verified: rak4631 .heap section 124,572 -> 132,764 B; both v6- and v7-script boards link with .data at 0x20004000). Safety net: Bluefruit.begin()'s return value - previously discarded - is now checked. If a future SoftDevice or Bluefruit config change raises the requirement past the reservation, the node logs a critical error instead of silently running without BLE, with instructions to re-measure via CFG_DEBUG=1 ("SoftDevice's RAM requires: 0x...").	2026-07-06 14:13:18 -05:00
Benjamin Faershtein	f8a9ae9f55	[codex] Fix ESP32 Paxcounter startup with Bluetooth disabled (#10871) ... * Fix ESP32 paxcounter startup with Bluetooth disabled * Address paxcounter startup review feedback	2026-07-03 18:58:32 -05:00
Thomas Göttgens	7509c1a6dd	Seeed Tracker X1 Support (#10834) ... * Seeed Tracker X1 Support * silence CPPCheck * fix macro order regression and adapt stm32hal for new radiolib * STM32 is a radiolib upstream fix ( https://github.com/jgromes/RadioLib/issues/1825 ) * fix STM32 regression * Seeed Tracker X1 Support * silence CPPCheck * fix macro order regression and adapt stm32hal for new radiolib * STM32 is a radiolib upstream fix ( https://github.com/jgromes/RadioLib/issues/1825 ) * fix STM32 regression * address copilot OCD * Split behavior only when the two LEDs are on distinct pins. * bring naming in line with the other seeed devices * update env name to fit convention too * guarantee evaluation order * Guard sensor readings against null values, make sensor use less power	2026-07-02 07:39:52 -05:00
Tom	3becaf2d95	emdashes begone (#10847)	2026-07-01 19:01:27 -05:00
5 people	0eaad08735	Fix T1000-E QMA6100P I2C probing (#10713) ... * Fix T1000-E QMA6100P I2C probing * Refactored to make generic * address copilot comments * fix(i2c): address copilot comments on QMA6100P scanning and hardware init - ScanI2CTwoWire: gate bounded QMA6100P probing to addresses 0x12/0x13 only (Copilot comment: avoid blocking other I2C device detection on nRF52) Falls back to normal Wire probing for all other addresses, allowing BMM150 and other devices at overlapping addresses to be properly detected. - Nrf52Twim: suppress cppcheck redundantAssignment on ENABLE register write (intentional disable→configure→enable pattern for hardware safety; explicit disable ensures known state before configuration even if already disabled) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Nrf52Twim: gate on HAS_QMA6100P; suppress cppcheck redundantAssignment The TWIM helper is only used by the QMA6100P probing path (T1000-E, the sole board defining HAS_QMA6100P). It was gated only on ARCH_NRF52, so it compiled as dead code on every other nRF52 board and was analyzed by the rak4631 cppcheck job -- which failed on a redundantAssignment false positive. Gate the header and source on HAS_QMA6100P so the file is only built/analyzed where it is actually used. Also keep an inline suppression on the ENABLE re-assignment (a required volatile disable->reconfigure->enable sequence) for the case where the T1000-E build is run through cppcheck locally. Fixes the rak4631 cppcheck CI failure for PR #10713. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: nomdetom <nomdetom@protonmail.com> Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com> Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>	2026-06-30 14:52:06 -05:00
3 people	2fdb722e00	Add gpsd support to portduino/native (#10781) ... * Add gpsd support to portduino * copilot fixes * emit gps config in emit_yaml * Fix formating to standard * Address coderabitai issues --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>	2026-06-30 14:44:36 -05:00
Tom	fd62322876	Log tidy (#10798) ... * clarify channel number formatting and fix the fixed-width padded nodeIDs * use the explicit 0x%08x for packets and nodeIDs throughout * LLM instructions	2026-06-30 12:44:25 -05:00
Ben MeadorsandAustin	0baf8b1db6	Portduino WASM hello world (still experimental) (#10793) ... * Add ARCH_PORTDUINO_WASM build: meshtasticd in WebAssembly over WebUSB Compile the full portduino firmware to WebAssembly (Emscripten) so a real node runs in a browser tab or headless Node, driving a LoRa radio over WebUSB through a CH341 — the desktop Ch341Hal path with its libusb backend swapped for a WebUSB one. The native/desktop portduino build is unchanged. New build env under src/platform/portduino/wasm/ (excluded from the native build_src_filter): WebUSB libpinedio backend, config/FS/region/MAC/PhoneAPI glue, wasm setup/loop, JS WebUSB runtime, and build stubs. bin/build-portduino-wasm.sh runs a standalone cached emcc build to build/wasm/meshnode.{mjs,wasm}. Six firmware sources gain #ifdef ARCH_PORTDUINO_WASM guards (single-threaded cooperative emscripten_sleep loop, continuous RX, US region default, std RNG, no-popen exec); none affect non-wasm builds. * PortDuino WASM: CI build job, cross-platform libdeps, configurable adapter bin/build-portduino-wasm.sh: auto-detect native-macos (macOS) vs native (Linux/CI) libdeps with a NATIVE_ENV override, and use the SAME env's Crypto with an XEdDSA-present guard. Drops the heltec-v3/Crypto borrow — the meshtastic/Crypto pin already ships XEdDSA; the native libdeps cache was just stale. No env pin change. Add .github/workflows/build_portduino_wasm.yml: build the ARCH_PORTDUINO_WASM target in CI (ubuntu + emsdk, native libdeps) so it can't silently bit-rot; asserts build/wasm/meshnode.{mjs,wasm}. src/platform/portduino/wasm: add wasm_set_lora_* setters so the JS host can configure any CH341 LoRa adapter (module, USB ids, DIO/TCXO, SPI speed, pins); wasm_config_apply falls back to the MeshToad defaults when unset. * WASM: build as a first-class [env:wasm] via platform-wasm (retire emcc script) Replace the standalone emcc build (bin/build-portduino-wasm.sh) with a normal PlatformIO env, `pio run -e wasm`, using the new meshtastic/platform-wasm platform (emcc/em++ + Asyncify, the WASM sibling of platform-native). The portduino WebAssembly node is now built the same way as every other target. - variants/native/portduino/platformio.ini: add [env:wasm] (platform pinned to platform-wasm, board wasm). Translates the script's source set into a curated build_src_filter, the ~30 EXCLUDE_* defines, and the lib set; defines ARCH_PORTDUINO_WASM in-repo so correctness doesn't hinge on the platform's board.json. - extra_scripts/wasm_link_flags.py: the firmware-specific emcc *link* settings (EXPORT_NAME, EXPORTED_RUNTIME_METHODS, EXPORTED_FUNCTIONS, ASYNCIFY_IMPORTS). PlatformIO feeds build_flags to compile only, so these must ride LINKFLAGS; without it the WebUSB Asyncify seam and the JS host's runtime methods are dropped (the _wasm_* exports survive only via EMSCRIPTEN_KEEPALIVE). - PortduinoGlue.{h,cpp}: guard the yaml-cpp dependency out of the WASM build (#ifndef ARCH_PORTDUINO_WASM around the include, emit_yaml/loadConfig/ readGPIOFromYaml). The browser node configures via the wasm_set_lora_* setters and dead-strips the YAML path; this drops the host yaml-cpp build dependency entirely. Native is unchanged (guards are inert there). - portduino_glue_wasm.cpp / portduino_main_wasm.cpp: repair EM_ASM JS that a formatter had mangled (!== -> != =, regex split) in the prior landing; the emcc link succeeds regardless, so CI now runs `node --check meshnode.mjs`. - .github/workflows/build_portduino_wasm.yml: build via `pio run -e wasm` (artifacts under .pio/build/wasm/), trigger on the shared inputs the env inherits (root platformio.ini, bin/platformio-*.py). - NodeDB.cpp: drop the dead ARCH_PORTDUINO_WASM region-default branch (region now defaults the same as native). - Crypto renovate pins: add the missing gitBranch so they track upstream. Output: .pio/build/wasm/meshnode.{mjs,wasm} (ES module, factory createMeshNode). Verified: pio run -e wasm (against the published platform archive), node --check, module instantiates in Node with all exports; native-macos + Docker native unit tests (450/450) still pass. * Fix name on the Piggystick * wasm: pin platform-wasm at the GPL-3.0-relicensed commit platform-wasm's LICENSE was always GPLv3 (matching this firmware), but its platform.json/README still declared Apache-2.0 (mis-copied from platform-native). That's fixed upstream in b83fa5b; bump the [env:wasm] pin to it. Build output is unchanged (license metadata only). Verified: pio run -e wasm against b83fa5b. * wasm: make reboot() actually restart the node (was a no-op) In wasm the reboot path is live (main.cpp -> Power::powerCommandsCheck -> Power::reboot), but Power::reboot's ARCH_PORTDUINO arm tore down SPI/Wire/Serial and then called the no-op ::reboot() stub — leaving the node running with a dead radio until the tab was manually reloaded. Triggers include an admin/phone reboot, factory reset, the "reconfigure failed" path, and the 60 s stuck-TX hardware watchdog (RadioLibInterface). - Power::reboot(): add an ARCH_PORTDUINO_WASM arm (before ARCH_PORTDUINO, since the wasm build defines both) that skips the host teardown and just calls ::reboot(). notifyReboot already let modules persist. - ::reboot() (glue): hand off to the JS host — browser reloads the tab (NodeDB state survives via IDBFS, same identity returns); headless calls Module.onReboot if provided, else logs. Loose !=/== so clang-format doesn't mangle the EM_ASM JS. - README: document the reboot handoff + the Module.onReboot hook. Verified: pio run -e wasm + node --check (EM_ASM intact); native-macos unaffected. * wasm: rename env to native-wasm and run it in the main CI matrix Rename [env:wasm] -> [env:native-wasm] for consistency with the portduino native family (native, native-macos, native-tft). The build dir follows to .pio/build/native-wasm/ (artifact is still meshnode.{mjs,wasm}); the PIOENV guard in extra_scripts/wasm_link_flags.py, the README, and the companion wrapper move with it. The board stays `wasm`. Also wire the build into normal CI: build_portduino_wasm.yml becomes a reusable workflow (workflow_call) invoked as the `build-wasm` job of main_matrix.yml, so the WebAssembly node is built like every other platform instead of on a separate path trigger. * native-wasm: auto-locate the Emscripten SDK (pre-build script) `pio run -e native-wasm` failed with "emcc not found" whenever it was invoked from a shell that hadn't sourced emsdk_env.sh — a VS Code task, an IDE build button, a bare terminal. Add a pre: extra script that probes the usual emsdk locations ($EMSDK_ENV, $EMSDK, ~/emsdk, ./.emsdk, the sibling companion checkout), sources emsdk_env.sh, and imports the resulting environment so the platform builder and emcc see PATH/EMSDK/EM_CONFIG. No-op when emcc is already reachable (CI), silent when no SDK is found (the platform emits its own error). * wasm: address PR review feedback - js/bridge.js: import CH341 from "./ch341.js" (sibling in this layout), not "../src/ch341.js" which doesn't resolve here. - js/ch341.js: a zero-length transferIn while MISO bytes are still outstanding now throws instead of breaking out with a partially-filled buffer — silent SPI corruption becomes a loud error, matching the comment above it. - libpinedio_webusb.c: webusb_set_auto_cs honors the AUTO_CS option (? 1 : 0) instead of the always-on ? 1 : 1. Runtime behavior is unchanged — Ch341Hal sets AUTO_CS=0 right after pinedio_init (RadioLib drives the active-low NSS); the option just isn't set yet at init, so this now correctly defaults off. - SX126xInterface.cpp: the RX-start error log now names the method actually called (startReceive vs startReceiveDutyCycleAuto) instead of hardcoding the duty-cycle name in the WASM branch. * native-wasm: drop the emsdk bootstrap shim (now in platform-wasm) The Emscripten SDK auto-location moved into the platform-wasm builder, so the firmware no longer needs its own pre: extra script. Remove extra_scripts/wasm_emsdk_env.py and bump the platform pin to the build that carries the bootstrap. The wasm_link_flags.py post script stays — those exported fns / runtime methods / Asyncify import seam are firmware-app-specific. * wasm: use the canonical companion name (meshtasticd-wasm-node) The companion repo was renamed meshtastic-web-node -> meshtasticd-wasm-node; fix the stale name in the wasm README and bump the platform pin to the build that promotes the canonical name in its emsdk auto-location. * wasm: re-entrancy guard for the API/region entry points + flaky-open retry Two robustness fixes for the browser node: - Re-entrancy guard. The node is single-threaded + Asyncify: while setup()/loop() is suspended inside a WebUSB transfer, the JS event loop is free, so a stray DOM/timer callback that re-enters a wasm_* entry point starts a second Asyncify unwind ("async operation already in flight" abort) or clobbers shared PhoneAPI state (observed as a "PhoneAPI::available unexpected state" flood). Add a g_wasm_in_firmware flag set around setup()/loop() (portduino_main_wasm.cpp); the wasm_set_region / wasm_api_to_radio / wasm_api_from_radio / wasm_api_available entry points now reject a mid-tick call (return busy) instead of corrupting or aborting. The host must still call them between ticks — this is the safety net the design lacked, not a substitute for the JS queue. - CH341 open retry (js/bridge.js). First-connect WebUSB is flaky — the interface is briefly unclaimable right after the grant, or held by a prior session, giving a transient "Could not open SPI: -1". Retry the open with a short backoff, closing the device between attempts so claimInterface starts clean. * wasm: exclude emscripten-only sources from cppcheck The `check` board matrix runs `pio check` (cppcheck) over all of src/, including src/platform/portduino/wasm/. cppcheck can't parse the EM_ASYNC_JS/ EM_JS macros (Syntax Error: AST broken at libpinedio_webusb.c:39, internalAstError) and these sources are not part of any checked board build ([env:native-wasm] is board_level=extra, compiled by the build-wasm CI job). Suppress the wasm dir in suppressions.txt, the same way generated/ and .pio/ are already excluded. * wasm: coalesce FS.syncfs so two never run at once IDBFS syncfs is async; the explicit wasm_fs_sync (5s timer + post-save + beforeunload) could overlap a prior in-flight sync, warning "2 FS.syncfs operations in flight at once". Serialize: if a sync is running, mark a pending re-sync and let the in-flight one chain it on completion — at most one in flight, trailing writes still flushed. (Companion drops IDBFS autoPersist so this is the single persistence path.) * wasm: silence false-positive SAST on the emscripten glue - extra_scripts/wasm_link_flags.py: restore the trunk-ignore-all(ruff/F821, flake8/F821) header every other SCons extra_script carries; Import/env are SConscript-injected globals, so ruff/flake8 flag them as undefined. - .semgrepignore: exclude src/platform/portduino/wasm/js/ (browser WebUSB glue, not part of the firmware binary). The unsafe-formatstring rule false-positives on its benign retry/diagnostic console logs. * Update .github/workflows/build_portduino_wasm.yml Co-authored-by: Austin <vidplace7@gmail.com> --------- Co-authored-by: Austin <vidplace7@gmail.com>	2026-06-28 06:50:35 -05:00
Clive BlackledgeandBen Meadors	98fa4a67db	[feature] Free a little memory used by bluetooth if wifi is enabled or bluetooth is disabled (#10398) ... * esp32: release BTDM heap when Bluetooth inactive Release ESP-IDF BTDM memory after config load when Bluetooth is disabled or WiFi is enabled, recovering heap on ESP32 targets where BLE won’t be used for this boot. * Address BT memory release review comments --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com>	2026-06-25 17:05:27 -05:00
Ben Meadors	8f425ad3f5	Merge pull request #10776 from jessm33/fix/native-unknown-module ... Update native to exit when configured with an unknown module	2026-06-25 05:40:50 -05:00
Austin	a43cd75bd3	Add Lilygo T-Impulse-Plus (#10497) ... * Add Lilygo T-Impulse-Plus * Enable small screen layout 64x32 * trunk'd * Haptic Feedback (short and long press) * enable Charging Indicator * enable nrfutil uploads * trunk fmt * Add Lilygo T-Impulse-Plus * Enable small screen layout 64x32 * trunk'd * Haptic Feedback (short and long press) * enable Charging Indicator * enable nrfutil uploads * trunk fmt * enable proper device model * Dim the haptic duration a bit. * Fix GPS pins and speed. Module enable is active low, speed is 38400 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * set correct geometry. * Add custom_meshtastic_* metadata to t-impulse-plus platformio.ini --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> (cherry picked from commit cb867cc6c8)	2026-06-24 16:54:03 -05:00
jessm33	ec82aa116d	Update native to exit when configured with an unknown module instead of going in to sim mode.	2026-06-24 09:24:41 -04:00