Files
Thomas Göttgens 73c4110528 fix(phoneapi): resend my_info when the node num moves mid-session (#11732)
* 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.
2026-09-09 06:35:14 +00:00
..