mirror of
https://github.com/meshtastic/firmware.git
synced 2026-09-20 05:18:54 -04:00
cppcheck-RedirectablePrint
507
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Lynxie
|
4cb912ff55 | fix(gps): remember valid fixes across search cycle (#11697) | ||
|
|
cd6ac90f7e |
Add waypoint & geofence support with notifications for BaseUI and InkHUD (#10920)
* Implement GeofenceModule for waypoint crossing notifications and integrate with existing modules * Waypoint Applet Initial Support on InkHUD * undo tile change * Update screen when Waypoint shows or dissapears * Merge branch 'develop' into waypoint-geofence * Geofence on InkHUD * Update MapTile.h * Update WaypointStore.cpp * Notifications * remove GF from waypoint screen * Prevent Focus from closing the notifiaction banner * Trunk fix * cleanup * undo merge conflix mistake * Waypoint screen on BaseUI * Focus preserve fix * UI bugs * Allow Inkhud to remove waypoint * Respect Locked Waypoints * Trunk fix * Update WaypointStore.cpp * Use 8-digit hex formatting for waypoint IDs. 0x%x was inconsistent with the repo's own convention (0x%08x for 32-bit IDs, used elsewhere in this file). Fixed here and in two other spots I found with the same issue (WaypointModule.cpp, GeofenceModule.cpp). * Update ExternalNotificationModule.cpp * Reject invalid surrogate codepoints in waypoint icon rendering * Update WaypointModule.cpp * Update WaypointStore.cpp * Update WaypointStore.cpp * Update WaypointStore.cpp * trunk fix * fix warnings * power.h rename to Power.h * Update Power.h * Fix executable bit on bin/lint-ifdef-complexity.sh Lost during a prior merge from develop (Windows checkout doesn't preserve file mode), causing "execve failed: Permission denied" in the Trunk Check Runner CI job. develop has this file at 100755; restoring that here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Update README.md * Clean up waypoint and geofence integration * Minimize waypoint and geofence implementation * removed unnecessary gating * Geofence alert * trunk fix * Update test_main.cpp * Update WaypointStore.cpp --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Ben MeadorsandQuency-D
|
bfd1e1a231 |
Add Heltec RC32, RC52 and RCC6 boards, and LC760CA GNSS support (#11572)
* refactor(graphics): select Arduino_GFX panels with a capability flag TFTDisplay tested `defined(HACKADAY_COMMUNICATOR)` in a dozen places to mean "this panel is driven by Arduino_GFX rather than LovyanGFX". Every new Arduino_GFX board had to be appended to all of them. Move the decision into the variant as USE_ARDUINO_GFX so the display code stops naming individual boards. No behaviour change: the Hackaday Communicator is still the only board that sets it. * feat(boards): add Heltec RC32, RC52 and RCC6 Three boards around the same 128x220 NV3001B panel: RC32 (ESP32-S3), RCC6 (ESP32-C6) and RC52 (nRF52840). They differ only in how the panel bus is wired, so they share one branch in TFTDisplay behind TFT_NV3001B. RC32 and RC52 also carry a rotary encoder on a TCA6408 I2C expander. That lands as its own input source rather than as board conditionals inside i2cButton, which is the M5Stack UnitC6L button driver and stays untouched. On RC52 and RCC6 the panel is an add-on module, so probe it before reporting a screen. The probe reuses the bit-banged SPI helper that already backs the T114 ST7789 check. Arduino_GFX is pinned to the upstream commit that added the NV3001B driver; it has not shipped in a tagged release yet. Co-Authored-By: Quency-D <55523105+Quency-D@users.noreply.github.com> * feat(gps): detect and configure the LC760CA GNSS module The LC760CA is another Unicore part, so it joins the $PDTINFO probe family and reuses the CM121 message-rate setup. It answers with CC1161W. GNSS_MODEL_LC760CA goes immediately before GNSS_MODEL_GENERIC_NMEA: the sentinel has to stay last because isValidGnssModel() uses it as the exclusive upper bound on values the probe cache may hold. Placing the new model after it would leave LC760CA permanently uncacheable. Co-Authored-By: Quency-D <55523105+Quency-D@users.noreply.github.com> * fix(graphics): re-init the NV3001B after the panel rail comes back DISPLAYOFF de-asserts VTFT_CTRL, which cuts power to the panel, so the controller loses MADCTL, COLMOD and gamma. displayOn() only sends sleep-out and cannot restore them, leaving the panel dark or in the wrong format after wake. Re-run begin() once the rail has settled, and repaint in full since the re-init leaves display RAM undefined. Also stop the TCA6408 rotary polling from two threads at once. Registering as an InputPollable meant InputBroker's pollSoon task could call pollOnce() while runOnce() was mid-transfer on the main thread, with nothing serialising Wire or the decoder state. Drop InputPollable and have the interrupt wake the thread instead, the way ButtonThread does, so the bus and the decode stay on one thread. * fix(graphics): skip the NV3001B wake when re-init fails begin() reports whether the bus came up. Ignoring it meant a failed re-init still lit the backlight and drove a full-screen repaint at a panel that was never initialised. * chore(boards): ship the Heltec RC boards at release level release is the normal level for a variant; the matrix generator still builds each of these in this PR because they add a new platformio.ini. --------- Co-authored-by: Quency-D <55523105+Quency-D@users.noreply.github.com> |
||
|
|
0b906b4d15 |
T-Watch Ultra support (#8171)
* feat: T-Watch Ultra support * fix init touch controller * add framebuffer * update to device-ui * trunk fmt * update amoled driver reference * PMU cosmetics * power off lora * fix NodeDB defaults * trySetRTC when fixedPosition * haptic touch (only BaseUI) * init lora RF switch * update LovyanGFX 1.2.19 * earlyInitVariant() adaptations acc. #9438 * update device-ui / touch handling * Set NFC_CS disabled on boot * Get t-watch-ultra working better on BaseUI * Fix compilation * Fix flash reads on t-watch-ultra * Get baseui drawing to the screen correctly again on t-watch and add touch IRQ handling * Add PMU IRQ handling * Add IMU support * Change define to avoid collision * BaseUI changes to support t-watch-s3 rounded screen (#10786) * BaseUI changes to support t-watch-s3 rounded screen * Extend margin work to CannedMessages * Finish merge * Get audio working on watch-ultra * trunk fmt * added custom_meshtastic boilerplate * T-Echo-Plus: disable BHI260AP while assumingly not implemented * Drop the duplicate origBold declaration from the merge * Inset incoming message bubbles on rounded screens * Fix RTTTL tempo, WiFi screen margins, PMU guard and a duplicate define * fix compile errror (the 2nd time) * fix SDcard * fix/workaround CO5300 pixel flush to SPI * trunk fmt --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> |
||
|
|
c308d0aca4 |
feat: Support Elecrow ThinkNode M9 (#10908)
* thinknode-m9 variant * move lora to SPI1 device * enable SDcard * use HSPI * BaseUI tft -> HSPI * buzzer, webdav lib * fix build issues * M9 default to MUI, no BT, short ringtone * add keyboard long-press config * update variant * add ThingNode-M9 GPS string * GPS 115200 baud * Basic BaseUI support * Fixup power detection * Compass and KB fixes for M9 * add timed Lock::lock() * add SD card * point device-ui to thinknode m9 draft branch * trunk fmt * fix FusionCompass * Fix t-deck-tft linker arg list overflow in CI * SDcard/lora fix: SPI1 must not be declared twice in arduino 3.x -> reuse SPI1 defined in FSCommon.cpp * update battery parameters * reinit SD card when updating; fix PSRAM size * update lib versions * fix wakeup on key press (KB_INT) * fix default nag_timeout for TFT/MUI devices with buzzer * increase PSRAM and SD freq * trunk fmt * update lovyanGFX 1.2.26 * update device-ui commit reference * fix screen definition * remove DONE; maybe a keyword or other used identifier * fixed CI error nag_timeout * fix prepareSleep initialization * trunk fmt * reduce SD SPI frequency * update device-ui * fix SDcard issue * stage * fix device-ui commit reference * fix device-ui commit * update device-ui commit (fixed keyboard lag) * fix QMI8658 * trunk fmt * update .ini meta information, align SD freq * fix device-ui reference to target (ready to merge) * device-ui for all other targets * make the rabbit happy * trunk fmt * fixed lock screen * fix compile error * SPI lock timeout * apply device-ui fix * revert bad RadioLib commit hash in platformio.ini Co-authored-by: mverch67 <71137295+mverch67@users.noreply.github.com> * fix wrong commit hash change * fix fix commit fix * I love changing random numbers in random files --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
Ben Meadors
|
5e54262fe1 |
refactor(io): unique_ptr ownership for motion sensors and I2C keyboard (#11458)
* refactor(io): unique_ptr ownership for motion sensors and I2C keyboard - AccelerometerThread / MagnetometerThread: the owned MotionSensor becomes unique_ptr, removing the manual delete/null bookkeeping in clean(). Deletion behavior is unchanged (MotionSensor's destructor is virtual). - KbI2cBase: the TCA keyboard was a reference member bound to an anonymous heap allocation - ownership was invisible and nothing could ever free it. It becomes unique_ptr with an out-of-line destructor (the base type is only forward-declared in the header). - GeoCoord::pointAtDistance returned shared_ptr with no shared ownership anywhere (and no callers); return by value instead. No behavior change. * fix(io): make TCA8418KeyboardBase destructor public for unique_ptr ownership * refactor(gps): delete dead pointAtDistance instead of converting it Per review: zero callers in this repo or device-ui, and the math was wrong at both ends (rangeMetersToRadians multiplies meters by 1852, treating meters as nautical miles). Remove it, its now-unused helper, and the <memory> include the old shared_ptr signature pulled in. |
||
|
Manuel
|
778041ec06 | fix esp32 time sync (RTC / NTP) (#11494) | ||
|
Ben MeadorsandClaude Opus 5
|
230da77642 |
fix(time): convert the millis() rollover sites #11291's CI guard cannot see (#11483)
* MeshPacketQueue: fix millis() rollover in the late-packet drop test replaceLowerPriorityPacket() read `backPacket->tx_after < now`, with `now` taken from millis() on the line above. tx_after is an absolute deadline, so that comparison inverts while the deadline sits on the far side of the 32-bit wrap: a queued late packet reads as not-yet-due for the rest of the wrap window, or every late packet reads as droppable at once. The same statement ordered two deadlines against each other with `backPacket->tx_after > p->tx_after`, which has the same problem. #11291 swept every site where millis() sits next to the comparison operator, and its CI guard matches that shape. Stashing the clock in a local first is the same bug written so the guard cannot see it. Both tests now subtract before comparing: the due test through Throttle::deadlinePassedAt(), and the ordering through the elapsed-since-now form already used in AdminModule's oldest-slot scan. The snapshot comes from Time::getMillis() so the deadlines and the test read one clock, per the convention deadlinePassedAt() documents. The `dt` the log line reports is now derived from the same elapsed value rather than recomputed. Behaviour is otherwise unchanged, save the boundary: deadlinePassedAt() is inclusive, so a deadline landing exactly on `now` reads as due rather than one millisecond early. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * RadioLibInterface: don't widen a uint32_t deadline delta into a 64-bit long TRANSMIT_DELAY_COMPLETED tested whether the front packet was still waiting with long delay_remaining = txp->tx_after ? txp->tx_after - millis() : 0; if (delay_remaining > 0) ... The subtraction is uint32_t. Where long is 32-bit - every embedded target - an already-due deadline lands negative and the packet transmits, which is why this has never been visible on device. Where long is 64-bit (portduino, and the native test build) the same value zero-extends to ~4.29e9, reads as positive, and the packet is rescheduled 49.7 days out. It stays parked until some later notifyLater() with overwrite happens to reset the timer. That is not an edge case. notifyLater() schedules through setIntervalFromNow(), so the thread wakes at or after the deadline; being a millisecond past due is the ordinary path through this branch. Ask Throttle instead. deadlinePassedAt() is the unsigned half-range test, so there is no signed conversion to get wrong at any width, and the remaining delay handed to notifyLater() is computed from the same snapshot. On 32-bit the behaviour is identical, including at the boundary: a deadline equal to now transmitted before and still does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ExpressLRSFiveWay: convert the two remaining raw window checks to Throttle runOnce() dismissed the alert frame with `now > alertingSinceMs + 2000` and chose its poll rate with `now < keyDownStart + 20000`, both against a millis() snapshot in a local. Same rollover inversion as any other naive compare, and invisible to the millis-deadline-check guard because millis() is not adjacent to the operator. update() in the same file was already on Throttle. hasElapsed()/isWithinTimespanMs() with the stored event give the full ~49.7 day range and need no snapshot. Sentinels are unchanged in meaning: `alerting` is the armed flag for alertingSinceMs and is tested first, and keyDownStart == 0 reads as "recent" for the first 20s of uptime exactly as `now < 0 + 20000` did - a poll rate either way. The arm sites move to Time::getMillis() so the writes land on the clock Throttle reads, which also puts them within reach of Time::setTestMillis(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * GPSUpdateScheduling: record whether a search is running, don't infer it elapsedSearchMs() answered "am I searching?" by ordering two raw millis() stamps: searchStartedMs > searchEndedMs. Whichever stamp lands on the far side of the 32-bit wrap reads as the larger one, so the answer inverts once per wrap cycle, in both directions: - a search that started before the wrap and ended after it keeps reading as "searching". elapsedSearchMs() then grows without bound and searchedTooLong() aborts a search that is not running. - a search that started after the wrap, following one that ended before it, reads as "idle". elapsedSearchMs() returns 0, so an unproductive search is never aborted and the receiver stays powered until it locks. Both self-heal at the next informSearching(), which bounds the damage to one GPS cycle - but the ordering test cannot be made wrap-correct, because the two stamps carry no information about which wrap they belong to. It does not need to be. Whether a search is in progress is a fact the three inform*() calls already have in hand; the ordering was only ever standing in for it. Add the flag and set it there. elapsedSearchMs() keeps its unsigned subtraction, which was always the correct part. The file's clock reads move to Time::getMillis() so the suite can drive them across the wrap. Behaviour-preserving in production - Time::getMillis() is millis() unless a test injects a clock. test_gps_update_scheduling/ gains seven cases: the idle/searching/ended states, elapsed exactness across the wrap, both inversion directions above, and reset(). The two wrap cases fail on the old predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * MessageStore: date boot-relative messages in uptime seconds A message received before the wall clock is trustworthy is stamped boot-relative and healed by upgradeBootRelativeTimestamps() once the RTC arrives. Both the stamp and the "same boot?" test were millis() / 1000, which wraps every 49.7 days: a stamp taken before the wrap reads as newer than `bootNow` afterwards, so `m.timestamp <= bootNow` declines to heal it and the message shows "???" until it ages out. MessageRenderer's own copy of the test falls the same way and prints invalidTime. Neither produces a wrong time - the guard is what fails safe - but Time::getUptimeSecs() landed in #11291 for exactly this, and does not wrap for 136 years. Both sites take it, which makes the comparison exact rather than merely fail-safe. While here, the autosave tick had its own hand-rolled deadline helper - `reachedMs(now, target)` as `(int32_t)(now - target) >= 0`. Wrap-correct, but a competing idiom for what Throttle::isWithinTimespanMs() already answers, and the signed cast is the form #11291 replaced everywhere else. Deleted; the stamps read Time::getMillis() so the whole path is on one clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WebServer: drop the hand-rolled millis() wrap branch getAdaptiveInterval() special-cased the wrap by hand: if (currentTime >= lastActivityTime) timeSinceActivity = currentTime - lastActivityTime; else timeSinceActivity = (UINT32_MAX - lastActivityTime) + currentTime + 1; Those two expressions are the same number - unsigned subtraction already computes the difference modulo 2^32 - so this is not a bug, just eight lines reimplementing what Throttle does. It also reads like a site that has thought about the wrap and settled it, which makes it a bad example to copy. Two isWithinTimespanMs() calls against the stored activity stamp, matching ethApiServer's shape for the same adaptive-interval decision. The stamps move to Time::getMillis() so the writes and the reads share a clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * MeshPacketQueue: only order elapsed times once both deadlines have passed The late-packet eviction I rewrote compared how long ago each deadline passed: backElapsed < (uint32_t)(now - p->tx_after) That is only an ordering when both deadlines are in the past. An incoming packet whose tx_after is still in the future subtracts to a near-2^32 elapsed, which reads as the most overdue packet in the queue rather than the least - so a full queue would drop the overdue packet it was about to transmit in favour of one that is not ready yet. The comparison it replaced, `backPacket->tx_after > p->tx_after`, got this right away from the wrap; I lost it in the conversion. Classify before ordering: p->tx_after must be unset, or passed, before its elapsed time means anything. Two expired deadlines still order by which is further overdue, which is what the branch is for. Caught by CodeRabbit on #11483. test/test_meshpacket_queue/ pins the branch: the future-dated arrival that started this, both directions of the both-expired ordering, the undelayed arrival, and all of it again with the deadlines and `now` on opposite sides of the wrap. maxLen is 1 so the suite reaches the branch without dragging in CompareMeshPacketFunc and a NodeDB. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ExpressLRSFiveWay: treat "no key pressed yet" as no activity keyDownStart is 0 until the first press of a boot, and the fast-poll window read that as a press at time zero: 100ms polling for the first 20s of uptime with no activity at all, re-triggering once per millis() wrap. The arithmetic this replaced (`now < keyDownStart + 20000`) did the same, so it is not a regression - but the sentinel is exactly what the conventions say to test before the elapsed comparison, and "has there been recent key activity" has an honest answer here. 250ms is the documented floor for not missing presses, so an idle node simply starts there and moves to 100ms on the first press. Also trims the wrap-cases comment in test_gps_update_scheduling to the two-line house limit. Both from CodeRabbit review on #11483. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
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>
|
||
|
|
54d6ce833e |
gps: avoid pow() in GPS_HARDSLEEP threshold heuristic (#11179)
* gps: avoid pow() in GPS_HARDSLEEP threshold heuristic GPS::down() used pow(seconds, 1.22) to pick between GPS_SOFTSLEEP and GPS_HARDSLEEP - a curve fit the surrounding comment already describes as "not particularly accurate". On flash-constrained builds where this was the only pow() call site (e.g. wio-e5), it single-handedly pulled in the full double-precision libm pow/rem_pio2 chain for a heuristic threshold decision. Replaces it with gpsHardsleepThresholdMs(), a piecewise-linear lookup over the same curve, sampled at 16 points and verified to track the original formula within ~0.5% for inputs >=10s and ~1.6% for 5-10s (worse only in relative terms below 5s, where the absolute difference is at most a couple of seconds - negligible against update intervals measured in tens of seconds to hours). Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * gps: trim comments to repo convention (1-2 lines) Addresses a CodeRabbit nitpick: the explanatory comments in GPSUpdateScheduling.cpp and test_gps_update_scheduling/test_main.cpp had grown into multi-line blocks with provenance detail that belongs in the commit message, not inline. Trims each to 1-2 lines, keeping only the essential rationale/bounds. Signed-off-by: Andrew Yong <me@ndoo.sg> Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> * Refactor main function to setup and loop for tests Signed-off-by: Thomas Göttgens <tgoettgens@gmail.com> * gps: extend the hardsleep threshold table below 5s and tighten its tests The 0s-to-5s chord read 42% high at 1s, 22% at 2s and 12% at 3s, against the ~1.6% the comment claimed. Adding 1s, 2s and 3s sample points brings the worst error below 10s to 1.60% at 7s. Above 10s it is 0.55% at 728s, unchanged. Tests: sample off-breakpoint values only, including both worst-error inputs (7s and 728s). Replace the 3000ms absolute floor, which made the 1s assertion unfalsifiable given a true value of 2750ms, with 2% and 0.75% bounds. Add breakpoint-exactness and clamp-boundary coverage. --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Signed-off-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: Austin <vidplace7@gmail.com> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> |
||
|
Andrew Yong
|
2f6906974e | gps: replace GeoCoord::latLongToMeter's spherical trig with equirectangular approximation (#11184) | ||
|
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> |
||
|
Jonathan BennettandClaude
|
512154ff97 |
feat(BaseUI): show 'GPS Time Only' when GNSS has time but no position fix (#11361)
The position frame's drawGpsCoordinates() only distinguished 'No GPS present' / 'No GPS Lock' / coordinates, so a GNSS that had decoded valid time but no fix displayed identically to a cold chip. - GPSStatus: add per-acquisition hasTime flag (5th ctor param, accessor, matches() term, updateStatus() copy) - GPS::runOnce(): publish immediately on the gotTime rising edge so the flag reaches observers on the time-only path, which previously never published; done directly rather than via the end-of-loop block so fixHoldEnds is preserved and hold/power behavior is unchanged. Safe without a location: PositionModule ignores invalid positions. gotTime is already cleared on each GPS_ACTIVE entry, so the state is not sticky across acquisitions. - UIRenderer::drawGpsCoordinates(): the 'No GPS Lock' line becomes 'GPS Time Only' when time is valid. The drawGps() header renderer is intentionally untouched (its branches need separate de-clobbering work). Claude-Session: https://claude.ai/code/session_01CcrasD4QsatunDreANDgCx Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
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> |
||
|
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> |
||
|
|
4b4e82bd72 |
SenseCAP Indicator: RP2040 peripherals for the main firmware (#6220)
* indicator: RP2040 peripherals for the main firmware The SenseCAP Indicator RP2040 co-processor serves as a generic peripheral bridge over a serial protobuf link (interdevice.proto): - FakeI2C implements TwoWire and tunnels write and read transactions, so the standard sensor drivers and the I2C scan work unmodified on the bridged second bus (WIRE1) - FakeUART forwards GPS NMEA to the regular GPS driver - SD card access with chunked file transfers, paged directory listings and card statistics; device-ui loads map tiles and map styles from the card behind the RP2040 - link at 2M baud with 4KB chunks, message structs kept off task stacks Log messages carrying their own bracket tag render it like a thread name. Replaces the earlier IndicatorSensor/COBS approach. * indicator: address review Correlate responses with request ids, serialize the shared TX buffer, reject oversized frames, fix RX buffer overflow and NMEA truncation, full-length file paths. * indicator: assign the GPS FakeUART at runtime Static initialization order across translation units is undefined, so createGps() assigns and null-checks the bridged serial instead. Bound the NMEA length defensively. * indicator: bump device-ui pin to 27e6c0c * indicator: ping/pong link probe, non-blocking runOnce, FakeI2C locking The RP2040 sends nothing unsolicited without a GPS module attached, so wait_ready now probes with the new ping message instead of listening passively. runOnce skips its pump while a requester holds link_lock, keeping the main loop from blocking for a full request timeout. FakeI2C serializes transactions between the UI task and the main loop with an owner-tracked lock held from beginTransmission to transaction end. * indicator: link resync, config-honoring GPS, bridged-bus routing, stats validity Frame resync scans to the next magic instead of flushing the RX buffer, and the pump handles all buffered frames per pass. The RX drain reads in bulk and the protobuf encoder gets the correct buffer bound. GPS honors the gps_mode setting on the Indicator instead of always running. RTC, I2C keyboard and motion sensor drivers resolve WIRE1 through ScanI2CTwoWire::fetchI2CBus so bridged buses reach the right transport. FakeUART implements flush/availableForWrite/const-write from the Stream contract and fences its cross-core ring buffer. SdCardInfo.stats_valid is passed through to device-ui, and the remote FS backend gains the remove operation used for cleanup of failed tile saves. * indicator: retry lost link round trips, I2CResult UNSPECIFIED Remote FS operations retry once on a transport timeout. Correlation ids drop late responses of the first attempt; a retried append whose first attempt landed is recognized by the offset conflict carrying the resulting file size. Definitive failures are not retried, missing-tile probes stay a single round trip. Regenerated bindings add the I2CResult.Status UNSPECIFIED zero value so an empty result cannot decode as success. * indicator: nack responses, rename bridge classes to I2CProxy/UARTProxy A request the co-processor cannot decode or handle is nacked, so the requester fails fast instead of burning its timeout. All requests stage the shared tx_message under link_lock. FakeI2C and FakeUART are renamed to I2CProxy and UARTProxy after the pattern they implement, with their instances following suit. Drops dead code (unused NO_NEWS_PAUSE, unreachable not-running branches, doubled include guards) and the GPS pin log line that is meaningless on the tunneled port. * indicator: refuse a co-processor that speaks another protocol version The ping/pong handshake now carries InterdeviceVersion. A pong reporting a version other than ours means the RP2040 runs firmware that does not match this build, so the bridge stays shut down for the session and the mismatch is logged with both versions. Requests fail fast instead of being misinterpreted by the other side. * indicator: regen protos, interdevice protocol version 2 * indicator: per-task I2C contexts, gated handshake, retryable link failures The bridged I2C bus is shared between the main loop and the UI task, and TwoWire has no transaction bracket a lock can span: drivers drain the read buffer with available()/read() long after requestFrom() returned. Each calling task therefore gets its own staging and read buffers instead of a lock that could be left held (or that could not protect the read buffer anyway). The transaction is staged inside the link, under its lock. No request is sent before the co-processor has completed the version handshake, and runOnce keeps probing until it does, so a co-processor that boots slowly or reboots on its watchdog no longer leaves the bridge dead for the session. Requests in flight are counted, not flagged: two threads can be in a request and the first one out must not clear the other's state. File operations are retried on a lost frame and on a co-processor busy with card maintenance, but not on a refusal (nack) or a definitive failure, and they release the SPI lock while they wait so a slow link does not starve the radio. * indicator: fail safe on a peer mismatch, wait out card maintenance FileStatus moved to a fresh tag: reusing the tag of the removed success flag made every failure status decode as success on a peer that predates it. A card being mounted (busy) is retried rather than reported as an empty slot, and a co-processor busy with card maintenance is waited out: mounting takes seconds and the free space scan of a large card walks its whole FAT, which is not a reason to report a missing tile. The bridged I2C bus releases the SPI lock as well, so the keyboard scan on the UI task cannot starve the radio either. Slot claims in the I2C proxy are atomic, NMEA is not sent to a peer we refuse to talk to, and the handshake is completed by the unsolicited ping the co-processor sends when it has booted, which also reports a reboot. * indicator: regen protos, FileStatus back on the original tags * indicator: regen protos, ping/pong carry the InterdeviceVersion enum * indicator: point the protobufs submodule at the merged interdevice protos * indicator: pin device-ui to the branch with the remote SD support * indicator: honor the txOnly flag of flush, report dropped GPS writes flush() through a Stream pointer discarded the receive buffer: the flag is txOnly, and HardwareSerial::flush() keeps what has been received. write() reported bytes as written even when the link refused to send them. The link probe uses Throttle for its rate limit. * indicator: decide the log tag on the formatted message, hex request ids The thread tag was suppressed based on the printf template, which disagrees with the rendered message it is compared against: a format starting with a conversion could produce two tags, and one without a trailing bracket-space lost the tag entirely. vprintf now receives the thread name and picks. Also shifts only the bytes actually buffered after a frame, throttles with Throttle and logs request ids as hex. * indicator: SD mount, eject and format commands over the link * indicator: bound how long a busy card state blocks the UI task * indicator: a busy co-processor must not block the UI task for ever The busy retry re-armed its own budget on every busy answer, so a co-processor that stayed busy kept the caller in the loop with no way out. Transport retries and the wait for a busy card are now separate budgets that only count down. * indicator: start each request from an aligned receive buffer A byte run lost mid-response (a UART overflow during a 4KB tile chunk, when the display starves the RX interrupt) misaligns the assembly buffer. The buffer was never reset, so the poison outlived the request and cascaded into the following chunks of the same tile: one glitch dropped a whole multi-chunk tile, while single-chunk tiles resynced in the idle gap and survived. Each request now flushes the buffer first, bounding a glitch to the one chunk it hit. Adds resync/decode/timeout counters, logged rarely, to see the rate. * indicator: enlarge the LVGL heap for low-zoom map tiles The heap was 3MB and the image cache reserves 1.5MB of it, so a low-zoom map tile could not find a large enough contiguous block to decode and rendered white. 5MB of the 8MB PSRAM fixes it with room to spare. * indicator: advance the device-ui and protobufs pins to the merged commits Point the protobufs submodule at the merged SD command protos (protobufs #986) so it matches the checked in interdevice sources, and bump the device-ui archive to the current indicator branch tip that carries the SD button and format UI. * Update device-ui library dependency URL * remove cutom sdkconfig * remove duplicated synchronisation (after PR11278 is in place) * set commit reference to updated RemoteSDService class * Add board_level configuration for release * fix cppcheck errors --------- Co-authored-by: Manuel <71137295+mverch67@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: mverch67 <manuel.verch@gmx.de> |
||
|
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> |
||
|
Tom
|
2024bb8384 |
Arrival time fix perhaps (#11274)
* Add explicit presence for MeshPacket.rx_time (arrival time) rx_time is now proto3 optional with a has_rx_time presence bit, matching the rx_rssi treatment. A node with no GPS and no phone connected yet has no time source at all, so a bare 0 was indistinguishable from a genuine 1970-01-01 reading; downstream consumers (replay packets, JSON serialization) now check has_rx_time instead of the value. * Dedupe rx_time stamping into a shared helper; trim a debug log string Extract the repeated haveTime/rx_time/has_rx_time stamp logic (5 call sites across Router.cpp, MeshBeaconModule.cpp, MeshService.cpp) into Router::computeRxTimeStamp()/stampRxTime(). Also shorten the new RTC.cpp LOG_DEBUG string. Saves 48 bytes of flash on rak4631 (measured), no behavior change. * Fix has_rx_rssi presence carried unconditionally through StoreForward replay preparePayload() set has_rx_rssi = true unconditionally on replay, regardless of whether the packet's rx_rssi at store time was a genuine measurement (e.g. MQTT-relayed packets carry no real RSSI). Store the presence bit alongside rx_rssi in PacketHistoryStruct and restore it on replay instead. Flagged by Copilot on #11271 (same root cause the has_rx_time explicit presence work fixes) but never addressed before that PR merged. * Trim comment blocks to the repo's 1-2 line guideline .github/copilot-instructions.md:338 caps code comments at 1-2 lines; several blocks added across the rx_time explicit-presence work ran well past that. Also consolidates Time.cpp's file-level doc comment into Time.h, where the rest of the Time:: API contract already lives. No behavior change. * Add rx_time explicit-presence test coverage - test_meshpacket_serializer: has_rx_time=false fixture plus tests asserting JsonSerialize/JsonSerializeEncrypted emit 0 rather than leaking the millis() placeholder, alongside the has_rx_time=true baseline. - test_stream_api: two tests driving a real PhoneAPI handshake (want_config_id through STATE_SEND_PACKETS) that simulate a phone time-giving transaction arriving before vs. after a queued packet is drained - covering both the reconciled and the ships-with-placeholder-absent paths of MeshService::reconcilePendingRxTimes(). * Fix three correctness issues flagged in review - Time.h: drop the reserved-identifier include guard (_MT_TIME_H); pragma once already covers it, matching convention elsewhere (e.g. RTC.h). - Time.cpp: rebase getMillis64()'s wrap accumulator when the test seam swaps clock sources, so a real<->injected clock jump isn't miscounted as a genuine 32-bit wrap. - NodeInfoModule: the 12h reply-suppression window is a local dedup duration, not a wall-clock reading - switch it to Time::getMillis64() so RTC-quality jumps and replayed packets' stale rx_time can't perturb it. - StoreForwardModule: has_rx_time was derived from *current* RTC quality at replay time rather than stored at capture time, so a history entry saved while time-blind could be misreported as a valid epoch once the clock later improved. Persist the presence bit in PacketHistoryStruct instead. * tryfix CI * post review fixes * more test fixes |
||
|
Thomas Göttgens
|
023351a979 |
Remove dead code (#11082)
Removed: - MessageStore::addFromPacket and addFromString, superseded by tryAddFromPacket - GeoCoord rangeRadiansToMeters, distanceTo, bearingTo - Router::rawSend, declared virtual with no override and no caller - ContentHandler handleHotspot, handleFs, handleAdminSettings, handleAdminSettingsApply, handleDeleteFsContent and their commented route registrations, plus the now unreachable htmlDeleteDir and the handleUpdateFs declaration that had no definition - ContentHelper replaceAll - OnScreenKeyboardModule popup chain: showPopup, clearPopup, drawPopup, drawPopupOverlay and their state, unreachable since the frame based UI was replaced by baseUI - DebugRenderer drawDebugInfoTrampoline, drawDebugInfoSettingsTrampoline and the orphaned drawFrameSettings - NodeListRenderer calculateMaxScroll, drawColumns and a stale extern haveGlyphs declaration with no definition - UIRenderer::haveGlyphs, Screen::blink, NotificationRenderer::showKeyboardMessagePopupWithTitle, VirtualKeyboard::getInputText - InkHUD touchNavLeft, touchNavRight, Applet::getActiveNodeCount, ThreadedMessageApplet::saveMessagesToFlash - TwoButton::setHandlerUp, TwoButtonExtended setHandlerUp, setJoystickDownHandlers, setJoystickUpHandlers - CannedMessageModule LaunchRepeatDestination, isCharInputAllowed, hasMessages - TrafficManagementModule resetStats, recordRouterHopPreserved, saturatingIncrement - UnitConversions::MetersPerSecondToMilesPerHour - EncryptedStorage getSessionRemainingSeconds - BMI270Sensor::writeRegisters, GPS::hasFlow, FSCommon copyFile, SerialConsole consolePrintf, buzz playLongPressLeadUp, memGet displayPercentHeapFree |
||
|
Thomas Göttgens
|
a808e992a1 | Add native Windows build of meshtasticd (#11031) | ||
|
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> |
||
|
Andrew YongandBen Meadors
|
dd509a8ecf |
Compute GeoCoord's UTM/MGRS/OSGR/OLC lazily instead of eagerly (#10996)
GeoCoord's constructor unconditionally computed all five coordinate representations (DMS, UTM, MGRS, OSGR, OLC) via setCoords(), even though most callers only ever read one. The UTM conversion alone pulls in a chain of libm trig functions (atan, __kernel_tan, __ieee754_acos, __ieee754_pow, __kernel_rem_pio2/__ieee754_rem_pio2) that then have to be linked in regardless of whether anything is ever displayed. The only screen consumer (UIRenderer.cpp's GPS coordinate display) already dispatches on a single configured format and reads exactly one representation per call - never more than one. Compute DMS eagerly (cheap, most commonly needed) and defer UTM/MGRS/OSGR/OLC to first access via their own getters, tracked with per-representation mutable dirty flags, so a caller that never touches a given representation never pulls in its conversion code or the trig functions it needs. On stm32wl, GeoCoord's heavy constructor is reached only through NMEAWPL.cpp's NMEA/CalTopo serial export (GPS-gated, so wio-e5 only), which only ever reads the DMS getters - so this recovers 8,288 bytes flash there (of the 10,172-byte ceiling if the whole feature were cut) with the feature fully intact and zero behavior change on any platform. Updated test_geocoord_extreme_coords_no_oob (the existing regression test for a historical out-of-bounds crash in the UTM/MGRS conversion on extreme lat/lon) to explicitly call each representation's getter, since it previously relied on the constructor eagerly triggering all four conversions - which this change intentionally defers. Verified: full native test suite passes (586/586 test cases, including this one under ASan), and rak4631 (nRF52, screen-equipped, where the UTM/MGRS/OSGR/OLC getters are actually reachable) builds successfully. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Ben Meadors
|
b4dd76a4db |
Harden against crafted-packet crashes + adversarial fuzzing (#10862)
Audit and fuzzing of the RF-packet decode -> dispatch -> display/phone paths for the "crash a node or phone with a crafted packet" surface, beyond the XEdDSA authenticity work. Crash fixes (reproduced under AddressSanitizer / UBSan): - GeoCoord::latLongToUTM/latLongToMGRS read fixed letter tables out of bounds on extreme latitude_i/longitude_i from a received Position, and narrowed out-of-range easting/northing doubles to unsigned (float-cast-overflow UB). Clamp the UTM zone, the easting/northing narrowing, and the band/col/row indices. Regression: test_geocoord_extreme_coords_no_oob. - EnvironmentTelemetry/AirQualityTelemetry render attacker floats via String(float), which on nRF52/RP2040/STM32/portduino formats into a fixed char[33] (dtostrf) and overflows near FLT_MAX. Clamp the rendered metrics via UnitConversions::displaySafeFloat (finite + magnitude <= 1e9), unit-tested in test_type_conversions. Defense-in-depth + robustness: - TraceRouteModule::printRoute: fix an snr_back[-1] OOB read (wrong count in the guard) and stop formatting the INT8_MIN "unknown SNR" sentinel as a dB value. - WaypointModule/NodeDB: sanitize untrusted strings before the OLED renderer and the phone-facing ClientNotification (belt-and-suspenders vs PB_VALIDATE_UTF8). - MeshService::sendToPhone: withhold NODEINFO/WAYPOINT packets whose nested string won't cleanly decode, protecting strict phone protobuf decoders without affecting mesh relay. Tests: new test_fuzz_decode (protobuf decode + UTF-8 sanitizer fuzz) and test_fuzz_packets (perhapsDecode / module-handler / traceroute / phone-gate fuzz), all under AddressSanitizer; native-suite-count 25 -> 27. Full suite 515/515 green. |
||
|
Tom
|
3becaf2d95 | emdashes begone (#10847) | ||
|
|
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> |
||
|
webgeek1234andJonathan Bennett
|
9cef69a9d3 |
feat(gps): Do not start gps if lora region is unset (#10386)
* Disable gps thread on startup if lora region is unset There is little reason to waste battery on the gps if the data cannot yet be used. * fix goobered merge Refactor GPS enabling logic and remove duplicate code. * trunk --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> |
||
|
Ben Meadors
|
9e3a9c370d |
Enhance RTC handling with unit test support for system time fallback (#10642)
* Enhance RTC handling with unit test support for system time fallback
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
(cherry picked from commit
|
||
|
Jonathan Bennett
|
ae3493a00c | Set the time from GPS every 30 minutes (#10737) | ||
|
Benjamin Faershtein
|
882ca0a216 |
Improve GPS stale probe recovery (#10714)
* Improve GPS stale probe recovery * Address GPS review feedback |
||
|
Jonathan BennettandCopilot Autofix powered by AI
|
bbcc35e209 |
Stm32 general (#10700)
* Attempt to generalize ARCH_STM32 * Trunk * One More ARCH_STM32 * Whoops, one snuck in there * Fix comment to reflect define change Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
HarukiToredaandCopilot Autofix powered by AI
|
a9b98f47e9 |
GPS: cache model and baudrate and skip full sweep every startup (#10544)
* GPS cache * trunk and CRLF fix * Fix GPS.cpp formatting for trunk * Format GPS.cpp for trunk clang-format * Show gps model instead of model number * Potential fix for pull request finding Useful fix Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update GPS.cpp * Update GPS.cpp * Trunk fix * Update GPS.cpp --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Ben Meadors
|
784e3748e2 | Merge remote-tracking branch 'origin/master' into develop | ||
|
|
33e2bb70e6 |
Enhance GPS search failure handling backoff logic (#10404)
* Enhance GPS search failure handling backoff logic * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Remove stray submodule gitlink for .claude worktree A 160000 (gitlink) entry for .claude/worktrees/naughty-payne-60fdb7 pointing at |
||
|
jessm33andjessm33
|
6e810741f3 |
Fix GPS initialization logic for Portduino configuration (#10395)
Co-authored-by: jessm33 <root@example.com> |
||
|
jessm33andjessm33
|
cdc47a2aea |
Fix GPS initialization logic for Portduino configuration (#10395)
Co-authored-by: jessm33 <root@example.com> |
||
|
Ben Meadors
|
603cce2988 | Add informSearchFailed method to update GPS power state handling (#10394) | ||
|
Ben Meadors
|
bc17d004ab | Merge branch 'master' into develop | ||
|
|
4ccdd80090 |
Add search duration check for exceeding 15 minutes (#10293)
* Add search duration check for exceeding 15 minutes Added a condition to check if the search duration exceeds 15 minutes, indicating too long of a search. * trunk * Fix searchedTooLong: move 15-min cap before UINT32_MAX check, cache elapsed, add constexpr Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/b7f74430-9e7e-4a6f-8095-6176c1eee972 Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Update src/gps/GPSUpdateScheduling.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove dead UINT32_MAX branch from searchedTooLong Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/6dad5b56-902e-4d0e-90c1-038a9c2df364 Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Ben Meadors
|
814f1289f6 | Merge remote-tracking branch 'origin/master' into develop | ||
|
Ben Meadors
|
99abfebc4a |
Fix TransmitHistory to improve epoch handling (#10017)
* Fix TransmitHistory to improve epoch handling
* Enable epoch handling in unit tests
* Improve comments and test handling for epoch persistence in TransmitHistory
* Add boot-relative timestamp handling and unit tests for TransmitHistory
* loadFromDisk should handle legacy entries and clean up old v1 files after migration
* Revert "loadFromDisk should handle legacy entries and clean up old v1 files after migration"
This reverts commit
|
||
|
Ben Meadors
|
4a534f02a4 | fix(gps): prevent GPS re-enablement in NOT_PRESENT mode | ||
|
Larry Doolittle
|
deb45e471c |
spelling fixes (#9801)
45 corrections in c++ source comments limited to 27 files in src/gps src/graphics src/input |
||
|
Thomas Göttgens
|
56fd9c7813 |
fix a lot of low level cppcheck warnings (#9623)
* simplify the observer pattern, since all the called functions are const getters. * use arduino macro over std: for numerical values and refactor local variables in drawScrollbar() * oh, so Cppcheck actually complained about const pointers not being const. * slowly getting out of ifdef hell * fix inkHUD warnings as well * last 2 check warnings * git checks should fail on low defects from now on |
||
|
Jorropo
|
167bcf2863 |
fix typo in PIN_GPS_SWITCH (#9648)
Wasn't caught by CI. |
||
|
Jorropo
|
df14561560 |
convert GPS global and some new in gps.cpp to unique_ptr (#9628)
Trying this to see if anything bad happen if I were to replace most raw pointers with unique_ptr. I didn't used std::make_unique since it is only supported in C++14 and onwards but until we update esp32 to arduino 3.x the ESP32 xtensa chips use a C++11 std. |
||
|
Jonathan BennettandCopilot
|
644fa5b54e |
Power off control pin on Thinknode m5 during deepsleep and add RTC (#9510)
* Power off control pin on Thinknode m5 during deepsleep * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Finish HAS_RTC cleanup * Add RTC for Thinknode M5 * Don't double-init Wire * Specify the RTC chip directly rather than use SensorRtcHelper. Saves a bit of flash, and avoid mis-detection --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Jonathan Bennett
|
200e79e800 | You get an RTC, and you get an RTC! (delete HAS_RTC as it wasn't actually doing much) (#9493) | ||
|
Jonathan Bennett
|
ad4b1d9c2b | re-enable RTC support on THINKNODE M3 and M6 | ||
|
Andrew Yong
|
90778a4e78 |
feat(GPS): Support Softsleep with WAKE-UP pin on PA1010D (#9078)
Support softsleep by defining PIN_GPS_STANDBY on CDTop CD-PA1010D. This differs from existing MTK GPS e.g. L76K, where pulling PIN_GPS_STANDBY (WAKE-UP pin) low is not sufficient to put the GPS module in standby. An additional `$PMTK225,4*2F` must be sent to enter "Backup Mode", which is exited by bringing PIN_GPS_STANDBY (WAKE-UP pin) high. Refer to datasheet[0] §1.9.3 "Backup Mode". 0: https://cdn-learn.adafruit.com/assets/assets/000/084/295/original/CD_PA1010D_Datasheet_v.03.pdf Signed-off-by: Andrew Yong <me@ndoo.sg> |
||
|
Ben MeadorsandCopilot
|
8894a0b711 |
Consolidate LoRa params / preset logic and fix display of preset values (#9413)
* Consolidate LoRa params / preset logic and fix display of preset values * Move preset coercion logic to RadioInterface * Fix some warnings * Fix warnings * STM32 fix * Add unit tests * Update src/mesh/RadioInterface.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |