Commit Graph
68 Commits
Author SHA1 Message Date
Tom de6b23190a Test suite rebuild (#11322)
* docs(nodedb): make the native node cap unambiguous

The native node cap was stated in four places that disagreed, and the disagreement
already caused a wrong diagnosis: a saturated 200-node database looked arithmetically
impossible because the cap had been read as 248, computed from a header that does not
apply on this platform. The real value is 198.

On portduino MAX_NUM_NODES is not a compile-time constant at all - the variant defines
it as `portduino_config.MaxNodes`, resolved at runtime, default 200 and settable per
host with `General: MaxNodes`. variant.h is reached before mesh-pb-constants.h, so that
header's ARCH_PORTDUINO branch never fires and its plausible-looking 250 is dead code.

- #error-guard the dead branch rather than leave a wrong number where people grep. The
  guard found a real defect: seven translation units reach mesh-pb-constants.h without
  configuration.h (SerialConsole.cpp, StreamAPI.cpp, PacketAPI.cpp, ServerAPI.cpp,
  PiWebServer.cpp, ServiceEnvelope.cpp, MeshtasticOTA.cpp, and test/TestUtil.cpp), so
  each was compiling with a different MAX_NUM_NODES - and therefore a different
  PACKETHISTORY_MAX - than the rest of the build. Each now includes configuration.h
  first. It cannot be included from mesh-pb-constants.h itself: that reaches
  SerialConsole.h through DebugConfiguration.h and closes a cycle.
- Name the bare 250 in getMaxNodesAllocatedSize() NODEDB_MIGRATION_LOAD_CEILING. It is a
  decode allowance for files written by larger-cap firmware, not a cap, and it read like
  one.
- Fix docs/node_info_stores.md, which named the wrong source and a "10-250" range that
  is wrong for native, and the copilot-instructions tunables line that said "portduino
  250".

* test(harness): give each suite its own scratch HOME and report leftovers

Native suites shared one directory. Every suite that constructs a NodeDB loads and
saves ~/.portduino/default/prefs/ - nodes.proto, config.proto, channels.proto,
module.proto, device.proto, warm.dat, transmit_history.dat - and nothing cleared it,
so state leaked suite -> suite within a run and run -> every run after it. A test run
could also rewrite a real meshtasticd node database on the same machine.

Per-run isolation does not fix this: the leak is generated inside a single run, so the
boundary has to be per suite.

bin/pio-test-isolate.sh runs each suite in its own scratch $HOME, registered as
test_testing_command for env:native and env:coverage so a bare `pio test` and CI get the
same boundary, not just bin/run-tests.sh. It runs the binary unchanged and exits with its
exit code, so PlatformIO's pass/fail is untouched. Overriding HOME here rather than
around `pio` also sidesteps the blocker that a bare HOME= breaks pio's own
~/.platformio/penv/bin/pio lookup.

Leftovers are reported as a second axis, PASS/FAIL x CLEAN/DIRTY, because an unintended
write has no matching assertion by definition - nobody writes TEST_ASSERT for a save they
do not know is happening. The harness asserts it from outside, so it applies to every
suite without the author opting in.

- Only the *set of changed paths* is asserted, never contents. Hashes answer the boolean
  "did this change?" and nothing more; content baselines over protobuf bytes would churn
  on every NodeInfoLite field added, which is how snapshot suites become noise.
- Deliberate writes are declared in test/state-manifest.tsv - one central file, suite /
  flags / mandatory reason. run-tests.sh prints the opt-out count on every run.
- Granularity follows the state flag, so the two ship together: per-test by default
  (TestUtil redefines RUN_TEST to checkpoint after each test, naming the exact test that
  dirtied things), suite boundary for state=per-suite, where carrying state across test
  cases is the declared behaviour.
- A declared write that does NOT happen is reported as MISSING, not folded into DIRTY. It
  catches silently broken persistence; a warning for now, since some are conditional.
- Graded AMBER, not RED. With isolation in place DIRTY means "undeclared", not
  "dangerous", and a check that lands red on day one gets switched off.

Guard the guard, both halves: state_assert_empty() refuses to run a suite against a
sandbox that is not empty (otherwise the after-diff measures against the wrong baseline
and reports CLEAN while meaning nothing), and bin/test-state-check.sh drives the real
wrapper with fixtures asserting CLEAN / CLEAN / DIRTY / MISSING plus both directions of
the empty assertion. A checker that silently matches everything would otherwise pass
forever.

--write-manifest proposes entries for a human to paste and justify; it never applies
them, and neither does CI.

* test(harness): stop reporting Unity's exit code as a signal

A native suite ends in exit(UNITY_END()), and UNITY_END() returns the failure count.
PlatformIO's native runner reads that non-zero exit code as a POSIX signal number, so
four failures print "Program received signal SIGILL", five print "SIGTRAP", and the suite
is classified [ERRORED] rather than [FAILED].

There is no crash. The signal name tracks the failure count and nothing else - it moved
SIGILL -> SIGTRAP when a diagnostic probe added a fifth failure - and it cost hours of
hunting a memory bug that did not exist, on an env (native) that carries no sanitizer at
all. It also explains the phantom extra test case in the totals: the runner adds a
synthetic entry for the signal it thinks it saw.

run-tests.sh now says so inline whenever a signal line appears, and the three
agent-facing docs say it too.

* test(admin): isolate NodeDB and globals per test

setUp() did `if (!nodeDB) nodeDB = new NodeDB();` and never deleted it, so 83 of the 85
tests shared one never-reset database and never restored config, owner, devicestate or
channelFile. The fixture that does restore them was opt-in and armed by exactly two
tests. The setUp comment claiming the rest "set their own config/region state and are
unaffected" was not true - the admin handlers under test write all four globals.

Route every test through the fixture instead: setUp saves the globals and installs a
fresh NodeDB, tearDown restores and deletes it. The two tests that armed it themselves no
longer need to.

All 85 pass, so nothing was silently relying on the shared state. It costs about 7% of
the suite's runtime (a NodeDB construction is a loadFromDisk plus, with a region set, key
generation) - worth paying to write the phase 3 tests against a clean fixture rather than
83 tests' residue.

Also cap the per-test attribution in the run summary at five entries; the full list stays
in the suite's sandbox.

* test(fs): cover the bounded file-manifest walk

getFiles() runs on every phone sync via STATE_SEND_FILEMANIFEST, and nothing asserted any
of its bounding behaviour. It does execute unasserted from test_stream_api's handshakes,
but the cap, the depth limit, the wasLimited paths, overlong-path rejection and capacity
release were all unguarded.

Eight tests, all describing what the code does today: today's code is already correct
here, since #10778 landed the by-reference collectFiles(), the 64-entry cap, the strlcpy
bounds and the swap-idiom release. They pass on arrival, which is the point - this is the
baseline a later change has to leave alone.

Two things they do not cover, and cannot:

- Moving reserve() outside the __cpp_exceptions guard. Exceptions are on natively, so the
  #else branch is not compiled. The suite's job there is to prove that change alters
  nothing observable.
- The file.name() null guard. No in-tree backend returns null; the guard is defensive.

The manifest-release test pins the swap idiom rather than calling
PhoneAPI's releaseFilesManifest(), which is file-local. It asserts capacity() == 0, not
just size() == 0 - a size-only check passes on clear(), which is the bug #7924 shipped.

Suite count 43 -> 44, recounted against the directories rather than copied.

* test(admin): assert node-DB metadata saves skip the radio reload

