diff --git a/bin/lint-unset-sentinel-millis.sh b/bin/lint-unset-sentinel-millis.sh index 1ff34da200..d3348cb8f4 100755 --- a/bin/lint-unset-sentinel-millis.sh +++ b/bin/lint-unset-sentinel-millis.sh @@ -85,7 +85,7 @@ set -uo pipefail # Millisecond fields this rule watches. See the notes above before editing. -SENTINELS='rebootAtMsec|shutdownAtMsec|enterDfuAtMsec|alertBannerUntil|pulseOffAt|delayedPulseAt|ntp_renew|tx_after|suppressTouchTapUntilMs|fixHoldEnds|lastChipRecoveryMs|activeReceiveStart|rxTimeMsec|lastInterruptTime|lastSentReply|lastSort|lastTxStart|lastHeartbeat|lastAveraged|lastSampleMs|lastIaqMs|last_format_ms|nextRepeatX|nextRepeatY|_cached_next_run' +SENTINELS='rebootAtMsec|shutdownAtMsec|enterDfuAtMsec|alertBannerUntil|pulseOffAt|delayedPulseAt|ntp_renew|tx_after|suppressTouchTapUntilMs|fixHoldEnds|lastChipRecoveryMs|activeReceiveStart|rxTimeMsec|lastInterruptTime|lastSentReply|lastSort|lastTxStart|lastHeartbeat|lastAveraged|lastSampleMs|lastIaqMs|last_format_ms|nextRepeatX|nextRepeatY|_cached_next_run|connect_time_ms|directionStartTime|downStartTime|fileage|keyDownStart|lastAuthFailure|lastContactMsec|lastDirectResponseMs|lastDiskSave|lastDownLongEventTime|lastDrawMsec|lastGpsSend|lastHeadingAtMs|lastHeapLogTime|lastHeapWarning|lastLfsFormatMs|lastMillis|lastPressLongEventTime|lastRemoteSessionMs|lastSentStatsToPhone|lastSentToPhone|lastSetFromPhoneNtpOrGps|lastTraceRouteTime|lastUpLongEventTime|lastUpdateMs|last_probe|last_report_to_map|lastrun_ntp|navBarLastShown|pressStartTime|startSendConditions|suppressFromMs|touchResumeAtMs|upStartTime' for target in "$@"; do [[ -f $target ]] || continue @@ -155,21 +155,200 @@ for target in "$@"; do return (c ~ /[A-Za-z0-9_]/) } + # Brace depth, and which depths are a class/struct BODY rather than a function body. Needed + # because a typed declaration means opposite things in the two places: inside a function it is a + # throwaway local that shadows the field, but at class scope it IS the field, with an initializer + # that can read the clock - src/modules/SerialModule.h does exactly that. Treating the second as a + # local silently excused a real arm site. + function update_scope(s, i, c) { + for (i = 1; i <= length(s); i++) { + c = substr(s, i, 1) + if (c == "{") { + depth++ + if (pending_class) { class_body[depth] = 1; pending_class = 0 } + } else if (c == "}") { + delete class_body[depth] + if (depth > 0) depth-- + } + } + } + + # Is the statement being judged sitting directly in a class body? `opened` is the net brace + # count seen earlier in this same statement, which update_scope() has not applied yet: a body + # opened on this very line (a one-line inline method) puts the statement inside a function, not + # in the class body. + function at_class_scope(opened) { + # The class body opened on this very statement, so its own brace is the class brace: exactly + # one unmatched brace means class scope, two or more means a method body inside it. + if (stmt_class_brace) return (opened == 1) + return ((depth in class_body) && opened <= 0) + } + + # Net unmatched `{` in the first `at` characters of the statement being judged. + function braces_before(s, at, i, c, n) { + n = 0 + for (i = 1; i < at && i <= length(s); i++) { + c = substr(s, i, 1) + if (c == "{") n++ + else if (c == "}") n-- + } + return n + } + # A local declaration that happens to reuse a sentinel name shadows the field and carries none # of its contract, so it is not this rule business. Detected by a type-ish token immediately # before the name - `uint32_t tx_after = millis() + d;` declares a local, `tx_after = ...` does # not. Kept narrow: only the spellings this tree actually uses for a millis value. + # + # Only honoured inside a function body; see update_scope() for why class scope is different. function is_declaration(s, at, head) { + if (at_class_scope(braces_before(s, at))) return 0 head = substr(s, 1, at - 1) sub(/[ \t]*(\*|&)?[ \t]*$/, "", head) return (head ~ /(^|[^A-Za-z0-9_])(uint32_t|uint64_t|int32_t|unsigned[ \t]+long|unsigned[ \t]+int|unsigned|long|int|auto|size_t|TickType_t)$/) } + # Does this statement read a clock directly? Matches millis(), Time::getMillis() and any wrapper + # whose name ends in millis, which is how almost every clock read in this tree spells itself, plus + # Zephyr k_uptime_get_32() - the nRF54L15 BLE code has no millis() at all and wraps at 32 bits just + # the same, so a sentinel armed from it needs the same guard. + function reads_clock(s) { return (s ~ /[Mm]illis[ \t]*\(/ || s ~ /k_uptime_get_32[ \t]*\(/) } + + # Already routed through a helper that dodges 0, so it is the fix rather than the defect. Note + # stampMillis() belongs here even though its name ends in millis: a local read through it holds a + # value that is already non-zero, so a write from that local is safe and must not be flagged. + function is_safe_arm(s) { return (s ~ /skipZero/ || s ~ /timerEndsAtMillis/ || s ~ /stampMillis/) } + + # Does the expression apply + or - to an already-dodged value at the OUTERMOST level? A dodged + # value is safe to store or copy, but not to do arithmetic on: stampMillis() guarantees only its + # own result, and `now + 5000` can carry a non-zero stamp straight back onto 0 - 0xFFFFEC78 + 5000 + # is exactly 0. That sum is what Time::timerEndsAtMillis() exists to dodge, so it has to be + # reported rather than excused. + # + # Depth-aware on purpose: the operator inside Time::skipZero(getMillis() - msAgo) is at depth 1 + # and is fine, because the helper wraps the result. So is the `? :` in the ternary arming form, + # which has no top-level + or - at all. + function toplevel_arith(s, i, n, c, d, prev) { + n = length(s); d = 0; prev = "" + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (c == "(") d++ + else if (c == ")") d-- + else if (d == 0 && (c == "+" || c == "-")) { + # not a unary sign, and not part of -> or ++/-- + if (prev != "" && prev != "(" && prev != "," && prev != "=" && prev != "+" && + prev != "-" && prev != "*" && prev != "/" && prev != "?" && prev != ":" && + substr(s, i + 1, 1) != ">") + return 1 + } + if (c != " " && c != "\t") prev = c + } + return 0 + } + + # Remember a local that was just assigned from a clock, so `field = now` a few lines later is + # recognised as the raw arm it really is. Without this the rule is blind to the commonest shape + # in the tree - `unsigned long now = millis();` at the top of a runOnce(), then half a dozen + # `xStartTime = now;` writes below it - and listing those fields would buy no protection at all. + # + # Deliberately shallow: one hop, within one function, name-based. It records ` = ` + # and ` = `, and it FORGETS the name when the same local is + # reassigned from anything else, so a variable reused for something unrelated stops matching. + # Taint is dropped at every function boundary (see the reset below), because a name that means a + # clock in one function usually means nothing in the next. + function note_taint(s, lhs, rhs, eqp, semi) { + eqp = index(s, "=") + if (eqp == 0) return + if (substr(s, eqp + 1, 1) == "=") return # `==` is a comparison + if (substr(s, eqp - 1, 1) ~ /[-+*\/%&|^!<>=]/) return # `+=`, `!=`, ... are not plain + lhs = substr(s, 1, eqp - 1) + rhs = substr(s, eqp + 1) + # This assignment only. Without the cut, a second statement on the same line teaches taint + # for the first - the same defect the judging path was fixed for. + semi = index(rhs, ";") + if (semi > 0) rhs = substr(rhs, 1, semi - 1) + # Take the last identifier on the left, which skips any type and `*`/`&` decoration. + if (!match(lhs, /[A-Za-z_][A-Za-z0-9_]*[ \t]*$/)) return + lhs = substr(lhs, RSTART, RLENGTH) + sub(/[ \t]+$/, "", lhs) + if (lhs == "") return + if (is_safe_arm(rhs) && !toplevel_arith(rhs)) { + delete tainted[lhs] + normalized[lhs] = 1 # holds a value that has already dodged 0 + } else if (reads_clock(rhs) || rhs_is_tainted(rhs)) { + tainted[lhs] = 1 + delete normalized[lhs] + } else { + delete tainted[lhs] # reused for something else - stop trusting the name + delete normalized[lhs] + } + } + + # Is any tainted local read in this expression, as a whole token? Token-bounded so a tainted + # `now` does not match `nowMs` or `snowfall`. Local names are plain identifiers, so using one + # as a match() pattern carries no regex metacharacters. + function rhs_is_normalized(s, name, t, p, before, after) { + for (name in normalized) { + t = s + while (match(t, name)) { + p = RSTART + before = (p == 1) ? " " : substr(t, p - 1, 1) + after = substr(t, p + length(name), 1) + if (before !~ /[A-Za-z0-9_]/ && after !~ /[A-Za-z0-9_]/) return 1 + t = substr(t, p + length(name)) + if (t == "") break + } + } + return 0 + } + + function rhs_is_tainted(s, name, t, p, before, after) { + for (name in tainted) { + t = s + while (match(t, name)) { + p = RSTART + before = (p == 1) ? " " : substr(t, p - 1, 1) + after = substr(t, p + length(name), 1) + if (before !~ /[A-Za-z0-9_]/ && after !~ /[A-Za-z0-9_]/) return 1 + t = substr(t, p + length(name)) + if (t == "") break + } + } + return 0 + } + BEGIN { LINE_CAP = 12 } # give up accumulating a statement after this many lines { code = strip_noncode($0) + # A class or struct header whose body opens on this line or the next. Anchored at the start + # of the line, because the keyword appears mid-line in shapes that are not class bodies at + # all: `template ` on a function, and an elaborated type in a parameter list such as + # `void g(struct Bar *b)`. Both used to mark the following FUNCTION body as class scope, which + # then reported every typed local in it. Not a forward declaration either, which ends in a + # semicolon with no brace. + stmt_class_brace = 0 + if (code ~ /^[ \t]*(class|struct)[ \t]+[A-Za-z_][A-Za-z0-9_]*/) { + # The body may open on this line or the next. `class Foo;` is a forward declaration and + # opens nothing; `class Foo { uint32_t t = millis(); };` opens AND closes here, so the + # trailing semicolon cannot be used to rule it out. + if (code ~ /\{/) { + # Body opens on this line. stmt_class_brace judges THIS statement (a one-liner + # whose member sits after the brace); pending_class is still needed so + # update_scope() registers the body for the lines that follow. + stmt_class_brace = 1 + pending_class = 1 + } else if (code !~ /;[ \t]*$/) { + pending_class = 1 # body opens on a later line + } + } + + # A closing brace in column 1 is the end of a function as this tree formats code, and a + # local called `now` there has nothing to do with the one in the next function. clang-format + # is enforced repo-wide, so this is reliable enough for a one-hop heuristic. + if ($0 ~ /^\}/) delete tainted + # An opt-out is sticky until the next statement that actually contains code is judged. That # is what lets it sit on its own line above the write, however many comment lines intervene, # without leaking past the statement it was written for. @@ -222,8 +401,15 @@ for target in "$@"; do # variable inherits whatever that one did, and anything already routed through # the helpers is the fix rather than the defect. Matching `millis` loosely # covers millis(), Time::getMillis() and any wrapper ending in millis. - if (rhs !~ /[Mm]illis[ \t]*\(/ || rhs ~ /skipZero/ || rhs ~ /timerEndsAtMillis/) - continue + # Arithmetic applied on top of an already-dodged value can wrap it back onto 0, + # so it is reported even though a helper appears in the expression. + if ((is_safe_arm(rhs) || rhs_is_normalized(rhs)) && !toplevel_arith(rhs)) { + continue # stored or copied straight through - safe + } + if (is_safe_arm(rhs) && !toplevel_arith(rhs)) + continue # already routed through the helpers + if (!reads_clock(rhs) && !rhs_is_tainted(rhs) && !rhs_is_normalized(rhs)) + continue # not a clock read, directly or via a local holding one if (pending_ok) continue # opted out, with a reason, at the write if (pending_bare) @@ -235,12 +421,21 @@ for target in "$@"; do hit_name[k] " is 0-means-unset - arm it with Time::timerEndsAtMillis(delay), or Time::skipZero(Time::getMillis()) for a stamp (see src/UptimeClock.h)", "unset-sentinel-millis" } + # Learn from this statement before dropping it: `now = millis()` here is what makes + # `field = now` below recognisable. Done after judging so a sentinel write cannot + # taint its own name. + if (nhits == 0) note_taint(stmt) + # Comment-only lines carry an opt-out toward the write below them, so they must not # clear it; a statement with real code in it consumes it. if (stmt ~ /[^ \t]/) { pending_ok = 0; pending_bare = 0 } stmt = "" nhits = 0 } + + # Last, so every brace on this line counts toward the scope of the NEXT line: a declaration + # sits at the depth its own line opened with. + update_scope(code) } ' "$target" done diff --git a/bin/test-lint-unset-sentinel-millis.sh b/bin/test-lint-unset-sentinel-millis.sh index 94c509f415..ab0923182f 100755 --- a/bin/test-lint-unset-sentinel-millis.sh +++ b/bin/test-lint-unset-sentinel-millis.sh @@ -41,6 +41,27 @@ run_case() { fi } +# run_case_h - same, but the fixture is a HEADER, so class-scope +# cases can be pinned. A typed declaration means opposite things in a class body and a function body. +run_case_h() { + local name="$1" expect="$2" body="$3" + local dir="$WORK/case_h" + rm -rf "$dir" + mkdir -p "$dir/src" + printf '%s\n' "$body" >"$dir/src/fixture.h" + + local got want + got=$(cd "$dir" && "$LINT" src/fixture.h | awk -F: '{print $2}' | paste -sd, -) + want=$(printf '%s' "$expect" | paste -sd, -) + + if [[ $got == "$want" ]]; then + echo "PASS $name" + else + echo "FAIL $name: expected lines [$want], got [$got]" + FAILURES=$((FAILURES + 1)) + fi +} + # --- must be reported --------------------------------------------------------- run_case "bare millis() sum" "2" 'void f() { @@ -209,6 +230,82 @@ run_case "opt-out covers both writes on its own line only" "3" 'void f() { rebootAtMsec = millis() + 5000; }' +# --- clock held in a local --------------------------------------------------- +# +# The commonest shape in the tree: one `now = millis()` at the top of a runOnce(), then several +# writes from it. Without these the rule is blind to every such field and listing one buys nothing. + +run_case "stamp copied from a tainted local" "3" 'void f() { + unsigned long now = millis(); + rebootAtMsec = now; +}' + +run_case "deadline built from a tainted local" "3" 'void f() { + uint32_t now = Time::getMillis(); + tx_after = now + delay; +}' + +run_case "several writes from one tainted local" "3 +4 +5" 'void f() { + unsigned long now = millis(); + pulseOffAt = now; + rebootAtMsec = now + 5000; + lastSort = now; +}' + +run_case "taint carried one hop through another local" "4" 'void f() { + uint32_t now = millis(); + uint32_t alsoNow = now; + lastSort = alsoNow; +}' + +run_case "tainted local still fixable via the helpers" "" 'void f() { + unsigned long now = millis(); + rebootAtMsec = Time::skipZero(now); +}' + +run_case "tainted local with an opt-out" "" 'void f() { + unsigned long now = millis(); + // unset-sentinel-ok: heldX carries the armed state + nextRepeatX = now + JOY_REPEAT_INTERVAL_MS; +}' + +# --- the taint must NOT spread further than one function, one name ----------- + +run_case "untainted local is not flagged" "" 'void f() { + uint32_t now = packet->rx_time; + rebootAtMsec = now; +}' + +run_case "similarly named local is not tainted" "" 'void f() { + uint32_t now = millis(); + rebootAtMsec = nowMs; +}' + +run_case "taint dropped when the local is reassigned from something else" "" 'void f() { + uint32_t now = millis(); + now = packet->rx_time; + rebootAtMsec = now; +}' + +run_case "taint does not cross a function boundary" "" 'void f() { + uint32_t now = millis(); +} +void g() { + rebootAtMsec = now; +}' + +run_case "taint from a comparison is not recorded" "" 'void f() { + if (now == millis()) {} + rebootAtMsec = now; +}' + +run_case "compound assignment does not taint" "" 'void f() { + now += millis(); + rebootAtMsec = now; +}' + # --- one write must not be judged by its neighbour on the same line ---------- # # rhs used to run to the end of the accumulated statement, so a neighbour decided this write. @@ -239,6 +336,120 @@ run_case "multi-line statement still sees its whole right-hand side" "2" 'void f millis() + 43200 * 1000; }' +# --- stampMillis() is the read-side dodge, not a raw clock ------------------- +# +# Its name ends in millis, so the clock-read test matches it. It must still count as safe, or every +# site that normalises at the read and then stores the local gets flagged. + +run_case "storing a dodged local straight through is safe" "" 'void f() { + uint32_t now = Time::stampMillis(); + lastSort = now; +}' + +# A dodged value is safe to store or copy, NOT to do arithmetic on: stampMillis() guarantees only its +# own result, and 0xFFFFEC78 + 5000 is exactly 0. That sum is what timerEndsAtMillis() is for. +run_case "arithmetic on a dodged local can wrap back onto 0" "3" 'void f() { + uint32_t now = Time::stampMillis(); + rebootAtMsec = now + 5000; +}' + +run_case "arithmetic on a direct helper call is reported too" "2" 'void f() { + rebootAtMsec = Time::stampMillis() + 5000; +}' + +run_case "an operator INSIDE the helper call is fine" "" 'void f() { + lastSort = Time::skipZero(Time::getMillis() - msAgo); +}' + +run_case "copying a dodged local one more hop stays safe" "" 'void f() { + uint32_t now = Time::stampMillis(); + uint32_t alsoNow = now; + lastSort = alsoNow; +}' + +run_case "stampMillis directly in the write is safe" "" 'void f() { + lastSort = Time::stampMillis(); +}' + +run_case "a raw read after a safe one re-taints the local" "5" 'void f() { + uint32_t now = Time::stampMillis(); + lastSort = now; + now = millis(); + rebootAtMsec = now; +}' + +# --- class scope versus function scope --------------------------------------- +# +# A typed declaration is a shadowing local inside a function, but AT CLASS SCOPE it is the field +# itself, with an initializer that can read the clock - src/modules/SerialModule.h does that today. +# Excusing the second as a local silently skipped a real arm site. + +run_case_h "class member initialised from the clock is reported" "2" 'class Foo { + uint32_t lastSort = millis(); +};' + +run_case_h "local inside an inline method is still excused" "6" 'class Foo { + void tick() + { + uint32_t lastSort = millis(); + } + uint32_t lastDrawMsec = millis(); +};' + +run_case_h "member already routed through the helpers is quiet" "" 'class Foo { + uint32_t lastSort = Time::stampMillis(); +};' + +run_case_h "forward declaration does not open a class body" "3" 'class Foo; +void f() { + lastSort = millis(); +}' + +# --- class scope must not swallow ordinary function bodies -------------------- +# +# The keyword appears mid-line in shapes that are not class bodies, and a body opened on the same +# line puts the statement inside a function. All three reported every typed local in the body. + +run_case "template on a function is not a class body" "" 'template void f(T x) { + uint32_t lastSort = millis(); +}' + +run_case "struct in a parameter list is not a class body" "" 'void g(struct Bar *b) { + uint32_t lastSort = millis(); +}' + +run_case_h "one-line inline method is a function body" "" 'class Foo { + void tick() { uint32_t lastSort = millis(); } +};' + +run_case_h "class with a multi-line method: member yes, local no" "6" 'class Foo { + void tick() + { + uint32_t lastSort = millis(); + } + uint32_t lastDrawMsec = millis(); +};' + +# note_taint gets the same per-write cut the judging path has. +run_case "taint is not learned from a neighbour on the same line" "" 'void f() { + uint32_t a = 0; uint32_t now = packet->rx_time; + rebootAtMsec = now; +}' + +# --- a class body that opens and closes on one line --------------------------- +# +# The trailing semicolon cannot be used to rule out a class header, because the whole body fits on +# the line; and that line\'s own brace is the CLASS brace, not a function body. + +run_case_h "one-line class body reports its member initialiser" "1" 'class Foo { uint32_t lastSort = millis(); };' + +run_case_h "one-line class with a one-line method excuses the local" "" 'class Foo { void tick() { uint32_t lastSort = millis(); } };' + +run_case_h "forward declaration opens nothing" "3" 'class Foo; +void f() { + lastSort = millis(); +}' + # --- scope ------------------------------------------------------------------- # test/ builds raw wrap values on purpose, so the rule must not reach into it. diff --git a/src/Power.cpp b/src/Power.cpp index f230318f67..ee14b3c639 100644 --- a/src/Power.cpp +++ b/src/Power.cpp @@ -19,6 +19,7 @@ #include "NodeDB.h" #include "PowerFSM.h" #include "Throttle.h" +#include "UptimeClock.h" #include "WaypointStore.h" #include "buzz/buzz.h" #include "configuration.h" @@ -1296,7 +1297,7 @@ void Power::logHeapUsage() memaudit::logBreakdown("periodic"); lastHeapLogFree = heapFree; - lastHeapLogTime = millis(); + lastHeapLogTime = Time::skipZero(Time::getMillis()); #endif } diff --git a/src/UptimeClock.h b/src/UptimeClock.h index 18895fd550..853fe7cbcf 100644 --- a/src/UptimeClock.h +++ b/src/UptimeClock.h @@ -58,6 +58,13 @@ inline uint32_t timerEndsAtMillis(uint32_t delayMs) return skipZero(getMillis() + delayMs); } +/// getMillis() for 0-means-unset stamps, with the 0 tick called 1. Use at the read when the value +/// is both stored and compared against stamps: skipZero() only at the store makes `now - stamp` wrap. +inline uint32_t stampMillis() +{ + return skipZero(getMillis()); +} + // skipZero() is the whole 0-means-unset contract in one expression, and it is constexpr, so pin it // here rather than only in test_uptime_clock: a build that breaks it stops at this header instead of // shipping a deadline that reads as never-set. The two obvious "simplifications" are what these diff --git a/src/gps/RTC.cpp b/src/gps/RTC.cpp index 58c00e12ee..ca5ebe146a 100644 --- a/src/gps/RTC.cpp +++ b/src/gps/RTC.cpp @@ -332,7 +332,7 @@ RTCSetResult perhapsSetRTC(RTCQuality q, const struct timeval *tv, bool forceUpd currentQuality = q; lastSetMsec = now; if (currentQuality >= RTCQualityNTP) { - lastSetFromPhoneNtpOrGps = now; + lastSetFromPhoneNtpOrGps = Time::skipZero(now); } // This delta value works on all platforms diff --git a/src/graphics/BaseUIEInkDisplay.cpp b/src/graphics/BaseUIEInkDisplay.cpp index 0506ba68dd..15d00f1ee7 100644 --- a/src/graphics/BaseUIEInkDisplay.cpp +++ b/src/graphics/BaseUIEInkDisplay.cpp @@ -1,6 +1,7 @@ #ifdef MESHTASTIC_INCLUDE_NICHE_GRAPHICS #include "./BaseUIEInkDisplay.h" +#include "UptimeClock.h" #include "configuration.h" #include "main.h" @@ -73,7 +74,7 @@ void BaseUIEInkDisplay::display() // Keyframe path. Returns true if a frame was pushed (sets lastDrawMsec). bool BaseUIEInkDisplay::forceDisplay(uint32_t msecLimit) { - const uint32_t now = millis(); + const uint32_t now = Time::stampMillis(); if (lastDrawMsec != 0 && (now - lastDrawMsec) < msecLimit) return false; diff --git a/src/graphics/EInkDisplay2.cpp b/src/graphics/EInkDisplay2.cpp index de058bc1f9..aec2c2caf2 100644 --- a/src/graphics/EInkDisplay2.cpp +++ b/src/graphics/EInkDisplay2.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #include "graphics/Backlight.h" @@ -59,7 +60,7 @@ bool EInkDisplay::forceDisplay(uint32_t msecLimit) // No need to grab this lock because we are on our own SPI bus // concurrency::LockGuard g(spiLock); - uint32_t now = millis(); + uint32_t now = Time::stampMillis(); uint32_t sinceLast = now - lastDrawMsec; if (adafruitDisplay && (sinceLast > msecLimit || lastDrawMsec == 0)) diff --git a/src/graphics/EInkParallelDisplay.cpp b/src/graphics/EInkParallelDisplay.cpp index a61b1bae97..d8d0f9475c 100644 --- a/src/graphics/EInkParallelDisplay.cpp +++ b/src/graphics/EInkParallelDisplay.cpp @@ -1,4 +1,5 @@ #include "EInkParallelDisplay.h" +#include "UptimeClock.h" #ifdef USE_EINK_PARALLELDISPLAY @@ -208,7 +209,7 @@ void EInkParallelDisplay::display(void) const uint16_t h = this->displayHeight; // Simple rate limiting: avoid very-frequent responsive updates - uint32_t nowMs = millis(); + uint32_t nowMs = Time::stampMillis(); if (lastUpdateMs != 0 && (nowMs - lastUpdateMs) < EPD_RESPONSIVE_MIN_MS) { LOG_DEBUG("rate-limited, skipping update"); return; @@ -367,11 +368,11 @@ void EInkParallelDisplay::display(void) startAsyncFullUpdate(forceFull ? CLEAR_SLOW : CLEAR_FAST); } - lastUpdateMs = millis(); + lastUpdateMs = Time::stampMillis(); previousImageHash = imageHash; // Keep same behavior as before - lastDrawMsec = millis(); + lastDrawMsec = Time::stampMillis(); } #ifdef EINK_LIMIT_GHOSTING_PX @@ -420,7 +421,7 @@ bool EInkParallelDisplay::forceDisplay(uint32_t msecLimit) if (!displayReady) return false; - uint32_t now = millis(); + uint32_t now = Time::stampMillis(); if (lastDrawMsec == 0 || (now - lastDrawMsec) > msecLimit) { display(); return true; diff --git a/src/graphics/Screen.cpp b/src/graphics/Screen.cpp index 6b2ed5b481..8d9c35ae42 100644 --- a/src/graphics/Screen.cpp +++ b/src/graphics/Screen.cpp @@ -493,7 +493,7 @@ float Screen::estimatedHeading(double lat, double lon) static double oldLat, oldLon; static float b = -1.0f; static uint32_t lastHeadingAtMs = 0; - const uint32_t now = millis(); + const uint32_t now = Time::stampMillis(); const uint32_t gpsUpdateIntervalSecs = Default::getConfiguredOrDefault(config.position.gps_update_interval, default_gps_update_interval); uint32_t effectiveUpdateIntervalSecs = gpsUpdateIntervalSecs; diff --git a/src/graphics/draw/UIRenderer.cpp b/src/graphics/draw/UIRenderer.cpp index 9a7638280c..58d759679b 100644 --- a/src/graphics/draw/UIRenderer.cpp +++ b/src/graphics/draw/UIRenderer.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #if HAS_SCREEN #include "CompassRenderer.h" @@ -2212,12 +2213,12 @@ void UIRenderer::drawNavigationBar(OLEDDisplay *display, OLEDDisplayUiState *sta if (navBarVisible && !navBarPrevVisible) { EINK_ADD_FRAMEFLAG(display, DEMAND_FAST); // Fast refresh when showing nav bar cosmeticRefreshDone = false; - navBarLastShown = millis(); + navBarLastShown = Time::skipZero(Time::getMillis()); } if (!navBarVisible && navBarPrevVisible) { - EINK_ADD_FRAMEFLAG(display, DEMAND_FAST); // Fast refresh when hiding nav bar - navBarLastShown = millis(); // Mark when it disappeared + EINK_ADD_FRAMEFLAG(display, DEMAND_FAST); // Fast refresh when hiding nav bar + navBarLastShown = Time::skipZero(Time::getMillis()); // Mark when it disappeared } if (!navBarVisible && navBarLastShown != 0 && !cosmeticRefreshDone) { diff --git a/src/input/ExpressLRSFiveWay.cpp b/src/input/ExpressLRSFiveWay.cpp index e9efeda52e..7d1e4de639 100644 --- a/src/input/ExpressLRSFiveWay.cpp +++ b/src/input/ExpressLRSFiveWay.cpp @@ -80,7 +80,7 @@ void ExpressLRSFiveWay::update(int *keyValue, bool *keyLongPressed) if (keyInProcess == NO_PRESS) { // New key down if (newKey != NO_PRESS) { - keyDownStart = Time::getMillis(); + keyDownStart = Time::skipZero(Time::getMillis()); // DBGLN("down=%u", newKey); } } else { diff --git a/src/input/LinuxJoystick.cpp b/src/input/LinuxJoystick.cpp index 8951a00b1c..d8f99ccec0 100644 --- a/src/input/LinuxJoystick.cpp +++ b/src/input/LinuxJoystick.cpp @@ -167,10 +167,12 @@ int32_t LinuxJoystick::runOnce() uint32_t now = millis(); if (heldX != 0 && (int32_t)(now - nextRepeatX) >= 0) { emitEvent((heldX < 0) ? INPUT_BROKER_LEFT : INPUT_BROKER_RIGHT); + // unset-sentinel-ok: heldX carries the armed state, so 0 is a legal deadline nextRepeatX = now + JOY_REPEAT_INTERVAL_MS; } if (heldY != 0 && (int32_t)(now - nextRepeatY) >= 0) { emitEvent((heldY < 0) ? INPUT_BROKER_UP : INPUT_BROKER_DOWN); + // unset-sentinel-ok: heldY carries the armed state, so 0 is a legal deadline nextRepeatY = now + JOY_REPEAT_INTERVAL_MS; } diff --git a/src/input/RotaryEncoderInterruptBase.cpp b/src/input/RotaryEncoderInterruptBase.cpp index c177403bf0..20e40d58e7 100644 --- a/src/input/RotaryEncoderInterruptBase.cpp +++ b/src/input/RotaryEncoderInterruptBase.cpp @@ -1,4 +1,5 @@ #include "RotaryEncoderInterruptBase.h" +#include "UptimeClock.h" #include "configuration.h" RotaryEncoderInterruptBase::RotaryEncoderInterruptBase(const char *name) : concurrency::OSThread(name) @@ -48,13 +49,14 @@ int32_t RotaryEncoderInterruptBase::runOnce() InputEvent e = {}; e.inputEvent = INPUT_BROKER_NONE; e.source = this->_originName; - unsigned long now = millis(); + unsigned long now = Time::stampMillis(); // Handle press long/short detection if (this->action == ROTARY_ACTION_PRESSED) { bool buttonPressed = !digitalRead(_pinPress); if (!pressDetected && buttonPressed) { pressDetected = true; + // unset-sentinel-ok: pressDetected is the armed flag; no read tests the stamp against 0 pressStartTime = now; pressAndTurnFired = false; } diff --git a/src/input/TrackballInterruptBase.cpp b/src/input/TrackballInterruptBase.cpp index 2be2495e12..e30cddd90e 100644 --- a/src/input/TrackballInterruptBase.cpp +++ b/src/input/TrackballInterruptBase.cpp @@ -203,20 +203,20 @@ int32_t TrackballInterruptBase::runOnce() if (e.inputEvent == INPUT_BROKER_NONE) { if (this->action == TB_ACTION_UP && !digitalRead(_pinUp) && !directionDetected) { directionDetected = true; - directionStartTime = millis(); + directionStartTime = Time::skipZero(Time::getMillis()); e.inputEvent = this->_eventUp; // send event first,will automatically trigger every 50ms * 3 after 500ms } else if (this->action == TB_ACTION_DOWN && !digitalRead(_pinDown) && !directionDetected) { directionDetected = true; - directionStartTime = millis(); + directionStartTime = Time::skipZero(Time::getMillis()); e.inputEvent = this->_eventDown; } else if (this->action == TB_ACTION_LEFT && !digitalRead(_pinLeft) && !directionDetected) { directionDetected = true; - directionStartTime = millis(); + directionStartTime = Time::skipZero(Time::getMillis()); e.inputEvent = this->_eventLeft; } else if (this->action == TB_ACTION_RIGHT && !digitalRead(_pinRight) && !directionDetected) { directionDetected = true; - directionStartTime = millis(); + directionStartTime = Time::skipZero(Time::getMillis()); e.inputEvent = this->_eventRight; } } diff --git a/src/input/UpDownInterruptBase.cpp b/src/input/UpDownInterruptBase.cpp index d597c8d8f4..5ad0d64ff0 100644 --- a/src/input/UpDownInterruptBase.cpp +++ b/src/input/UpDownInterruptBase.cpp @@ -1,4 +1,5 @@ #include "UpDownInterruptBase.h" +#include "UptimeClock.h" #include "configuration.h" UpDownInterruptBase::UpDownInterruptBase(const char *name) : concurrency::OSThread(name) @@ -50,7 +51,7 @@ int32_t UpDownInterruptBase::runOnce() { InputEvent e = {}; e.inputEvent = INPUT_BROKER_NONE; - unsigned long now = millis(); + unsigned long now = Time::stampMillis(); // Read all button states once at the beginning bool pressButtonPressed = !digitalRead(_pinPress); diff --git a/src/mesh/IndicatorSerial.cpp b/src/mesh/IndicatorSerial.cpp index 74608b5fa9..e146178954 100644 --- a/src/mesh/IndicatorSerial.cpp +++ b/src/mesh/IndicatorSerial.cpp @@ -1,6 +1,7 @@ #ifdef SENSECAP_INDICATOR #include "IndicatorSerial.h" +#include "UptimeClock.h" #include "concurrency/LockGuard.h" #include "mesh/comms/UARTProxy.h" #include @@ -61,7 +62,7 @@ void SensecapIndicator::probe_link() msg.data.ping = meshtastic_InterdeviceVersion_INTERDEVICE_VERSION_CURRENT; stamp_request(msg); send_uplink_unlocked(msg); - last_probe = millis(); + last_probe = Time::skipZero(Time::getMillis()); } // Read whatever is available on the link and process complete packets diff --git a/src/mesh/NextHopRouter.cpp b/src/mesh/NextHopRouter.cpp index f6c8857f46..00ccc814ef 100644 --- a/src/mesh/NextHopRouter.cpp +++ b/src/mesh/NextHopRouter.cpp @@ -201,7 +201,7 @@ void NextHopRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtast p->relay_node, wasAlreadyRelayer, weWereSoleRelayer); origTx->next_hop = p->relay_node; } - noteRouteLearned(p->from, p->relay_node, Time::getMillis()); // M3: anchor freshness (hot or overflow route) + noteRouteLearned(p->from, p->relay_node, Time::stampMillis()); // M3: anchor freshness (hot or overflow route) #if HAS_TRAFFIC_MANAGEMENT // Mirror the confirmed (and now unique-resolved) hop into the TMM overflow cache so it // survives even when the source isn't (or is no longer) in the hot NodeDB. @@ -313,7 +313,7 @@ std::optional NextHopRouter::getNextHop(NodeNum to, uint8_t relay_node) // a health record that still matches the stored byte; a next_hop set by another path (e.g. // TraceRouteModule) with no matching record is left authoritative. const RouteHealth *h = findRouteHealth(to); - if (h && h->lastNextHop == node->next_hop && isRouteStale(*h, Time::getMillis())) { + if (h && h->lastNextHop == node->next_hop && isRouteStale(*h, Time::stampMillis())) { LOG_INFO("Next hop 0x%x for 0x%08x stale (age/fails); flood and clear", node->next_hop, to); node->next_hop = NO_NEXT_HOP_PREFERENCE; // clear persisted route clearRouteHealth(to); // clear RAM health @@ -345,7 +345,7 @@ std::optional NextHopRouter::getNextHop(NodeNum to, uint8_t relay_node) uint8_t hint = trafficManagementModule->getNextHopHint(to); if (hint && hint != relay_node) { const RouteHealth *h = findRouteHealth(to); - if (h && h->lastNextHop == hint && isRouteStale(*h, Time::getMillis())) { + if (h && h->lastNextHop == hint && isRouteStale(*h, Time::stampMillis())) { LOG_INFO("TMM next hop 0x%x for 0x%08x stale (age/fails); flood and clear", hint, to); trafficManagementModule->clearNextHop(to); // clear overflow route (setNextHop won't store 0) clearRouteHealth(to); // clear RAM health @@ -444,7 +444,7 @@ int32_t NextHopRouter::doRetransmissions() { // Same clock Throttle reads, so setNextTx() deadlines and this test can't diverge under an // injected test clock. - uint32_t now = Time::getMillis(); + uint32_t now = Time::stampMillis(); int32_t d = INT32_MAX; // FIXME, we should use a better datastructure rather than walking through this map. @@ -617,6 +617,7 @@ void NextHopRouter::noteRouteLearned(NodeNum dest, uint8_t nextHop, uint32_t now h->lastNextHop = nextHop; h->consecutiveFailures = 0; } + // `now` is a parameter, so guard at the store too: 0 is the empty-slot marker. h->learnedAtMsec = Time::skipZero(now); } @@ -626,7 +627,7 @@ void NextHopRouter::noteRouteSuccess(NodeNum dest, uint32_t now) if (!h) return; // only routes we actually learned have health to refresh h->consecutiveFailures = 0; - h->learnedAtMsec = Time::skipZero(now); + h->learnedAtMsec = Time::skipZero(now); // a parameter, so guard at the store too } void NextHopRouter::noteRouteFailure(NodeNum dest) diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 17f063a64f..aa5ff2d48e 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -245,7 +245,7 @@ static void clearAuthSlot_LH(const PhoneAPI *p) PhoneAPI::PhoneAPI() { - lastContactMsec = millis(); + lastContactMsec = Time::skipZero(Time::getMillis()); std::fill(std::begin(recentToRadioPacketIds), std::end(recentToRadioPacketIds), 0); } @@ -437,7 +437,7 @@ bool PhoneAPI::checkConnectionTimeout() bool PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength) { powerFSM.trigger(EVENT_CONTACT_FROM_PHONE); // As long as the phone keeps talking to us, don't let the radio go to sleep - lastContactMsec = millis(); + lastContactMsec = Time::skipZero(Time::getMillis()); memset(&toRadioScratch, 0, sizeof(toRadioScratch)); if (pb_decode_from_bytes(buf, bufLength, &meshtastic_ToRadio_msg, &toRadioScratch)) { diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index a3c86cd0ef..7be23ee810 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -181,7 +181,7 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas // M3: an end-to-end ACK proves the directed route to the ACK's sender currently works, // so clear its failure count and refresh freshness (keeps a good route pinned). if (!isBroadcast(getFrom(p))) - noteRouteSuccess(getFrom(p), Time::getMillis()); + noteRouteSuccess(getFrom(p), Time::stampMillis()); } else { stopRetransmission(p->to, nakId); } diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index c4d670f830..55a10ebaa4 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1151,7 +1151,7 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) JSONFile.close(); } JSONFile.open(portduino_config.JSONFilename + "_" + datetime, std::ios::out | std::ios::app); - fileage = millis(); + fileage = Time::skipZero(Time::getMillis()); } } if (portduino_config.JSONFilter == (_meshtastic_PortNum)0 || portduino_config.JSONFilter == p->decoded.portnum) { diff --git a/src/mesh/TransmitHistory.cpp b/src/mesh/TransmitHistory.cpp index 35144ec0d7..4b2ebc8a37 100644 --- a/src/mesh/TransmitHistory.cpp +++ b/src/mesh/TransmitHistory.cpp @@ -1,6 +1,7 @@ #include "TransmitHistory.h" #include "FSCommon.h" #include "SPILock.h" +#include "UptimeClock.h" #include "gps/RTC.h" #include @@ -81,7 +82,7 @@ void TransmitHistory::loadFromDisk() void TransmitHistory::setLastSentToMesh(uint16_t key) { - lastMillis[key] = millis(); + lastMillis[key] = Time::skipZero(Time::getMillis()); uint32_t now = getTime(); if (now >= 2) { const uint8_t flags = (getRTCQuality() == RTCQualityNone) ? ENTRY_FLAG_BOOT_RELATIVE : ENTRY_FLAG_NONE; @@ -94,7 +95,7 @@ void TransmitHistory::setLastSentToMesh(uint16_t key) // after boot so a crash-reboot loop can't avoid persisting. if (lastDiskSave == 0 || !Throttle::isWithinTimespanMs(lastDiskSave, SAVE_INTERVAL_MS)) { if (saveToDisk()) { - lastDiskSave = millis(); + lastDiskSave = Time::skipZero(Time::getMillis()); } } } @@ -151,7 +152,7 @@ uint32_t TransmitHistory::getLastSentAbsoluteMillis(uint32_t storedEpoch) const return 0; } - return millis() - msAgo; + return Time::skipZero(Time::getMillis() - msAgo); } uint32_t TransmitHistory::getLastSentBootRelativeMillis(uint32_t storedSeconds) const @@ -167,7 +168,7 @@ uint32_t TransmitHistory::getLastSentBootRelativeMillis(uint32_t storedSeconds) if (secondsAgo > BOOT_RELATIVE_RECOVERY_WINDOW_SEC) { return 0; } - return millis() - (secondsAgo * 1000); + return Time::skipZero(Time::getMillis() - (secondsAgo * 1000)); } uint32_t secondsAhead = storedSeconds - now; @@ -175,7 +176,7 @@ uint32_t TransmitHistory::getLastSentBootRelativeMillis(uint32_t storedSeconds) return 0; } - return millis(); + return Time::skipZero(Time::getMillis()); } uint32_t TransmitHistory::getLastSentToMeshMillis(uint16_t key) const @@ -286,7 +287,7 @@ void TransmitHistory::loadFromDisk() {} void TransmitHistory::setLastSentToMesh(uint16_t key) { - lastMillis[key] = millis(); + lastMillis[key] = Time::skipZero(Time::getMillis()); } uint32_t TransmitHistory::getLastSentToMeshEpoch(uint16_t key) const diff --git a/src/mesh/api/PacketAPI.cpp b/src/mesh/api/PacketAPI.cpp index c8adda5204..3590cfdb76 100644 --- a/src/mesh/api/PacketAPI.cpp +++ b/src/mesh/api/PacketAPI.cpp @@ -2,6 +2,7 @@ // First, in its own block so the include sorter keeps it there: configuration.h supplies the // variant defines mesh-pb-constants.h needs (portduino resolves MAX_NUM_NODES at runtime). +#include "UptimeClock.h" #include "configuration.h" #include "MeshService.h" @@ -59,7 +60,7 @@ bool PacketAPI::receivePacket(void) data_received = true; powerFSM.trigger(EVENT_INPUT); - lastContactMsec = millis(); + lastContactMsec = Time::skipZero(Time::getMillis()); meshtastic_ToRadio *mr; auto p = server->receivePacket()->move(); diff --git a/src/mesh/eth/ethOTA.cpp b/src/mesh/eth/ethOTA.cpp index b99ff73046..458b9cf5fc 100644 --- a/src/mesh/eth/ethOTA.cpp +++ b/src/mesh/eth/ethOTA.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #if HAS_ETHERNET && defined(HAS_ETHERNET_OTA) @@ -119,7 +120,7 @@ static bool authenticateClient(EthernetClient &client) uint8_t clientHash[OTA_HASH_SIZE]; if (!readExact(client, clientHash, OTA_HASH_SIZE)) { LOG_WARN("ETH OTA: Timeout reading auth response"); - lastAuthFailure = millis(); + lastAuthFailure = Time::skipZero(Time::getMillis()); return false; } @@ -136,7 +137,7 @@ static bool authenticateClient(EthernetClient &client) if (diff != 0) { LOG_WARN("ETH OTA: Authentication failed"); client.write(OTA_ERR_AUTH); - lastAuthFailure = millis(); + lastAuthFailure = Time::skipZero(Time::getMillis()); return false; } diff --git a/src/mesh/http/WebServer.cpp b/src/mesh/http/WebServer.cpp index 8a44895241..befc5d8776 100644 --- a/src/mesh/http/WebServer.cpp +++ b/src/mesh/http/WebServer.cpp @@ -111,7 +111,7 @@ static void handleWebResponse() static uint32_t lastHeapWarning = 0; if (lastHeapWarning == 0 || !Throttle::isWithinTimespanMs(lastHeapWarning, 30000)) { LOG_WARN("Low heap (%u bytes), not accepting HTTPS connections", freeHeap); - lastHeapWarning = millis(); + lastHeapWarning = Time::skipZero(Time::getMillis()); } } } diff --git a/src/mesh/wifi/WiFiAPClient.cpp b/src/mesh/wifi/WiFiAPClient.cpp index 35cb5653eb..8f53187279 100644 --- a/src/mesh/wifi/WiFiAPClient.cpp +++ b/src/mesh/wifi/WiFiAPClient.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #if HAS_WIFI #include "NodeDB.h" @@ -313,7 +314,7 @@ static int32_t reconnectWiFi() tv.tv_usec = 0; perhapsSetRTC(RTCQualityNTP, &tv); - lastrun_ntp = millis(); + lastrun_ntp = Time::skipZero(Time::getMillis()); } else { LOG_DEBUG("NTP Update failed"); } diff --git a/src/modules/DropzoneModule.cpp b/src/modules/DropzoneModule.cpp index 100b87662c..4adcf8c4a7 100644 --- a/src/modules/DropzoneModule.cpp +++ b/src/modules/DropzoneModule.cpp @@ -2,6 +2,7 @@ #include "DropzoneModule.h" #include "Meshservice->h" +#include "UptimeClock.h" #include "configuration.h" #include "gps/GeoCoord.h" #include "gps/RTC.h" @@ -39,13 +40,13 @@ ProcessMessage DropzoneModule::handleReceived(const meshtastic_MeshPacket &mp) snprintf(matchCompare, sizeof(matchCompare), "%s conditions", owner.short_name); if (received >= strlen(matchCompare) && strncasecmp(incomingMessage, matchCompare, strlen(matchCompare)) == 0) { LOG_DEBUG("Received dropzone conditions request"); - startSendConditions = millis(); + startSendConditions = Time::skipZero(Time::getMillis()); } snprintf(matchCompare, sizeof(matchCompare), "%s conditions", owner.long_name); if (received >= strlen(matchCompare) && strncasecmp(incomingMessage, matchCompare, strlen(matchCompare)) == 0) { LOG_DEBUG("Received dropzone conditions request"); - startSendConditions = millis(); + startSendConditions = Time::skipZero(Time::getMillis()); } return ProcessMessage::CONTINUE; } diff --git a/src/modules/KeyVerificationModule.cpp b/src/modules/KeyVerificationModule.cpp index eb6c496413..29594f64ab 100644 --- a/src/modules/KeyVerificationModule.cpp +++ b/src/modules/KeyVerificationModule.cpp @@ -3,6 +3,7 @@ #include "CryptoEngine.h" #include "HardwareRNG.h" #include "MeshService.h" +#include "UptimeClock.h" #include "gps/RTC.h" #include "graphics/draw/MenuHandler.h" #include "main.h" @@ -400,7 +401,7 @@ void KeyVerificationModule::resetToIdle() memset(hash1, 0, 32); memset(hash2, 0, 32); if (sessionFromRemote) - lastRemoteSessionMs = millis(); // start the cooldown when the session ends, not when it opened + lastRemoteSessionMs = Time::skipZero(Time::getMillis()); // start the cooldown when the session ends, not when it opened sessionFromRemote = false; currentNonce = 0; currentNonceTimestamp = 0; diff --git a/src/modules/PositionModule.cpp b/src/modules/PositionModule.cpp index 213237341a..68a7ccdf66 100644 --- a/src/modules/PositionModule.cpp +++ b/src/modules/PositionModule.cpp @@ -35,6 +35,7 @@ PositionModule::PositionModule() if (transmitHistory) { uint32_t restored = transmitHistory->getLastSentToMeshMillis(meshtastic_PortNum_POSITION_APP); if (restored != 0) { + // unset-sentinel-ok: the enclosing restored != 0 already rules out the unset value lastGpsSend = restored; LOG_INFO("Position: restored lastGpsSend from transmit history"); } @@ -550,7 +551,7 @@ int32_t PositionModule::runOnce() if (node == nullptr) return RUNONCE_INTERVAL; - uint32_t now = Time::getMillis(); + uint32_t now = Time::stampMillis(); // Local-only delivery, so it runs regardless of mesh opt-in state or channel utilization. // Only send while the queue is empty (phone assumed connected), like telemetry. The cadence @@ -709,7 +710,7 @@ void PositionModule::trySmartBroadcast(const meshtastic_PositionLite &selfPos, u if (!sendOurPosition()) return; - lastGpsSend = nowMs; + lastGpsSend = Time::skipZero(nowMs); // nowMs is a parameter, so guard at the store as well if (transmitHistory) transmitHistory->setLastSentToMesh(meshtastic_PortNum_POSITION_APP); LOG_DEBUG("Sent smart pos@%x:6 to mesh (distanceTraveled=%fm, minDistanceThreshold=%im, timeElapsed=%ims, " @@ -729,7 +730,7 @@ void PositionModule::handleNewPosition() meshtastic_PositionLite selfPos; if (!nodeDB->copyNodePosition(node->num, selfPos)) return; - trySmartBroadcast(selfPos, Time::getMillis()); + trySmartBroadcast(selfPos, Time::stampMillis()); } } diff --git a/src/modules/Telemetry/AirQualityTelemetry.cpp b/src/modules/Telemetry/AirQualityTelemetry.cpp index 2eb596bd96..bfb0ab73bb 100644 --- a/src/modules/Telemetry/AirQualityTelemetry.cpp +++ b/src/modules/Telemetry/AirQualityTelemetry.cpp @@ -1,4 +1,5 @@ #include "DebugConfiguration.h" +#include "UptimeClock.h" #include "configuration.h" #if HAS_TELEMETRY && !MESHTASTIC_EXCLUDE_AIR_QUALITY_SENSOR @@ -244,7 +245,7 @@ int32_t AirQualityTelemetryModule::runOnce() } else if (phoneDue && phoneAllowed) { // Mesh transmission isn't due yet, but we can still update the phone. if (sendTelemetry(NODENUM_BROADCAST, true)) { - lastSentToPhone = millis(); + lastSentToPhone = Time::skipZero(Time::getMillis()); // Correct the awake time, trimming to 0 const unsigned long elapsed = millis() - startAirQualityTelemetryCycle; awakeAheadOfTimeMs = elapsed >= awakeAheadOfTimeMs ? 0 : awakeAheadOfTimeMs - elapsed; diff --git a/src/modules/Telemetry/DeviceTelemetry.cpp b/src/modules/Telemetry/DeviceTelemetry.cpp index 8d3834137e..1b6b074e00 100644 --- a/src/modules/Telemetry/DeviceTelemetry.cpp +++ b/src/modules/Telemetry/DeviceTelemetry.cpp @@ -41,7 +41,7 @@ int32_t DeviceTelemetryModule::runOnce() sendTelemetry(NODENUM_BROADCAST, true); if (lastSentStatsToPhone == 0 || Throttle::hasElapsed(lastSentStatsToPhone, sendStatsToPhoneIntervalMs)) { sendLocalStatsToPhone(); - lastSentStatsToPhone = Time::getMillis(); + lastSentStatsToPhone = Time::skipZero(Time::getMillis()); } } return sendToPhoneIntervalMs; diff --git a/src/modules/Telemetry/EnvironmentTelemetry.cpp b/src/modules/Telemetry/EnvironmentTelemetry.cpp index a4143de299..e638a5db53 100644 --- a/src/modules/Telemetry/EnvironmentTelemetry.cpp +++ b/src/modules/Telemetry/EnvironmentTelemetry.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #if HAS_TELEMETRY && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR @@ -466,7 +467,7 @@ int32_t EnvironmentTelemetryModule::runOnce() // Just send to phone when it's not our time to send to mesh yet // Only send while queue is empty (phone assumed connected) sendTelemetry(NODENUM_BROADCAST, true); - lastSentToPhone = millis(); + lastSentToPhone = Time::skipZero(Time::getMillis()); } } if (sleepOnNextExecution) { diff --git a/src/modules/Telemetry/HealthTelemetry.cpp b/src/modules/Telemetry/HealthTelemetry.cpp index 6ec316e701..10b8d68153 100644 --- a/src/modules/Telemetry/HealthTelemetry.cpp +++ b/src/modules/Telemetry/HealthTelemetry.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #if !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR && !MESHTASTIC_EXCLUDE_HEALTH_TELEMETRY && !defined(ARCH_PORTDUINO) @@ -90,7 +91,7 @@ int32_t HealthTelemetryModule::runOnce() // Just send to phone when it's not our time to send to mesh yet // Only send while queue is empty (phone assumed connected) sendTelemetry(NODENUM_BROADCAST, true); - lastSentToPhone = millis(); + lastSentToPhone = Time::skipZero(Time::getMillis()); } } if (sleepOnNextExecution) { diff --git a/src/modules/Telemetry/PowerTelemetry.cpp b/src/modules/Telemetry/PowerTelemetry.cpp index 684ce20133..f73fbf8120 100644 --- a/src/modules/Telemetry/PowerTelemetry.cpp +++ b/src/modules/Telemetry/PowerTelemetry.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #if !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR @@ -105,7 +106,7 @@ int32_t PowerTelemetryModule::runOnce() // Just send to phone when it's not our time to send to mesh yet // Only send while queue is empty (phone assumed connected) sendTelemetry(NODENUM_BROADCAST, true); - lastSentToPhone = millis(); + lastSentToPhone = Time::skipZero(Time::getMillis()); } } if (sleepOnNextExecution) { diff --git a/src/modules/TraceRouteModule.cpp b/src/modules/TraceRouteModule.cpp index 310cf4bc1b..4549f19df9 100644 --- a/src/modules/TraceRouteModule.cpp +++ b/src/modules/TraceRouteModule.cpp @@ -1,6 +1,7 @@ #include "TraceRouteModule.h" #include "MeshService.h" #include "NodeDB.h" +#include "UptimeClock.h" #include "graphics/Screen.h" #include "graphics/ScreenFonts.h" #include "graphics/SharedUIDisplay.h" @@ -534,7 +535,7 @@ const char *TraceRouteModule::getNodeName(NodeNum node) bool TraceRouteModule::startTraceRoute(NodeNum node) { LOG_INFO("TraceRoute startTraceRoute: node=0x%08x", node); - unsigned long now = millis(); + unsigned long now = Time::stampMillis(); if (node == 0 || node == NODENUM_BROADCAST) { LOG_ERROR("Invalid trace route node: 0x%08x", node); @@ -700,7 +701,7 @@ void TraceRouteModule::launch(NodeNum node) LOG_INFO("TraceRoute first init"); } - unsigned long now = millis(); + unsigned long now = Time::stampMillis(); if (initialized && lastTraceRouteTime > 0 && now - lastTraceRouteTime < cooldownMs) { unsigned long wait = (cooldownMs - (now - lastTraceRouteTime)) / 1000; bannerText = String("Wait for ") + String(wait) + String("s"); @@ -842,7 +843,7 @@ void TraceRouteModule::drawFrame(OLEDDisplay *display, OLEDDisplayUiState *state #endif // HAS_SCREEN int32_t TraceRouteModule::runOnce() { - unsigned long now = millis(); + unsigned long now = Time::stampMillis(); if (runState == TRACEROUTE_STATE_IDLE) { return INT32_MAX; diff --git a/src/modules/TrafficManagementModule.cpp b/src/modules/TrafficManagementModule.cpp index f12fbfbcdc..f0892e4590 100644 --- a/src/modules/TrafficManagementModule.cpp +++ b/src/modules/TrafficManagementModule.cpp @@ -1,4 +1,5 @@ #include "TrafficManagementModule.h" +#include "UptimeClock.h" #if HAS_TRAFFIC_MANAGEMENT @@ -1523,7 +1524,7 @@ bool TrafficManagementModule::shouldRespondToNodeInfo(const meshtastic_MeshPacke // request declined above never spends the budget). false forwards the request instead of consuming // it. Rationale in https://meshtastic.org/docs/development/reference/traffic-management-internals "Throttling direct // responses". - if (!directResponseAllowed(getFrom(p), p->to, clockMs())) { + if (!directResponseAllowed(getFrom(p), p->to, Time::skipZero(clockMs()))) { TM_LOG_DEBUG("NodeInfo direct response throttled for 0x%08x; forwarding request", getFrom(p)); return false; } @@ -1627,7 +1628,7 @@ bool TrafficManagementModule::directResponseAllowed(NodeNum requester, NodeNum t reqSlot->lastReplyMs = nowMs; tgtSlot->key = target; tgtSlot->lastReplyMs = nowMs; - lastDirectResponseMs = nowMs; + lastDirectResponseMs = Time::skipZero(nowMs); // a parameter, so guard at the store as well return true; } diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 1c6cd57a0c..8108ba050c 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -3,6 +3,7 @@ #include "NodeDB.h" #include "PowerFSM.h" #include "ServiceEnvelope.h" +#include "UptimeClock.h" #include "configuration.h" #include "main.h" #include "mesh/Channels.h" @@ -870,5 +871,5 @@ void MQTT::perhapsReportToMap() packetPool.release(mp); // Update the last report time - last_report_to_map = millis(); + last_report_to_map = Time::skipZero(Time::getMillis()); } diff --git a/src/platform/extra_variants/t5s3_epaper/variant.cpp b/src/platform/extra_variants/t5s3_epaper/variant.cpp index 548fec421d..39ebcdc231 100644 --- a/src/platform/extra_variants/t5s3_epaper/variant.cpp +++ b/src/platform/extra_variants/t5s3_epaper/variant.cpp @@ -1,3 +1,4 @@ +#include "UptimeClock.h" #include "configuration.h" #ifdef T5_S3_EPAPER_PRO @@ -555,7 +556,7 @@ struct TouchLightSleepEndObserver { } touchStateEpoch++; - touchResumeAtMs = millis(); + touchResumeAtMs = Time::skipZero(Time::getMillis()); touchIndicatorRefreshPending = !isTouchInputEnabled(); #ifdef MESHTASTIC_INCLUDE_NICHE_GRAPHICS // Clear sleep-time touch overlay after wake. @@ -602,7 +603,7 @@ bool readTouch(int16_t *x, int16_t *y) LOG_DEBUG("touchscreen1: wakeup() on deferred resume"); touch.wakeup(); touchNeedsWake = false; - suppressFromMs = millis(); + suppressFromMs = Time::skipZero(Time::getMillis()); return false; } diff --git a/src/platform/nrf54l15/NRF54L15Bluetooth.cpp b/src/platform/nrf54l15/NRF54L15Bluetooth.cpp index 9ce0326316..8cef2fb59c 100644 --- a/src/platform/nrf54l15/NRF54L15Bluetooth.cpp +++ b/src/platform/nrf54l15/NRF54L15Bluetooth.cpp @@ -21,6 +21,7 @@ #include "BluetoothCommon.h" #include "BluetoothStatus.h" #include "PowerFSM.h" +#include "UptimeClock.h" #include "concurrency/OSThread.h" #include "configuration.h" #include "main.h" @@ -387,7 +388,7 @@ static void connected_cb(struct bt_conn *conn, uint8_t err) k_mutex_unlock(&ble_mutex); memset(lastToRadio, 0, sizeof(lastToRadio)); - connect_time_ms = k_uptime_get_32(); + connect_time_ms = Time::skipZero(k_uptime_get_32()); last_att_time_ms = connect_time_ms; char addr[BT_ADDR_LE_STR_LEN]; diff --git a/src/platform/stm32wl/main-stm32wl.cpp b/src/platform/stm32wl/main-stm32wl.cpp index 1f363edfa9..efeadedfeb 100644 --- a/src/platform/stm32wl/main-stm32wl.cpp +++ b/src/platform/stm32wl/main-stm32wl.cpp @@ -1,4 +1,5 @@ #include "FSCommon.h" +#include "UptimeClock.h" #include "configuration.h" #include "error.h" #include "gps/GPS.h" @@ -228,7 +229,7 @@ void preFSBegin() if (g_lfsCorruptMagic != LFS_CORRUPT_MAGIC) return; g_lfsCorruptMagic = 0; - lastLfsFormatMs = millis(); + lastLfsFormatMs = Time::skipZero(Time::getMillis()); RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_FLASH_CORRUPTION_UNRECOVERABLE); fsFormat(); LOG_INFO("LittleFS format complete; restoring default settings"); diff --git a/test/test_uptime_clock/test_main.cpp b/test/test_uptime_clock/test_main.cpp index daf4cbed31..db0adacdb0 100644 --- a/test/test_uptime_clock/test_main.cpp +++ b/test/test_uptime_clock/test_main.cpp @@ -104,6 +104,61 @@ void test_timerEndsAtMillis_dodges_the_wrap_tick_itself() TEST_ASSERT_EQUAL_UINT32(1u, Time::timerEndsAtMillis(0)); // getMillis()==0, delayMs==0: sum is 0 } +// --- stampMillis(): one value for storing a stamp AND measuring against it --- + +void test_stampMillis_matches_getMillis_away_from_the_wrap() +{ + Time::setTestMillis(123456u); + TEST_ASSERT_EQUAL_UINT32(123456u, Time::stampMillis()); +} + +void test_stampMillis_calls_the_zero_tick_one() +{ + Time::setTestMillis(0); + TEST_ASSERT_EQUAL_UINT32(1u, Time::stampMillis()); +} + +// The regression this helper exists for. Applying skipZero() at the STORE while a reader measures +// elapsed time against a raw clock splits the two sides apart on the wrap tick: the stamp is 1 while +// now is still 0, so `now - stamp` is UINT32_MAX and a brand new stamp reads as ~49.7 days old. Any +// elapsed-since guard - a cooldown, a debounce, a long-press threshold - then fires when it must not. +void test_store_side_dodge_alone_makes_a_fresh_stamp_read_as_ancient() +{ + Time::setTestMillis(0); + const uint32_t rawNow = Time::getMillis(); // what an un-normalised reader holds: 0 + const uint32_t stampedAtStore = Time::skipZero(rawNow); // the old shape: dodge only at the store + TEST_ASSERT_EQUAL_UINT32(0u, rawNow); + TEST_ASSERT_EQUAL_UINT32(1u, stampedAtStore); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, (uint32_t)(rawNow - stampedAtStore)); // the whole problem +} + +void test_reading_through_stampMillis_keeps_elapsed_at_zero_on_the_wrap_tick() +{ + Time::setTestMillis(0); + const uint32_t now = Time::stampMillis(); // read once, used for the store AND the comparison + const uint32_t stamp = now; // store it as-is; no second dodge needed + TEST_ASSERT_EQUAL_UINT32(1u, now); + TEST_ASSERT_EQUAL_UINT32(0u, (uint32_t)(now - stamp)); // fresh reads as fresh + + // Once the clock moves on, the measured age is short by exactly the 1 ms the dodge introduced: + // the stamp was taken at tick 0 and recorded as 1, so 400 ticks later it reads as 399 old. That + // is the whole cost of the scheme, and it is the same 1 ms skew skipZero() already documents - + // pinned here so nobody "corrects" it to 400 and reintroduces a raw read on one side. + Time::advanceTestMillis(400u); + TEST_ASSERT_EQUAL_UINT32(399u, (uint32_t)(Time::stampMillis() - stamp)); +} + +// A stamp stored one tick BEFORE the wrap, read one tick after, must still measure 1 ms - the dodge +// must not disturb ordinary wrap-crossing arithmetic. +void test_stampMillis_measures_across_the_wrap_boundary() +{ + Time::setTestMillis(0xFFFFFFFFu); + const uint32_t stamp = Time::stampMillis(); + TEST_ASSERT_EQUAL_UINT32(0xFFFFFFFFu, stamp); + Time::advanceTestMillis(1u); // wraps to 0, which stampMillis reports as 1 + TEST_ASSERT_EQUAL_UINT32(2u, (uint32_t)(Time::stampMillis() - stamp)); +} + // --- getMillisMonotonic(): the published wrap carry --- void test_monotonic_matches_millis_before_any_wrap() @@ -373,6 +428,11 @@ void setup() RUN_TEST(test_advanceTestMillis_wraps_like_millis); RUN_TEST(test_skipZero_maps_zero_to_one); RUN_TEST(test_skipZero_leaves_nonzero_values_alone); + RUN_TEST(test_stampMillis_matches_getMillis_away_from_the_wrap); + RUN_TEST(test_stampMillis_calls_the_zero_tick_one); + RUN_TEST(test_store_side_dodge_alone_makes_a_fresh_stamp_read_as_ancient); + RUN_TEST(test_reading_through_stampMillis_keeps_elapsed_at_zero_on_the_wrap_tick); + RUN_TEST(test_stampMillis_measures_across_the_wrap_boundary); RUN_TEST(test_timerEndsAtMillis_is_an_ordinary_sum_away_from_the_wrap); RUN_TEST(test_timerEndsAtMillis_dodges_a_sum_that_wraps_to_zero); RUN_TEST(test_timerEndsAtMillis_dodges_the_wrap_tick_itself);