cppcheck reports returnByReference on XModemAdapter::getForPhone():
`meshtastic_XModem` carries a 128-byte payload buffer plus header fields,
so returning it by value copied the whole struct on every call.
Return `const meshtastic_XModem &` instead, and mark the method const -
it is a pure read of xmodemStore, with resetForPhone() being what drains
it. Every caller either copies into a value or reads a single field, so
no call site changes.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com>
limitPower() converts the member `power` in place (regulatory clamp, then the
TX_GAIN_LORA/FEM subtraction) and relied on applyModemConfig() having just
re-seeded it. Since #10025 every driver calls it from both reinitChip() and
programModemParams(), and the recovery paths added in #11676/#11678 run the two
back-to-back, so each recovery re-converts an already-converted value. On a
RAK13302 (22-entry gain table) one recovery walks a 30 dBm request
30 -> 22 -> 13 dBm and a second one down toward the -9 dBm floor, while
config.lora.tx_power still reads 30. Seed `power` from config.lora.tx_power at
the top of limitPower(); applyModemConfig() always writes the resolved value
back there, so a single call is unchanged.
Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com>
cppcheck (iterateByValue) flagged five range-for loops in the NodeDatabase
pb_callbacks that copy a whole protobuf entry off the vector only to pass
its address to pb_encode_submessage(), which takes a const void *. Bind by
const reference instead.
Drops one meshtastic_NodePositionEntry, NodeTelemetryEntry, NodeStatusEntry,
NodeEnvironmentEntry and NodeInfoLite_Legacy copy per node per encode pass;
these run on every nodes.proto save.
The nodes_tag loop in NodeDB.cpp is intentionally left as a by-value copy:
it mutates item.snr_q4/item.snr to the on-disk quantized form before
encoding, so it is a working copy rather than a redundant one. cppcheck
does not flag it.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* fix(phoneapi): resend my_info when the node num moves mid-session
The first region set mints the PKI key and moves my_node_num to
crc32(public_key) live. my_info only went out during the want_config_id
handshake, so an already-connected client kept addressing the old number and
its admin packets NAKed PKI_SEND_FAIL_PUBLIC_KEY until it reconnected.
PhoneAPI tracks the number it last reported and re-sends my_info from
STATE_SEND_PACKETS when it no longer matches. createNewIdentity() nudges
fromNum so clients poll.
Fixes#11718
* fix(phoneapi): key the MyInfo re-announce off a one-shot state
Review follow-up. The per-connection reportedNodeNum field is gone: adding
per-instance members to PhoneAPI is documented as breaking USB-CDC enumeration
on the nRF52 Adafruit framework, and the baseline was never set for
SPECIAL_NONCE_ONLY_NODES, which skips STATE_SEND_MY_INFO and so emitted an
unexpected my_info after config_complete_id.
MeshService::identityMoved is set with the nudge and cleared once the notify
pass has reached every observer, so PhoneAPI::onNotify arms STATE_RESEND_MY_INFO
on each connected client in that single pass and stores nothing per connection.
The test now drives NodeDB::createNewIdentity() and MeshService::loop() instead
of writing my_node_num directly, and asserts the transport wake-up. Nodes-only
sync asserts no trailing my_info. drainToIdle() honours its read cap.
* fix(phoneapi): restart the dump when the node num moves mid-sync
Review follow-up. A client still in its config dump has already been sent the
old my_info and has no steady state for the one-shot to fall back from, so the
notify pass cleared identityMoved without covering it and the client finished
syncing on the obsolete number.
PhoneAPI::onNotify now restarts such a client's dump, which is the existing
re-handshake path. Skipped for a client that has not reached my_info yet and for
SPECIAL_NONCE_ONLY_NODES, which never sends one.
test_node_num_change_mid_dump_restarts_sync renumbers mid-dump and asserts the
restart, the new number, and that no part of the config is lost. Verified to
fail without the fix.
* fix(phoneapi): make the identity-move signal survive a concurrent notify pass
Review follow-up. The identity move can run off the loop task: a local admin
set_config reaches AdminModule through Router::sendLocal() on whichever task
delivered it. A bool cleared by MeshService::loop() could therefore be set and
cleared without any client being armed, losing the re-announce.
A generation counter replaces the bool. loop() snapshots it with fromNum before
notifying and only advances the seen counter afterwards, so anything bumped
during the pass is still pending. The same snapshot fixes a notify for a
fromNum bump that arrived mid-pass being marked delivered.
test_node_num_change_mid_dump_restarts_sync now asserts the whole restarted
dump: header order, channels, both config sections, our node record, nonce.
Also trims the MyInfo redaction comment to the two-line cap.
* fix(nodedb): keep self at index 0 after a live renumber, restart nodes-only syncs
Review follow-up. createNewIdentity() removed our old row and appended the new
one, leaving index 0 pointing at some other node. PhoneAPI's own-nodeinfo read
and the demote/evict scans that skip index 0 to protect us both rely on that
slot being self, so a renumbered node handed every client a stranger's record as
its own. Pinned the way nodeDBSelfCare() does it.
onNotify no longer exempts SPECIAL_NONCE_ONLY_NODES from the mid-sync restart.
That dump carries no my_info, but it does carry the self record, which the move
invalidates the same way. Such a client also gets the re-announce once its sync
lands in STATE_SEND_PACKETS, which it previously never did.
The generation counters are atomic. Every interleaving was already safe, since
observers read the live counter and the seen counter only advances to a pre-pass
snapshot, but the concurrent plain accesses were a data race on paper.
* fix(meshservice): make fromNum atomic
Review follow-up. The counter is bumped from whichever task queued the packet
and read by loop(). It is private to MeshService, so the type change covers
every access.
* docs(agents): exempt test headers from the two-line comment limit
The one-or-two-line comment rule rests on "the diff and commit message
carry the rationale". For a test that premise is false. A test outlives
the PR that added it, and the next person to read it is reading it
because it failed - months later, in someone else's change, with the
original discussion out of reach. That reader has one decision to make:
real regression, or an expectation that has gone stale? The assertions
alone cannot answer it, so the justification has to live in the file.
The new "Test comments" section requires three things of a test header -
what is under test by symbol and file, why that behavior is required, and
the specific regression that returns if the assertions are deleted or
relaxed - and grants whatever length they need.
Authoritative statement lives in that section of
.github/copilot-instructions.md, anchored at #test-comments. AGENTS.md
and CLAUDE.md link to it rather than restate it. The test/** entry in
.coderabbit.yaml carries the one permitted copy, since a YAML instruction
cannot follow a link.
This documents existing practice rather than changing it: 57 of the 72
suites with a test_main.cpp already open with three or more comment
lines, test/test_gps_fix_hold/test_main.cpp with eleven. The rule as
written forbade all of them, and automated reviewers acted on it.
Scoped to the single comment bullet, not to the whole "General Style"
section. Unlike "Naming Conventions", that section also holds the logging
tiers and the Throttle rule, which bind test code as hard as src/; a
blanket preamble would have quietly exempted tests from millis()
discipline as well.
The exception is bounded. The CodeRabbit entry still flags narrative that
carries no contract - debugging journey, changelog prose, restating what
the assertions plainly do - and per-case comments that merely repeat the
test name. The documentation-does-not-live-here rule is untouched.
* docs(agents): cut the test-comment rule to its essential statements
A section about comment length had no business running to 373 words.
Canonical section down to 194: dropped the elaboration of why the commit
message is unavailable, the aside about which bullet reviewers most often
cut, the "not licence for narrative" preamble, and the closing flourish.
What remains is what an agent has to act on - the premise that fails for
tests, the three things a header states, the instruction to reject a
shorten-this review comment, and the worked example.
Pointer files carry scope, not argument. The AGENTS.md bullet is one
sentence and a link, matching the Test naming pointer directly above it.
The .coderabbit.yaml copy keeps its imperatives, since a YAML instruction
cannot follow the link to find them, but loses the causal explanation it
did not need in order to act.
* Hide Navigation Bar on EInk Shutdown
* Revert "Hide Navigation Bar on EInk Shutdown"
This reverts commit 744812555b.
* Hide Navigation Bar on EInk Shutdown
* It's Hide, not Drop
Every PR targeting develop has been red since 2026-09-07 on the seven ESP32
check jobs, while the nRF52, RP2040 and STM32 jobs pass on identical source.
gh-action-firmware#61 moved the ESP32 container images onto the pioarduino
core. Its esp32 platform ships its own tool-cppcheck 2.20.1 and reinstalls it
over anything the repo pins, so ESP32 now analyses with cppcheck 2.20 while
every other platform still resolves platformio/tool-cppcheck 1.21100.230717,
i.e. 2.11. 2.20 parses far more of this tree than 2.11 ever managed, so checks
that were always enabled fired for the first time: 388 defects, ESP32 only.
#11776 cleared the two unknownMacro errors. Of the 386 left, two are worth
acting on and are fixed rather than suppressed:
* SerialModule dereferenced a null Position in NMEA/CALTOPO mode. `decoded`
stays NULL when pb_decode_from_bytes() fails, but printWPL() was called
with *decoded regardless, so a malformed position payload on our portnum
crashed the node. Emit the waypoint only on a successful decode.
* InkHUD's 12-hour clock passed a signed 12 to a %u conversion.
The rest are style and performance suggestions - functionStatic and the
const-correctness family account for 351 of them. Suppress those check ids so
the gate means the same thing on every platform again, scoping the one-off
ones to their file so a new occurrence elsewhere still fails. Burning them
down is worth doing deliberately, not under a CI outage.
Verified in the CI container images: all seven previously failing ESP32
environments pass, and rak4631, tracker-t1000-e and t-echo-plus still pass
under cppcheck 2.11.
pioarduino's cppcheck 2.20 reports functionStatic for all 20 methods of the
no-op graphics::Screen defined under !HAS_SCREEN: none of them touch a member,
so it offers to make them static. The advice is wrong here - the stub exists
only to mirror the real Screen's instance API so call sites like
screen->setFrames(...) compile on screenless boards, so the methods have to
stay non-static member functions.
The header is included by 85 translation units, so this fired 1700 times on
the two screenless esp32 boards in the check matrix (heltec-ht62-esp32c3-sx1262
and tlora-c6) - the entire src/graphics low-severity count for those boards.
bin/check-all.sh passes --fail-on-defect=low, so it was failing those jobs.
Wrap the stub in an inline cppcheck-suppress-begin/end block, matching the
inline-suppression style already used elsewhere in the tree.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lib_ignore matches the name from the library manifest, and iLabs_Hearth
declares "iLabs Hearth" with a space. The directory name used in #11757
therefore never matched, and the Pico builds still compile the library and
still fail on its ESP_SERIAL_PORT #error.
* fix(ci): build with pioarduino core instead of upstream platformio
The espressif32 platform is the pioarduino fork, but setup-base installed
upstream platformio and then ran pio upgrade, so every firmware build ran
a core the platform is not built against.
Upstream 6.2.0, released 2026-09-05, moved its tool-scons core dependency
to ~4.41101.0 (SCons 4.11.1). The lazy "import SCons.Tool.FortranCommon"
that smart_link() uses to choose a linker fails there, so every ESP target
died at link-action resolution before compiling a file. Core resolves
tool-scons as a core dependency, so a platform_packages pin cannot help:
core installs its own version and removes the pinned one.
pioarduino core pins SCons 4.8.1 by URL rather than by range, so upstream
releases cannot reach it. Dropping pio upgrade as well, since it re-pulled
the latest upstream core regardless of what pip installed.
Only setup-base changes. The matrix-generation jobs install platformio to
parse the ini files and never build firmware, and the native test suites
pass on upstream core because that platform does not reach smart_link.
* Revert "fix(ci): pin tool-scons to 4.8.1 for ESP targets (#11756)"
This reverts commit a8912b1eb.
The pin never took effect. PlatformIO Core resolves tool-scons as a core
dependency, so it installs its own version and removes the pinned one:
Installing platformio/tool-scons @ 4.40801.0
Installing platformio/tool-scons @ ~4.41101.0
Removing tool-scons @ 4.40801.0
Switching to pioarduino core in the preceding commit fixes this properly,
and leaving a platform_packages entry that fights a core dependency would
only be misleading.
arduino-pico gained a bundled iLabs_Hearth library that supplies its own
Preferences.h. NodeDB.cpp includes <Preferences.h> inside an ARCH_ESP32
guard, and the library dependency finder runs in chain mode, which matches
include directives without evaluating the preprocessor. It therefore pulls
the library into every Pico build, and Hearth.cpp refuses to compile on a
board that does not define ESP_SERIAL_PORT:
iLabs_Hearth/src/Hearth.cpp:121:2: error: #error "iLabs Hearth requires a
board variant that defines ESP_SERIAL_PORT ..."
The library only reaches the build because the platform resolves
framework-arduinopico from an arduino-pico master commit; the
platform_packages entry here pins the name arduino-pico, which is a
different package and does not override it.
Ignoring the library is enough, since no Pico target uses Matter. The
sibling iLabs_ESP-NOW ships ESP32_NOW.h and ATLink.h, which nothing
includes, so it needs no entry.
PlatformIO Core now pulls tool-scons ~4.41101.0 (SCons 4.11.1), which
overrides the 4.8.1 the pioarduino espressif32 platform asks for. In
4.11.1 the lazy "import SCons.Tool.FortranCommon" that smart_link() uses
to pick a linker raises ModuleNotFoundError, so every ESP environment
fails at link-action resolution, before a single file is compiled:
*** [.pio/build/<env>/firmware-<env>.elf] ModuleNotFoundError :
No module named 'SCons.Tool.FortranCommon'
The package is not at fault; FortranCommon.py is present in
tool-scons-4.41101.0 and imports cleanly outside SCons. The failure comes
from the module state SCons's own tool loader leaves behind.
Scoped to esp32_common, which all six ESP architectures extend. nRF52,
STM32 and rp2040 are unaffected and keep the toolchain they have.
* feat(t-connect-pro): add LilyGo T-Connect-Pro variant
ESP32-S3R8, 16MB flash, 8MB octal PSRAM. SX1262 LoRa, 480x222 ST7796 LCD
with CST226SE touch, W5500 ethernet and a 10A relay on EXT_NOTIFY_OUT.
LoRa, display and ethernet share one SPI bus (SCK 12 / MISO 13 / MOSI 11),
so every peripheral stays on SPI2_HOST.
board_level is extra and HW_VENDOR falls through to PRIVATE_HW until a
HardwareModel enum value is allocated.
* fix(w5500): serialize shared-bus SPI access with spiLock
Arduino's ETHClass reaches SPI through SPIClass, whose mutex is invisible to
LovyanGFX. On a board where both share a bus the MAC reads glitched frame
headers, and the resulting ESP_LOGE flood blocks the W5500 RX task on the
console UART until the task watchdog reboots the device.
SharedBusEthernet installs esp_eth directly so its custom_spi_driver
callbacks can take spiLock, the mutex the radio, display, SD and sensors
already share. It derives from NetworkInterface, so localIP(), connected(),
config() and the GOT_IP events are unchanged.
Selected by ETH_SHARED_SPI; boards without it keep the stock ETHClass path.
Measured on T-Connect-Pro under a 150 x 1472 byte flood with the display
active: 34948 truncated frames, 3 reboots and 20% packet loss before,
none after.
* fix(cst226se): honour reset pin, screen rotation and skip wrong-model probes
Drive TOUCH_RST when the variant defines one, and stop passing I2C pins to
begin() so SensorLib does not re-init a bus the scan already owns.
Derive touch geometry from SCREEN_ROTATE the way TFTDisplay does, so a
rotated panel maps to the landscape UI rather than the raw panel size.
Use TouchDrvCST226 rather than the TouchDrvCSTXXX wrapper. Pinning the model
does not stop the wrapper walking CST816 and CST92xx, whose retries cost
about 3.5s of boot. T-Beam behaviour is unchanged.
* style: trim comments to the two-line limit
Follows the comment rule in .github/copilot-instructions.md, which the
original commits missed.
* feat(t-connect-pro): use the T_CONNECT_PRO hardware model
Depends on meshtastic/protobufs#1062. Does not build until that merges and
the generated headers are synced, since meshtastic_HardwareModel_T_CONNECT_PRO
does not exist yet.
Drops -D PRIVATE_HW, which becomes a no-op once HW_VENDOR resolves, and
promotes board_level to release.
* chore(t-connect-pro): mark as community supported
Support level 3, matching the other unlicensed LilyGo boards.
* fix(w5500): roll back partial init when begin() fails
begin() returns early when ethHandle is set, so a failure after
esp_eth_driver_install() left the handle populated and every later call
returned true with no working driver.
teardown() releases the event handler, netif glue, netif, driver, PHY and MAC
in reverse creation order, and every failure path now uses it.
* refactor(w5500): drop config the custom SPI driver never reads
spi_devcfg and spi_host_id are only read by w5500_spi_init, which esp_eth
skips when custom_spi_driver is set, so the device config fields were dead.
Also drops the handle() accessor, its only caller is the class's own event
handler, the eventRegistered flag, since unregistering an unregistered
handler is safe, the redundant _esp_netif guard around destroyNetif(), and
the TFT_CS indirection, which this panel path does not read.
* fix(w5500): fail begin() when the event handler cannot register
The return value was ignored, so a failed registration still started Ethernet
and returned true while onEthEvent never fired. WiFiAPClient would then miss
ETH_CONNECTED, GOT_IP and DISCONNECTED, leaving the link up with the firmware
believing it was down.
* fix(position): stamp the broadcast cadence only when a position packet was actually sent
* fix(position): honor the router verdict and always fall back to nodeinfo when no position goes out
* fix(position): snapshot Time::getMillis() for the throttle stamps and trim the added comments
* fix(position): consume the radio-generation change only on a position send that went out
* refactor(position): share one smart-broadcast path and drop the duplicated fresh-position guard
* fix(position): persist smart broadcasts to the transmit history
* fix(telemetry): don't broadcast the noise floor default as a real reading
LocalStats.noise_floor has no has_/presence bit, so RadioLibInterface's
NOISE_FLOOR_DEFAULT (-120 dBm) placeholder was indistinguishable on the
wire from a genuine -120 dBm reading whenever no valid RSSI sample had
ever been collected (radio not idle when sampled, or every reading
falling outside the plausible bounds).
Gate the assignment on hasNoiseFloorSamples() and log a warning instead
of shipping the placeholder. The field stays at its zero-init default
in that case.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Address review: trim the noise_floor comment and demote the log to debug
An empty sample buffer is expected during early boot, so LOG_WARN overstated it.
* Restore the periodic noise floor feeder lost in merge e55947595
updateNoiseFloor() shipped in #9347 with three call sites. Review removed the
onNotify() one because sampling on the ISR path can overflow the 256-byte radio
FIFO; the maintainer's guidance was to keep a periodic call from thread context
instead. The remaining completeSending() and startReceive() calls then vanished
in merge commit e55947595 "Merge upstream develop into noise-floor", which took
develop's rewrite of both functions wholesale. No commit since has called it.
That left DeviceTelemetry::getLocalStatsTelemetry() as the only caller, so the
20-sample window advanced at most once per local stats send: one sample every 15
minutes, five hours to fill, and the internal 5s throttle never reached.
Sample from the existing AGC maintenance tick, before periodicRadioMaintenance()
because resetAGC() recalibrates the frontend and biases an RSSI read taken right
after it. NOISE_FLOOR_UPDATE_INTERVAL_MS stays at 5s as an inner floor; the 60s
call site sets the real rate. The window now fills in about 20 minutes.
* Trim the noise floor change
Drop the updateNoiseFloor() call on the telemetry path: the 60s tick feeds the
window now, so it contributed about one sample in fifteen and cost an SPI-gated
read on the local stats path.
The else-branch existed only to log; the "Sending local stats" LOG_INFO in the
same function already reports noise_floor=0 when no sample exists.
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
* docs(agents): document that test names are snake_case, not camelCase
Automated reviewers repeatedly ask for test suite directories and test_*
functions to be renamed to camelCase to match the src/ convention. That
rename breaks the harness and the rule was never written down.
Authoritative statement lives in the "Test naming" section of
.github/copilot-instructions.md, anchored at #test-naming. AGENTS.md and
CLAUDE.md link to it rather than restate it.
bin/run-tests.sh enumerates suites with -name 'test_*' but matches
PlatformIO verdict lines against test_[a-z0-9_]+, lowercase only, so an
uppercase suite directory is counted missing and the run reports AMBER
instead of GREEN. RUN_TEST in test/TestUtil.h passes #func to Unity and to
the state-checkpoint hooks, making the function name the only attribution a
CI failure carries.
.coderabbit.yaml gains a test/** path_instruction stating the rule inline,
since YAML cannot follow the link.
* docs(agents): separate the suite-directory and test-function naming rules
Review feedback on the previous commit was correct on both points.
The section called the test-function form snake_case while every example used
camelCase segments. The tree holds 743 test functions with an uppercase segment
and 675 without, so snake_case was wrong for more than half of them. Split the
two rules that were conflated: suite directories are strictly test_[a-z0-9_]+,
while test functions require only the test_ prefix and underscore separators,
with segment case free. States what is actually forbidden - dropping the prefix,
or collapsing the segments into one camelCase identifier.
The canonical-copy policy forbade restating the rule anywhere, then restated it
in AGENTS.md and .coderabbit.yaml. Name the YAML entry as the single permitted
copy, since a YAML instruction cannot follow a link, and reduce the AGENTS.md
bullet to a pointer.
* docs(agents): make the AGENTS.md and CLAUDE.md pointers neutral
Both still carried the "snake_case, not camelCase" label that b4fdd9168
corrected in the canonical section, so an agent reading either pointer got the
whole-name rule the canonical section now rejects. Describe the scope instead of
restating the rule: the src/ naming rule does not apply under test/.
* feat(meshtasticd): add RAK19714 USB SX1262 pinmap
Add a CH341 USB preset so meshtasticd can use the RAK19714 without a hand-written config.
* change filename to lowercase(lora-usb-rak19714.yaml) so autoconf can find it. Remove redundant power limit
* Rename lora-usb-RAK19714.yaml to lora-usb-rak19714.yaml
change filename to lowercase(lora-usb-rak19714.yaml) so autoconf can find it.
---------
Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
The T-Deck Pro V1.1 boots into Critical Fault #3 (NoRadio) because
LORA_EN (GPIO 46) is never driven high, so the SX1262 never receives
power and is never detected.
Before #9438 this pin was driven from src/main.cpp under
`#elif defined(T_DECK_PRO)`, which covered both the t-deck-pro and the
t-deck-pro-v1_1 environments, since both build with -D T_DECK_PRO.
#9438 moved that block into variants/esp32s3/t-deck-pro/variant.cpp and
linked it with a build_src_filter added only to the t-deck-pro
environment. t-deck-pro-v1_1 had been added five days earlier and has
neither a variant.cpp nor a build_src_filter, so it silently lost the
pin setup: earlyInitVariant() is a weak symbol with an empty default in
main.cpp, so a missing strong override produces no compile or link
error.
Give t-deck-pro-v1_1 the same variant.cpp as t-deck-pro, plus the
build_src_filter needed to actually link it. This also restores the
LORA_CS, SDCARD_CS and PIN_EINK_CS pre-init lost at the same time; all
three share the SPI bus and need their chip selects deasserted before
the bus is used.
Fixes#11708
Co-authored-by: Claude <noreply@anthropic.com>
* fix(metadata): report all compiled-out module configs
Add MQTT_CONFIG, NEIGHBORINFO_CONFIG, STOREFORWARD_CONFIG and
TELEMETRY_CONFIG bits to getDeviceMetadata().excluded_modules,
guarded by the same macros that gate the modules in
src/modules/Modules.cpp (MESHTASTIC_EXCLUDE_MQTT,
MESHTASTIC_EXCLUDE_NEIGHBORINFO, MESHTASTIC_EXCLUDE_STOREFORWARD,
HAS_TELEMETRY). Widen three existing conditions: PAXCOUNTER_CONFIG
now also reports when an ESP32 build sets MESHTASTIC_EXCLUDE_PAXCOUNTER;
BLUETOOTH_CONFIG now also reports when HAS_BLUETOOTH is 0 on nRF52/ESP32;
NETWORK_CONFIG collapses the per-arch nRF52/RP2040 arms into a single
!HAS_NETWORKING check.
Clients read excluded_modules to decide which module config screens to
show. Four bits were never set when the module was compiled out, and
three were set only for a subset of the affected builds, so clients
offered config screens for modules absent from the firmware: MQTT on
every STM32WL target, TELEMETRY on nrf54l15 and minimize builds,
NEIGHBORINFO on russell and several nRF52 RAK boards.
The change is confined to getDeviceMetadata(). The bitmask is advisory:
clients use it to hide menu entries and it does not touch the module
config wire protocol. No PhoneAPI, NodeDB or AdminModule changes.
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>
* fix(metadata): exclude Store & Forward on unsupported architectures
Define MESHTASTIC_EXCLUDE_STOREFORWARD for any build that is neither
ARCH_ESP32 nor ARCH_PORTDUINO.
StoreForwardModule registers only on those two architectures, but the
macro was previously set only by minimize builds and a few variant
flags, so getDeviceMetadata() still advertised STOREFORWARD_CONFIG on
nRF52, RP2040 and STM32WL. Every other use of the macro is already
nested in an ARCH_ESP32/ARCH_PORTDUINO block, so ESP32 and Portduino
builds are unaffected.
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>
---------
Signed-off-by: Andrew Yong <me@ndoo.sg>
* fix(pki): reject a restored pre-2.8 low-entropy key at set time, explain the swap
Restoring/setting a private key is a private-key change: the public key is
*generated* from it. The low-entropy blacklist check in generateCryptoKeyPair
runs against the stored public_key at entry, which is empty on a bare key
restore — so a known pre-2.8 weak key derived from the provided private key was
never caught at set time. It was only detected on the next boot (once the weak
public key had been persisted and re-checked), which looks to the user like
their saved key silently "did not stick", and their node number
(== crc32(public_key)) had quietly changed too.
- NodeDB::generateCryptoKeyPair: in the provided-private-key branch, re-check
the *derived* public key against LOW_ENTROPY_HASHES. If it matches, replace it
with a fresh secure keypair and set keyIsLowEntropy so the reason is surfaced.
- AdminModule set-config(security): when the restore path regenerated a rejected
low-entropy key, send a client warning at set time explaining the key can't be
restored and the node number changed. Scoped to that branch so a stale flag
from a boot-time regeneration can't fire on unrelated security sets.
No protobuf changes; reuses the existing ClientNotification warning path.
Signed-off-by: Garth Vander Houwen <garthvh@yahoo.com>
* fix(pki): gate low-entropy restore warning on successful keygen
generateCryptoKeyPair returns false on an unset LoRa region before
resetting keyIsLowEntropy, so the set-time warning could fire on a stale
flag. Capture the return value and require both.
Shorten the rationale comments to two lines each.
* fix(pki): clear key sizes when a restored private key derives nothing
The provided-private-key branch sets private_key.size and public_key.size
to 32 before regeneratePublicKey() runs. On failure it returned false with
both sizes still set, and AdminModule persisted that pair; every later
keygen then re-derived from the same dead key. Clear both on the failure
path so the next keygen mints a fresh identity.
Add test_admin_radio coverage for the set-time restore path: a derived
low-entropy key warns and rotates, a stale keyIsLowEntropy flag with
keygen blocked does not warn, and a failed derivation clears both sizes.
* fix(pki): validate a restored public key that is itself blacklisted
A restore supplying both private_key and public_key reached neither keygen
branch, so a whole pre-2.8 low-entropy pair was accepted and persisted at
set time and only caught on the next boot. Re-derive when the supplied
public key is blacklisted, which routes it through the same rejection and
warning as the bare-private-key restore. A non-blacklisted keypair import
is unaffected.
Install the test crypto stub through a helper and drop it in
restoreAdminRadioGlobals(), so a failed assertion's longjmp cannot leak a
freed engine into later tests.
* fix(pki): only warn about a swapped key when one was actually swapped
keyIsLowEntropy is set from the stored public key at function entry, so a
restore whose supplied public key is blacklisted set it even when keygen
merely re-derived the public key from a private key that was kept. The
warning then claimed a new key had been generated and the node number
changed, which was only half true. Gate it on the private key actually
being replaced.
* fix(pki): re-check a freshly minted keypair against the blacklist
Both mint sites called crypto->generateKeyPair() once and trusted the
result, so an entropy source still producing known-weak keys could persist
another blacklisted identity. Route both through a helper that re-checks
and retries a bounded number of times, then logs if it cannot do better.
Pass the caller's own copy of the private key to generateCryptoKeyPair()
instead of config.security.private_key.bytes, which aliased the memcpy
destination inside it.
* fix(pki): fail keygen when every replacement stays blacklisted
generateBlacklistCheckedKeyPair() logged an error after exhausting its
retries but left the compromised keypair in place and its callers marked
the keygen successful, persisting exactly the identity the check exists to
reject. Return a flag, clear both key sizes on exhaustion, and abort both
callers so the next keygen starts clean.
Match the declaration guard to the definition's, and derive the expected
mint count in the retry test from the configured one.
* refactor(pki): drop the keygen retry loop, fail on the first weak mint
Retrying cannot help: an entropy source that lands on one of the twelve
blacklisted keys is broken, and a second call to it produces the same
result. With real entropy the odds are ~2^-250, so the loop never runs
twice in practice either. Check once and fail, which is the same guarantee
in a third of the code.
* fix(pki): check the derived key on the stored-private-key path too
factory_reset_config keeps the private key and clears the public one, so
the entry check sees no stored key, reports "not low entropy" and takes the
regenerate branch, which adopted whatever it derived. A preserved pre-2.8
key was therefore accepted for a whole boot cycle before the next boot
caught it - the same silent revert this PR exists to remove.
Hoist the post-derive blacklist check into a helper and use it on both
derive paths.
* fix(pki): clear key sizes when stored-private derivation fails too
The stored-private-key path set public_key.size to 32 up front and left it
there when regeneratePublicKey() failed, so config claimed a pair the node
never got - the same defect already fixed on the provided-key path.
Both paths now derive through one helper that clears on failure and vets
the derived key, replacing the separate blacklist-replace helper.
---------
Signed-off-by: Garth Vander Houwen <garthvh@yahoo.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
* Fix architecture name for Seeed Wio Tracker L2
* Normalize custom_meshtastic_architecture against the board MCU
The declared value reached the manifest unchecked, which is how esp32s3 shipped
here and in the -tft env that extends it. infer_architecture() already derives
the canonical spelling from the board MCU, so prefer it when the two disagree
and print the override.
Scanned all 113 envs declaring an architecture; this variant was the only
mismatch.
* Simplify the architecture override
---------
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
* fix(stm32wl): advertise canShutdown if HAS_LSE
Define HAS_CPU_SHUTDOWN on HAS_LSE STM32WL builds and, on that path,
report canShutdown in getDeviceMetadata() from the runtime
stm32wlRtcAvailable() check.
canShutdown was always false on STM32WL: HAS_CPU_SHUTDOWN was never set
for the architecture and pmu_found is never set there, so apps hid the
shutdown control even though deep-sleep shutdown works on HAS_LSE builds
via cpuDeepSleep() -> STM32LowPower::shutdown(). Reading the runtime
check keeps the report accurate when the LSE crystal fails to lock,
where cpuDeepSleep() resets instead of sleeping.
The #else branch is untouched, so non-STM32WL and non-HAS_LSE builds
report canShutdown exactly as before.
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>
* fix(stm32wl): reject HAS_CPU_SHUTDOWN without HAS_LSE
Add an #error in architecture.h when an STM32WL build has
HAS_CPU_SHUTDOWN set but HAS_LSE unset.
getDeviceMetadata() takes the stm32wlRtcAvailable() branch under
HAS_CPU_SHUTDOWN, but that function is compiled only under HAS_LSE, so a
build forcing HAS_CPU_SHUTDOWN=1 with HAS_LSE=0 would reference it with
no declaration or definition. No current variant does this, and
architecture.h derives HAS_CPU_SHUTDOWN from HAS_LSE in the same block,
so ordinary builds are unaffected.
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>
---------
Signed-off-by: Andrew Yong <me@ndoo.sg>
* feat: log heap watermark, largest free block and subsystem breakdown
The periodic heap line reported only free/total, which cannot distinguish a
leak from fragmentation, and the MemAudit per-subsystem breakdown was only
ever printed at boot.
Add ESP.getMinFreeHeap()/getMaxAllocHeap() wrappers to MemGet (0 on platforms
that cannot report them) and include both in the 5-minute line, then log the
MemAudit breakdown on the same tick. A falling watermark is a leak; a steady
watermark with a shrinking largest block is fragmentation, and the breakdown
names the tagged subsystem that moved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E9ZacpGtsA6DWavqr5Ty2i
* docs: correct the watermark interpretation in logHeapUsage comment
A single step down in the minimum-free watermark is a transient allocation,
not proof of a leak; it takes repeated new lows across samples.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E9ZacpGtsA6DWavqr5Ty2i
---------
Co-authored-by: Claude <noreply@anthropic.com>
Increase the cache-control age limits for R2 uploads.
Releases: Cache for 1 day in browser, 1 month on CDN
Nightly: Cache for 1 hour in browser, 1 day on CDN
Add metadata to R2 uploads so we can track which commit and which GitHub Actions run produced the upload.
* ci(test): shard the native test suite across a matrix
Replace the single sequential runner with a matrix populated by
bin/test-shards.py from the test/ tree: areas over --max-suites are
split, smaller ones packed, and a --max-shards budget bounds the
fan-out. A collector job merges the per-shard JUnit reports, checks the
union against the canonical suite set, and states the verdict. Native
PlatformIO Tests remains as the single required check over the matrix.
Drop the --without-testing warm build. PlatformIO links every native
test program to the same $BUILD_DIR/$PROGNAME, so the area run relinked
each suite regardless. ccache carries the shared src objects between
shards instead; one shard is flagged cache_writer so a single entry is
saved.
The coverage-event-policy and coverage-channel-table envs and the
attribution canary move into their own matrix rows and job.
Harden the new paths: bound the matrix row count so a branch cannot size
the fan-out, reject multi-line or empty $GITHUB_OUTPUT values, fail the
whole-run attribution gate on an empty expected set, upload exact report
and tracefile names instead of globs, and pass the repo path to
bin/lib/shuffle.sh as an argument rather than into bash -c source text.
12 shards, largest 9 suites.
* ci(test): minimal test toolchain, cap shard runtime, fix pack overflow
Add .github/actions/setup-native-test, used by the shard and canary jobs
in place of setup-native. It drops the redundant second checkout, both
submodules (src/mesh/generated is tracked, meshtestic is the hardware
harness), cppcheck, and the adafruit-nrfutil, poetry and meshtastic pip
installs, and folds in ccache and lcov. setup-base and setup-native are
unchanged, so the firmware matrix and every other consumer keep theirs.
Cap the shard job at 30 minutes. A lost runner held one for 48 of the
360 GitHub allows by default, and there are twelve of them.
pack() could exceed --max-suites: ceil(total / cap) is a lower bound and
whole areas do not divide, so three areas of 6 at cap 10 put 12 in one
of two bins. Grow the bin count until every bin fits.
Validate the fixed-env test_filter tokens against SUITE_RE. PlatformIO
accepts globs there, and those tokens reach the same word-split and the
same attribution gate as discovered names. Split with read -ra so a
token cannot glob against the workspace either.
Report the suite count rather than the length of the -f argument array,
which counted every name twice.
Trim comments to the one or two lines AGENTS.md asks for.
* ci(test): quote the $GITHUB_OUTPUT redirects
Applied to all five, including the three that predate this branch, so the
file is consistent rather than half-converted.
boards/seeed-sensecap-indicator.json was the only board file carrying
"f_boot": "120000000L". Under platformio/espressif32 6.x that key only
selected a prebuilt bootloader image. Under pioarduino HybridCompile,
which this board uses since it moved to the 3.3.11-based core (#11238),
f_boot becomes the compile-time clock for both flash and PSRAM, so every
2.8 build of the Indicator is compiled with CONFIG_ESPTOOLPY_FLASHFREQ_120M,
CONFIG_SPI_FLASH_HPM_ON and CONFIG_SPIRAM_SPEED_120M (octal PSRAM at
120 MHz is an experimental ESP-IDF feature). The device hangs in early
flash/PSRAM init before the boot watchdog is disarmed and reset-loops
with RTCWDT_RTC_RST and no bootloader output, also after a full erase
and install.
Without the key the build falls back to f_flash (80 MHz) like every
other ESP32-S3 board, reports "80MHz for both Flash and PSRAM", and
produces a bootloader byte-identical to the T-Deck's 2.8 bootloader.
Fixes#11691
- In enter_dfu, arm enterDfuAtMsec = millis() + 5s and return instead of
resetting inline; the want_response ACK then goes out the normal path
and Power::powerCommandsCheck() calls enterDfuMode() at the deadline.
Nudge the deadline off 0 in the rare case the addition wraps to it,
since powerCommandsCheck() reads 0 as unarmed. The delay is the
client's detach window - and the margin a WebSerial web flasher needs
(meshtastic/web-flasher#426).
- In enterDfuMode(), stop the GPS and drain/end every configured UART
before the reset. The ROM bootloader autobauds off the first byte on
USART1 (PB6/PB7) or USART2 (PA2/PA3), and on every WL variant a
console UART or the GPS stream sits on those pins. Factor the drain
into quiesceSerial() and reuse it in cpuDeepSleep().
- Move earlyBootCheck from constructor(101) to .preinit_array, ahead of
the core's premain()/SystemClock_Config() whatever the link order, and
reset RCC before jumping to system memory.
The handler used to reset the MCU inline, before the ACK was sent and
while the client still held the console UART. The STM32WL ROM bootloader
autobauds off the first byte received; a stray byte during the handoff
(a trailing protobuf frame, a port-close DTR/RTS glitch) desynced it and
left the device unreachable at any baud until a hard reset.
STM32WL only: every hunk is behind #if defined(ARCH_STM32) or lives in
main-stm32wl.cpp. nrf52, rp2040 and the rest are unchanged.
Known limitation: gps->disable() only issues a UBX sleep command, so a
non-u-blox or otherwise free-running GPS with no hardware enable/standby
pin keeps transmitting on its UART past this point. If that UART is
USART1 (PB6/PB7) or USART2 (PA2/PA3), the ROM bootloader can still
autobaud onto the GPS stream instead of the host. New STM32WL hardware
designs should keep GPS UARTs off those two bootloader-autobaud pins, or
provide a way to power down or hold the GPS in reset before DFU.
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>
vidplace7