set_favorite_node, set_ignored_node and toggle_muted_node each persist a NodeInfoLite bit
and nothing else. MeshService::reloadConfig() gates its region re-derivation and
configChanged notification on saveWhat & (SEGMENT_CONFIG | SEGMENT_CHANNELS), so a
SEGMENT_NODEDATABASE-only save already skips the live radio reconfigure.

Pure characterization - all three pass on develop. Worth pinning because that reconfigure
is the path implicated in the WisMesh Tag favourite-node crash, and develop asserts
nothing about it: widening the saveWhat mask or reordering the check would currently go
unnoticed.

Ported from the config-save series along with ConfigChangedCounter (an Observer<void *>
counting configChanged notifications, the only externally visible signal that the reload
branch was taken) and TEST_NODE_NUM. They join the existing suite, so no suite-count
change.

* refactor(menu): extract the mute toggle into a named function

The node menu's mute action was inline in a banner-callback lambda, and that lambda only
ever runs via screen->showOverlayBanner() - which is why nothing in MenuHandler.cpp was
reachable from a test. Lift the `selected == Mute` branch into
menuHandler::toggleNodeMuted(uint32_t) and call it from the lambda.

Behaviour-neutral by construction: same statements, same order, same bare saveToDisk().
The null check moves into the function, so the call site no longer needs its own lookup.
Verified by the native build and suite; the byte-identical-image check on a
headroom-constrained nRF52 board was not run locally - CI's firmware-size comment covers
it.

Three tests come with it, all describing today's behaviour:

- the bit flips both ways and no configChanged fires (develop never calls reloadConfig on
  this path);
- an unknown node is a no-op rather than a write;
- and the segment mask. Flipping one NodeInfoLite bit currently rewrites all five
  segments via bare saveToDisk(). That is asserted deliberately, with the comment naming
  it as characterization of a known defect: a pending fix narrows it to
  SEGMENT_NODEDATABASE, and when it lands this assertion is expected to change, which
  makes the improvement visible in the diff instead of silent.

saveToDisk() is not virtual, so the mask is observed through its effect - remove the five
prefs files, toggle, and see which reappear.

* docs(test): make every suite count a pointer to the canonical one

test/native-suite-count is the registered total and is machine-checked against test/test_*
on every full run and by the suite-count-check CI job. Every other statement of the count
is a copy that drifts: copilot-instructions said 12, AGENTS.md said 19, and the real
number is 44.

Replace both literals with a pointer to the file, say explicitly that no document should
state the count as a literal, and reframe the two suite listings as descriptions rather
than inventories - they carry per-suite information the count does not, so they stay, but
nothing should infer completeness from their length. Register the new FS suite in both.

* test(harness): randomise suite order, reproducibly

Landed last, deliberately. Randomising an order-dependent suite set does not find bugs so
much as convert a silent pass into intermittent red, and the first instinct is to revert
the randomisation rather than fix the coupling. Phases 1-2 removed the coupling; this
keeps it removed.

Both runners previously hid order dependence behind a fixed order that happened to differ
between them, and neither order was chosen: CI's area rules put admin first, PlatformIO's
local discovery is reverse alphabetical and put it last. CI was green by accident.

- bin/run-tests.sh --shuffle / --seed <n>. The seed defaults to HEAD's short SHA: one
  order per commit, so a red is replayable and attributable to the diff instead of flaky,
  while the project keeps exploring orders. Printed at the start and carried into the
  RESULT line, so a verdict is replayable from that line alone; the full order is printed
  on failure, because for an order-dependent failure the order is the diagnostic.
- The shuffle is a Fisher-Yates over a MINSTD generator rather than awk's rand(), whose
  sequence differs between gawk and mawk. A seed that does not reproduce the same order on
  another machine is not a seed.
- Shuffling needs one `pio test -f <suite>` invocation per suite - PlatformIO orders by
  its own os.walk() over test/ and filters only select - which measures at about 4.7s per
  suite of extra startup.
- CI shuffles its area order, seeded from GITHUB_SHA and printed with the command to
  replay it locally. Intra-area order stays PlatformIO's; controlling it there would mean
  per-suite invocations, which is a cost worth deciding separately.

Also records the 16 measured entries in test/state-manifest.tsv, each with its reason,
taken from a full run's --write-manifest output rather than guessed.

* test(default): cover the region-throttle interval overload

getConfiguredOrDefaultMsScaled(configured, default, nodes, TrafficType) is the overload
every telemetry and position module actually calls, and nothing referenced TrafficType
anywhere under test/. All four of its behaviours were unguarded: the no-region guard, the
throttle <= 1 short-circuit, the multiply, and the 64-bit overflow clamp.

The throttles are real, not hypothetical - EU_866 carries PROFILE_LITE, which sets both
positionThrottle and telemetryThrottle to 10, so a change here moves broadcast spacing in
that region by an order of magnitude.

Each test pins numOnlineNodes at the congestion threshold and uses ROUTER, which never
congestion-scales, so the coefficient is 1 and the throttle is the only variable. The
overflow case needs a base above INT32_MAX/10, hence three days rather than one.

* ci(test): keep pull-request suite order fixed, seed the rest

Shuffling the area order on every run - including pull_request - would turn a
contributor's PR red for an ordering they did not choose, which is how a randomisation
gets reverted instead of the coupling being fixed. That is the exact dynamic the ordering
work was sequenced last to avoid, and the previous commit walked straight into it.

- pull_request keeps the fixed declared area order.
- push and schedule shuffle, seeded from the commit SHA: deterministic per commit,
  printed, attributable, and never blocking someone else's PR.
- A suite_order_seed input on workflow_call and workflow_dispatch overrides both, so a
  specific failing order can be replayed anywhere, including on a PR.

The run log prints which mode it took, the resulting order, and the local command to
replay it.

* ci(test): satisfy CKV_GHA_7 and yamllint on the seed input

The seed is reachable through workflow_call, which callers can pass programmatically. The
workflow_dispatch copy tripped checkov's "workflow_dispatch inputs MUST be empty" rule,
and suppressing it was not worth it: replaying a specific order is a local operation, and
the run log already prints the exact bin/run-tests.sh command to do it.

* style(menu): apply the node-ID format convention

RadioInterface.cpp documents the rule: 0x%08x in logs, !%08x in user-facing
display. MenuHandler held every remaining exception - seven logs printing bare
%08X, and two display labels doing the same.

Repo-wide there are now no bare %08X node IDs left in log calls.

* ci(test): pass workflow inputs through env, not shell interpolation

suite_order_seed and github.event_name were spliced into the run: script as
${{ }} text, so a value carrying shell metacharacters would execute as code on
the runner rather than being read as data. semgrep (run-shell-injection) and
zizmor (template-injection) both flag it.

Both now arrive as environment variables and are read as "$VAR".

* refactor(test): share the seeded shuffle between the harness and CI

bin/run-tests.sh and test_native.yml each carried a byte-identical copy of the
MINSTD Fisher-Yates awk. The workflow prints "replay locally: ./bin/run-tests.sh
--shuffle --seed $seed" after a shuffled CI run, and that instruction is only
true while the two agree - drift would be announced by a replay quietly
reproducing a different order than the one that failed.

Extract shuffle_suites() to bin/lib/shuffle.sh and source it from both.
Permutations verified identical across seeds before and after the move.

* fix(test): correct the shared-state MISSING check and summary join

Three defects in the new harness:

state_classify() matched declarations two different ways - state_path_declared()
for "undeclared", a hand-rolled regex for "missing". Interpolating an entry into
an ERE also let a metacharacter in a manifest name match a file that is not the
declared one. Both directions now go through the one helper.

