Repository navigation
fix: review fixes for repeater admin, routing, privacy, robustness and small screens - #105
Open
berikobama wants to merge 77 commits into
Open
berikobama wants to merge 77 commits into
berikobama wants to merge 77 commits into
Conversation
…d telemetry Repeater login, repeater CLI commands and telemetry requests called handler methods that openhop_core 1.1 no longer has (set_login_callback, set_command_response_callback) or used the old two-argument set_response_callback(hash, callback). Every attempt raised AttributeError or TypeError before anything was transmitted, so Admin and Request Telemetry could never work against openhop_core >= 1.1.3 (the version pyproject requires). Register waiters the way 1.1 matches replies: login and CLI by the target's full public key, telemetry by (public key, reflected request tag). The login password is now stored inside the try block, so a failed send no longer leaves it in the handler's password table for the rest of the session. The new tests pin the handler signatures, so a future API change fails loudly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MeshCore firmware has no logout command: the companion's CMD_LOGOUT only drops the local connection state. The CLI "logout" sent here drew an "Unknown command" reply from the repeater, which then showed up as a direct message from it. Logout now only forgets the session locally, like the firmware companion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
get_public_key() returned LocalIdentity.get_shared_public_key(), the X25519 key used for ECDH, not the Ed25519 identity key that adverts, path hashes and contacts use. As a result the QR code and Settings showed a key nobody can message, discovery responses advertised that wrong key, the "me" path badge prefix was wrong, and the "skip our own advert echoed by a repeater" check never matched, so the node listed itself as a peer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Admin view refreshed its repeater list only on repeater_login, repeater_logout, advert_received and contact_received events. Nothing emits the last two: adverts heard on the air arrive as plain "packet" events, plus mesh.contact.* and mesh.network.node_discovered. So a repeater that appeared while the app was running never showed up in Admin until a restart (or the user's own login), and could not be logged into. Compare what the list shows (repeater names, keys, favourites, logged-in set) after each event batch instead of enumerating event types, as the Peers view already does with its snapshot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add "Discover Nearby" to the advert menu. It sends MeshCore's zero-hop CONTROL discover request (filter: repeaters, room servers, sensors; random tag) and collects the answers for 12 s through openhop_core's ControlHandler tag callback. Firmware repeaters answer with their node type, the SNR they heard the request at, and their full public key; each answer becomes a peer with a direct (zero-hop) route, so Admin login works without waiting for an advert. A discover answer carries no name, so unknown nodes are asked with an anonymous ANON_REQ_TYPE_OWNER request (no login needed; the reply is "name\nowner"). Without an answer the node is listed as e.g. "Repeater 13ab5c7e" until its next advert renames it in place; the rename keeps the peer row, contact, login session, DM thread, saved password and favourite, and never takes a name that belongs to another key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MeshcoreClient() and OpenHopCoreSession() open the default database through XDG_STATE_HOME even when a test injects its own stores, and tests/integration/test_settings_apply.py builds a real session via update_settings(). Running the suite on a machine that also runs the app therefore opened and migrated the user's live database (and seeded rows into it), and anything that configured logging wrote into the live app.log. An autouse fixture now points XDG_STATE_HOME, XDG_DATA_HOME, XDG_CONFIG_HOME and the log directory at a per-test temp dir. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… names The log directory was hard-coded to ~/.local/state/meshcore-uconsole while the database follows XDG_STATE_HOME, so with XDG_STATE_HOME set the log landed somewhere else than the rest of the app's state, and every process (tests, mock-mode runs, a second instance) shared and rotated the same app.log. Resolve the log directory through paths.state_dir() when logging is set up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The identity seed (the node's private key) was written with the process umask, typically 0644, and so were the database with its WAL/SHM files (channel secrets, repeater admin passwords, message history), the logs and exported logs. Any other local account or process could read them. Set umask 077 at startup, create the seed with O_EXCL and mode 0600, create the state directories 0700, and tighten files that earlier versions already wrote 0644 when they are next opened. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… of logs The file log runs at DEBUG and is bundled by Export Logs for bug reports, yet it held private content: - openhop_core's Dispatcher logs every decrypted direct message and channel message (private channels included) at INFO; - GPS fixes and telemetry replies were logged with full coordinates; - the click debug hook logged the text of any tapped container, i.e. message bubbles. A PrivacyFilter on both handlers now withholds the text of decrypted messages (keeping channel and sender), the GPS and telemetry lines no longer include coordinates, and the debug hook only reads labels of buttons. The filter also escapes control characters: names and messages come from the air, and a newline or terminal escape sequence in them could forge log lines or drive the terminal showing the log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…shared Telemetry replies included the live GPS position (or the fixed position) even with "Share position" off. allow_telemetry defaults to on, favourites-only to off, the requester picks whether location is included, and anonymous requests need no contact entry, so any node that had heard one advert could ask where the device currently is. Location is now part of a telemetry reply only when share_position is on, the switch that already governs the position in adverts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The app replaced openhop_core's ANON_REQ handler with one that treated sub-type 0x03 as a telemetry request. In an anonymous request 0x03 is ANON_REQ_TYPE_BASIC (remote clock), and anonymous senders are by definition not contacts, so any node could fetch battery voltage and position without being known, without openhop's rate limiter, and with replies that failed for requests arriving over one or more hops (create_path_return without path_len_encoded). Drop the override. Telemetry stays available to contacts through the authenticated REQ path; BASIC, OWNER and REGIONS go back to openhop_core's firmware-compatible, rate-limited handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ilter Every CONTROL discover request was answered immediately, whatever node types it asked for, with no limit. Anyone in radio range could make the handheld transmit on demand: a presence and direction-finding beacon, battery drain, and enough airtime to break the EU868 10 % duty cycle (each answer is about half a second at SF8/62.5 kHz). A repeater's own discover.neighbors, which only asks for repeaters, also got an answer it then discards. Answer only requests whose filter includes chat nodes, at most 4 per 2 minutes like firmware repeaters, after a short random delay so several answerers don't collide, and keep a reference to the send task so it can't be garbage collected mid-flight. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
libadwaita before 1.4 always parses a toast title as Pango markup (the use-markup property only arrived in 1.4), and Debian bookworm ships 1.2. Toasts carry node names from the air (the Discover summary) and error texts, so a name like "R&D <Hill>" blanked the toast with a Gtk warning, and markup in a name could restyle it. All toasts now go through compat.new_toast(), which escapes the title. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Settings > Export Logs used Gtk.FileDialog, which only exists from GTK 4.10; on Debian bookworm (GTK 4.8.3) the button raised AttributeError. Add compat.choose_save_path(), which uses Gtk.FileDialog where available and Gtk.FileChooserNative otherwise, and use it for the export. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ms them Pressing Enter in these dialogs calls compat.activate_dialog_response(). On libadwaita < 1.5 that used Adw.MessageDialog.response(), which only emits the response signal: the channel was added (or the scope set) but the dialog stayed open, and a second Enter ran the action again. Close the dialog after the response, without the extra "close" response that close() would emit, as a click on the button does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the uConsole's 1024x576 logical screen the right edge of every page and the header's status and Connect controls were cut off. Measured minimum window sizes (mock data): 1164 px wide with a peer selected, 1046 px otherwise, and 706 px tall after opening packet details. Causes: - the page Gtk.Stack is homogeneous, so every page took the size of the largest one; - the Analyzer toolbar's filter toggles needed 1030 px in a plain Box; - the Peers detail action buttons sat in one non-wrapping row. Size the stack by the visible page, put the Analyzer filters in a horizontal scroller, and let the Peers action buttons wrap. The largest minimum is now 962x418. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Messages view detected new messages by comparing the number returned by list_messages_for_channel(limit=100) with the previous number. Once a channel held 100 messages that number stayed 100, so new messages in the open channel were never shown until the user switched channels. Unread counts of the other channels were only refreshed when the number of channels changed, so they stayed stale. Track the id of the last shown message instead (rebuild when it leaves the window) and refresh every channel's unread label on each update. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The settings form read the fixed latitude/longitude as `_parse_float(...) or current.latitude`, so entering 0 kept the old value, and an empty field raised "cannot be empty" on save. Once entered, a fixed position could not be removed, and it keeps going out in adverts and telemetry when position sharing is on. An empty field now clears the position (0.0, the app's "unset"), 0 is accepted, and values outside +-90/+-180 are rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ng signal values
While paused the analyzer stopped advancing its event cursor. The event store
keeps a bounded window and since() returns at most 200 events, so packets that
arrived during a pause were skipped on resume (a reviewer saw 0 of 300 shown
after resuming). Pausing now only stops rendering; collected packets are drawn
when the stream resumes.
Missing RSSI/SNR were replaced with -112 dBm / -9.0 dB ("or" defaults), which
also turned a real SNR of 0.0 into -9.00 and made unmeasured packets look
measured. Missing values now show as n/a.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Several layers each kept only the newest part of a burst: - poll_events() processed every drained event but returned only the newest `limit` (100) of them to the UI, including the app's own buffered events; - the event store's idle pump didn't reschedule after a full batch, so a backlog waited for the next event or the 30 s safety timer; - the analyzer asked the store for at most 200 events per update; - the client's event buffer was drained with extend() then clear(), so an event appended by a worker thread in between was lost. Return everything that was drained, reschedule the pump after a full batch, let the analyzer read the store's whole window, and drain the buffer with deque.popleft(). A reviewer's 300-packet burst now arrives complete (it was 0 of 300, then 100, in earlier runs). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_run_async() waited on future.result() without a timeout, and the only timeout was an asyncio.wait_for() inside the event loop. disconnect() stops and closes that loop with tasks still pending, so a caller in the middle of an operation waited forever: Discover kept saying "still listening", Admin stayed busy and Telemetry stayed on "Requesting..." until the app was restarted. openhop_core also calls sys.exit() on GPIO errors. Raised inside a task, that re-raises out of the loop and kills the event loop thread, with the same hang. Pending tasks are now cancelled before the loop stops (callers get an error right away), every wait has a deadline on the caller's side too, and SystemExit inside a radio call becomes a RuntimeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Send, Advert, Discover and the Admin actions connect implicitly from worker threads, and nothing serialised connect(): with autoconnect still running, pressing Advert could run a second session.start(), initialising the radio twice on the same GPIO lines and orphaning the first node task. connect() and disconnect() now share a lock. - The 8 s connect timeout was shorter than session.start()'s own radio.begin() retry back-off (2 + 4 + 6 s), so the later retries never ran; it is now 30 s. - A failed connect replaced the session without stopping it, so the next attempt found the GPIO lines still held. The failed session is now stopped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Python's sqlite3 runs DDL outside its implicit transactions, so every ALTER of an upgrade committed on its own, while schema_version was bumped once at the end. If the upgrade was interrupted (power loss on a battery handheld, or an exception in a backfill), the next start re-ran the ALTERs against columns that already existed and failed with "duplicate column name" every time, leaving the app unable to start. Wrap the whole upgrade, backfills and version bump in BEGIN IMMEDIATE ... COMMIT (SQLite DDL is transactional) and roll back on any error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
poll_events() caught only OSError around storing packets, and nothing around peer/message processing. A sqlite3.Error (database locked, disk full, I/O error) is not an OSError, and a malformed field raised ValueError: either escaped poll_events(), so the rest of the already-drained batch, including received messages, was lost. When it happened in the 30 s safety pump, GLib also removed that timer for good. Each event is now processed on its own and failures are logged with their traceback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The headless commands built their radio config from built-in defaults plus environment variables only: 910.525 MHz, SF7, CR 4/5, preamble 17, gpiochip 0 and the name "uconsole-node", while `doctor` and the GTK app use the saved settings and all of them use the node's real identity key. On a configured EU device `listen` heard nothing, and `advert` would have renamed the node mesh-wide at a US frequency. They now load the saved settings with the environment on top, as the app does; --node-name only overrides the name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
disconnect() ends every repeater session, but the Admin panel kept showing "Logged in (admin)" with the command line and Logout enabled; the next command then silently reconnected and timed out after 20 s with no session. When the repeater list's state changes (it includes the logged-in set), the selected repeater's login controls are now refreshed too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MeshCore repeaters only run CLI commands from admin sessions (simple_repeater checks isAdmin()), but the command entry, Send and quick-command buttons were enabled for guest logins too, where every command silently timed out after 20 s. They are now enabled only when the login is an admin one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"reboot" was a one-click quick-command button between "get repeat" and "ver"; a trackball slip took a remote repeater offline. It now asks for confirmation first, styled like the other destructive confirmations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Peers were created and updated from the raw "packet" event, which openhop_core emits for every received frame before deduplication and before AdvertHandler checks the advert's signature. A node in radio range could forge adverts without owning any key and rewrite a contact's key, route, map position, repeater flag or name, and any captured advert could be replayed later. Take peers only from mesh.network.node_discovered, which openhop_core publishes after the signature check, dedupe and own-advert filter (and from the app's own discovery results). Ignore adverts whose signed timestamp is not newer than the last one seen for that key. Since that event has no sender_id, derive the peer id from the key prefix as packet events did, and decode the hop list from its inbound path for display. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Peers and contacts are keyed by display name, and an advert with an existing name replaced that peer's public key in memory, in the database and in the contact book. Any node anywhere on the mesh could advertise "Alice" with its own (validly signed) key: the user's DMs to Alice were then encrypted to the impostor, the favourite (and so telemetry favourites-only) moved to the impostor, and a repeater's saved admin password would be encrypted to it on the next login. A name now belongs to the first key seen with it; another key advertising the same name becomes a separate peer, "Alice (9f3a1c)". ContactBook.add_contact() no longer rebinds a name to a different key either. A verified advert with a new name from a known key renames that peer in place (generalising the discovery-placeholder rename), so a node that renames itself no longer appears twice, loses its favourite, or keeps receiving DMs under its old name. An end-to-end check through openhop_core's AdvertHandler confirms that after a validly signed impostor advert the DM to "Alice" is readable by Alice only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The command entry consumed every Up and Down key press, even with no command history to move through, so keyboard users could not move focus out of it with the arrow keys. The keys are now only consumed while they actually step through the history. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
After logging in as admin, the Status tab stayed in front, so the command line was neither visible nor focused and the user had to switch tabs first. An admin login now shows the Console tab with the command line focused (a guest login stays on Status, since guests have no CLI). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The map refreshed its peer markers only when the number of located peers changed, so a peer that moved stayed at its old position and a renamed peer kept its old label as long as the count was unchanged. It now compares what the markers show (name, position, repeater flag), and rebuilds a marker whose label or style changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
GpsdProvider kept returning the last position after the fix was lost (gpsd reporting mode < 2), after the gpsd connection dropped, and even after the provider was stopped. That stale position went into adverts and telemetry (when sharing is on) and onto the map as the device's current location. It now returns a position only while it has a current fix, and forgets it on stop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
When gpsd is not reachable, the app reads the GPS UART itself and switches the GPS module's power on (GPIO 27 on the uConsole AIO). On disconnect it drove that pin LOW and then GPIO.cleanup() floated it, powering the GPS off under gpsd, other applications and the board's own power tools, which share the rail. The fallback now leaves the rail as it is when it stops. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
`meshcore-console import-channel --secret <hex>` required the private channel key as an argument, where it ends up in the shell history and is readable in /proc/<pid>/cmdline by every process. --secret is now optional: without it the key is asked for at a hidden prompt, or read from stdin when piped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
Every OpenHopCoreSession opens its own SQLite connection (for openhop_core's channel-secret lookups), and the client builds a new session on every disconnect, failed connect and settings change while disconnected. The old sessions' connections were never closed. Sessions now have close(), and the client calls it on each session it replaces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
Every received frame is emitted twice, as a "packet" and a "raw_packet" event, and both were written to the packet history, each with its own commit on the GTK thread. The analyzer only ever shows "packet" events, so half of the 1000 kept rows were unusable (about 200 of the 400 loaded at startup) and every packet cost two database writes. Only "packet" events are stored now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
On connect the contact book was seeded with each peer's route rebuilt from its stored last_path, which is the inbound path of an advert: the hops from the advertiser to us, i.e. the reverse of a route a direct packet can follow, and possibly days old. So after a restart, DMs to peers two or more hops away were again sent along a backwards route (the earlier routing fix only covered adverts heard while running). Seeded contacts now start with an unknown route; DMs flood until openhop_core learns the route from a PATH return. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
`meshcore-console send --peer NAME` started a bare session whose contact book was never filled, so every send failed with "Contact '<name>' not found". The stored peers are now loaded into the contact book first, as the GTK client does on connect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
Events the client generates itself were added to the history when queued and again when poll_events() delivered them, so list_recent_events() returned each of them twice. History is now recorded once, on delivery. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The listen loop only compared the elapsed time after an event arrived, so on a quiet channel `meshcore-console listen --duration 180` never stopped. The duration is now a deadline on the listen itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
…g the request An incoming request is tried against every contact whose key starts with the request's 1-byte source hash. The key conversion ran outside the per-contact try, so one contact with an invalid stored key raised out of the handler and the request went unanswered for every other contact sharing that hash byte. The key handling is now part of the per-contact attempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The app's REQ handler bypassed the rules openhop_core's own request handler and firmware apply: - there was no replay check, so a captured telemetry request replayed later was answered again (another transmission, and fresh battery/position for whoever replayed it); like firmware's per-client last_timestamp, a request not newer than the contact's last one is now ignored; - it was registered without local_delivery, so direct packets still in transit (hops left on their path) reached it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The map always fetched OpenStreetMap tiles over the internet. "Center on device" loads the tiles around the live GPS position, so the tile server learns the user's IP address and approximate location, and the on-disk tile cache becomes a record of where the device has been. A new "Download Map Tiles" setting (on by default, so nothing changes unless it is turned off) detaches the online tile source; the map then shows markers only, with a note saying why the map is blank. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
…n't have The Admin console's "telemetry" quick button sent the CLI text "telemetry", which no MeshCore repeater firmware implements (checked against v1.16 and v1.18): the repeater answered "Unknown command". Telemetry is a binary request (available from the Peers view). The button now sends "clock", which shows the repeater's time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
PathVisualization is a FlowBox aligned START. GTK then gives it the width it asks for at its allocated height, and at the one-row height its parent reserves that is a single badge: the badges were stacked one per row inside a one-row allocation, drawn over the sections below. In Peers that is the public key; the message and packet details had the same overlap. Let the FlowBox fill the width, so it wraps by the width it really has and gets the height of the rows it draws, and keep each arrow+badge at its natural size inside its cell. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
…shared get_device_location() only fell back to the fixed position from Settings when "share position" was on, so with sharing off and no GPS fix the map had no device marker and "Center on device" found nothing, although the position was entered. That setting decides what goes on air; send_advert() and the telemetry reply already check it themselves, and nothing else transmits get_device_location(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The map always opened on the middle of the contiguous US at zoom 4 (mock mode: San Francisco), whatever the device's position, and forgot where the user left it. Now it opens on, in order: the current GPS fix; the view it was left at; this device's fixed position from Settings; the area covering the peers with known positions; the whole world. While the user hasn't moved the map since starting, the first GPS fix centres it once, so a moved van doesn't keep opening where it parked last. The view is saved a few seconds after the map stops moving (one write per pan, not per frame) and when the window closes, as its own row in the settings table: it is app state, not a setting, and update_settings() has side effects (a SETTINGS_UPDATED event, a fresh session). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The path of a packet holds its relays in the order they relayed it: each repeater appends its hash, so the sender's side comes first and the relay that this device heard comes last. The path views in Peers, Messages and the Analyzer start the chain at this device but drew the hops in list order, so every route of two or more hops was reversed: the relay next to the sender was drawn next to "Me", and the one this device actually hears was drawn next to the sender. Seen on air: all 787 multi-hop packets the uConsole stored on 2026-10-28/29 end with the hash of the one repeater it hears directly, which the views drew at the far end. The chain now walks the hops backwards from this device. Hop counts and everything else that reads paths were not affected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
With a channel open at its newest message, every arriving message ended up just below the bottom edge: the view stopped at the end of the messages before it, as far short as the new bubble was tall (69 and 90 px in the mock for a one- and a three-line message). The view still counted as "at the bottom", so it never caught up and stayed one message behind; on air it looked like chats only sometimes scrolled. New bubbles change the scroll range only when the next frame lays them out, and the scroll to the end ran from an idle callback, which can run before that frame. The view now follows the range instead: whenever it grows (or the view shrinks) while the view was at the bottom, the view moves to the new end. Opening a channel starts at the bottom (or at the first unread message, as before) even when the previous channel was scrolled up. Unchanged: a view scrolled up to read older messages stays where it is when a message arrives, and shows the jump-to-latest button. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
a29defc moved the view to the end from the adjustment's "changed" signal. That signal arrives while the chat is being laid out, and a position set there is lost: the adjustment said "at the end", but the messages stayed where they were, so the newest one was still just below the bottom edge. The view now moves in an idle callback right after the layout, at a priority above redraws: at the default idle priority it ran up to 1.7 s after the layout in the mock, now right after that frame is drawn. The callback checks again that the view should still follow, so it can no longer override a jump to the first unread message: a channel with unread messages opens there and follows new ones only once scrolled to the end. An empty channel follows from its first message. Checked in the mock by which messages are actually visible, three runs each: after each of six arriving messages the newest is visible, a view scrolled up stays put and shows the jump-to-latest button, and an unread channel opens at its first unread message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
The details of a message grew with their content: for a message relayed over six repeaters they were 358 px tall (route, signal, time, buttons), and the chat panel then needed 566 px. On the uConsole's 576 px screen the window grew past the bottom edge (834 px in the mock), so the details' Close button and the message entry could not be reached. The details now scroll inside at most 220 px; the message list keeps the rest of the panel. Short details take only their own height, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
cwill747
requested changes
Oct 31, 2026
cwill747
left a comment
cwill747
left a comment
Owner
There was a problem hiding this comment.
Adversarial review of the locally cloned PR (tip e6fa249). Existing suite: 301 passed, 1 skipped. Reviewer regressions: 4 failed, 1 passed — each failure reproduces one of the findings below (tests in review-pr-105/test_adversarial.py). Full report in review-pr-105/REVIEW.md.
DM threads got their own ids ("dm:<name>"), but nothing kept a group from
taking one: importing a private channel named "dm:bob" created a group with
Bob's DM thread id, so the next "Message Bob" opened that group and the reply
went out with send_group_text to everyone holding its key. Bob's DMs filed
into it as well.
A group name in the DM namespace is now refused wherever a group can come
from: the channel secrets store (and with it import_channel, "+ Add Channel"
with or without a key, and radio_cli import-channel) and ensure_channel. What
was stored before is not used: secret rows under such a name are left out of
the channels handed to openhop_core, a stored channel whose kind does not
match its id is not loaded (both stay in the database), and a group message
naming one is dropped. send_message() takes DM or group from the id itself,
so a DM thread's id can never broadcast. import_channel's check for a DM of
the same name goes: since the DM namespace, a group name cannot hit one.
Found by cwill747's review of cwill747#105.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
Saved admin passwords were stored by peer name, and the Admin view pre-fills the one saved for the selected repeater's name. A name only stays with its key while the peer is known: the peer list evicts logged-out repeaters when it is full, and once "Hill repeater" was evicted, any key could advertise the name, own it, and get the old password pre-filled; the next login sent it to that key. The store is now keyed by the repeater's full public key, and the client looks a password up through the key of the peer holding the name now, so a password never follows a name to another key. A repeater that comes back with its key has its password again, also under an alias name. A rename no longer has to move the password. The v10 migration rebuilds repeater_passwords keyed by public_key. Each password moves to the key of the one peer of its name; one whose name no peer (or more than one) has is dropped, as there is no telling whose it was. Found by cwill747's review of cwill747#105. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
A second key advertising a name already owned gets a name of its own, "Alice (333333)", but that name was not checked in turn: when a node had already advertised "Alice (333333)" itself, the next "Alice" from a key starting 333333 was filed into that node's peer, and its public key was replaced while its peer id, favourite and name-keyed state stayed. _claim_name now tries longer key prefixes, then the full key with a counter, until a name is free or this key's own; a keyless legacy peer is taken over only by a key that has no peer yet. Updating a peer from an advert of another key, or renaming onto a name that is held, is refused instead of overwriting. Found by cwill747's review of cwill747#105. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
A DM thread's id was its peer's lowercased name ("dm:alice"). A rename
relabelled the thread but could not move its id, so after Alice advertised
herself as Bob the old thread showed "Bob" while Bob's next DM opened a
second thread "dm:bob": two conversations with one peer, the history split
between them. The same happened when a discovery placeholder learned its
real name. And "Bob" and "bob", two keys, shared one thread: the second one's
DMs showed up in the first one's conversation.
A thread is now "dm:<public key>". Incoming DMs carry the sender's key
(contact_pubkey), and ensure_dm_channel() takes the named peer's key, so a
rename only relabels the thread. A peer whose key is not known still gets a
name id.
The v11 migration moves each thread to the key of the one peer with its
peer_name and merges threads that land on one key, so a conversation split
by a rename is whole again. A thread of no single known key keeps its id.
Found by cwill747's review of cwill747#105.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt
Owner
|
Looks good, just want to run it through my uconsole before merging |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The fixes from a code review of the app and from running it on a uConsole: one commit per fix, 77 commits, each with a message that explains the cause and the change, so the PR can be reviewed (and bisected, or partly reverted) commit by commit. Every commit passes ruff check + format, compileall and the test suite on its own;
cz checkpasses for the whole range.No dependency or packaging changes (
openhop-core>=1.1.3, as onmain). Two commits are small features that fixes depend on or that came out of field use; they are marked below.Tested on: uConsole CM5 + HackerGadgets AIOv2 (SX1262), Debian 12 bookworm (libadwaita 1.2, GTK 4.8), openhop_core 1.1.3, on air in a live MeshCore network with a v1.16 repeater. Protocol behaviour was checked against the MeshCore firmware sources (v1.16.0 and v1.18.0). New tests cover each fix where it can be tested without a radio: 301 pass (1 skipped) at the tip.
Related issues
Open:
a7022e9only makes a failed connect release the GPIO lines and lets the retries run.Follow-ups to closed issues (the issue's fix landed; these fix what was still wrong in the same area):
c8d9438floods until a route is learned;48a00bdretries DMs and reports delivery.Direct#52 (hops always "Direct") and suport multi-byte path mode #70 (multi-byte paths): the hop count was the raw encoded path-length byte (DMs showed "255 hops", multi-byte paths wrong counts), and a message could take the path/SNR of another packet.3fcf99e,47b1d2c.e8cb316); it was missing from the map when not shared (b9c2bae); the map always opened on the US (601d6e2). Also Add dot with self on map #36 (own dot on the map).4255193), Export Logs crashed on GTK 4.8 (918d39d), Enter in the Add Channel / Region Scope dialogs ran the response twice on theAdw.MessageDialogfallback (f5bbc6f).0e19dd3).3e2c91c).aafcb9edeletes the key with the channel and says so in the confirmation dialog. Related: Private channel key functionality #101 / feat: support private channel keys when adding channels (#101) #103, and8d23f2a(import-channel no longer needs the key on the command line).3c75e58).clock(485fc3c).d6c2220).XDG_STATE_HOME(cea4e6d).fd8e561,a7c85be).Behaviour changes worth a look
c8d9438).cc8aa1b,adb4711,64bdeda); the peer list is bounded (75536ac).069555d, v10 migration). Older builds cannot read the newrepeater_passwordstable, so going back after the upgrade loses saved passwords.687fc6e); the identity seed, database and logs are created owner-only (4deab07).dm:are refused (12e341b,0148be6,077fde7; existing rows are migrated, v9 and v11).57c8512).4fc3544,da28aac).3e2c91c): existing installs using that preset power the radio, not the GPS.ui.map_view), not as a setting (601d6e2).a5a507c).60b67d8) and a switch to stop downloading map tiles (c90bb53).All 77 commits, by area
Repeater admin
93ce68cfix(repeater): use the openhop_core 1.1 handler API for login, CLI and telemetryd8eb6fffix(repeater): make logout local instead of sending a CLI "logout"21f75e2fix(ui): rebuild the admin repeater list whenever its repeaters change60b67d8feat: discover nearby repeaters and ask them for their names1af9c4ffix(admin): show the logged-out state after a disconnect62ca82cfix(admin): enable the command line only for admin logins49a8331fix(admin): ask before rebooting a repeaterb63190afix(admin): say "still waiting" instead of ignoring requests while busye89a711fix(admin): let Up/Down leave the command line when there is no historydfd804dfix(admin): open the console after an admin login485fc3cfix(admin): replace the "telemetry" quick command, which repeaters don't haveIdentity, security and privacy
6133712fix: use the Ed25519 identity key as the node's own public key4deab07fix(security): keep the identity seed, database and logs owner-only687fc6efix(logging): keep message text, positions and control characters out of logs3c75e58fix(privacy): send the position in telemetry replies only when it is shared5cc2eb6fix(privacy): leave anonymous requests to openhop_core's handlerd3e00d5fix(privacy): rate-limit discovery answers and honour the node-type filtercc8aa1bfix(security): build peers only from adverts openhop_core has verifiedadb4711fix(security): never let another key take over a contact's name077fde7fix(channels): keep group channels out of the DM thread ids069555dfix(admin): bind a saved repeater password to the repeater's key64bdedafix(peers): give a name collision a name nobody else holds0148be6fix(messages): key DM threads by the peer's public key75536acfix(security): bound the peer list8894882fix(requests): skip a contact with an unusable key instead of dropping the requestfab3433fix(requests): don't answer replayed requests or transit copiesRouting and messages
c8d9438fix(routing): flood DMs until a route is known, and stop taking routes from advertsd0bae8cfix(routing): don't restore routes from stored advert paths at startup12e341bfix: give DM threads their own channel ids48a00bdfix(messages): send without freezing the UI, retry DMs, and report deliveryd2ba4c1fix(messages): stop dropping a repeated DM sent minutes later47b1d2cfix(messages): pair each decoded message with its own packet3fcf99efix(messages): show the real hop count instead of "255 hops"59dafb0fix(messages): show late CLI replies in Admin and credit room-post authors57c8512fix(messages): keep message history per channelaafcb9efix(channels): remove a private channel's key with the channel, after warninga29defcfix(messages): keep the newest message in view as messages arrive1b28109fix(messages): scroll to a new message after the layout, not during itClient robustness and storage
62822e6fix: deliver bursts of events to the UI without dropping the oldesta93e4b5fix(client): never leave a radio call waiting forevera7022e9fix(client): serialise connect/disconnect and clean up a failed connectdacd2b5fix(db): run schema upgrades in one transaction388e36ffix(client): keep processing a batch of events when one of them failsd61624ffix(settings): fall back to the default for an unreadable settings row80d4be1fix(client): close the database connection of a replaced sessionbfd50f2fix(analyzer): store each received packet oncec386a77fix(client): record app-generated events once in the event historyRadio, GPS and hardware
4fc3544fix(radio): add the EU/UK Narrow preset and use MeshCore 1.16 preamble lengthsda28aacfix(radio): pass MeshCore's sync word to the radio explicitly3e2c91cfix(presets): power the AIOv2 LoRa module from GPIO 16, not the GPS raild6c2220fix(gps): report a gpsd position only while there is a current fixb1b60cafix(gps): don't power the GPS rail off when the serial fallback stopsUI: small screens, libadwaita 1.2, keyboard
4255193fix(ui): show toast text literally on libadwaita 1.2918d39dfix(ui): make Export Logs work on GTK 4.8f5bbc6ffix(ui): close Add Channel and Region Scope dialogs when Enter confirms them0e19dd3fix(ui): keep the main window within a 1024x576 screene6fa249fix(messages): keep a message's details within the screen360f5b3fix(ui): keep the open channel and unread counts updatingc312982fix(peers): keep details, focus and selection when the peer list refreshesfd8e561fix(ui): reach channel actions from the keyboardaac8cd5fix(ui): stop leaking a popover per channel right-clicka7c85befix(ui): make node badges usable from the keyboard10f499dfix(ui): URL-encode the node name in the contact QR code1ca7fd4fix(ui): stop network paths drawing over the public keya5a507cfix(ui): draw a path from this device outwards, not backwardsbd7350bfix(analyzer): keep collecting packets while paused, and stop inventing signal valuesMap and settings
e8cb316fix(settings): allow a fixed position to be cleared or set to 07ed9dd3fix(map): update markers when peers move or are renamedc90bb53feat(map): add a switch to stop downloading map tilesb9c2baefix(map): show this device's fixed position on the map even when not shared601d6e2fix(map): open where the map was left, or at this device, not on the USACLI
af4fc1efix(cli): use the saved settings for listen, send and advert8d23f2afix(cli): let import-channel take the key without the command linec69342bfix(cli): letsendreach the peers the app knowsdd741c2fix(cli): endlisten --durationon time even when nothing is receivedTests and logging
d61ab35test: keep the test suite out of the real app state directoriescea4e6dfix(logging): keep the log in the state directory that XDG_STATE_HOME namesAI disclosure
This PR was written with an AI assistant (Claude, via Claude Code), working under my direction: the review, the code, the tests and the commit messages are AI-generated. I tested the result on my own uConsole hardware, on air. Please review it with that in mind.
🤖 Generated with Claude Code
https://claude.ai/code/session_018DBViqusT2T1L2jPFYWKkt