The end-to-end tests asserted reply[7] == PERM_ACL_ADMIN, which follows
the constant wherever it goes — the one thing these tests exist to stop.
They now assert the literal bytes from openhop_core's acl_conformance
table, so both repos are pinned to the same numbers rather than to each
other's vocabulary. Checked out against 2a55dd6 with these tests copied
in, all seven fail.
Also cover the fail-closed import guard, which was the only untested
path protecting against a mixed-version install, plus its happy path so
a guard that always fires cannot pass unnoticed.
acl.py kept its own copy of the role constants, so an installation could
pair the new numbering with a core that still derives is_admin from the
0x02 bit. That combination announces a room server's READ_WRITE clients
(role 2) as admins on the login reply's is_admin byte — silently, and
exactly where this change is meant to make the two bytes agree. The
dependency is a mutable openhop_core@dev, so nothing prevented it.
Import the roles and acl_is_admin/acl_role from openhop_core instead of
duplicating them, and raise an actionable ImportError when they are
absent: refusing to start beats misannouncing a client's privileges. The
drift test no longer skips on an old core, so CI reports the mismatch
rather than hiding it.
The ACL client list now reports the real role, so a room server's
read-write clients emit "read_write" — a value the OpenAPI schema did
not allow, which generated clients and validators reject.
Add it to the enum, document all four roles, and add a test asserting
every role_name() we can emit is declared in the schema.
OpenHop numbered the ACL roles GUEST=0x01, ADMIN=0x02, READ_WRITE=0x01.
Firmware ClientACL.h uses GUEST=0, READ_ONLY=1, READ_WRITE=2, ADMIN=3 in
the low two bits, so our admin byte decoded as READ_WRITE in every stock
MeshCore client (KiekR showed admin sessions as guest and hid the admin
controls), and GUEST/READ_WRITE were indistinguishable from each other.
Adopt the firmware values and split the guest password by server type,
as firmware does: a repeater's guest password grants GUEST (fetch base
telemetry, no settings changes) and a room server's grants READ_WRITE
(post and read messages). Blank-password read-only access stays GUEST,
matching simple_room_server's allow_read_only branch.
Also fix the mesh-CLI admin gate in text.py, which shadowed a local
PERM_ACL_ADMIN = 0x02 and tested the bit rather than the role — under
the corrected numbering that would have let READ_WRITE clients run admin
commands. The web ACL list now reports the real role name instead of
collapsing every non-admin to "guest".
Requires openhop_core with acl_is_admin() in the login reply
(openhop_core fix/login-perms); the two must land together.
Fixes#388
The OTA updater could stamp a build with a version that did not come from
the code being installed, which left nodes stuck on a version they could
not upgrade away from.
Two independent defects combined:
* _fetch_latest_version() returned the newest repository tag whenever the
GitHub lookup failed. On a dynamic channel such as dev, that value is
from a different branch entirely, so the UI reported "up to date" while
tracking a channel that was hundreds of commits ahead. The fallback also
swallowed _RateLimitError, defeating the backoff path already built for
it.
* _do_install() then passed that predicted version to setuptools_scm as
SETUPTOOLS_SCM_PRETEND_VERSION (and as the pretend-version argument of
pymc-do-upgrade), so pip built the channel's code but recorded the
predicted version as the installed one. When the two matched, every
later upgrade — including Force Reinstall — resolved as "requirement
already satisfied" and did nothing.
Observed on a node tracking dev: installed 1.1.1, latest 1.1.1, no update
offered, while dev was at 1.1.2.dev201.
A version lookup that cannot be resolved now raises, so a failed check is
reported as a failed check instead of a wrong version, and the install
derives its version from the checkout pip clones.
The remaining pretend-version uses in manage.sh are scoped to this
distribution. The unscoped variable applies to every setuptools_scm-based
project built in the same pip invocation, not only ours: dependencies
normally arrive as wheels and are unaffected, but one built from an sdist
inherits our version. With it set, pip wheel on the portend sdist produces
portend-9.9.9; portend and jaraco.functools both build with setuptools_scm.
buildroot-manage.sh already exports the scoped variant.
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.
Enhanced the `persist_discovery_result` function to retain advert metadata such as `node_name`, `latitude`, and `longitude` from existing records when updating neighbor information. This prevents overwriting valuable metadata with `None` values during SQLite updates. Added a corresponding test to verify that existing metadata is preserved correctly during the discovery process.
Added a new `served` field to the neighbor scopes API response, which includes the node's own advertised scopes. This allows clients to identify which scopes they already share with neighbors without needing to re-derive the rules. Updated the OpenAPI specification to reflect this change and added a new private method to retrieve the served scopes. Additionally, rebuilt the UI assets to incorporate these changes.
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.
MeshCLI._auto_add_discovery_result reached for storage_handler.record_advert and
returned early when it was absent. record_advert lives on StorageCollector, but
MeshCLI is constructed with the SQLiteHandler (main.py -> TextHelper ->
MeshCLI), which exposes only store_advert. The guard therefore always failed in
production: `discover.neighbors` discovered nodes and recorded none of them, so
`neighbors` listed whatever adverts happened to arrive rather than what
discovery found. Tests missed it because they injected a hand-rolled double that
did have record_advert.
Extract the shared persistence into discovery.persist_discovery_result, which
accepts either storage object its callers actually hold: record_advert when
present (store plus the MQTT/Glass advert publish), falling back to
store_advert. neighbors_publisher._enrich_discovery_result, which had its own
working copy against the StorageCollector, now calls it too, so the two paths
build an identical advert row from one implementation.
The regression test specs its mock off the real SQLiteHandler class rather than
hand-rolling the surface, so a double can no longer claim a method the wired-in
object does not have. It fails against the previous code with KeyError
'auto_added'.
- 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.
Enhance the PacketRouter to ensure that a companion does not receive its own transmitted packets. The `_companion_bridges_for_packet` method now tags packets with the originating companion's hash, preventing redelivery to the same bridge during fan-out. This change applies to various packet types, including advertisements and group messages. Additionally, tests have been added to verify that injected packets are correctly filtered, ensuring that the originating bridge does not process its own transmissions while still allowing other bridges to receive them.
SIGTERM did not stop the daemon. Observed: the process stayed alive 18+
minutes with all three companion listen sockets still bound and the serial
port still held, so a restart could not reopen the radio. Two defects
compounded.
Cleanup never ran. The signal handler cancelled run(), whose finally then
awaited _shutdown() from inside an already-cancelled task -- so the first
await raised CancelledError, run() returned, and asyncio.run() tore the loop
down before a single shutdown step executed. Not one step logged. Running
cleanup in a sibling task instead does not fix it either: run() returns as
soon as the dispatcher stops and asyncio.run() cancels every leftover task
on the way out, which showed up as 'frame server :5050' being cancelled
mid-stop. The handler now unwinds run() cooperatively by stopping the
dispatcher, so run_forever() returns on its own and cleanup runs in a task
that is not being cancelled. Cancelling is kept only as a fallback for a
failure before the dispatcher exists.
Nothing bounded the steps. Frame servers, bridges, router and Glass had no
timeout, so one stuck step stranded every step after it -- including
releasing the radio. Each step is now bounded by SHUTDOWN_STEP_TIMEOUT_S and
logged by name, so a hang is both survivable and diagnosable, and the sync
steps (HTTP stop, sensor manager, GPS, radio cleanup) run off-loop so a
blocking close cannot stall the sequence. The dispatcher is stopped first so
RX ends before its radio is released.
Even with cleanup fixed, a single non-daemon thread that never returns hangs
SIGTERM forever: interpreter finalization joins them with no timeout, which
is where the original 18-minute hang sat (main thread parked in
Py_FinalizeEx -> wait_for_thread_shutdown). Report any that are still alive
by name so the offender can be fixed at the source, excluding asyncio's own
executor workers since asyncio.run() joins those under its own timeout. Then
arm a daemon watchdog that forces the process down SHUTDOWN_EXIT_GRACE_S
after cleanup finishes, so a future stray thread costs a delayed exit rather
than a stuck service.
Verified on hardware: SIGTERM and SIGINT both exit in 1s with zero warnings,
all ports and the serial device released, and MQTT publishing its offline
status before disconnecting.
Startup config mistakes were handled inconsistently: an identity collision
exited cleanly, but a missing or invalid config file (FileNotFoundError /
RuntimeError from load_config) and a missing identity key (RuntimeError) dumped
a full traceback -- and load_config ran outside main()'s try, so those errors
escaped the fatal handler entirely.
Add repeater/exceptions.py with ConfigurationError(RuntimeError) and re-parent
IdentityConfigurationError onto it (it stays a RuntimeError, so existing
except-sites are unaffected). Raise ConfigurationError for the missing/invalid
config file, the config-load failure, and the missing identity key. Move
load_config and daemon construction inside main()'s try and catch
ConfigurationError there, logging just the message and exiting 1; unexpected
failures still log with a traceback.
A configured-identity collision (IdentityConfigurationError from the startup
preflight) is an actionable config problem, not a crash, but the top-level
fatal handler logged every exception with exc_info=True, burying the message
under a full traceback. Catch IdentityConfigurationError in main() and log just
the message before exiting 1; unexpected errors still get the traceback.
Local identities occupy two routing/persistence namespaces: companions
(companion_bridges[hash] plus the companion_* tables keyed by the hash byte)
and server-side identities (the repeater and every room server, which share
the login/text/protocol helper handlers[hash] slot and the room_* tables). A
one-byte prefix collision is only unrepresentable when both identities share a
namespace; a companion and a server-side identity live in physically separate
stores, and the packet router (_consume_via_local_candidates) already offers a
colliding packet to both and lets HMAC pick the owner.
Key IdentityManager state by (hash_byte, namespace) instead of the bare hash so
the guard rejects only same-namespace collisions. This keeps blocking the pairs
that actually break -- companion<->companion (bridge overwrite plus
companion_prefs PRIMARY KEY corruption) and server<->server, i.e.
repeater<->room-server and room-server<->room-server (handlers[hash] overwrite
plus room_* corruption) -- while allowing a companion to share a prefix with the
repeater or a room server, which was previously rejected despite being only a
cosmetic label clash.
Build a core RegionMap from the node's served regions and wire it into
the dispatcher and every companion bridge, so a flood reply is re-scoped
to the region its request arrived under (or left plain for a wildcard /
direct request) -- matching firmware simple_repeater::sendFloodReply.
Previously replies went out plain, so a reply to a request in region B
was dropped by B-only repeaters.
The map is built once from the node-wide transport_keys table (each named
region -> RegionEntry, flags=REGION_DENY_FLOOD for deny-flood regions; the
'*' wildcard is deliberately not an entry so plain floods reply plain). A
single shared instance reaches the dispatcher and all bridges. Public
regions rely on name-hashing for their key; a stored key is carried only
when it is genuinely custom material the name would not reproduce, keeping
reply-matching aligned with the forwarding transport-code check.
Region edits at runtime (CLI, web API, Glass sync) all funnel through the
transport_keys CRUD methods, which now fire a post-commit change callback;
the daemon rebuilds the map and reassigns a fresh instance to the
dispatcher and every live bridge (atomic rebind, safe against an in-flight
find_match on the RX thread).
Requires openhop_core with Dispatcher.region_map / CompanionBridge.region_map.
The PATH and protocol-response paths recorded the companion dedupe
unconditionally after the bridge fan-out, so a delivery where every
bridge raised was still suppressed for the full dedupe TTL — the client
lost the packet even though later mesh copies arrived. Have the fan-out
report delivery (at least one bridge completed without raising) next to
authentication, and mark the dedupe only on delivery, which is the
behaviour the marking helper's contract already documented. One healthy
bridge still counts as delivered, so duplicate suppression of repeated
mesh copies is unchanged.
yellowcooln