`paste -sd'; '` does not join with "; ": with -s, paste cycles through a
multi-character delimiter one character per join, so paths rendered as
"a;b c;d e". Replaced with an awk join.

test-state-check.sh ran on after a failed cd instead of stopping (SC2164).

./bin/test-state-check.sh: 6/6 fixtures pass, MISSING included.

* fix(portduino): bound General.MaxNodes

MaxNodes was validated only for <= 0. Any positive value, including a typo'd or
pasted-in one, propagates to MAX_NUM_NODES and scales both the node DB and the
nodes.proto decode ceiling - failing at boot with no obvious cause.

The ceiling is a sanity bound, not a capability limit; raise it if a host
genuinely needs more.

* docs(nodedb): reconcile the capacity tables

The property matrix omitted the ESP32-S3 100-node flash tier that the platform
table above it lists, and neither mentioned that the WASM build overrides
MaxNodes to 80 in wasm_config_apply().

* fix(nodedb): make mesh-pb-constants.h self-sufficient on portduino

The ARCH_PORTDUINO #error assumed it was unreachable in a normal build. It is
not: the vendored device-ui sources include this header without configuration.h,
which broke both native-tft docker builds.

Include configuration.h here instead, ahead of every compile-time default -
variant.h overrides MAX_RX_TOPHONE as well as MAX_NUM_NODES, so placing it lower
in the file just moves the divergence to a redefinition. The #error stays as a
backstop for the case where that include genuinely stops providing the cap.

Verified with the native env's own flags: a TU including only this header now
compiles, normal-order use of both macros compiles, and NodeDB.cpp compiles.

* fix(portduino): raise the MaxNodes ceiling to 16000

Marked artificial: nothing in the node DB fails at 16001. 16000 sits just under
the 16384 (128 x 128) population where HopScalingModule saturates its sampling
denominator and starts dropping nodes, so a host inside the bound still gets
meaningful hop recommendations.

* lint(trunk): advise on node IDs logged as bare %08x

RadioInterface.cpp documents the convention - 0x%08x in logs, !%08x in display -
but nothing enforced it, which is how the MenuHandler cluster drifted. 22 call
sites in PacketHistory, NodeInfoModule and PositionModule are still off it.

A trunk linter rather than a CI grep job, because trunk checks changed files:
new violations get flagged without a 22-site cleanup landing in an unrelated PR.
Modelled on the existing too-many-defined definition.

Scoped to values it can tell are IDs - an ID-shaped argument (->num, .from,
getNodeNum) or message text naming one. A 32-bit hex that is not an ID is out of
scope, so the CRC32 logs in ethOTA.cpp are correctly ignored.

Emits "note", trunk's only non-blocking level: "warning" and "info" both exit
non-zero and would gate CI, which is not what a log-format nit deserves. The
pre-existing sites are line-scoped in the allowlist, so a new bad call in those
same files is still caught.

* lint(trunk): stop exempting the known node-id-format sites

The seeded allowlist made the rule green by declaring the backlog acceptable.
Empty it instead, so the 22 pre-existing sites are reported and get cleaned up
by whoever next edits those files.

Costs nothing to do: the rule emits "note", so these are non-blocking either
way. The allowlist stays for its real purpose - a value the linter misreads as
an ID.

* style: log node and packet IDs as 0x%08x

Clears the 22 sites the node-id-format linter reports, so the rule starts from
zero rather than from a backlog nobody can see - trunk suppresses pre-existing
findings by default, so left alone these would not have surfaced on edit the way
an empty allowlist implies.

Format strings only; no argument or control flow changes. The !%08x
user-facing display forms are deliberately untouched - that is the other half of
the same convention.

* test(harness): build once up front, so suite timings mean something

run-tests.sh fused build and run in a single pio invocation, so whichever suite
PlatformIO's directory walk reached first absorbed the entire src compile and
reported it as its own duration. On a real run that made a 0.03s suite report
13m21s, and hid the build cost from every other number in the summary.

Do what .github/workflows/test_native.yml already does: one --without-testing
build pass, then run with --without-building. Measured on a full 44-suite run -
the build is now a single reported figure and 968 test cases execute in 1.9s,
with no suite above 0.084s.

Build output goes to its own log rather than $LOG: the outcome regexes match
"error:" and "[ERRORED]", so a compiler diagnostic sharing that file would read
as a test failure.

Both red paths now keep the log they quote from. $LOG and the build log are
mktemps the EXIT trap removes, so the three grepped lines were previously all
anyone ever saw - and the cause is usually further up than the first [FAILED].

* test(harness): keep the run log on every red path

bin/pio-test-isolate.sh already keeps a failing or DIRTY suite's sandbox and log
under .pio/test-state/<suite>/. What was missing is the cross-suite view: $LOG is
a mktemp the EXIT trap deletes, so run-tests.sh quoted three grepped lines from a
file that no longer existed by the time anyone looked.

Preserve it as .pio/build/<env>/test-failure.log from both red paths - including
"no success summary found", which said "see log" while preserving nothing, and
which is exactly the case where the build died before any suite ran and so left
no per-suite sandbox either.

Cleared at the start of every run, so a green run cannot leave a red one's log
lying around looking current.

* fix(test): report the real failure count on a shuffled red

A shuffled run is one `pio test` invocation per suite, all appending to the
same log, so the log carries one PlatformIO "N test cases:" summary per suite.
verdict_red() took `tail -1`, which reports whatever the LAST suite did: a
failure in suite 3 printed a "0 failed" summary from suite 44 directly under
"RED - failures detected:".

Sum the summaries instead. A single summary line - every unshuffled run - is
passed through verbatim, so the familiar output is byte-identical.

The patterns are passed to the awk helper as strings rather than /regex/
literals: awk evaluates a regex literal in argument position as `$0 ~ /re/`,
so the callee would receive 0 or 1 and silently sum garbage.

* fix(test): do not emit an empty suite name for an empty shuffle

`printf '%s\n' "$@"` with no arguments still writes one empty line, and both
callers read shuffle_suites through mapfile, so an empty suite list arrived as
a single suite named "". Return before the printf when there is nothing to
shuffle.

* test(harness): state and enforce the Linux host requirement

The native harness is a Linux tool: bash 4+ (mapfile), GNU coreutils and GNU
find (-printf, md5sum, -executable). Most of that predates this branch -
mapfile and both find predicates are already on develop - but none of it was
written down, so the requirement was there to be discovered rather than read.

Refuse to start on a non-Linux uname instead of degrading. On a BSD userland
this would not fail cleanly: it would mis-hash the sandbox and mis-read the
suite list, and still print a verdict. A state check that silently measures
the wrong thing is worse than one that declines to run.

Carrying a per-host fallback was the alternative, and it buys a second code
path that nothing in CI exercises. bin/test-native-docker.sh already exists
for macOS and non-Linux hosts, and the native-macos PlatformIO env is a build
target for meshtasticd, not a test host - the isolation wrapper is registered
for env:native and env:coverage only.

Documented in the script header, test/README.md, and both agent docs.

* fix(test): terminate every suite with exit(UNITY_END())

Two sites across two suites ended on a bare UNITY_END(). That ends the
reporting, not the suite: setup() returns, the runtime goes on calling loop(),
and the process runs forever. PlatformIO does not notice - it reports a suite
from its Unity output, not from process exit - so the suite passes, the run
goes green, and the binary stays resident. Thirteen of them had accumulated on
one dev box, the oldest 19 hours old.

The costs are quiet by construction:

- the per-suite sandbox is deleted underneath a live process, so its
  CLEAN/DIRTY verdict describes what the suite had written when the harness
  stopped looking, not what it left behind;
