* fix(portduino): detach the CH341 poll thread when it detaches its own interrupt on Windows
* fix(portduino): stop a superseded CH341 poll thread on Windows
* fix(portduino): wait for CH341 poll threads before deinit closes the device
* fix(portduino): stop a superseded CH341 poll thread before it rewrites pin state
* fix(portduino): keep a same-thread re-arm's CH341 pin state sentinel intact
* fix(portduino): close the CH341 attach/deinit race and recognize a superseded poll thread
* style(portduino): trim the new CH341 comments to the two-line house limit
* fix(gps): wake AG3335 from software RTC sleep
On the Airoha trackers, GPS_HARDSLEEP simply dropped PIN_GPS_EN. Cutting VCC
with no prior command leaves the receiver in hardware RTC mode, from which
nothing in the firmware ever brings it back: the only Airoha wake sequence
that existed, wakeAirohaForActiveProbe(), is reachable from probe() alone and
never from the normal GPS_HARDSLEEP -> GPS_ACTIVE transition. The tracker
stops producing fixes until it is rebooted.
Park the receiver in software RTC mode with $PAIR650,0 before the power cut,
and pulse GPS_RTC_INT after VCC comes back to bring it out again. The receiver
may already have auto-slept and missed the first command, so it is resent
until it acks, bounded at 400 ms rather than repeated a fixed number of times:
setPowerState() runs on the GPS thread and from the notifyDeepSleep observer,
and every mesh thread shares loopTask on nRF52, so an unconditional wait here
stalls LoRa servicing, the screen and buttons for its full duration on every
sleep cycle, on top of holding the receiver powered that much longer.
The RTC_INT pulse becomes a shared helper so the probe path and the power
state machine no longer carry separate copies, and the wake is guarded on
GPS_RTC_INT as well as GNSS_AIROHA, since the family flag is not a promise
that the board routed that line.
Also drops the raw digitalWrite(PIN_GPS_EN, LOW) calls that followed
writePinEN(false) in GPS_HARDSLEEP and GPS_OFF, and the one in
toggleGpsMode(). writePinEN() already drives the pin through the GpioVirtPin
chain built in createGps(); the raw writes duplicated it while bypassing both
that abstraction and the RAK4631/WISMESH_TAP guard inside writePinEN().
Affects tracker-t1000-e, seeed_mesh_tracker_X1 and wio-t1000-s.
Co-Authored-By: WayenWeng <jinyuan.weng@seeed.cc>
* fix(gps): keep the $PAIR650 retry inside its stated budget
The while-condition was evaluated after getACK, so a final attempt starting
just under the budget could add another ack window on top of it. Stop starting
attempts once a whole window no longer fits, which makes 400 ms a real ceiling
rather than a soft one.
* fix(gps): gate the AG3335 park on a wake path, the probed model, and a miss count
Review found three holes in the soft-RTC park, all from review by Thomas
Goettgens.
The sleep was guarded on GNSS_AIROHA while the wake was guarded on
GNSS_AIROHA && GPS_RTC_INT, so a board that did not route RTC_INT would have
been parked with no way back out - worse than the bare power cut this is
meant to fix. Both now derive from a single HAS_AIROHA_SOFT_RTC, so they
cannot be guarded separately again.
The retry stamped 'start' from raw millis() but tested it with
Throttle::isWithinTimespanMs, which reads Time::getMillis(). Under
Time::setTestMillis() the two diverge and the loop either falls through or
never exits; getACK just below already uses the injectable clock.
The park was gated only at compile time, so a probe that fell back to
GENERIC_NMEA would still be sent PAIR650 and block for the full budget every
cycle. It now checks the probed model the way the constellation setup at
L975 does, and gives up after three consecutive unacked sleeps so a receiver
that is present but wedged cannot stall loopTask indefinitely.
* fix(gps): drive the Airoha probe wake from the capability, not the board
Requested by Manuel Verch. wakeAirohaForActiveProbe() asserted EN and pulsed
RTC_INT only under TRACKER_T1000_E, so seeed_mesh_tracker_X1 and wio-t1000-s
got a bare $PAIR382 during probe even though both route the same pins. Keying
it on HAS_AIROHA_SOFT_RTC gives every board that routed RTC_INT the physical
wake, which is what the probe's own hardware reset needs undone, and lets a
future variant opt in by declaring the pins rather than by name.
The 1000 ms $PAIR382 repeat loop goes with it: it was compensating for the
pulse being absent at this point, so a single command after the pulse is
enough, and T1000-E boots a second sooner.
A board that declares GNSS_AIROHA without routing RTC_INT keeps the bare
command. Waking and parking must match, per the previous commit, but probing
only reads, so it cannot strand the receiver the way the park can. The EN
write is guarded on PIN_GPS_EN the way createGps() already guards its own.
---------
Co-authored-by: WayenWeng <jinyuan.weng@seeed.cc>
* feat(nodedb): greet only while the node store is under half full
The ad-hoc greeting in MeshService::handleFromRadio() was gated on
!isFull(), so a node kept sending unsolicited NodeInfo right up to the
last free slot - on a dense mesh that is the regime where the store is
already churning and the greeting is least likely to buy a lasting
entry.
Add NodeDB::isHalfEmpty(), true only when strictly more than half the
slots are free, and gate the greeting on it instead. The comparison is
written as 2 * numMeshNodes < cap so a half-full store reads false with
no integer rounding, and MAX_NUM_NODES is read into a local because
portduino resolves it through a runtime call.
The helper keeps the MINIMUM_SAFE_FREE_HEAP term that !isFull() used to
contribute: low heap disqualifies the store regardless of occupancy, so
a sparse database on a memory-starved device still does not transmit.
Admission is untouched - updateFrom() and getOrCreateMeshNode() still
fill to capacity. Only greeting stops early.
* fix(nodeinfo): raise the minimum greeting window to 30 minutes
The !shorterTimeout branch of NodeInfoModule::allocReply() used a
10-minute base, so a node that had just greeted one neighbour could
greet the next ten minutes later. Raise the base to 30 minutes.
This is the floor, not the window: getConfiguredOrDefaultMsScaled()
still multiplies by the congestion coefficient for the roles that scale,
so a busy mesh stretches it further. ROUTER/ROUTER_LATE and the
tracker/sensor roles bypass the scaling and get a flat 30 minutes.
The interactive paths are unaffected - they pass shorterTimeout and keep
their own 60-second gate. The periodic broadcast is unaffected too:
default_node_info_broadcast_secs is 3 hours with a 1-hour minimum, both
clear of the new floor, so the timer is not swallowed by the throttle.
* fix(nodeinfo): a send restarts the routine broadcast countdown
sendOurNodeInfo() left the OSThread schedule alone, so an ad-hoc send
had no effect on the periodic broadcast: run() anchors the next run at
runned() + interval, and nothing re-anchored it when the send came from
a greeting, a PKI decrypt failure or a completed key verification. The
routine copy could follow minutes behind an ad-hoc one, putting two
NodeInfos on the air for no gain.
Call setIntervalFromNow() with the configured broadcast interval once
the packet is queued, so the next periodic copy is a full interval from
the send rather than from the last tick.
It sits on the return-true path only: a send vetoed by allocReply() -
throttle, airtime ceiling, reply suppression - must not be able to
silence the routine broadcast. Calling it from inside runOnce() is
harmless, since run() then applies the same interval from a last_run of
effectively now.
* test(nodeinfo): cover the send window, the countdown reset and the greeting gate
Three behaviours from this branch had no coverage: isHalfEmpty()'s exclusive
boundary, the 30-minute send floor, and the countdown reset on a send.
isHalfEmpty() goes to test_nodedb_blocked, which already owns the full-store
cases and clears the hot store per test. Three tests sweep the cap over the
sizes real deployments have - portduino resolves MAX_NUM_NODES from
General.MaxNodes on every read, so a predicate that cached it would greet at the
wrong occupancy - and pin the band where admission outlives greeting. That suite
had no tearDown; it has one now, restoring the cap so an assertion firing
mid-sweep cannot leak a 2-node cap into the tests after it.
test_nodeinfo_send_window is new because nothing in the tree stands up
NodeInfoModule's send path. Six tests: the floor at 30 minutes with 10 refused,
the interactive 60-second gate staying separate, the countdown re-armed by a
broadcast and by an ad-hoc unicast, left alone by a refused send, and a preset
change consumed only by a send that goes out.
The scaling above 40 online nodes is deliberately not retested here -
getConfiguredOrDefaultMsScaled() is test_default's contract, per preset and per
role. These tests pin the base and leave the multiplier alone.
NodeInfoModule gains two PIO_UNIT_TESTING accessors for the countdown:
concurrency::OSThread is a private base, so a test shim cannot reach it and only
the class itself can. They compile out of a shipping build.
The heap term in isHalfEmpty()/isFull() stays uncovered: memGet.getFreeHeap()
returns UINT32_MAX on portduino, so a native test could only pin a stub.
* chore(trunk): exempt test_nodedb_blocked from the trufflehog Lob detector
test_removeNodeByNum_presentNodeOnFullDb is exactly 35 characters after the
test_ prefix, which is the length of a Lob API key, and trufflehog's detector
matches the bare identifier. The name is years old; it surfaces now only because
this branch touches the file, and the pre-push gate reports a finding in a
changed file as new.
Added to the ignore block that already carries the same detector's hex-literal
false positives, with the reason stated alongside them. Nothing in that file is
a credential.
* fix(nodeinfo): exempt a licensed station from the floor, delay only on a real send
Two review findings on the 30-minute window.
Ham mode sets node_info_broadcast_secs to 600 s for the FCC minimum call-sign
announcement (AdminModule.cpp). The new floor refused every one of those sends
until 30 minutes had passed, so a licensed station's call sign went out three
times less often than the regulation asks - a regression the old 10-minute base
did not have. A licensed station now keeps its own interval whenever that is
shorter than the floor. The exemption is exactly the licensed case because
nothing else can get under the floor: a set-config clamps the field to an hour,
and the userprefs path clamps identically.
sendOurNodeInfo() ignored what sendToMesh() returned, so a packet the router
declined - no interface, queue full - still re-armed the routine broadcast and
still reported success, which let runOnce() consume a pending channel change
for a send that never reached the air. Only ERRNO_OK and ERRNO_SHOULD_RELEASE
now count; sendToMesh() has already released the packet in both cases.
Both are pinned by tests that fail without them, measured: the licensed case
fails at "11 min is past it, and the floor must not override it", the declined
send at "a declined send is not a send". The licensed test carries an unlicensed
control on the same configuration, so deleting the floor outright would not
satisfy it.
* test(nodeinfo): assert the deadline the scheduler reads, from an aged last_run
The countdown cases asserted Thread::interval, which is not what schedules the
next run: shouldRun() keys off _cached_next_run, and the two ways of writing it
differ. setIntervalFromNow() recomputes it from now; Thread::setInterval()
recomputes it from last_run. Swap the call in sendOurNodeInfo() for the latter
and the period still reads three hours while the deadline lands wherever the
last tick was - firing the routine copy right behind an ad-hoc send, the exact
thing the reset exists to prevent. Every test passed.
Assert the deadline instead, from a fixture where the two answers are
distinguishable: ageLastRunForTests() calls Thread::runned() with an hour-old
timestamp, the state a periodic thread is genuinely in between runs, so a
deadline off last_run lands an hour early against a five second tolerance.
Measured: with setInterval() in place of setIntervalFromNow(), the new case
fails by 3600004 ms and the eight others pass, including the one asserting the
period - which is what says the old assertion could not see this.
runned() and _cached_next_run are protected in Thread and OSThread is a private
base, so the hooks live on NodeInfoModule, with the two already there.
Raised by Copilot on #11897.
* fix(nodeinfo): a declined send must not start the throttle window either
allocReply() stamped TransmitHistory when it built the packet, before anything
had been sent. The previous commit made sendOurNodeInfo() report a router
rejection instead of swallowing it, but the stamp was already written by then,
so a packet that never reached the air still started the window - and with the
floor now at 30 minutes, that silences the node for half an hour over a send
that failed.
allocReply() has two callers and only one of them can see the outcome: the
module framework sends its own reply through currentReply, with no post-send
hook a module can reach (MeshModule::sendResponse is not virtual). So the stamp
stays there for that path, and sendOurNodeInfo() defers it across its own
allocReply() call and stamps once the router has accepted the packet.
deferHistoryStamp mirrors the shorterTimeout member alongside it - same
call-scoped signal, same lifetime.
test_sendWindow_aRejectedSendDoesNotStartTheWindow asserts both halves: no stamp
after the rejection, and the retry immediately after goes out. The existing
rejected-send case checked the first failure and the countdown only, which is
how this survived it.
248/248 across every suite that touches NodeInfoModule (admin_session_repro,
admin_radio, nodeinfo_send_window, traffic_management, fuzz_packets) plus
transmit_history, whose subject this is.
Raised by CodeRabbit on #11897.
* fix(router): relay a PKI unicast with a known party in LOCAL_ONLY and KNOWN_ONLY
A frame the relay cannot decrypt takes the OPAQUE_RELAY_ONLY path in
Router::perhapsHandleReceived() and returns before handleReceived(), so it never
reaches a module. The rebroadcast_mode rule for opaque traffic is therefore the
IS_ONE_OF list in relayOpaquePacket(), and LOCAL_ONLY and KNOWN_ONLY were not on
it: a node in either mode dropped every opaque frame, including a PKI unicast
with a party it knows. The gate in RoutingModule::handleReceivedProtobuf() that
used to allow exactly that is unreachable for these packets and no longer
decides anything.
Add both modes to the list, with the identity rule the unreachable gate carried:
a PKI-shaped unicast (channel 0, not broadcast) with `from` or `to` known to us.
An unreadable broadcast and a unicast between two strangers stay dropped in
these modes, which is what the proto documents - they ignore foreign meshes.
This is not only direct messages. Remote administration and key verification are
PKI unicasts too, and a KNOWN_ONLY relay was black-holing those between two
other nodes just the same.
CORE_PORTNUMS_ONLY reached this list the same way in #11843; this is the
remaining pair.
* test(rebroadcast_mode): pin the relay decision per mode, in both directions
What this node carries for other nodes, per DeviceConfig.rebroadcast_mode, for
packets it can read and packets it cannot. The harness pushes a real PKI frame
through Router::perhapsHandleReceived() and counts what reaches the radio.
Both directions are load-bearing, and each is guarded by cases the other leaves
green. Measured by rebuilding the firmware three ways:
- as shipped: 9/9 pass.
- with the two modes taken back out of relayOpaquePacket(): the known-party
and remote-admin cases fail, the stranger and foreign-mesh cases still pass.
- with the modes listed but the identity qualifier deleted: the stranger and
foreign-mesh cases fail, the known-party cases still pass.
So a revert and an over-broadening each fail their own tests, and neither can be
satisfied by breaking the other.
The suite and its harness come from the opaque-packet-handling branch and
compile against develop unmodified. Two assertions were dropped because they
pin behaviour this branch does not add: delivery of unreadable frames to the
phone, and a queued copy of our own suppressing the originator's repeat, which
needs relayOpaquePacket() to consult the TX queue.
* fix(test): guard the harness include, drop a comment its test outlived
Two review findings on the imported suite.
The harness builds real PKI frames through CryptoEngine entry points a
MESHTASTIC_EXCLUDE_PKI build does not declare, but it was included above the
guard, so the empty-suite branch that exists for those builds could not compile.
Move the include inside the guard and pull the base includes the stub branch
needs above it.
The phone-delivery case was dropped when the suite came across - this branch
does not deliver unreadable frames to the phone - but its comment stayed behind
and now described the test below it, which is about the signature policy.
* test(rebroadcast_mode): make the from-known and channel-0 operands load-bearing
The qualifier has three operands, and the suite only exercised one of them.
Every existing case that a known party carries has a known DESTINATION: the
remote-admin case marks both parties, the known-destination case marks the
target. Rewriting the identity test to consult p->to alone passed all nine. And
the only non-PKI-shaped frame in the suite was a broadcast, which
!isBroadcast(p->to) rejects before p->channel is ever read, so deleting the
channel gate passed all nine too.
Add the two cases that close it: a known SOURCE with a destination we have never
heard of, which must relay in both modes, and a known party on a channel hash we
do not hold, which must not - that is someone else's channel traffic, addressed,
not PKI.
Measured both ways. With the from operand and the channel gate removed from
relayOpaquePacket(), the two new cases fail and the other nine pass; with the
shipping code, 11/11.
Raised by CodeRabbit on #11898.
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* Send an ack over PKC when no channel can carry it
PKI needs only the two keys, so a DM can reach us over a channel we do not
carry. Its ack is a ROUTING packet, which wouldEncryptWithPKC() excludes, so
today it is channel-encoded, fails at setActiveByIndex() with NO_CHANNEL, and is
never sent. The sender sees nothing and retransmits to exhaustion for a message
that was in fact delivered.
Fall back to PKC for exactly that case. This is the one place an ack is
deliberately made opaque to relays; normally that costs next-hop learning and
intermediate retransmission cancel, which is why ROUTING is PKC-excluded in
general, but here there is no readable alternative to lose, because without this
the ack does not exist.
The predicate is scoped as tightly as that argument reaches: a unicast ROUTING
packet we originate, carrying a request_id, to a destination whose key we hold,
under the same ham/sim/private-key preconditions PKC always has, and only when
the channel index does not resolve. It tests channels.getHash() rather than
setActiveByIndex() so it has no side effect; generateHash already returns -1 for
an invalid key, so the two agree on which indexes are unusable.
Four cases in test_packet_signing pin the corners: the fallback fires, it does
not paper over an ack with no destination key, it does not catch a non-ack on
the same unusable channel, and an ack on a channel that does resolve still goes
out readable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8
* Short-circuit the fallback so an out-of-range channel logs no error
wouldEncryptWithPKC() reaches channels.getName(chIndex) before its portnum
exclusion, and getByIndex() logs "Invalid channel index" on the way past. With
the general predicate tested first, an ack on an out-of-range index printed that
error and then went on to encode successfully. Test ackFallback first so the
case that is about to succeed never asks.
Also record why the range check leads inside the predicate: getHash() is a bare
hashes[i] with no bounds test of its own.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8
---------
Co-authored-by: Claude <noreply@anthropic.com>
drawCommonFooter() black-fills the bottom (connection_icon_height + 2) rows
before drawing the API-connected link icon. On colour builds that fill spans the
whole screen width; the monochrome path fills only the icon's own width.
The body grid does not shrink with the panel. textSixthLine is 58 rows down
whatever the display height, while the footer band starts at
SCREEN_HEIGHT - 1 - connection_icon_height. On a 64-row panel that is row 58 --
exactly the sixth body line -- so the bar erases the last thing the frame drew.
On BaseUI that is the sixth body row, the LoRa frame's ChUtil bar (rows 52-59)
and the bottom of the clock. Taller panels put the band clear of the grid and
are unaffected: at 80 rows it starts at 74, and on high-res it is far below.
It only shows once a client is connected, since the function early-returns on
!isAPIConnected(), which is why it reads as the blue link icon eating the
screen rather than as a layout bug.
Only the icon's own rect is registered for colour tinting, so the wide fill buys
the tint nothing. Keep it where it clears the body, and fall back to the
icon-width fill -- what the monochrome path already does -- where it would not.
Panels with room below the body are byte-identical to before.
Seen on a 128x64 HUB75 running meshtasticd; esp32s3/visualizer-hub75 is the
same geometry.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(games): correct the high-score announcement argument order
GAMES_HIGH_SCORE_STRING is "New %s high score %lu by %s!" but the arguments
were passed as (name, initials, score): the initials string was formatted
through %lu and the score integer through %s. That is a format/argument
mismatch, so the announcement printed garbage at best and dereferenced the
score as a pointer at worst.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(input): report which physical gamepad button produced an event
A joystick event only carried the action it was mapped to, so a consumer could
not tell two buttons apart once they shared one action, and games were limited
to the handful of actions the broker defines.
Carry the originating evdev button code in InputEvent::kbchar, encoded into a
reserved 0xC0..0xDF range that misses printable ASCII and every
INPUT_BROKER_MSG_ value (SystemCommands switches on kbchar without looking at
inputEvent, so a collision there would reboot the node rather than move a
paddle). D-pad events are axes, not buttons, and keep leaving kbchar at 0 --
which is exactly what lets a consumer tell stick from button.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(portduino): let one joystick action bind several buttons
Input.JoystickButtons took a single evdev code per action, so a pad's A and Y
could not both select, and the shoulder buttons could not sit alongside the
D-pad. Accept a list of codes as well as a bare scalar; the config writer
inverts its code->action map back out, emitting a list only where an action
has more than one button.
ConfigCheck gains a real checker for the section (it was previously waved
through as free-form) covering the three ways a mapping silently does nothing:
an action name the driver does not know, an evdev name where the numeric code
belongs, and one code claimed by two actions. Two fixtures and shell-test
cases cover the clean list form and those three faults.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(games): use the gamepad's extra buttons, and return home when idle
Games now receive the physical button alongside the action, so a pad with more
than two usable buttons controls more than two things:
- Snake: a shoulder button mapped to left/right turns relative to the snake's
heading (L counter-clockwise, R clockwise) while the D-pad keeps steering
absolutely. The two are told apart by kbchar, not by hardcoding one pad's
codes.
- Breakout: the ball now rides the paddle after each serve until the player
fires it with B or A, so a life is not lost to a ball already in flight when
the player looks up. The paddle also keeps its position between lives. A game
can claim BACK for the duration (Game::wantsBackButton) so B serves instead
of pausing, and releases it once the ball is live.
- Start (BTN_BASE4 / BTN_START) is mapped to select like any other button, so
it launches games and drives the menus; inside a running game GamesModule
picks it out of kbchar and pauses instead.
Separately, the games frame no longer holds a walked-away device hostage: after
15 s with no input it returns to the home frame, so the device still reads as a
Meshtastic node. The timer is suspended while a picker or banner is up (e.g.
high-score initials entry, which the input handler never sees) so it cannot
yank the user out mid-entry.
Screen::isInteractionBusy() generalises the old module-intercept check --
modal module, intercepting module, game, or an open interactive overlay --
and MessageRenderer uses it before popping an incoming-message banner. A
transient banner REPLACES an active overlay, so an arriving message could
otherwise discard a half-entered high score. The message is still stored, its
thread still selected, and the unread indicator still set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(ui): compose freetext on the on-screen keyboard from a gamepad
A gamepad can drive the on-screen keyboard but cannot type, so on a host with
a joystick and no configured keyboard device the OSK is the only way to compose
freetext. Set osk_found there, and gate the "Freetext" menu entries on whether
the device can enter text at all (physical keyboard, OSK, or touchscreen
virtual keyboard) rather than on kb_found alone -- those entries were hidden on
exactly the devices that needed them.
The OSK prompt that CannedMessageModule already had inline in the message
selector becomes showOnScreenKeyboard(), so the menu path can reach it too.
Menus call in from a banner callback and the banner is torn down as soon as
that callback returns, which would take the keyboard down with it, so the menu
path defers the launch to runOnce().
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(games): address review on frame fallback and joystick input gating
Breakout: the paddle suppression was far too broad. aLinuxJoystick is
constructed on every Linux host whether or not a gamepad is configured
(InputBroker.cpp), so `aLinuxJoystick && kbchar == 0` was true everywhere and
swallowed LEFT/RIGHT from the keyboard, trackball and ExpressLRS -- on a host
with no joystick attached at all. Gate on the stick actually driving the paddle
instead: LinuxJoystick assigns heldX before it emits and only auto-repeats while
heldX is set, so every axis LEFT/RIGHT arrives with a zone held and nothing else
does. kbchar == 0 still distinguishes an axis from a shoulder button mapped to
left/right, which must keep nudging the paddle.
Screen: showHomeFrame() did nothing when the home frame was hidden, since
setFrames() only assigns positions.home for !hiddenFrames.home. That stranded
the games inactivity bounce on the frame it was trying to leave. Fall back to
the messages frame, which setFrames() always adds.
Test: rename test_ballWaitsOnPaddleUntilLaunched to
test_ball_waitsOnPaddleUntilLaunched, matching the repo convention and its
neighbours in the file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(games): give games the whole InputEvent so Breakout can identify the source
Follow-up to review on #11917. The previous narrowing still could not tell
sources apart: kbchar == 0 is shared by the joystick's D-pad axis and by every
other driver that sends a bare LEFT/RIGHT, so while the D-pad was held a
keyboard or touchscreen press was still discarded. heldXZone() proves the axis
is driving, not that this particular event came from it.
Pass the event itself to Game::handleInput() rather than (ev, kbchar). Games
that only care about the action read event->inputEvent; Snake keeps using
kbchar for shoulder steering; Breakout now also checks event->source against
LinuxJoystick's origin name, so only that driver's own axis repeats are
suppressed.
Chose the event over a third positional parameter so the signature does not
have to grow again the next time a game needs something the event already
carries.
All three conditions in Breakout are load-bearing: source says it came from
this gamepad, kbchar == 0 says it is the axis rather than a shoulder button
mapped to left/right, and heldXZone() != 0 says the axis is what is driving
right now so tick() already has it covered.
LinuxJoystick::originName() exposes the name the driver stamps into
InputEvent::source, alongside the existing heldXZone()/heldYZone() accessors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Cancel superseded runs of Check PR Labels and Semgrep Differential Scan
Both workflows trigger on pull_request without a concurrency group, so
every push, label or edit on a PR queues a fresh run while the earlier
ones keep their place in the org's shared runner queue. Check PR Labels
listens to six event types, so opening, labelling and editing one PR
queued three identical runs within a minute today.
Give each the same head_ref-keyed group with cancel-in-progress that
CI, Tests and the trunk check already use. Develop pushes are
unaffected: they fall through to run_id, as before.
* Key the new concurrency groups by PR number, not head_ref
Fork PRs often come from a branch named develop, so two of them share
github.head_ref and one would cancel the other's run. The PR number is
unique per PR.
custom_meshtastic_images pointed at heltec-mesh-node-t1.svg, which does
not exist in web-flasher. The file there is heltec-meshnode-t1.svg. The
wrong path returns HTTP 200 with a 7 KB HTML error page rather than the
203 KB SVG, so it does not surface as a 404.
HWCDC::isPlugged() is a SOF watchdog that reads false transiently while USB
is connected and working. runOnce() answered that with a 20 s sleep, and
nothing wakes the thread on RX, so host traffic sat in the CDC RX ring and
reached the API as a burst. Cap the sleep at 250 ms, the rate readStream()
already idles at.
IS_USB_SERIAL only tested ARDUINO_USB_CDC_ON_BOOT, so ARDUINO_USB_MODE=0
boards ran the same check against a USB-Serial/JTAG peripheral that is not
attached to the PHY and never sees a SOF. Gate the check on IS_USB_HWCDC.
Measured on tlora-t3s3-v1, 900 s of 1 Hz ToRadio/FromRadio round trips:
before 12 stalls, rtt_max 19.96 s, console asleep 26.4% of wall time.
After 0 stalls, rtt_max 0.147 s, p50 unchanged at 0.028 s.
Fixes#11864
The SFA30 probe only compared the requestFrom() length, which equals the
requested length for any device that ACKs, so an LPS33HW/LPS35HW at 0x5D
was reported as SFA30. Probe WHO_AM_I first so ST sensors never receive
the SFA30 command, and require valid Sensirion CRC-8 on every word of the
device marking response.
Fixes#11880
* fix(mqtt): rewrite the region root topic only on the default broker
A region change rewrote any root starting with "msh", on any broker. Custom roots such as msh/home were clobbered, and private brokers had their topics moved even though a regional broker is regional already. An empty root, which MQTT treats as the default, was never updated.
Region changes now go through MQTT::applyRegionRootTopic(), which rewrites the root only on the default broker and only when the root is empty, the default, or a msh/<region> the firmware wrote itself.
* fix(mqtt): parse the broker address before the default-server check
Persist module config only when the root actually changed. Replace a stray NUL byte in the test with the \0 escape.
* fix(mqtt): count the regional roots as the default root topic
meshtastic/protobufs#1045: admin.proto's AS3935_config and
telemetry.proto's AS3935Config collide after name mangling. The
flash-persisted message is renamed to AS3935State upstream.
Field numbers are unchanged, so existing /prefs/as3935.dat files still
decode.
Depends on the protobufs rename and the regenerated sources landing
first.
Co-authored-by: Claude <noreply@anthropic.com>
* fix(api): retain the unwritten tail on a short TCP API write
ServerAPI closed the session whenever stream->write() returned fewer bytes than
requested. A short write is transmit-buffer backpressure, not a dead socket, and
it is most likely during the back-to-back frames of the initial NodeDB dump, so
a node at its node cap dropped clients on effectively every connect.
Route TCP frames through StreamFrameWriter, the retained-tail path the USB CDC
console already uses: the remainder is re-offered on the next pass and the
session is closed only when the link itself is gone. Poll at 25ms while output
is still undelivered, since nothing wakes the thread when the socket frees
transmit space.
Fixes#11822
* fix(api): block log re-encoding while a TCP frame is retained
emitLogRecord() writes into txBufLog and StreamFrameWriter can now hold that
buffer as a retained tail, so a second log record would overwrite bytes the
transport has not sent yet. Gate encoding on the retained-frame state, matching
SerialConsole.
No caller reaches this today (emitLogRecord() is only used by SerialConsole),
but retaining the buffer at all is new here.
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* fix(nrf52): serialise the warm-node ring against LittleFS on the shared flash cache
On nRF52840 the warm-node store writes its 3-page record ring straight through
flash_nrf5x_write/erase/flush, holding only spiLock. Every LittleFS writer instead
holds Adafruit_LittleFS's own mutex, and two of them run on other tasks entirely:
Bluefruit's bond saves on the callback task, and - since phone config writes moved
into BLE context - a whole saveToDisk on the BLE task. Neither takes spiLock.
Both writers share one 4 KB page cache, one SoftDevice flash semaphore and one
result word. flash_cache_write repoints that cache when the requested page differs
from the cached one, so a second writer arriving mid-write flushes the first
writer's page and re-points the buffer; the first writer's remaining memcpy then
lands in the wrong page's image. Ring records end up inside LittleFS metadata, or
the reverse. The collision also exhausts the flash layer's 20 x 1 ms busy-retry
budget against an 85 ms page erase, and flash_cache_flush discards the failure, so
32 LittleFS blocks vanish with no error reaching the filesystem. What the user sees
is a torn directory pair on the next mount, a format, and critical error 13.
Take the filesystem mutex in the five ring entry points that reach flash, after
spiLock and never before - the order every existing path already uses. The ring
touches no LittleFS call itself, so the non-recursive mutex is never re-entered.
Longest new hold is a page rotation at roughly half a second, against a 2 s
supervision timeout and a 90 s watchdog. Non-nRF52840 backends are untouched.
* fix(nodedb): make saveProto report a failed readback or rename
SafeFile::close() already verifies the .tmp by hash and renames it over the
live file, and saveProto captured that result, logged it, and then returned
the pb_encode status alone. A torn or half-programmed page therefore counted
as a successful save: for the fullAtomic files the old contents silently
survived, for nodes.proto (written in place) the file was simply gone, and
saveToDisk's recovery path never fired for the one failure it exists for.
* fix(nodedb): retry a failed save before formatting, and never format on a low rail
saveToDisk answered any failed write with an immediate fsFormat(), which is
where most "critical error 12/13" reports and the total config wipe behind
them come from. A write that fails once is far more often a busy SoftDevice
or a VDD dip mid-save than a corrupt filesystem, so:
- retry twice, 150 ms apart, re-checking powerHAL_isPowerLevelSafe() before
each attempt and before the format; on a low rail return false and leave
the filesystem alone (the next save lands once the rail recovers, and boot
already waits for a safe level)
- check fsFormat()'s result instead of assuming it worked
- after a successful format rewrite every segment, not only the ones this
call asked for: the format took config.proto and the node identity with it,
so a nodes-only save that ended in a format used to come back up as a new
node
- with encrypted storage a format also destroys the DEK; skip the resave
rather than land the private key and PSKs on flash in plaintext
RP2040 feeds its watchdog across the delays, as the neighbouring code does.
* fix(nrf52): quiesce flash before every software reset and power-off
The Adafruit flash layer keeps one 4 KB page image and one SoftDevice flash
semaphore for the whole chip. Every reset path we own - Power::reboot(),
enterDfuMode() (admin enter_dfu_mode_request, which arrives on the BLE task
since #10967), cpuDeepSleep()'s reset and system-off arms, and the
wio-t1000-s secure DFU handler - went straight to NVIC_SystemReset or
sd_power_system_off while another task could be half-way through a page
program or erase. A reset in that window leaves the page erased or partly
programmed; LittleFS finds the torn metadata on the next mount and the
corruption handler formats the filesystem.
nrf52FlashQuiesce() takes spiLock and the LittleFS mutex, waits out whatever
write is in flight, flushes the page cache, and keeps both locks because the
caller resets next. The corruption-reboot handler and __assert_func are left
alone: they run inside the filesystem call stack or a fault, where taking the
mutex would deadlock.
nRF54L is a second copy of these paths since #11867 and still defines
ARCH_NRF52, so it gets the same function on the same core flash layer.
* fix(nrf52): quiesce flash before the library BLE DFU handler jumps to the bootloader
On every board except wio-t1000-s the Nordic DFU service is the framework's
BLEDfu, whose START_DFU handler runs on the callback task and jumps to the
bootloader with no regard for a flash write in progress on the loop task.
That is the OTA path the Apple app and nRF Connect use (Android sends
enter_dfu_mode_request instead, which the previous commit covers).
QuiescingBLEDfu re-installs the control-point write callback after
BLEDfu::begin() and wraps the library's: flush under both locks, then drop
the LittleFS mutex before handing over, because the library reloads the bond
keys through LittleFS on its way to the jump and the mutex is not recursive.
spiLock stays held across the handler: every LittleFS writer on the BLE task
takes it first, the loop task cannot preempt the callback task, and the
handler never blocks after the flush, so nothing can dirty flash before
bootloader_util_app_start(). If the handler returns, nothing jumped, and the
lock is released.
The library callback is a file-static, so it is read back out of the
characteristic through a pointer-to-member obtained via a using-declaration;
that is well-formed C++ and compiles under the pinned GCC 9.3 with LTO.
* test(nodedb): pin the save-failure contract of saveProto and saveToDisk
A failed rename must come back as false from saveProto, a one-off unsafe
rail reading during a write must be retried and land, and a rail still
unsafe at the retry gate must make saveToDisk return false with the
filesystem untouched. The rail is scripted through a strong
powerHAL_isPowerLevelSafe() over the weak native default; on Windows the
default is strong, so only the rename case runs there. The format branch
itself is unreachable natively (a FLASH_CORRUPTION critical error exits
the portduino process), which is what the survival assertions pin.
* fix(nodedb): only format when the filesystem itself is unreadable
Making saveProto honest about write failures gave the recovery path a new way in:
any persistent write failure now reached fsFormat(), which takes every file with
it. A busy or lock-protected nRF52 flash fails every write for as long as it lasts,
so two retries are not enough to tell that apart from a corrupt filesystem, and
guessing wrong costs the node its config, keys and bonds.
Reads settle it. They never touch the SoftDevice write path that a busy flash
fails on, so if /prefs still walks and a stored proto still opens and reads, the
metadata chain is intact and the write failure was transient - return false and
let the caller try again later. Genuine corruption is not silently tolerated: lfs
asserts on it, and the nRF52 handler reboots and formats on the way back up.
Covered by a test that fails without this: a save whose rename cannot succeed,
against an otherwise healthy filesystem, must leave devicestate untouched.
* trunk: exempt Unity test entry points from trufflehog
trufflehog's Lob detector matches "test_" followed by alphanumerics, which
describes every Unity test function name. It fired on a new test in
test_nodedb_save_retry and will fire again on the next suite added. Scoped to
test/**/test_main.cpp, alongside the existing gitleaks exemption for the
synthetic node-DB fixtures.
* fix(nodedb): feed the RP2040 watchdog around the format and the resave
saveToDisk() only feeds the watchdog at the top of each retry. The last
retry, the readable probe, fsFormat() and the five-segment resave then
share one 8 s budget (watchdog_enable in main-rp2xx0.cpp) with no loop
left to feed it. A timeout during the resave leaves the filesystem empty
and the node boots on defaults with a new identity - the exact outcome
this PR exists to prevent, reached by a different road.
Feed once before the probe and again before the resave. Both feeds sit
outside any lock: filesystemStillReadable() takes spiLock itself, and
the format has already released it. ARCH_RP2040 covers rp2040 and rp2350
alike, and the blocks compile out everywhere else, so no other platform
and no native test changes.
Raised by @caveman99 in review.
* fix(nodedb): narrow the save-probe comment and name the full-filesystem case
The comment on filesystemStillReadable() claimed "real corruption asserts
in lfs and formats on reboot". That does hold on nRF52 - nrf52.ini builds
with -DLFS_NO_ASSERT and force-includes cpp_overrides/lfs_util.h, whose
LFS_NO_ASSERT arm routes LFS_ASSERT to the lfs_assert() in main-nrf52.cpp,
which stamps NRF52_MAGIC_LFS_IS_CORRUPT and resets into the format - but
NodeDB.cpp compiles for ESP32, RP2040 and portduino too, where nothing of
the sort is wired up. It is also not true on nRF52 under POFWARN, where
lfs_assert() deliberately skips the stamp. Drop the claim rather than
qualify it three ways.
The log line now names what a field log actually needs to tell apart: a
filesystem that still reads but cannot be written is either busy or full.
Raised by @caveman99 in review.
* fix(sx128x): quiesce flash before the 2.4GHz region reset
reinitChip() saves the region, waits 2 s and resets. On nRF52 that was
the last software reset still going straight to NVIC_SystemReset with a
page program possibly in flight, so "every software reset" in the earlier
commit did not quite hold.
The quiesce stays inside the ARCH_NRF52 arm on purpose. The #else arm
logs and falls through to lora.setCRC() further down, which re-enters
spiBeginTransaction(); a quiesce hoisted above the #if would take spiLock
and never give it back, self-deadlocking portduino and stm32wl. Routing
this through Power::reboot() is wrong for the same class of reason:
setupModules() runs before initLoRa, so its notifyReboot observers and
waypointStore.saveToFlash() are live and would add a flash write to an
aborted radio init.
Raised by @caveman99 in review.
* fix(nrf52): only quiesce on the DFU control write that actually resets
QuiescingBLEDfu wrapped every control-point write, so a write that was
never going to reset still blocked the Bluefruit callback task on spiLock,
forced an early page-cache commit and held back the GATT authorize reply.
Only START_DFU resets; gate on that.
Deliberately no "request->len &&" term. The library's own test is
`request->data[0] == START_DFU` with no length check (BLEDfu.cpp:110 in
both the nRF52 and nRF54 cores), and Bluefruit hands the callback a copy
of a reused event buffer, so a zero-length write carrying a stale 0x01
still resets inside the library. A len term here would let exactly that
reset run unquiesced, which is the case this wrapper exists for. Reading
data[0] is always in bounds: ble_gatts_evt_write_t declares uint8_t
data[1] and the copy covers it.
Raised by @caveman99 in review.
* Prove explicit acks with Routing.ack_proof
Explicit acks are ROUTING_APP packets, and ROUTING_APP is excluded from PKC, so
an ack travels under channel encryption alone - and the default channel key is
public. Anyone in range can forge one, and the client grants its strongest
delivery claim on the strength of the ack's unauthenticated `from`.
Where the acknowledged packet was PKI encrypted the endpoints already share a
Curve25519 secret, so the recipient can prove receipt in ~10 encoded bytes:
ack_proof = HMAC-SHA256(shared_key,
"ack" | LE32(from) | LE32(to) | LE32(request_id)
| routing)[0..8)
where `routing` is the encoded Routing message without the ack_proof field,
taken as received with that byte range removed rather than re-encoded. Excising
keeps the value a function of the received bytes alone, so it does not depend on
two implementations' encoders agreeing and does not drop fields this build has
never heard of. For the same reason the sender appends the field rather than
setting it on a decoded struct and re-encoding.
What this does and does not buy. It buys an authenticated delivery receipt from
the actual recipient, which is the property a forged ack costs a user and which
matters where people act on a delivery confirmation. It does NOT protect the
retransmission loop, and must not be described as if it does:
perhapsGenerateImplicitAckForOwnOverheard clears a pending retransmission on any
overheard rebroadcast of our own (from, id), header-only and keyless, so
replaying the originator's own ciphertext stops their retries more cheaply than
forging an ack. No ack authentication of any kind closes that path.
Advisory only, and deliberately not a step toward enforcement. A rule requiring
a proof once a peer has sent one would make a missing proof destroy the only
delivery signal we have, on state the user cannot see: the proof needs the peer
to hold our key, and peer-side eviction, a downgrade or a factory reset are all
invisible to us. A valid proof marks the ack verified; anything else behaves
exactly as today. The client renders the difference.
Verification costs one X25519 and nothing caches the shared secret, so
ackProofPermitsAction gates on findPendingPacket first - otherwise a forged ack
naming any packet id, which is visible in the cleartext header, would force a DH.
Depends on meshtastic/protobufs#1094 for the generated field.
* test: correct a comment that predates ack_proof being generated
The wire-roundtrip test described ack_proof as an unknown field, which was true
while the prototype hand-encoded it. The field is generated now, so this build
understands it - but older firmware does not, which is the case the assertion
actually covers.
* Fix the EXCLUDE_PKI test build, and format
Two copies of test_proof_binds_error_reason and test_proof_binds_direction were
sitting in the #else branch of test_ack_proof, where Identity, makeIdentity,
makeAck, becomeNode, crypto and ACK_PROOF_SIZE do not exist. They were also
unregistered, so they were dead code that only served to break the build. The
native suite never compiles that branch, so a local run could not see it.
Also apply clang-format to a declaration that had been wrapped by hand.
* Exempt test_ack_proof from trufflehog's Lob false positive
Same detector and same shape as the three suites already listed here: it
stitches nearby hex literals into one candidate string, and this suite's node
numbers and request ids (0x0A0A0A0A, 0x0B0B0B0B, 0xABCD1234) happen to match a
Lob API key. The suite holds no literal key material - every key it uses comes
from crypto->generateKeyPair at runtime.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Initial firmware support for Axiometa Genesis Mini
* Report the real AXIOMETA_GENESIS_MINI hardware model
The protobufs now carry AXIOMETA_GENESIS_MINI = 148, so the board no
longer has to masquerade as private hardware.
On ESP32 the -D PRIVATE_HW flag never selected the model on its own -
architecture.h has no arm for it, so the board fell through to the
PRIVATE_HW default at the end of the chain. Give it its own arm and drop
the flag.
* fix(input): sample the encoder button after light-sleep wake
The edge that wakes the device lands while beforeLightSleep() has the
interrupts detached, and a button that is still held produces no further edge
until it is released. The thread stayed parked at INT32_MAX, so the press was
never sampled - no event was emitted, PowerFSM's GPIO-wake branch reads
BUTTON_PIN rather than the encoder pin, and the node dropped straight back into
light sleep with the press swallowed entirely. The second press worked, the
first did not.
Sample once on wake, and only when the button is asserted, so a timer or radio
wake leaves the thread alone. The press then follows the ordinary path and
InputBroker drops the event because the screen was off, so it wakes the screen
and does nothing more - the same behaviour every other input device has.
Rotation stays deliberately non-waking: only the button pin is armed in
doLightSleep(), and the abState re-seed discards a shaft moved during sleep
rather than replaying it as detents.
Reported by CodeRabbit on #11852.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: rcarteraz <robert.l.carter2@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Moves python dependencies declared in `setup-base` to a separate requirements.txt file (which renovate can keep updated) and aligns it with gh-action-firmware.
Also removes `pio upgrade`, this is pointless (we just installed the latest platformio in the previous step)
RP2xx0 had no __assert_func, so newlib's ran: it prints to stdio and abort()
reaches arduino-pico's _exit, a breakpoint loop. Before rp2040Loop() arms the
watchdog that hangs the node until power is removed; afterwards it costs a
silent stall of up to 8 s.
Install one along the lines of the nRF52 handler: log the failed expression
and reboot through watchdog_reboot().
Refs #11795
Jonathan Bennett