One pair of statements for all neighbours and one for a prefix, bound as a
parameter, rather than a WHERE clause formatted into the SQL, which bandit
flags.
neighbor.remove now resolves its targets in SQL: zero-hop repeaters, the
nodes `neighbors` lists, all of them or those whose key starts with the
given prefix, with their cached scopes. A keyed remove no longer deletes a
companion's, room's or distant repeater's advert history that happens to
share the prefix, and a locked database leaves every neighbour in place
instead of some of them. The prefix is compared literally rather than
LIKE-matched.
The prefix delete's None branch, which cleared every advert, is gone along
with get_neighbors' raise_errors flag, which nothing uses now.
get_neighbors answers {} on a storage error, which `neighbor.remove all` took
for "no neighbours" and answered OK. Add an opt-in raise_errors to
get_neighbors (and the storage collector passthrough) and use it here, so a
failed read surfaces as an error.
Bandit flags an f-string that holds SQL even when only placeholders are
interpolated. Start from a plain SELECT and append the IN (?, ...) clause,
as the other variable-length queries in this file do.
Stored entries now record when they last logged in, so the web UI can
show whether a provisioned grant is still in use after a restart.
Firmware keeps this in RAM only; last_activity still starts at 0 on
load, so "not active since restart" keeps its meaning.
- acl_entries gains a nullable last_login column, added beside the
table's CREATE so a failed earlier migration cannot lock stored
admins out of a load that reads it.
- A login by a stored entry writes its time, best-effort, and not when
the stored second already matches it.
- A guest promoted by setperm keeps its login time; a row taken over
from another identity's label does not inherit that identity's.
- Key changes, renames and label adoption carry the time; a retried
load keeps the later of the live and stored times and writes a login
made while the store was unreadable.
/api/acl_clients adds client_name and client_type for each entry: the
name the node advertises, from an advert the repeater heard, or else a
companion's contact for it (a companion's own admin is often never heard
directly). Rebuilds the bundled UI (openHop_RepeaterUI feat/acl-management),
which shows the name in place of the one-byte address.
Codex confirmation round: rows a rekey's failed cleanup left at an old key
survived a later rename under the old label, which the rename did not
touch and the delete (by the new label) did not reach, so a room given
the old name and key could load them. A room owns only the rows under
its label at its current key; a rename, a rekey and a delete now drop
every other row under the labels involved. A failed sweep is retried by
the next one. Loading does not sweep: a room whose key was edited by hand
would otherwise lose rows that setting the key back recovers.
Codex confirmation round: deleting a room removed only its rows at its
current key. Rows a rekey's failed cleanup left at an old key kept its
label, so a room re-created with that name on that key loaded them. A
delete now drops every row under the room's label; names are unique, so
no other room holds it.
Codex round 5 (after the usage limit reset):
- A rekeyed or deleted room's old key kept answering logins, messages and
requests until a restart: its login, text and protocol handlers stayed
registered. The daemon now unregisters an identity from every packet
helper (_unregister_identity_everywhere), and update_identity and
delete_identity call it for the retired identity. Each helper removes
only its own handler, never another identity's on the same hash byte.
- Rows left at a key by a failed cleanup could grant their admins to a
later room on that key. A room loads only rows under its own label and
logs any others; moves copy, relabel and delete only the room's own
label's rows. A same-key rename relabels before the config is saved
and puts the label back if the save fails, so a room's own rows follow
its name. delete_identity reports an ACL cleanup that failed.
- Two overlapping hot reloads could start a superseded replacement
RoomServer whose sync loop then ran untracked. A swap starts its room
only if it is still the registered one.
Fourth review round (opencode; codex hit its usage limit):
- update_identity's hot reload was always refused: the identity manager
rejects a name or hash already registered, which the running room's own
entry always is. The ACL name index then lagged the config, so a renamed
room was unmanageable by name, and a later rekey left a second live,
writable ACL. The old entry is now unregistered first and restored if
the new registration is refused (IdentityManager.unregister_identity),
and a move falls back to the store key when the name misses.
- A key change whose save and cleanup both failed left a copy at the new
key that blocked every retry. A copy under the room's own label is
replaced on retry; another identity's entries still refuse the move.
- A registration now detaches the ACL it replaces before loading, warns
when it displaces another identity sharing its hash byte, and the no-store
guard for a key change lives in LoginHelper.move_room_acl.
- /acl_stats is documented as it has always answered; store_error and
store_errors say they clear once a later read succeeds; the example
config says room servers keep their own security settings.
Third review round (codex, opencode):
- A failed save whose rollback also failed left the ACL under a key the
config did not name. A key change now copies the entries, saves the
config, then drops the old ones, all under the ACL's lock: any failure
leaves the key the saved config names holding its entries.
- Moves and deletes found the live ACL by scanning for its store key, and a
rekey left two ACL objects for one identity, so a second move or a delete
could act on the stale one. LoginHelper finds ACLs by name, and a new ACL
detaches and drops the one it replaces.
- A room deleted and re-added with the same key reused the detached ACL and
never stored again. A detached ACL is never reused.
- A failed read at startup was never retried; the next login or change
retries it, and acl_clients reports it as store_errors.
- A key change with no store anywhere was treated as moved. It is refused.
- The owner lookup fell back to the 1-byte hash for a room with no ACL,
attributing another identity's entries to it. acl_stats used the hash too.
- A refused update_identity left its merged settings in the live config;
every refusal now restores the entry. A key that yields no public key is
refused rather than skipping the shared-key check.
- Removing from every ACL stopped at the first failed write; it now reports
which failed and which succeeded. Room eviction carries on past one.
- Settings on a reused ACL are applied under its lock.
Second review round (codex, opencode):
- Moving a room's rows and pointing its live ACL at them happened in two
steps, so a grant could land under the old key in between.
ACL.move_store now does both under the ACL's lock.
- update_identity accepted a key another identity already uses, and the
move merged the two ACLs. It now refuses such a key before changing
anything, and the store refuses to move onto a key that has entries.
- update_identity saved the new key before moving the ACL, so a failed move
left a saved key with an empty ACL. The move now runs first, and a failed
save moves it back. It also runs when the daemon has no login helper.
- A failed read at startup looked like an empty ACL. The ACL records the
error and /api/acl_info reports it as store_error.
- A deleted room's handlers stay live until a restart, and a login there
wrote its entries back. Deleting now detaches the live ACL from storage.
- /api/acl_info looked ACLs up by the 1-byte hash, so identities sharing one
could show each other's counts. It uses the name lookup acl_clients does.
- The acl_clients filter is an object in the spec, as returned.
Review (codex, opencode) found:
- A room renamed and rekeyed in one update lost its ACL, and a room later
created under the old name could adopt it. Room servers are no longer
adopted by label; update_identity moves their rows to the new key and
name in one transaction and points the live ACL at them. Only the
repeater, which is unique, still adopts by label after a key change.
- A removal racing a grant could leave the grant stored but not listed.
Each ACL change and its write now happen under one lock.
- A failed write was reported as OK. The store now raises; setperm answers
"Err - failed to save" and leaves the table as it was, and the API
returns an error. A login still succeeds, as its session is valid.
- Re-registering an identity (a rename) built a new ACL, zeroing every
session's activity and replay watermark, so the room sync loop stopped
pushing to logged-in clients. The live ACL is now reused.
- The repeater's max_clients default rises to firmware's 32: admins are
never evicted and now persist, so 5 or 10 filled permanently. Stored
entries past max_clients all load, with a warning.
- atoi takes ASCII digits only; non-repeater identity types get their own
label; the sync-loop skip logs at debug; `get acl` has help.
API for the web UI:
- POST /api/acl_set_permissions sets an entry by identity name.
- /api/acl_clients lists entries by identity name rather than the 1-byte
hash two identities can share, with permissions_value, persisted and
identity_pubkey. /api/acl_info adds acl_entries and stored_entries, and
authenticated_clients counts entries that have been active.
- /api/acl_remove_client reads client_pubkey, as its schema always said
(the UI sent it, and the endpoint read only public_key), and takes
identity_name.
Firmware keeps its ACL in /s_contacts, so a key provisioned over serial with
`setperm <pubkey> 3` logs in with a blank password forever after. Ours was
RAM-only and setperm was a stub, so that workflow could not work.
- New acl_entries table, keyed by the local identity's full 32-byte public
key rather than its 1-byte hash, which collides between identities. An
identity_label ("repeater", "room_server:<name>") lets an identity adopt
its rows after a key change, as firmware's ACL survives a new private key.
Shared secrets are derived on load, never stored.
- The repeater stores every entry with permissions, a room server admins
only (firmware saveFilter). A write happens only when an entry's stored
permissions change, so a returning admin costs no I/O.
- setperm follows ClientACL::applyPermissions and firmware's replies: atoi
permissions stored as the whole byte, a guest role deletes by key prefix,
other roles need the full key. It refuses an empty prefix and a key that
is not a valid ed25519 point, both of which firmware accepts.
- `get acl` works from the web CLI (the serial stand-in, passed local=True)
and still errors remotely. It was unreachable: "get " matched it first.
- A full table evicts the least active non-admin, as firmware putClient,
instead of refusing; it still refuses when every entry is an admin.
- Room servers keep a stored admin through inactivity eviction and do not
push to entries that have not logged in since load, as firmware skips
clients with no activity.
- Deleting a room server drops its stored entries.
With the contact book full and overwrite-oldest on, a new advert evicts
the oldest contact in memory and _persist_contact upserts the newcomer
to SQLite, but the evicted row is never deleted. The stored count grows
by one per eviction until the next full save. If the daemon exits
without one (a crash, a kill, or a shutdown step that times out), the
companion then refuses to start because its stored contacts exceed
max_contacts.
Delete the evicted contact's row when contact_deleted fires, next to
the single-row upsert.
The three readers of the CRC scalar all answered for one receiver. On a bridge
that is half the node: a backhaul going deaf moved no number an operator or an
observer could see, while the sum of radios[].errors in the same status message
already said so.
All three now read RepeaterHandler.get_crc_error_count(), which sums the
radios -- stats.errors in the MQTT status message, crc_error_count in /stats,
and n_recv_errors in the MeshCore wire response, which previously read
self.radio directly and so described the default endpoint. stats.errors and the
per-radio entries beside it now reconcile.
This matches what node_stats() already does with the airtime counters that sit
next to it: a node-level counter describes the node, and which radio
contributed what stays available per radio, in radios[] and in the crc_errors
table.
The sum prefers each radio's live hardware counter and falls back to the last
count the sampler read from a radio the fabric cannot hand back. These are
lifetime counters, so dropping a radio would walk the node's total backwards
and hand an observer reading deltas a negative interval. A single-radio node
still reports its one unlabelled counter, read live, exactly as before.
The snapshot that keeps non-default radios reporting the band they are
actually on was seeded lazily, on the first status publish. That publish
waits for a broker to connect, so a node whose MQTT is briefly unreachable
could take a live radio save first and bake the pending settings in as
though they were what the hardware got -- reproducing the wrong-band
attribution the snapshot exists to prevent.
Capture it in MeshCoreToMqttPusher.__init__ instead, together with the
default radio id. That id was being recomputed from live config on every
publish, but fabric.default_radio is itself restart-required: a pending
change would refresh the radio that was not retuned and freeze the one that
was, reporting both wrongly where rebuilding everything got one right.
Seeding at construction also removes the unlocked check-then-set on
_applied_radio_map, which the status-publish timer thread shared with
whatever triggered the first publish.
The figures in stats are one set for the whole node, so on a two-radio bridge
they cannot say which side is carrying the traffic or which side has gone deaf.
Airtime is summed across channels and utilisation is the highest of them, which
is right for a node-level number and useless for telling the local radio from
the backhaul. Noise floor and errors are the default radio's outright, so the
link radio is simply unreported.
Every radio now carries its own tx_air_secs, rx_air_secs, current_airtime_ms,
utilization_percent, noise_floor and errors in its radios[] entry, under the
same field names stats uses so there is no second vocabulary to learn. The node
already computes all three -- AirtimeBudgets.per_radio_stats, the per-radio
noise floor cache, and the radios' CRC counters -- and /stats has reported the
first two as airtime_radios and noise_floor_radios since per-radio analytics
landed. This is the observer feed catching up.
Additive by construction: radios[] is published only where it already was, on a
node with more than one band, and a consumer that ignores it reads exactly what
it read before. Telemetry is merged into the map rather than gating it, and
each source contributes on its own, so a radio the fabric cannot hand back
keeps its air settings and still attributes its packets.
Two figures worth knowing before comparing them:
- stats.errors is the *default radio's* CRC count, matching /stats and the
MeshCore wire response, so on a bridge it does not equal the sum of
radios[].errors. Making the node scalar a sum would change three readers
that have always meant one radio, which is a decision of its own.
- tx/rx_air_secs truncate to whole seconds per radio and again for the node,
so the per-radio values can sum to one second less than the node's.
stats.errors was a literal 0 on every heartbeat, so an observer could not tell
a quiet node from a deaf one. It was hardcoded twice: once where the collector
builds the live stats, and again in publish_status, where the defaults were
spread *after* live_stats and so overrode anything the provider reported. The
second one would have pinned the field to 0 even once the first was fixed.
The node does count receive errors -- the radio's CRC failure counter, already
sampled on a timer, persisted as per-radio deltas and reported on the MeshCore
wire response as n_recv_errors. Report that, and let the defaults be defaults.
get_crc_error_count() on the engine is the single reader of the hardware
counter, replacing the inline read in get_stats so /stats and the status
message cannot drift apart. It reports the default radio's count rather than
the node's sum, matching the wire response, /stats, the noise floor beside it
in the same payload, and record_crc_errors' rule that only the default radio's
delta is published. A bridge's link radio errors are stored against their radio
id and queryable from the crc_errors table; they are not in this scalar.
queue_len stays 0. The Python repeater has no TX queue to measure, which is
what it has always reported -- the ordering fix just stops that being a
coincidence.
Per-radio sampling made the status message describe the wrong radio on a Fabric
node. _get_live_stats read the newest row in the noise_floor table with no
radio filter, which was right while only the default radio was ever stored.
_record_noise_floor_async now stores every radio's sample, default first, so
the newest row is the *other* radio's -- a bridge published its backhaul's
noise floor as the node's.
That is the opposite of the contract the sampler documents: _sampling_radios
puts the default first precisely because /stats, the MQTT status message and
companion stats all report that radio, and record_noise_floor honours it by
publishing only the default's sample. Reading the table directly went around
the rule.
Read the engine's cached reading instead, which is default-only by
construction, is what /stats reports, and agrees with the collector's own
cached value on the WebSocket path -- all three now say the same thing. It also
drops a SQLite query per heartbeat, which matters on an SD-card-bound Pi.
One behaviour change: the old query looked back 30 minutes, so a node restarted
within that window published a pre-restart reading as current. Now the key is
omitted until the first sample lands, 30 seconds in. The field was already
optional, and a reading from before the restart was not a current one.
Single-radio nodes are unaffected: their sole sample is the default's either
way.
Multi-radio entries currently replace radio, sx1262, and transport
configuration sections wholesale. Partial sx1262 entries therefore discard
board settings supplied by the top-level hardware preset, including TCXO
control, DIO2 RF switching, and radio enable pins.
Merge mapping sections key-by-key instead, treating the top-level sections as
defaults and radios[] values as per-radio overrides. Keep radio_type as a
scalar override so heterogeneous fabrics continue to work.
Overlay makes two spellings of one setting reachable together, where the
entry's could lose to an inherited default it never wrote: en_pins beats
en_pin in SX1262Radio, and address/serial_number beat device_address/serial
in get_radio_for_board. An entry naming either spelling now displaces the
other, so the failure mode is not a radio that silently never powers up.
This also corrects duty-cycle metering. It builds its air profiles through
_merge_radio_entry, so a radios[] entry that set only its frequency was
metered on library defaults -- 62.5 kHz / SF8 / CR8 -- rather than on the
node's own top-level radio block, and reported no tx_power at all.
_inherited_air_profile's docstring already described the overlay semantics
this commit implements.
Inheritance reaches the MQTT radio map the same way, and there it needed a
guard. The map is rebuilt from config on every status publish, so once a
partial entry inherits the top-level radio block, saving the default radio's
bandwidth moved the reported bandwidth of every radio that omits one --
while only the default radio is retuned, the rest keeping their band until a
restart. Observers join that map against rx_radio_id to decide which band
carried a packet, so it would have attributed those radios' packets to a band
they were never on. build_radio_status_entries now takes the map built when
the radios were last really configured and keeps every non-default entry from
it, which is the line AirtimeBudgets._effective_air already holds for
metering; MeshCoreToMqttPusher seeds it on its first build.
Log each resolved radio config at debug level, redacting modem_tcp.token via
redact_modem_tokens_in_place, since what an entry inherited is otherwise
invisible and a wrong answer surfaces only as a radio that will not transmit.
Add regression coverage for TCXO/RF-switch inheritance, explicit false
overrides, partial radio air settings, CH341 device selection, per-radio
radio_type overrides, alias displacement, metering inheritance, token
redaction, and a live default-radio save leaving other radios' published
air settings alone.
Fixes#485
An observer derives channel utilisation from the change in transmitted plus
received airtime between two status messages, so a node that publishes only
tx_air_secs cannot be metered: LetsMesh reports no utilisation for an openHop
bridge, and a consumer that defaults the missing key to zero sees TX-only
occupancy, which badly understates a repeater that hears far more than it
repeats.
utilization_percent, already published, is not that figure and cannot stand in
for it. It is this node's spend against its own duty-cycle budget over a
60-second window, so at the default 3600 ms/min it reads 100% at 6% of wall
clock -- a "how close am I to my TX cap" gauge, not channel occupancy.
AirtimeBudgets already tracks received airtime per channel and node_stats()
sums it, so this is a reporting gap rather than a missing measurement. Both
counters go out as whole seconds, matching the firmware's uint32 fields and the
two paths that already report them that way -- the MeshCore wire response in
handler_helpers/protocol_request.py and companion stats in main.py.
Worth knowing when reading the figure: RX airtime is estimated per packet from
the modulation that carried it, over packets that decoded. CRC failures,
preamble-only detections and traffic on other channels are not counted, so it
is a floor on receiver busy time rather than a measurement of it. Firmware's
counter has the same character, so the two remain comparable.
publish() skipped a connection whose disallowed_packet_types covered the packet
without recording anything, and the empty results list is how the same method
decides no broker was reachable. A node with healthy, connected brokers that all
filter a type therefore warned "No active broker connections" for every packet
of it -- loudest for exactly the common types someone would choose to filter.
Record the deliberate skip, so the warning fires only when nothing was
reachable. publish_mqtt already does this for its own intentional skips; this is
the same idiom one method up rather than a new mechanism.
The disabled-connection branch was never affected, which is why the warning
looked correct in a node with a broker switched off.
The filter added in the previous commit asked whether the record carried
a drop_reason. Every TRACE record carries one by design - trace_received
on arrival, trace_forwarded once relayed - and TraceHelper reaches
storage through log_trace_record, which took the default. Traces
therefore stopped reaching the brokers entirely.
Classifying a packet as invalid is the caller's job, and the engine
already does it; drop_reason is not that classification, since
duplicates, policy drops and traces all carry one and all belong on the
observer feed. So record_packet takes the caller's judgement as final
and defaults to publishing: skip_mqtt_if_invalid (default True, meaning
"decide for me") becomes skip_mqtt (default False, meaning "this one is
invalid").
TRACE also builds its own record rather than going through
_build_packet_record, so it never picked up the ingress radio. Read it
off the packet there. There is no egress counterpart: a forwarded trace
is injected into the router later and only mutates the in-memory record,
so what reaches the brokers is always the reception.
Neither gate has ever blocked anything.
disallowed_packet_types resolves configured names to integer payload-type
codes, but the publish path compared them against the payload's `type`
field, which the MC2MQTT schema fills with the literal string "PACKET".
The code lives in `packet_type`, serialised as a decimal string. Read it
from there as an int. A payload with no packet type (status, diagnostic
streams) or an unparseable one is published rather than dropped: not
being classifiable is not evidence of being disallowed.
skip_mqtt_if_invalid was computed in the engine, passed through
record_packet, and never read. Honour it for the broker publish only:
a packet this node could not parse is still stored and still reaches
Glass and the dashboard, which are the surfaces an operator debugs
their own RF from. Only malformed adverts, empty payloads and over-long
paths are affected - duplicates and policy drops are valid packets this
node chose not to repeat, and observers still see those.
A two-radio bridge is one MeshCore node with one pubkey, and the path
byte it appends on repeat is the same whichever radio carried the
packet. Nothing on the air says which band a hop used, and an observer
receiving on one side can only infer the other by already knowing its
neighbour's frequency. The node itself does know, so say it.
Two halves, both needed:
- Packets gain rx_radio_id (the radio that heard it) and tx_radio_ids
(every radio that retransmitted it - a bridge repeating on both sides
lists both). tx_radio_ids is always a list, so a consumer never has to
branch on the JSON type, and is absent rather than empty when the
packet was not repeated.
- Status gains radios: [{id, radio}], mapping each id to its
MHz,kHz,SF,CR settings. Without it a radio id is just an
operator-chosen label. Ids are unique only within a node, so consumers
key on (origin_id, id). Rebuilt per publish, since air settings can
change from the web UI while the node runs.
Single-radio nodes publish neither and their payload stays byte-for-byte
what it was: the ids would say nothing the status message doesn't
already. A node whose radio settings cannot all be read publishes no map
at all rather than a partial one that would place a packet on the wrong
band.
On a Fabric node the top-level status radio field now reports the
default radio rather than the top-level radio: block, which is the base
entries inherit from and can describe a band no radio is on.
A duty cycle is a limit on a channel, not on a node, and one AirtimeManager
charging every radio at the top-level radio block's modulation is wrong twice
over on a bridge. The same packet occupies a 62.5 kHz channel about eight times
longer than a 500 kHz one, so whichever bandwidth that block names, the other
radio is mis-charged by that ratio; and the two share one allowance, so a busy
local radio spends the backhaul's airtime on a band it never transmits on.
There is now one budget per channel, built from each radio's own profile and
its own band's limit -- 868.0-868.6 MHz allows 1% where 869.4-869.65 allows
10%, and one number cannot describe both. Two radios tuned alike share a
budget, because they contend for the same spectrum. duty_cycle.shared_budget
restores a single node-wide allowance for anyone who wants the conservative
reading, and a single-radio node is metered exactly as it was.
Metering a channel means knowing which radio will carry a packet, and the
engine used to work that out by restating openhop_core's dispatch rules. It
asks now. openhop_core passes a packet's ingress radio to the TX policy, so the
choice is a function of the packet rather than of whatever arrived most
recently; the repeater asks before scheduling and gets the answer the send will
reach. That also closes a race that predates this: sticky and bridge read the
node's last RX at send time, which after a retransmit delay on a busy node is
another packet's arrival, and moves a reply onto the wrong band.
The core change is optional. The keyword in resolve_tx_radio_id's signature is
the version check, and against a core without it the tx_mode selectors are
registered in their original one-argument form and behave exactly as they did.
What changes there is the gate: with no way to know which radio will transmit,
every radio the fabric could pick is held to its own budget, so a node cannot
transmit past a legal limit by guessing the quiet channel. Which mode is in
force is logged at startup.
Also fixed, each reproduced by an adversarial review of the earlier attempt:
- A node with one radios[] entry had its modulation and limit overwritten by
the stale top-level block on every radio save from the web UI. The refresh
path now branches on whether profiles were read, not on how many radios
there are.
- build_radio_profiles is all-or-nothing by design -- it feeds the UI -- so one
radio_type: none collapsed metering onto the top-level block for the radio
still transmitting. Metering has its own builder that degrades per radio.
- Carrying the whole node's spend onto an unrecognised channel compounded over
repeated rebuilds (100 -> 400 -> 1600 ms) and wedged transmission for the
rest of the window. Carry-over is keyed by channel, which makes an unchanged
rebuild idempotent, and an unrecognised one starts from the busiest single
channel rather than the sum of them.
- A rebuild worked the previous default manager out from the config again,
which by then already named the new default -- preserving the wrong object
and orphaning the one NeighborScopeHelper holds for the life of the process.
It is remembered rather than recomputed.
- Every caller that reports one airtime figure was reporting the default
radio's channel: the MeshCore wire field total_air_time_secs, companion
stats, the RRD and SQLite history, and /stats. They read a node total again
-- totals summed across channels, utilisation the highest of them, because a
node at its limit on one channel is at a limit however quiet the other is.
tests/test_fabric_fanout.py's rig never bound a tx_mode selector, so all 49 of
those tests ran against a fabric with no TX policy at all. It binds the real
one now; that gap is why an on-air change went unnoticed through two reviews.
A bridge sends one logical packet through both radios, and the two do not fare
alike: 500 kHz and 62.5 kHz are different channels with different neighbours and
different contention. The packet row holds one set of LBT figures, from the
primary egress, so the other radio's attempts and backoffs were dropped on the
floor -- and an egress that failed was never stored at all, leaving the half of
the story an operator most wants to see only in the log.
A new packet_egress table keeps one row per physical send: packet id, timestamp,
radio, whether it went out, and its LBT attempts, total backoff and channel-busy
flag. Written only on a node with two or more radios; with one radio the packet
row already says all of this, and nothing about single-radio behaviour changes.
The rows are written on the storage writer thread beside the packet, addressed
by the id the insert returns, and are kept off packet_record itself so they do
not ride along on every websocket, Glass and MQTT publish. Retention ages them
out on the packet cutoff, and purging packets takes them along.
/lbt_diagnostics gains radios[]: the same summary and bucket shape per radio,
read from the new table over a covering index, so one radio's contention is
never read as the other's. The combined figures above it are unchanged and still
count each packet once. Per-radio entries carry no rf block -- the RRD series
behind it is node-wide, and hanging it off one radio would read as that radio's
own RSSI and loss -- and no packet-type breakdown, which would cost a heap
lookup per egress.
The fan-out now reads a send's LBT metadata whether or not it succeeded. The
single-egress path has always done so, and the figures on a failed send are
exactly the ones that explain it -- as far as they reach us: a radio that raises
instead of returning metadata (an SX1262 giving up on a busy channel) reports
nothing to read, so that row records the failure with no attempts rather than
inventing them.
Redundancy copies are recorded too. A multi-ack burst is real airtime on both
radios, and leaving it out would undercount exactly the traffic multi-ack adds
to a channel already busy enough to need it.
/db_stats lists packet_egress only once something has been written to it, so a
single-radio node reports the tables it always reported and never pays for
counting rows it does not have.
The bucket and summary maths moved out of get_lbt_diagnostics into three module
functions so the per-radio series is the same code as the combined one rather
than a second copy of 150 lines. The combined answer is unchanged.
A bridge has one receiver per band, and until now one of them stood for the
node: the background timer read the default radio and stored the answer with no
radio attached. On the 910.100/910.525 pair that reported the wide radio's noise
floor as the whole node's, and charged its CRC counter with both radios' errors.
Every physical radio is now sampled, one after another, and stored against its
own id. Migration 19 adds a nullable radio_id to noise_floor and crc_errors with
a covering index each, so a per-radio history reads only its own rows. Rows
written before this stay NULL: they were never measured per radio, and
labelling them would claim an attribution nobody made.
CRC baselines are per radio. A shared one charges every radio's errors to
whichever was sampled last, which reports swings that never happened.
Reads take an optional radio_id: /noise_floor_history, /noise_floor_stats,
/crc_error_count and /crc_error_history. Naming a radio also makes offset paging
stable, which the RF Health page depends on -- paging the merged series shifts a
page's contents every time the other radio samples. History rows name their
radio only on a node with two or more radios, so a single-radio response is
byte-for-byte what it was.
The default radio is sampled first and is the only one published. /stats, the
MQTT status message and companion stats read its cached value, and publishing
per radio doubles the observer-feed cadence -- its own change, not this one.
The StorageCollector facade carries the new arguments too. The web endpoints
reach the database through it, so the tests that drive them autospec it: a bare
mock accepts any keyword and would have let a facade that had not kept up
answer happily in tests and raise TypeError on a real node.
Adding the radio filter and per-radio split by concatenating SQL fragments tripped bandit B608, failing the pre-commit check. The fragments were constants and every value was bound, but the queries are now three fixed strings: a NULL radio parameter disables the rx_radio_id filter, and the per-radio split selects its own query. Results are unchanged.
On a Fabric node the analytics endpoints merged every radio together, with
one transmission per logical packet and nothing to tell a reception on one
radio from the other. Every packet row already records its ingress radio
and each radio that sent it, so the split needs no new data.
The attribution rules get_airtime_buckets introduced move into shared
helpers: packet_carriers() says which stored radio ids a row occupied, and
RadioResolver maps them onto the configured radios (one radio owns every
row; with two, an unknown id is unattributed, never guessed).
get_airtime_buckets now uses them with no change in output.
With two or more radio profiles:
- /packet_stats adds radios[] (received, duplicates, dropped, physical
transmissions, average RSSI/SNR) and unattributed counts. The legacy
totals come from the same grouped pass, so the 24 h aggregate the
WebSocket broadcasts does not gain a second scan. Totals still count
one transmission per packet.
- /route_stats adds the route mix of each radio's receptions, plus
originated for packets no radio received.
- /radio_packet_rates (new) returns receptions and transmissions per
radio per bucket, grouped in SQL on columns idx_packets_airtime covers.
A week-long window is an index-only scan instead of a Python loop over
every row: 0.17 s against 1.11 s for the airtime loop on a synthetic
1.55M-row table.
- /neighbor_links gives each link radios[], its stats as each radio heard
it. The tracker keeps them beside the merged figures without changing
its keys or entry count.
- /neighbor_link_history rows carry rx_radio_id, and the endpoint takes a
radio_id filter and a by_radio bucket split.
A single-radio node gets exactly the answers it had: the legacy queries
still run for it and every new field is omitted.
get_airtime_buckets selects rx_radio_id, tx_radio_id and tx_radio_ids to
charge airtime per radio, but idx_packets_airtime only covers timestamp,
length, payload_length and transmitted. SQLite therefore fell back to
idx_packets_timestamp with a heap lookup for every row in the window:
the access pattern the covering index was added to remove, and the one
that pushes the chart past the client timeout on an SD card. The test
meant to guard this asserted the plain timestamp index.
Migration 18 drops idx_packets_airtime and rebuilds it with the three
radio id columns. CREATE INDEX IF NOT EXISTS never reshapes an existing
index, so the base schema cannot do this. It keeps creating the original
four-column shape, which is safe on a database that predates the radio
id columns, and the migration widens it once migrations 16 and 17 have
added them. The rebuild reads the packets table once, at startup.
The bucket query is now a module constant shared with the test, which
asserts COVERING INDEX idx_packets_airtime for both airtime queries, so
a column added to the query cannot slip out of the index unnoticed.
A two-radio Fabric bridge runs each side at its own SF, so one combined
utilization line computed from a single caller-supplied profile describes
neither radio. Report airtime per radio instead.
build_radio_stack now records the effective air settings of every radio it
builds, in configured order, as radio_stack_meta["radio_profiles"]. The
same builder is exposed as build_radio_profiles() so /stats and the airtime
endpoint can read current settings from the live config rather than a boot
snapshot. Air settings are read the way get_radio_for_board reads them, and
a field that cannot be read is reported as null rather than defaulted, so
nothing is charted from settings that were never on the air.
get_airtime_buckets takes those profiles and charges each stored packet to
the radios that actually carried it: its ingress rx_radio_id and every
successful egress in tx_radio_ids, falling back to the scalar tx_radio_id
for pre-fan-out rows. A relayed packet therefore appears as RX on the radio
that heard it and TX on each radio that sent it, each at that radio's own
time-on-air. Duplicate receptions are included; they occupied the receiver.
With one profile every packet belongs to that radio, so single-radio and
pre-Fabric databases are attributed exactly as before. With two or more, a
packet whose radio cannot be identified is counted as unattributed rather
than guessed onto the default radio.
The response is additive: legacy buckets/rx_total/tx_total remain as the
combined series, and radios[] carries one series per radio alongside
unattributed_rx_count/unattributed_tx_count. Airtime comes from the shared
core estimator instead of a second inline Semtech formula, memoised per
(radio, length). The sf/bw_hz/cr/preamble query parameters are kept only as
a fallback; server profiles are authoritative and are never applied across
both radios of a bridge.
Add two Fabric options for an exactly-two-radio node:
- fabric.repeat_on_ingress (tx_mode: bridge): a relayed packet is sent
on the other radio first, then back out on the radio that heard it.
- fabric.local_tx_mode: all: locally originated packets (companions,
adverts, room/protocol replies) go out once on every radio, default
radio first.
MeshCore processing (policy, dedupe, path mutation) still runs once per
logical packet; only the physical TX is duplicated, each egress with its
own Packet, serialised through the TX lock and gated independently for
duty cycle and link health. One successful egress is a successful
forward. Multi-ack extras share the egress set.
inject_packet registers its single ACK waiter before the first TX so an
ACK heard mid-fan-out outlives the dispatcher's ACK cache. Fan-out fixes
flood scope and path-hash mode once before copying and metering, so all
radios carry identical bytes and the duty-cycle gates see them.
Packet records gain tx_radio_ids (JSON column + migration); tx_radio_id
remains the primary successful egress. Invalid option combinations are
rejected before hardware is opened. Defaults preserve existing behaviour.
`pre-commit run --all-files` fails on `dev` at the `ruff-format` hook, so every
open pull request is red on a check that has nothing to do with its own diff.
Both files arrived with 72da7ee (pr-436) formatted by an earlier ruff.
Purely cosmetic, and exactly what `ruff format` produces:
- `sqlite_handler.py` — a generator expression and an f-string concatenation
that both now fit inside the line limit unsplit.
- `test_companion_import_repeater_contacts.py` — the blank line ruff requires
after a module docstring.
"Import repeater contacts" wrote nothing whenever a contact-type filter was
supplied, and reported success having imported zero rows.
`adverts.contact_type` holds the *display* name written through
`handler_helpers.discovery.NODE_TYPE_NAMES` — "Chat Node", "Repeater",
"Room Server", "Sensor". The import API accepts MeshCore's names — "companion",
"repeater", "room_server", "sensor", validated in
`companion_endpoints.import_repeater_contacts`. `companion_import_repeater_contacts`
compared them directly:
query += f" AND contact_type IN ({placeholders})"
so "repeater" never matched "Repeater", "room_server" never matched
"Room Server", and "companion" never matched "Chat Node" at all. On a live
repeater with 285 adverts, every one of the four permitted values selected 0
rows, as did all four together; only an unfiltered import worked.
A second defect had the same cause. The adv_type lookup normalised the stored
name (`lower()`, spaces to underscores) but mapped it with the API-name table,
where "chat_node" is absent — so chat contacts imported as adv_type 0 rather
than 1, even on an unfiltered import. That is silently wrong rather than empty,
and needs a manual `UPDATE companion_contacts SET adv_type=1 WHERE adv_type=0`
to repair.
Both sites derived the mapping independently, which is how they drifted. They
now share one `_ADVERT_TYPE_BY_STORED` table keyed on the normalised stored
form: the filter reverse-maps the requested API names onto the stored keys and
compares `LOWER(REPLACE(TRIM(contact_type), ' ', '_'))`, and the adv_type comes
from the same table. Filtering and limiting stay in SQL. An unrecognised type
still selects nothing rather than silently widening the query.
Tested against the real stored forms: 6 of the 8 new tests fail on current dev
(each of the four type filters, all four combined, and the adv_type mapping)
and all 8 pass with this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`SQLiteHandler.__init__` runs `_init_database()` and then `_run_migrations()`.
`_init_database` created `idx_packets_upstream_time`, which spans
`upstream_hash` and `upstream_hash_size` -- columns that, on a database
predating migration 13, only appear once `add_upstream_hash_to_packets` has
added them in the *later* migration step.
Fresh installs were fine, because the `CREATE TABLE` carries both columns. Any
existing database raised `no such column: upstream_hash` part-way through
`_init_database`, which caught and logged it. Every statement after the failing
index was therefore skipped -- including the `room_messages` and
`room_client_sync` tables -- and the daemon started with a partially-created
schema while reporting itself healthy: service active, radio initialised,
companions serving, HTTP up, and no packets recorded.
The `add_upstream_hash_to_packets` migration already creates this index, right
after adding the columns it depends on, so the copy in `_init_database` was
redundant as well as too early. Remove it and leave the index to the migration
that owns those columns. All three paths still end up with it: a fresh database
gets the columns from `CREATE TABLE` and the index when the migration runs, an
un-migrated database gets both from the migration, and an already-migrated one
kept the index from its earlier run.
Observed on a Raspberry Pi 4 upgrading a ~13 MB production database from 1.1.1;
the repeater was off-air for four minutes before the cause was found. It
self-heals on a second start, because the migration still runs after the failed
init, which makes it easy to misread as harmless.
`tests/test_sqlite_upgrade_existing_db.py` builds a database with the
pre-migration-13 schema and asserts `_init_database` runs to completion, keyed
on tables created after the old failure point. Asserting only on the upstream
columns is not sufficient -- the migration adds those either way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 5s WebSocket broadcast used to carry uptime_seconds alone — while
the dashboard sidebar starved: its duty-cycle gauge, noise floor and
mode never travel over WS, and the heartbeat marks stats fresh, which
suppresses the UI's HTTP fallback poll. The vitals now ride every
beat, all cheap in-memory scalars (~200 bytes): utilization_percent,
noise_floor_dbm (kept on record, no per-beat SQL), the repeater mode
(so a change made over RF or CLI reaches the web badge), and the
advert tier counters the sidebar's OK/Drop line shows, injected as a
callback from the daemon like the other collector hooks.
In exchange the heavy get_packet_stats(24h) SQL aggregate is decimated
to one beat in six (30s): nobody reads a 24h cumulative at 5s
resolution, and the UI already treats both payload fields as optional,
so older dashboards keep working. Net wire traffic goes down.
Signed-off-by: Thibault VINCENT <root@devcat.org>
- Added tests for multi-radio stack functionality, including merging radio entries and building radio stacks.
- Implemented policy context tests to ensure correct handling of radio IDs in multi-radio scenarios.
Scope answers were built into the MQTT payload and then discarded, so the web UI
had nothing to show between cycles and no way to ask a single repeater. Adds a
store for them, a read endpoint, and a single-target query.
Migration 15 adds neighbor_scopes: one row per queried neighbour holding the last
answer (`scopes`, `responded_at`) alongside the last query's outcome
(`status`, `queried_at`). The two are kept apart deliberately -- a failed query
updates the outcome but leaves the answer in place, because the responder
rate-limits anonymous replies to 4 every 3 minutes and one timeout is weak
evidence that a neighbour's scopes changed. An empty answer is stored as a real
answer: it means the neighbour serves unscoped traffic only.
A row is written for any query the node attempted. `timeout` alone cannot carry
that: the sweep reports it both for a neighbour that was asked and stayed silent
and for one it never reached, which is what ScopeResult.transmitted separates.
`send_failed` is recorded too even though nothing reached the air -- it was
attempted and the duty cycle refused, and skipping it left a repeater that keeps
refusing reading as "never queried" however often it was asked.
Scope rows follow their neighbour out of the database, on all four paths that
delete adverts: the two explicit deletes, the 6-hourly retention cleanup, and a
purge of the adverts table. Without the last two the table grew without bound and
a purge left scope counts on screen for repeaters no longer listed.
GET /api/neighbor_scopes serves the table; it stays separate from the paginated
advert queries, which are read per contact type on every page load.
POST /api/query_neighbor_scopes asks one neighbour now. Unlike publish_neighbors
it holds the request open for the reply, since the response window is normally its
5 s floor; past 45 s it returns and leaves the query running so a late answer
still lands rather than throwing away spent airtime. It returns the stored view,
not the raw result, so a failed query cannot tell the client to forget scopes the
database still holds. Nothing is published -- the periodic cycle owns the topic.
A query and a cycle must not collide over the scope helper. A cycle holds it for
its whole run including the discovery window, so a query refuses while one is
active; a cycle defers while a query is in flight; and if the two still race, the
cycle abandons the pass on the short retry delay instead of publishing a table
with every scope missing or dying and re-spending its discovery broadcast. Queries
are tracked so shutdown cancels them rather than transmitting through teardown.
The endpoint needs json_in: it is not on globally for /api and cherrypy's Request
has no `json` attribute without it, so reading the body would have failed on every
real request. A test asserts the decorator rather than trusting a fabricated
cherrypy.request, which is how it went unnoticed.
Follow-up work on the neighbours feature, plus one crash found while reviewing it.
The crash: `mqtt_brokers.neighbors` is a settings block, but the per-broker key of
the same name is a boolean and config.yaml.example documents both, so
`neighbors: true` under mqtt_brokers is an easy hand-edit to make. Every reader
called .get() on it directly, raising AttributeError inside
NeighborScopeHelper.refresh_config -- which is built during daemon init, so the
daemon would not start at all. A shared neighbors_config_block() accessor now
ignores a non-mapping (with one startup warning instead of per-tick noise) and
the API save rewrites it as a proper block.
Payload counters, mirroring firmware buildNeighborsMessage:
- total_neighbors and queried_neighbors. The latter cannot be derived from
`status`: a neighbour the sweep never reached reports `timeout` exactly like one
that was asked and stayed silent. ScopeResult therefore carries a `transmitted`
flag, set only past the point where the injector confirms the request is on air.
- Firmware's third field, `truncated`, is deliberately not emitted -- it reports a
fixed PSRAM JSON buffer overflowing, which openhop does not have. total_neighbors
here consequently always equals the published row count.
- self.default_scope: the region this node stamps on outgoing floods, or `*` when
unset. Read from live config so `region default <name>` applies without a
restart, and `#`-stripped like the scopes string beside it. Firmware tracks this
internally but does not publish it, so this field is openhop-only.
POST /api/publish_neighbors runs a cycle immediately, the HTTP twin of the
`discover.scopes` mesh CLI command. Authenticated by default along with the rest
of /api, and it schedules the cycle on the event loop rather than holding the
request open for the minutes a cycle takes.
Schedule persistence: _next_publish_at is monotonic and _last_publish_at was
in-memory only, so every restart read as "due" and spent a discovery broadcast
plus one serialized scope query per neighbour. A generic daemon_state table
(migration 14) now records the last publish and start() resumes from it, never
sooner than a five-minute grace window and never later than one interval. Two
details this had to get right:
- The schedule keys off the last *successful* publish. A failed publish also sets
_last_publish_at and reschedules on the short retry delay; persisting that would
silently turn the retry into a full interval.
- _tick's disabled branch no longer clears the schedule before the feature has
been enabled in this process. At boot the MQTT connections may not be up, so
enabled() can briefly be false, and clearing there would discard the restored
schedule and re-run the sweep anyway -- the exact thing this prevents.
- api_endpoints: the broker rebuild is a strict field whitelist, so a client
that omits `neighbors` (every UI build predating this feature) reset every
broker to false on any MQTT save, silently disabling the feature. Fall back
to the stored per-broker value instead. An explicit false still turns it off.
- neighbor_scopes: `_query_one` cleared `_pending` in an `except Exception`,
which does not catch CancelledError. A shutdown cancel during the injector
await -- where a query spends most of its wall time -- left a dead pending
query that kept consuming RESPONSE packets and hiding them from every
companion bridge. One try/finally around the whole method.
- neighbor_scopes: refresh_config had no caller after construction despite
promising live updates; it now re-reads before each sweep, so a live
delays.direct_tx_delay_factor change resizes the response window.
- neighbor_scopes: truncate scopes at the first NUL as the responder's C-string
writer intends, rather than only rstrip-ing the cipher's zero padding.
- api_endpoints: accept the documented max_sweep_seconds and
duty_cycle_abort_seconds, reject unknown keys instead of reporting success on
a typo, and reject non-bool `enabled` / fractional interval_hours rather than
silently coercing them.
- mqtt_handler: check paho's rc before counting a neighbours publish as
delivered; the payload is uncapped, so a size/queue rejection is realistic.
- neighbors_publisher: retry after 15 minutes when a cycle publishes nothing,
instead of burning the full 24h interval.
- neighbors_publisher: track the manual `discover.scopes` cycle so shutdown can
cancel it, and move the already-running check onto the event loop.
Tests: router-hook integration (a matching response is consumed, an unrelated
one still reaches companions), cancellation, sweep re-entry, malformed and
hostile response payloads, config validation, and the broker save round-trip.
All ten regression tests fail against the pre-fix code.
Python port of the firmware's WITH_MQTT_NEIGHBORS feature (MeshCore PR #35 plus
the aba571ed pacing rework). Each cycle refreshes the zero-hop neighbour table
with a node-discovery broadcast, asks every neighbour for its region scopes, and
publishes the assembled table to the MQTT neighbors topic.
- NeighborScopeHelper: client side of the anon-regions request (the server side
already existed in openhop_core). Queries are strictly serialized -- firing
them as a burst makes the responses collide, which is what the firmware fix
addressed. router.inject_packet resolves at the firmware's logTx/logTxFail
boundary, so the response deadline is armed only once the request is on air
and the firmware's QUEUED/PENDING state machine collapses into an await.
The response window is sized from radio parameters, mirroring
neighborDiscoverQueryTimeoutMs().
- NeighborsPublisher: owns the schedule, the immutable per-cycle snapshot, and
the payload. The snapshot merges live discovery responses over the stored
table because get_neighbors() serves a 60s cache that store_advert() does not
invalidate.
- packet_router: offer PAYLOAD_TYPE_RESPONSE to the scope helper before the
companion fan-out; it consumes only on an authenticated tag match.
- Per-broker `neighbors: true` opt-in (default off) plus a
mqtt_brokers.neighbors.enabled master switch; interval is 12-336h as in
firmware, rejected rather than clamped.
- `discover.scopes` mesh CLI command triggers one cycle immediately.
The whole zero-hop table is published including neighbours that did not answer
(timeout / send_failed), matching the firmware payload. No 10 KB cap: that
existed for a fixed PSRAM buffer.
agessaman