- .gcda coverage and LeakSanitizer's report both flush from atexit handlers,
  so a suite that never exits contributes no coverage and gets no leak check;
- each survivor pins its own deleted 94 MB binary, which du cannot see.

One of the two is the #else of an architecture guard, which is the easiest one
to get wrong - it looks like there is nothing to clean up. test_mqtt has a
correct exit(UNITY_END()) in its live branch, so a "does this file call exit()
anywhere" check passes the file whole.

test_serial had two more. develop's serial-config validation rework
restructured that suite - the architecture guard is gone and both remaining
branches now exit correctly - so this commit no longer has anything to change
there; bin/lint-unity-exit.sh, added later on this branch, is what keeps it
that way.

test/README.md gets a section on it, since the skeleton showing the right
shape had not stopped this happening.

* test(harness): detect and reap suites that outlive their run

A suite that never exits was invisible: PlatformIO reports a suite from its
Unity output, so the run stayed green while the binary kept running. Two
checks, because they fail differently.

Runtime, in bin/pio-test-isolate.sh: the sandbox $HOME is mktemp-unique per
suite, so any process still holding it is a survivor of that suite. Matching
on the environment rather than a remembered PID identifies one whatever its
parentage - a fork, a grandchild, a process already reparented to init - none
of which a $! comparison catches. Reaped before the after-fingerprint is
taken, so that fingerprint measures a tree nobody is still writing to, and so
a run cannot leave processes accumulating on the host. Recorded as a sixth
summary column and graded AMBER: the tests did pass, but the CLEAN verdict and
the coverage were measured under a false assumption.

Author-time, as bin/lint-unity-exit.sh, wired into trunk at "note" like
node-id-format: every UNITY_END() must be wrapped in exit(). The rule is per
occurrence, and that is the point - a file-level "calls exit() somewhere"
check passes test_serial and test_mqtt, which have a correct one in their live
branch and a bare one in the #else. Running it over the tree turned up
test_mqtt, which the file-level pass had missed.

It allows `int rc = UNITY_END(); ...; exit(rc)`, used by test_packet_signing
to restore globals between the summary and the exit. That is where the rule
gives ground: capturing and never exiting would leak and is not flagged.
Flagging a correct idiom would push someone to "fix" working code.

bin/test-state-check.sh gains a survivor fixture, asserting the wrapper both
reports and reaps - a detector that only reports leaves the host accumulating
processes, which is half the harm. 8/8.

* fix(lint): make the unity-exit scanner statement-aware

The rule judged one physical line at a time, which reports two kinds of correct
code as bare:

    /* a comment that happens to
       mention UNITY_END() */          <- interior lines were never stripped

    exit(
        UNITY_END());                  <- exit( and the macro never met

On a probe of both, two of three findings were wrong. This is a note-level rule
whose whole job is advice, and bin/lint-node-id-format.sh already says why that
matters: a false positive costs more than a miss. One that cries wolf gets
ignored, and the real finding goes with it.

Carry /* ... */ state across lines and accumulate logical statements before
testing, with a 12-line cap so one unclosed call cannot swallow the rest of the
file - the same structure lint-node-id-format.sh uses, so the two custom linters
in bin/ work alike rather than each having its own idea.

Verified both directions: the develop-era sources still produce the same four
findings, the fixed tree produces none, and a probe covering block-comment
interiors, wrapped exit(), line comments, return UNITY_END() and capture-then-
exit reports only the genuinely bare calls - including a complete block comment
followed by real bare code on the same line, which the state machine has to
keep live.

Reported by CodeRabbit on #11322.

* fix(lint): tokenise instead of pattern-matching, and self-test it

Second round of review findings on the same scanner, all confirmed by direct
test before changing anything. Six defects, one root cause: layered regexes
cannot tokenise C++.

False positives (correct code reported):
  - UNITY_END() inside a string literal read as code

False negatives (real leaks missed):
  - a string containing "/*" opened comment state and swallowed later lines
  - greedy .* removed everything between two block comments on one line,
    taking a bare call with it
  - myexit(UNITY_END()) matched the exit() exemption as a substring
  - x == UNITY_END() and total += UNITY_END() matched the assignment exemption

Replaced with a character-level scan carrying comment state, and token-bounded
exemptions: exit must be a whole identifier, and the capture form must be a
plain `=`. Raw string literals are still not modelled - there are none under
test/, and delimiter tracking for a case that does not occur would be untested
code guarding untested code, so it is documented rather than guessed at.

Also drops the `return UNITY_END()` exemption. It only terminates from main(),
there is no main() under test/, and from a helper it just returns a count.

bin/test-lint-unity-exit.sh pins all fifteen cases, every false positive and
false negative found in review among them. The rule has been wrong twice in a
way that looked fine by inspection; it needed a self-test more than it needed
another careful reading.

Two further findings in the same review:

  - bin/run-tests.sh dropped PASSTHRU in shuffled mode, so `--shuffle -vvv`
    built verbosely and then ran quietly. The shuffled loop now forwards
    EXTRA_ARGS, which is PASSTHRU minus the -f pair it supplies per suite.
  - bin/run-tests.sh did not guard `cd "$ROOT_DIR"`.

And one that did not reproduce: the survivor fixture's glob does find the pid
file (verified with the lookup instrumented - the earlier failure was an
artifact of running the script from /tmp, where SCRIPT_DIR cannot resolve).
The assertion was still weak, because an empty pid took the "not running"
branch and passed vacuously. It now fails if the pid was never recorded, and
finds the file by search rather than assuming a directory depth.

Reported by CodeRabbit on #11322.

* fix(lint): report each UNITY_END occurrence at its own location

The self-test only asked "did the linter say anything", so it could not have
caught a wrong line, a wrong column, or a missing second finding. Fixtures now
assert the exact diagnostics as line:col, and the first run of that assertion
found two real problems.

The caret pointed at the wrong occurrence. For `exit(UNITY_END()); UNITY_END();`
the verdict was right but the column was 17 - the wrapped call - because the
scanner stripped terminating forms out of the whole statement and then reported
the first occurrence it had seen. Two bare calls on one line reported once.

Judged per occurrence now, by looking back through whitespace at what wraps it,
so both the count and the caret are right. That also needed a position map from
strip_noncode(): removing a comment or collapsing a literal shifts every later
column, and counting occurrences in the raw line does not recover it either -
TEST_MESSAGE("... UNITY_END() ..."); UNITY_END(); has two occurrences in the raw
text and one in the code.

Four of the expected columns I wrote by hand were also wrong, off by one. The
linter was right in every case; the assertions were not. They are computed from
the fixture text now rather than pasted from output, because a baseline accepted
from the tool it is testing asserts nothing.

17 fixtures, including the two-on-one-line case from review and its mirror.

Reported by CodeRabbit on #11322.
2026-08-06 14:05:07 +00:00
Thomas Göttgens df6e67f70b Signing and ingress hardening (#11282)
* Include warm-tier signers in the identity update gate

* Fail the send when PKI encryption fails

* Require signatures on licensed unicasts

* Include warm-tier signers in the NodeInfo downgrade drop

* Clamp hop fields on UDP multicast ingress

* Address review comments on signing hardening

Condense the updateUser rationale to two lines and stop calling the
Balanced-mode drop a broadcast now that licensed unicasts reach it.
2026-07-29 22:28:25 +00:00
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
2026-07-29 14:03:25 +00:00
1e982fa78c Sign plaintext packets in licensed mode (#10969)
* feat: sign licensed plaintext packets

Preserve and publish identity keys in licensed mode so existing XEdDSA signatures can authenticate plaintext traffic. Sign licensed broadcasts and direct messages when they fit, while keeping PKI encryption disabled and normal routing behavior unchanged.

* fix(security): persist licensed channel sanitation

* fix(security): close licensed migration lifecycle gaps

* fix(security): address licensed signing review feedback

* test: restore signing globals from Unity teardown

* fix(baseui): allow confirmed ham region selection

* fix(baseui): guard region picker validation

* style: trunk fmt

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Austin <vidplace7@gmail.com>
2026-07-27 02:57:15 +00:00
Benjamin Faershtein 08cfb3d683 Merge upstream develop into packet authentication policy 2026-07-20 11:48:59 -07:00
Thomas Göttgens 5ded0ec8c5 Gate identity learning on signature in NodeDB::updateUser (#11084) 2026-07-20 11:56:42 +02:00
Benjamin Faershtein 59ab12675a Merge develop into packet authentication policy 2026-07-19 15:33:16 -07:00
Andrew YongandBen Meadors b71c1adb26 stm32wl: add hardware RTC support (rak3172) (#10961)
* stm32wl: add hardware RTC support infrastructure

Wires the STM32WL chip's internal RTC (running off the LSE 32.768kHz
crystal) into meshtastic's existing time-of-day framework
(perhapsSetRTC()/readFromRTC()), following the same pattern already
used for I2C RTC chips (RV3028, PCF8563/85063, RX8130CE).

LSE is started and polled manually before ever calling into the
STM32RTC library, with our own bounded timeout - the library's own
internal LSE startup path has no bounded fallback and hangs forever
via Error_Handler() if the crystal never locks, so this is required
for a board with a missing/faulty crystal to boot normally rather
than hang.

Gated behind a new HAS_LSE variant flag (currently unset everywhere,
so this is inert until a variant opts in - see follow-up commit).

Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>

* gps: qualify RTC.h includes to avoid case-insensitive filesystem collision with STM32RTC

The stm32duino STM32RTC library (added to lib_deps in a follow-up
commit) ships its own src/rtc.h. On case-insensitive filesystems
(the macOS default), an unqualified #include "RTC.h"/<RTC.h> from
any file outside src/gps/ resolves to the library's rtc.h instead of
src/gps/RTC.h, since PlatformIO's LDF puts lib_deps include paths
ahead of the project's own -Isrc/gps.

Qualify every include as gps/RTC.h so it can't collide with any
same-named header a future dependency might ship, regardless of
filesystem case sensitivity. Purely mechanical, no behavior change.

Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>

* stm32wl(rak3172): enable hardware RTC support

Opts rak3172 into the HAS_LSE infrastructure added previously: sets
STM32WL_LSE_DRIVE to a conservative default and pulls in the
STM32RTC library. rak3172 has ~63KB flash headroom going in;
build-verified at 76.7% flash usage after this change (up from a
73.8% baseline), well within budget.

wio-e5 is not opted in here despite sharing the same STM32WLE5 chip
- it's already at 96.8% flash usage today (GPS + I2C sensor support
compiled in, unlike rak3172), leaving too little headroom to safely
add STM32RTC without first trimming something else.

Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>

* stm32wl: add docstrings for LSE/RTC setup functions

Addresses CodeRabbit's docstring coverage check on PR #10961.

Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>

* stm32wl: address CodeRabbit nitpicks on PR #10961

- Brace the single-statement HAS_LSE branch in perhapsSetRTC() to
  match the sibling readFromRTC() branch's style.
- Quote the RTC.h include in PhoneAPI.cpp for consistency with every
  other qualified include site.

Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Signed-off-by: Andrew Yong <me@ndoo.sg>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2026-07-16 19:00:45 -05:00
Benjamin Faershtein d6b12ea3f1 feat(security): enforce packet authenticity policies 2026-07-09 17:48:55 -07:00
Andrew YongandBen Meadors 6768bd08df fix(modules): null-check packetPool/clientNotificationPool/mqttClientProxyMessagePool allocations (#10951)
* fix(modules): null-check packetPool/clientNotificationPool/mqttClientProxyMessagePool allocations

Follow-up to fix/meshpacket-alloc-null-checks (PR meshtastic/firmware#10948),
which fixed the core routing path (Router/NextHopRouter/ReliableRouter/
MeshService/RadioLibInterface) after reproducing a HardFault on STM32WL
hardware under real mesh traffic. This covers the same allocCopy()/
allocZeroed() unchecked-return pattern in lower-frequency paths that were
out of scope for that PR: SimRadio.cpp (portduino sim RX/TX), NodeInfoModule,
the Telemetry modules' power-saving-sleep notifications, MQTT (map report,
client-proxy messages, config-validation notifications), PositionModule,
SerialModule, and KeyVerificationModule.

Sites where the allocation result was already read back through an
existing null check downstream (e.g. Telemetry's lastMeasurementPacket,
SimRadio's receivingPacket, AdminModule::sendWarning) were left as-is -
verified safe, not touched.

Router::allocForSending() (and everything that funnels through it, e.g.
allocDataProtobuf()) remains unguarded and out of scope here too - it's
called from 30+ sites across the codebase and needs its own audit.

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>

* fix(KeyVerificationModule): bound cn->message writes with snprintf

CodeRabbit flagged unbounded sprintf() writes into the fixed-size
ClientNotification.message buffer as a static-analysis nitpick while
reviewing this PR's allocation null-checks. Align these four sites with
the snprintf(dest, sizeof(dest), ...) pattern already used for the same
field elsewhere in this PR (SerialModule.cpp, MQTT.cpp).

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>

---------

Signed-off-by: Andrew Yong <me@ndoo.sg>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2026-07-09 10:34:58 -05:00
Ben Meadors 0e84c1a827 Harden XEdDSA unsigned-packet policy and add coverage (#10858)
Audit of the XEdDSA packet-signing implementation (#10478) surfaced several
issues in when unsigned packets are accepted on receive or emitted on send.
This fixes them and adds regression coverage.

- Unicast NodeInfo exchange no longer breaks against signer nodes: the
  NodeInfoModule downgrade drop is gated to broadcasts, since senders never
  sign unicast (want_response replies, directed exchanges).
- Replace the payload-size sign heuristic with an exact encoded-size gate
  (signedDataFits) and mirror it on the receive side, removing a dead band
  where 167-168 B broadcasts were signed then failed TOO_LARGE.
- Extract the receive policy into checkXeddsaReceivePolicy() and apply it to
  plaintext-MQTT decoded downlink, which previously skipped signature
  verification and downgrade protection entirely.
- Reject signatures whose length is neither 0 nor 64 as malformed, so a
  crafted partial signature can't inflate the size estimate and dodge the
  unsigned-downgrade drop.
- Hold cryptLock on the MQTT verify path (shared Ed25519 key cache).
- Clear any client-preset signature on packets we originate, on all builds.
- Randomized (hedged) signing per the Signal XEdDSA spec: bump the
  meshtastic/Crypto pin to the build where XEdDSA::sign mixes 32 bytes of
  caller randomness into the nonce as Z (meshtastic/Crypto#3), and seed those
  bytes in xeddsa_sign from HardwareRNG (checked, with a seeded-CSPRNG
  fallback). test_crypto pins that repeated signs differ and both verify.

Adds test coverage: test_packet_signing groups A-E (receive matrix, send
policy, NodeInfo backstop, encoding invariants, decoded-ingress policy),
test_mqtt end-to-end downlink cases, and a test_crypto randomization check.
2026-07-02 11:48:58 -05:00
8267bb22bd Packet Signing via XEdDSA (#10478)
* Test commit for XEdDSA support

* Update to Crypto lib in Meshtatic org

* Generate a new node identity on key generation (#7628)

* Generate a new node identity on key generation

* Fixes

* Fixes

* Fixes

* Messed up

* Fixes

* Update src/modules/AdminModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mesh/NodeDB.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Figured it out!

* Cleanup

* Update src/mesh/NodeDB.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mesh/NodeDB.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/modules/AdminModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update crypto commit hash

* Some fixes for xeddsa pr (#9610)

* fix: add null check for getMeshNode() in NodeInfoModule

getMeshNode() can return nullptr for unknown nodes. Dereferencing
without a check crashes the firmware when receiving NodeInfo from
a node not yet in the database.

* fix: enforce XEdDSA signature verification and prevent stripping

Previously, failed signature verification still allowed the packet
through, making signatures purely cosmetic. Now:

- Failed verification drops the packet (DECODE_FAILURE)
- Successfully verified nodes get HAS_XEDDSA_SIGNED bitfield set
- Unsigned packets from previously-signing nodes are rejected
- Log levels reduced from WARN/ERROR to DEBUG/WARN as appropriate

* fix: include packet metadata in XEdDSA signature

The signature now covers [fromNode | packetId | portnum | payload]
instead of just the payload bytes. This prevents:
- Replay attacks (different packetId fails verification)
- Reattribution (different fromNode fails verification)
- Portnum redirection (different portnum fails verification)

Also adds a key initialization check to xeddsa_sign (returns false
if XEdDSA keys are all zeros) and checks the return value in the
encode path.

* fix: handle existing key pair in AdminModule security config

When a user provides both a valid private key and public key via
admin config, the crypto engine's DH private key and owner public
key were never loaded. DMs and XEdDSA signing would silently break.

Add an else branch to load both keys into the crypto engine.

* perf: cache Ed25519 public key conversion in xeddsa_verify

curve_to_ed_pub() performs field element parsing, inversion, and
multiplication on every call. Since packets from the same node
tend to arrive in bursts, a single-entry cache avoids repeating
this expensive conversion for consecutive packets from one sender.

* fix: skip identity cleanup when node number is unchanged

createNewIdentity() was called on every generateCryptoKeyPair(),
including normal boots where the same key is regenerated. This
caused unnecessary NodeDB writes and old-node cleanup logic to
run when the node number hadn't actually changed.

Also fixes only zeroing byte[0] of the old node's public key
instead of clearing the entire array.

* fix: replace hardcoded 120 with derived XEDDSA_SIGNATURE_SIZE constant

The payload size check for XEdDSA signing used a magic number (120).
Replace with a derivation from DATA_PAYLOAD_LEN and XEDDSA_SIGNATURE_SIZE
so the limit adjusts automatically if constants change. This also
increases the max signable payload from 120 to 169 bytes, which is
still safe since the actual encoded size is checked after pb_encode.

* fix: add const qualifiers to XEdDSA verify and curve_to_ed_pub inputs

pubKey, payload, and signature parameters in xeddsa_verify are
input-only and should not be modified. Same for curve_pubkey in
curve_to_ed_pub.

* chore: remove commented-out old Crypto dependency in portduino.ini

* Leave out the admin module change for now

---------

Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>

* trunk

* protobuf re-update

* Protobufs

* Merge resolution fix

* Put XEDDSA on the right bit

* NodeDB update to new nodeInfoLite accessors, etc

* 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>

* 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>

* 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>

* 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>

* 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>

* Refine unsigned packet rejection logic in Router (#10534)

* use hardware random to fill the first 32 signature bytes with entropy prior to signing.

* Add XEdDSA packet-signing policy tests and update dependencies for macos

* Minor fixes

* integrate XEdDSA support and update dependencies across multiple modules

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Wessel <github@weebl.me>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com>
2026-06-13 06:45:56 -05:00
Ben Meadors 309d51a3e8 fix(NodeInfoModule): update user handling in allocReply to prevent global state clobbering 2026-06-09 21:00:40 -05:00
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 eb7e5c7acf.

* Add NodeInfoModule integration for RTC quality changes and trigger immediate checks

* Update test conditions for RTC quality checks
2026-03-27 15:38:41 -05:00
Ben Meadors e4c5bfd161 Fix NodeInfo suppression logic to ensure suppression only applies to external requests (#9947)
* Fix NodeInfo suppression logic to ensure suppression only applies to external requests

* Ensure NodeInfo reply suppression logic to only apply for external requests which are actually nodeinfo packets
2026-03-19 08:15:38 -05:00
Ben MeadorsandCopilot 3a74e049ab Add Transmit history persistence for respecting traffic intervals between reboots (#9748)
* Add transmit history for throttling that persists between reboots

* Fix RAK long press detection to prevent phantom shutdowns from floating pins

* Update test/test_transmit_history/test_main.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Test fixes and placeholder for content handler tests

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-02-25 20:41:07 -06:00
Wessel 1a7f560372 fix: zero entire public key array instead of only first byte (#9619) 2026-02-12 15:33:57 +01:00
Ben MeadorsandCopilot fb3bf783dd Implement graduated scaling for NodeInfo send timeout based on active mesh size (#9364)
* Implement graduated scaling for NodeInfo send timeout based on active mesh size

* Shorter timeout still needed for pubkey unkown and ad-hoc send

* Update src/modules/NodeInfoModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-01-19 20:00:41 -06:00
Ben Meadors 31e55d0b66 Be more judicious about responding to want_response in existing meshes (#9014)
* Be more judicious about sending want_response in existing meshes and responding to nodes we already heard from

* Turns out we don't actually use this
2025-12-19 13:56:10 -06:00
Ben Meadors bd9076b740 Remove risky change 2025-10-07 06:14:35 -05:00
Ben MeadorsandCopilot 7c5e2bc95a Clear out user.id except for sending to phone (#8202)
* Null out user.id except for sending to phone

* Fix

* Update src/modules/NodeInfoModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Copilot garbage

* This is unnecessary, because we don't stored user.id on userlite

* Don't need this

* Fix warning

* Just alter the protobuf

* Alter protobuf doesn't do anything with the altered data, so let's re-encode it

* Check inputbroker before access

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-10-04 06:42:36 -05:00
Ben Meadors a15d654767 Finish deprecating the Repeater role behavior (#8144)
* Finish deprecating the Repeater role behavior

* Validate

* Fixed bad if/else block

* Get your crap together!
2025-09-28 15:30:53 -05:00
Ben Meadors ac4bcd2f56 Cleanup 2025-09-11 18:57:30 -05:00
Ben Meadors e17c50bb86 Put guards in place around debug heap operations (#7955)
* Put guards in place around debug heap operations

* Add macros to clean up code

* Add pointer as well
2025-09-11 07:57:42 -05:00
Jonathan Bennett e7741c20e4 Add LOG_HEAP log type, and more heap debug messages (#7937) 2025-09-09 10:29:07 -05:00
Ben Meadors db238ef524 Log when this happened 2025-08-11 19:49:35 -05:00
Jonathan Bennett f2b935f48f Stop the bleeding with malicious NodeDB overwrites (#7596) 2025-08-11 15:52:28 -05:00
Jonathan Bennett c3b2b474c6 Drop NodeInfo packets if the is_licensed bit doesn't match owner (#7361) 2025-07-16 16:05:34 -05:00
4feaec651f Unify the native display config between legacy display and MUI (#6838)
* Add missed include

* Another Warning fix

* Add another HAS_SCREEN

* Namespace fixes

* Removed depricated destination types and re-factored destination screen

* Get rid of Arduino Strings

* Clean up after Copilot

* SixthLine Def, Screen Rename

Added Sixth Line Definition Screen Rename, and Automatic Line Adjustment

* Consistency is hard - fixed "Sixth"

* System Frame Updates

Adjusted line construction to ensure we fit maximum content per screen.

* Fix up notifications

* Add a couple more ifdef HAS_SCREEN lines

* Add screen->isOverlayBannerShowing()

* Don't forget the invert!

* Adjust Nodelist Center Divider

Adjust Nodelist Center Divider

* Fix variable casting

* Fix entryText variable as empty before update to fix validation

* Altitude is int32_t

* Update PowerTelemetry to have correct data type

* Fix cppcheck warnings (#6945)

* Fix cppcheck warnings

* Adjust logic in Power.cpp for power sensor

---------

Co-authored-by: Jason P <applewiz@mac.com>

* More pixel wrangling so things line up NodeList edition

* Adjust NodeList alignments and plumb some background padding for a possible title fix

* Better alignment for banner notifications

* Move title into drawCommonHeader; initial screen tested

* Fonts make spacing items difficult

* Improved beeping booping and other buzzer based feedback (#6947)

* Improved beeping booping and other buzzer based feedback

* audible button feedback (#6949)

* Refactor

---------

Co-authored-by: todd-herbert <herbert.todd@gmail.com>

* Sandpapered the corners of the notification popup

* Finalize drawCommonHeader migration

* Update Title of Favorite Node Screens

* Update node metric alignment on LoRa screen

* Update the border for popups to separate it from background

* Update PaxcounterModule.cpp with CommonHeader

* Update WiFi screen with CommonHeader and related data reflow

* It was not, in fact, pointing up

* Fix build on wismeshtap

* T-deck trackball debounce

* Fix uptime on Device Focused page to actually detail

* Update Sys screen for new uptime, add label to Freq/Chan on LoRa

* Don't display DOP any longer, make Uptime consistent

* Revert Uptime change on Favorites, Apply to Device Focused

* Label the satelite number to avoid confusion

* Boop boop boop boop

* Correct GPS positioning and string consistency across strings for GPS

* Fix GPS text alignment

* Enable canned messages by default

* Don't wake screen on new nodes

* Cannedmessage list emote support added

* Fn+e emote picker for freetext screen

* Actually block CannedInput actions while display is shown

* Add selection menu to bannerOverlay

* Off by one

* Move to unified text layouts and spacing

* Still my Fav without an "e"

* Fully remove EVENT_NODEDB_UPDATED

* Simply LoRa screen

* Make some char pointers const to fix compilation on native targets

* Update drawCompassNorth to include radius

* Fix warning

* button thread cleanup

* Pull OneButton handling from PowerFSM and add MUI switch (#6973)

* Trunk

* Onebutton Menu Support

* Add temporary clock icon

* Add gps location to fsi

* Banner message state reset

* Cast to char to satisfy compiler

* Better fast handling of input during banner

* Fix warning

* Derp

* oops

* Update ref

* Wire buzzer_mode

* remove legacy string->print()

* Only init screen if one found

* Unsigned Char

* More buttonThread cleaning

* screen.cpp button handling cleanup

* The Great Event Rename of 2025

* Fix the Radiomaster

* Missed trackball type change

* Remove unused function

* Make ButtonThread an InputBroker

* Coffee hadn't kicked in yet

* Add clock icon for Navigation Bar

* Restore clock screen definition code - whoops

* ExternalNotifications now observe inputBroker

* Clock rework (#6992)

* Move Clock bits into ClockRenderer space

* Rework clock into all device navigation

* T-Watch Actually Builds Different

* Compile fix

---------

Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>

* Add AM/PM to Digital Clock

* Flip Seconds and AM/PM on Clock Display

* Tik-tok pixels are hard

* Fix builds on Thinknode M1

* Check for GPS and don't crash

* Don't endif til the end

* Rework the OneButton thread to be much less of a mess. (#6997)

* Rework the OneButton thread to be much less of a mess. And break lots of targets temporarily

* Update src/input/ButtonThread.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix GPS toggle

* Send the shutdown event, not just the kbchar

* Honor the back button in a notificaiton popup

* Draw the right size box for popup with options

* Try to un-break all the things

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* 24-hour Clock Should have leading zero, but not 12-hour

* Fixup some compile errors

* Add intRoutine to ButtonThread init, to get more responsive user button back

* Add Timezone picker

* Fix Warning

* Optionally set the initial selection for the chooser popup

* Make back buttons work in canned messages

* Drop the wrapper classes

* LonPressTime now configurable

* Clock Frame can not longer be blank; just add valid time

* Back buttons everywhere!

* Key Verification confirm banner

* Make Elecrow M* top button a back button

* Add settings saves

* EInk responsiveness fixes

* Linux Input Fixes

* Add Native Trackball/Joystick support, and move UserButton to Input

* No Flight Stick Mode

* Send input event

* Add Channel Utilization to Device Focused frame

* Don't shift screens when we draw new ones

* Add showOverlayBanner arguments to no-op

* trunk

* Default Native trackball to NC

* Fix crash in simulator mode

* Add longLong button press

* Get the args right

* Adjust Bluetooth Pairing Screen to account for bottom navigation.

* Trackball everywhere, and unPhone buttons

* Remap visionmaster secondary button to TB_UP

* Kill ScanAndSelect

* trunk

* No longer need the canned messages input filter

* All Canned All the time

* Fix stm32 compile error regarding inputBroker

* Unify tft lineheights (#7033)

* Create variable line heights based upon SCREEN_HEIGHT

* Refactor textPositions into method -> getTextPositions

* Update SharedUIDisplay.h

---------

Co-authored-by: Jason P <applewiz@mac.com>

* Adjust top distance for larger displays

* Adjust icon sizes for larger displays

* Fix Paxcounter compile errors after code updates

* Pixel wrangling to make larger screens fit better

* Alert frame has precedence over banner -- for now

* Unify on ALT_BUTTON

* Align AM/PM to the digit, not the segment on larger displays

* Move some global pin defines into configuration.h

* Scaffolding for BMM150 9-axis gyro

* Alt button behavior

* Don't add the blank GPS frames without HAS_GPS

* EVENT_NODEDB_UPDATED has been retired

* Clean out LOG_WARN messages from debugging

* Add dismiss message function

* Minor buttonThread cleanup

* Add BMM150 support

* Clean up last warning from dev

* Simplify bmm150 init return logic

* Add option to reply to messages

* Add minimal menu upon selecting home screen

* Move Messages to slot 2, rename GPS to Position, move variables nearer functional usage in Screen.cpp

* Properly dismiss message

* T-Deck Trackball press is not user button

* Add select on favorite frame to launch cannedMessage DM

* Minor wording change

* Less capital letters

* Fix empty message check, time isn't reliable

* drop dead code

* Make UIRenderer a static class instead of namespace

* Fix the select on favorite

* Check if message is empty early and then 'return'

* Add kb_found, and show the option to launch freetype if appropriate

* Ignore impossible touchscreen touches

* Auto scroll fix

* Move linebreak after "from" for banners to maximize screen usage.

* Center "No messages to show" on Message frame

* Start consolidating buzzer behavior

* Fixed signed / unsigned warning

* Cast second parameter of max() to make some targets happy

* Cast kbchar to (char) to make arduino string happy

* Shorten the notice of "No messages"

* Add buzzer mode chooser

* Add regionPicker to Lora icon

* Reduce line spacing and reorder Position screen to resolve overlapping issues

* Update message titles, fix GPS icons, add Back options

* Leftover boops

* Remove chirp

* Make the region selection dismissable when a region is already set

* Add read-aloud functionality on messages w/ esp8266sam

* "Last Heard" is a better label

* tweak the beep

* 5 options

* properly tear down freetext upon cancel

* de-convelute canned messages just a bit

* Correct height of Mail icon in navigation bar

* Remove unused warning

* Consolidate time methods into TimeFormatters

* Oops

* Change LoRa Picker Cancel to Back

* Tweak selection characters on Banner

* Message render not scrolling on 5th line

* More fixes for message scrolling

* Remove the safety next on text overflow - we found that root cause

* Add pin definitions to fix compilation for obscure target

* Don't let the touchscreen send unitialized kbchar values

* Make virtual KB just a bit quicker

* No more double tap, swipe!

* Left is left, and Right is right

* Update horizontal lightning bolt design

* Move from solid to dashed separator for Message Frame

* Single emote feature fix

* Manually sort overlapping elements for now

* Freetext and clearer choices

* Fix ESP32 InkHUD builds on the unify-tft branch (#7087)

* Remove BaseUI branding

* Capitalization is fun

* Revert Meshtastic Boot Frame Changes

* Add ANZ_433 LoRa region to picker

* Update settings.json

---------

Co-authored-by: HarukiToreda <116696711+HarukiToreda@users.noreply.github.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Jason P <applewiz@mac.com>
Co-authored-by: todd-herbert <herbert.todd@gmail.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-06-21 06:36:04 -05:00
Ben Meadors 42a80d8aed Coerce user.id to always be derive from the nodenum (#6906)
* Coerce user.id to always be derive from the nodenum

* Additionally null it out on send

* Revert "Additionally null it out on send"

This reverts commit 22a2d68723.
2025-05-28 11:30:59 -05:00
Ben MeadorsandCopilot fc64bea698 Unmessagable implementation and defaults (#6811)
* Unmessagable implementation and defaults

* Router and router late are unmessagable by default

* Update src/modules/AdminModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-05-14 15:28:09 -05:00
GUVWAFandBen Meadors 4747e73f37 Space out periodic broadcasts of modules automatically (#5931)
* Space out periodic broadcasts of modules automatically

* Add warning for function usage

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2025-01-26 13:59:59 -06:00
Ben Meadors 982190936d More log reductions. I'll probably stop now ;-) (#5263) 2024-11-06 21:03:25 +08:00
Ben Meadors f769c50fa5 More reduction (#5256)
* Now with even fewer ings

* Ye

* Mo

* QMA6100PSensor
2024-11-04 19:15:59 -06:00
Ben Meadors 50dac38a1b Pass#2: Lots more savings in logs and string reduction surgery (#5251)
* Pass#2: Lots more savings in logs and string reduction surgery

* Don't need Thread suffix either

* Warn
2024-11-04 12:16:25 -06:00
Tom Fifield 276067065e Log cleanups (#5135)
* Log cleanups

change log levels, shorten log messages, delete commented out logs

* Address comments from review

* Remove full stops

* EDEBUG --> DEBUG
2024-11-04 06:09:23 -06:00
28b469dbf0 fix: don't broadcast public keys if the user is licensed (#5190)
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
2024-10-30 12:05:09 -05:00
Ben Meadors 4575352d8c Initial NODENUM_BROADCAST_NO_LORA implementation with NeighborInfo module (#5087)
* Initial NODENUM_BROADCAST_NO_LORA implementation with NeighborInfo module

* isBroadcast

* Trunkt
2024-10-19 12:48:00 -05:00
Thomas GöttgensandBen Meadors 05e4a639a1 remove newline from logging statements. (#5022)
remove newline from logging statements in code. The LOG_* functions will now magically add it at the end.

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2024-10-14 15:11:43 +11:00
GUVWAF 673fe294f3 Add rxDupe, txRelay and txRelayCanceled to LocalStats (#4936)
* Introduce `isFromUs()` and `isToUs()`

* Add rxDupe, txRelay and txRelayCanceled to LocalStats
2024-10-04 06:28:51 -05:00
Ben MeadorsandTom Fifield e8829b8f52 Refactor and consolidate time window logic (#4826)
* Refactor and consolidate windowing logic

* Trunk

* Fixes

* More

* Fix braces and remove unused now variables.

There was a brace in src/mesh/RadioLibInterface.cpp that was breaking
compile on some architectures.

Additionally, there were some brace errors in
src/modules/Telemetry/AirQualityTelemetry.cpp
src/modules/Telemetry/EnvironmentTelemetry.cpp
src/mesh/wifi/WiFiAPClient.cpp

Move throttle include in WifiAPClient.cpp to top.

Add Default.h to sleep.cpp

rest of files just remove unused now variables.

* Remove a couple more meows

---------

Co-authored-by: Tom Fifield <tom@tomfifield.net>
2024-09-23 08:58:14 -05:00
Jonathan Bennett 8ef72a5c08 Shorter nodeinfo timeout redux (#4458)
* Add shorterTimeout bool to sendOurNodeInfo

* Respond to likely PKI decode errors with a quick nodeinfo

* Protbufs

* Move to PKI_UNKNOWN_PUBKEY for PKI decode error
2024-08-14 17:17:53 -05:00
Jonathan Bennett d2ea430a3e Make SPI frequency and TOPHONE queue size configurable on Native (#4369)
* Make SPI frequency configurable on Native

* Make the tophone queue size configurable for Portduino

* The modified SPISettings must be configured in setup(), after config.yaml is processed

* make MeshService a pointer, so we can configure MAX_RX_TOPHONE at run time

* Got a little over excited with refactoring

* Silence a warning
2024-08-01 19:29:49 -05:00
Ben Meadors 0b48663cbc Don't alloc NodeInfo replies when channel utilization is > 40% (#3991)
* Don't alloc NodeInfo replies when channel utilization is > 40%

* Commit

* Logs
2024-05-30 08:49:01 -05:00
Jonathan BennettandBen Meadors dfcd0d14f6 Add MaxNodes to Native config (#3427)
* Add MaxNodes to Native

* It compiles...

* Convert nodedb to use new

* Closer but still broken.

* Finally working

* Remove unintended lines

* Don't include a pointer

* Capitalization matters.

* avoid rename in protocol regen

* When trimming the nodeDB, start with a cleanup

* Remove extra cleanupMeshDB() call for now

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2024-03-21 09:06:37 -05:00
Ben Meadors 0d1d79b6d1 Extract default intervals and coalesce methods into their own file / static class methods (#3425)
* Extract default intervals and coalesce methods into their own file / static class methods

* Missed pax

* Still managed to miss one
2024-03-17 08:18:30 -05:00
Ben Meadors e27f029d09 Bump minimum NodeInfo send to 5 minutes (#3423)
* Bump minimum NodeInfo send to 3 minutes

* 5
2024-03-16 19:56:42 -05:00
Ben Meadors ec6bdeed81 NodeInfo broadcast ensure default on 0 and enforce 1 hour minimum (#3415)
* NodeInfo broadcasts ensure defaults on 0 and enforce 1 hour minumum

* Doh!

* Hey that's not on config!
2024-03-15 07:12:03 -05:00
GUVWAFandBen Meadors fd8b1687a1 Update channel of node in updateUser and write to flash if needed (#3094)
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2024-01-15 20:11:35 -06:00
Thomas Göttgens 24c4ee9bfa local variable and class variable may not be named the same 2023-12-17 16:28:48 +01:00