From 78065715b6e90a47261da357163314c34505af04 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Sat, 3 Oct 2026 23:47:53 +0200 Subject: [PATCH] design: source chooser at startup Launched with no --server, udpscope silently connects to 127.0.0.1:8000 and offers no way to pick a source or see which ones it already knows. The design is a pre-flight dialog: it runs before ScopeModel exists, so a cancel costs no buffer and no socket and the existing pipeline is untouched. The list is built from the stored session files, filtered to those naming at least one signal -- a development machine accumulates one file per probe run on a throwaway port, 140 of them here -- and the session format gains multicast and interface keys so a row can reconnect exactly. Co-Authored-By: Claude Opus 4.6 --- .../specs/2026-10-03-source-chooser-design.md | 236 ++++++++++++++++++ 1 file changed, 236 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-03-source-chooser-design.md diff --git a/docs/superpowers/specs/2026-10-03-source-chooser-design.md b/docs/superpowers/specs/2026-10-03-source-chooser-design.md new file mode 100644 index 0000000..070e17a --- /dev/null +++ b/docs/superpowers/specs/2026-10-03-source-chooser-design.md @@ -0,0 +1,236 @@ + + + +# Source chooser at startup + +Launched with no `--server`, udpscope connects to `127.0.0.1:8000` because that is the +default in `Config`. There is no way to pick a source from the application, and no way to +see which sources it already knows about — a user who runs it against several streamers has +to remember each host and port and retype them. This adds a pre-flight dialog that lists +the sources with a stored session and offers a form for a new one. + +## Scope + +In: the chooser, the connection form, the stored connection settings a chooser row needs to +reconnect, and the probes that check them. + +Out: switching source without restarting; editing a stored source in place (forget it and +re-add); network discovery; a "test connection" button. Each is a separate feature and none +is needed to pick a source at startup. + +## When the chooser appears + +`Config` gains `bool server_specified = false`, set only by the `--server` branch of +`parse_args()`. `main()` runs the chooser if and only if it is false. `Config::server` keeps +its `127.0.0.1` default, so nothing downstream has to cope with an unset host. + +The condition is `--server` alone, not "no arguments": `--multicast` still needs a server to +fetch CONFIG from, so a run that gives a group but no server is a run that must still be +asked which host to ask. + +Every headless probe documented in CLAUDE.md passes `--server`, so none of them reaches the +chooser and none changes behaviour. That is the property that makes this safe to add to a +project whose entire regression net is those probes. + +## Lifecycle + +``` +main() + parse_args --server absent? + | yes + choose_source(cfg) gtk_init(), dialog, nested GMainLoop + | false (cancelled) -> return 0 + | true (chosen) + ScopeModel(cfg) \ + Acquisition(cfg) | unchanged + run_ui(cfg, ...) / +``` + +The chooser runs before `ScopeModel` exists, so a cancel costs no buffer allocation and no +socket, and `run_ui()`, `ScopeModel`, `Acquisition` and `RowSet` are untouched. `gtk_init()` +followed by a nested `GMainLoop`, and then later `gtk_application_new()` + +`g_application_run()` inside `run_ui()`, works: verified in a throwaway program before this +design was settled (`gtk_init()` is idempotent, and running your own loop is the documented +GTK4 replacement for the removed `gtk_dialog_run()`). + +Cancel exits **0**, not an error: declining to choose a source is not a failure. + +## Known sources + +`layout_store` gains: + +```cpp +struct KnownSource { + std::string host; + std::uint16_t port = 0; + std::string multicast; ///< empty = unicast + std::string interface_addr; + int rows = 0; ///< plots the session had + int signals = 0; ///< named slots across those plots + std::int64_t mtime = 0; ///< for the ordering; not shown +}; + +std::vector known_sources(); +``` + +It scans `config_dir()` for `*.toml`, splits each stem at the **last** `_` into host and +port (the host part may itself contain `_`, since `layout_path()` replaces unsafe characters +with it), parses the file with the existing `load_layout()`, and keeps the entry only when +at least one row slot names a signal. Sorted by `mtime`, newest first. + +This lives in `layout_store` because that file already owns `config_dir()`, the +`_.toml` scheme and the parser. A chooser that re-derived any of the three would +be a second expression of the same decision, which is how two parts of this codebase have +disagreed before. + +**The filter is what makes the list usable.** A development machine accumulates one session +file per probe run on a throwaway port — 140 of them at the time of writing, all with +`signals = ["", "", "", ""]`. Requiring a named signal means the list shows sources somebody +actually configured. + +`forget_source(const KnownSource&)` deletes the file, for the chooser's `×`. + +## Stored connection settings + +A session file is named `_.toml`, so its name cannot say whether that source was +multicast or which interface it joined on. `LayoutState` therefore gains: + +```cpp +std::string multicast; ///< empty = unicast +std::string interface_addr; ///< empty = default route +``` + +`save_layout()` emits them, when non-empty, in the **top block** (layout_store.cpp:325-338, +before the first `[[plot]]`). This is not a stylistic choice: in TOML a bare key written +after a table header belongs to that table, which is exactly how the `enabled` key was +parsed into the last `[[calibration]]` record and silently lost. `load_layout()` reads them +under `Section::Top`. A file written by an older build has neither key and loads as unicast. + +The values come from `Config` and never change at runtime, so `App` carries them beside +`source_host`/`source_port` and `collect_layout()` writes them back verbatim. + +The host in the filename stays the CONFIG server even for a multicast session, as today; the +group is an attribute of that source rather than its identity. + +## The chooser dialog + +New `src/source_dialog.{hpp,cpp}`: + +```cpp +/// Asks which source to open. Returns false if the user declined, in which case `cfg` is +/// untouched and the caller should exit. Fills `cfg.server`/`port`/`multicast`/ +/// `interface_addr` on true. +bool choose_source(Config& cfg); +``` + +A separate file rather than more of `src/ui.cpp`: that file is 7400 lines and already the +largest in the project, and this shares no state with it — it runs before the `App` struct +exists. + +A `GtkWindow` titled `udpscope — choose a source`, holding a `GtkListBox` inside a +`GtkScrolledWindow` (the list is unbounded) and an action row: + +``` + udpscope — choose a source + ───────────────────────────────── + 127.0.0.1:8000 3 plots, 4 signals × + 10.0.0.5:8000 1 plot, 2 signals × + 10.0.0.5:8001 2 plots, 3 signals, multicast 239.1.2.3 × + ───────────────────────────────────────────────────────────── + [ New host… ] [ Cancel ] +``` + +A multicast row is titled by its **server**, not by its group: that is the host the session +file is named after and the host CONFIG is fetched from. The group is a line of subtitle. + +- Each row: `host:port` in the normal face over a dimmed `N plots, M signals`, with + `multicast GROUP` appended when the session stored one. +- Activating a row — click or Enter, i.e. `GtkListBox::row-activated` — fills `cfg` from + that `KnownSource` and ends the loop with true. +- `×` calls `forget_source()` and removes the row. **No confirmation**: the gesture is the + confirmation, and what it destroys is one session's layout, which the app rewrites on the + next clean exit. It is a `GtkButton` inside the row, so it must not also activate the row; + it is packed at the end and stops there by having its own handler. +- `[New host…]` opens the connection form below. +- `[Cancel]`, `Escape` and the window's close button all end the loop with false. +- With no known sources the list is replaced by a dimmed line (`No saved sources yet.`) and + `[New host…]` takes the initial focus, so the dialog is never an empty box. + +## The connection form + +A second `GtkWindow`, transient for and modal on the chooser: + +| Field | Widget | Default | Empty means | +| --- | --- | --- | --- | +| Server | `GtkEntry` | `127.0.0.1` | — (required) | +| Port | `GtkSpinButton`, 1..65535 | `8000` | — | +| Multicast group | `GtkEntry` | empty | unicast | +| Interface | `GtkEntry` | empty | default route | + +`[Connect]` writes the four fields into `cfg` and ends both loops with true; `[Cancel]` +returns to the chooser. `[Connect]` is insensitive while `Server` is empty — the only +validation here. A *malformed* address is left to the acquisition layer, which already +reports it, and the starvation notice covers a source that accepts the socket and then says +nothing; duplicating that check in the dialog would be a second expression of it. + +These four fields and no others. `--buffer-mb`, `--max-window`, `--holdoff`, +`--recv-buffer` and `--plots` are tuning rather than "which source", and a form that asks +for them is a form a user has to read before using the app. + +Connecting to a host that already has a session file is not a special case: the file is +keyed on host and port, so `run_ui()` loads it exactly as it would have. + +## Error handling + +| Case | Behaviour | +| --- | --- | +| Cancel / close / Escape | exit 0, before any allocation or socket | +| `config_dir()` missing or unreadable | empty list, `[New host…]` only | +| A session file that fails to parse | `load_layout()` already returns what it parsed; an entry with no named signal is filtered out like any other | +| A chosen source that never answers | unchanged from today: CONNECT times out, the client reconnects, the starvation notice fires | +| `forget_source()` fails to unlink | the row stays; nothing else to do and nothing was lost | + +## Verification + +Probes, reported on stderr in the style of the existing ones, so this is checked by the same +net as everything else rather than a separate harness. + +- `UDPSCOPE_PROBE_SOURCES=1` — print the discovered list and exit without showing the + dialog: + + ``` + [sources] 3 known: 127.0.0.1:8000 plots=3 signals=4 | 10.0.0.5:8000 plots=1 signals=2 | + 239.1.2.3:8000 plots=2 signals=3 multicast=239.1.2.3 + ``` + + Assertions: only sources with a named signal appear, order is newest first, and a config + directory full of probe leftovers reports them **not at all**. + +- `UDPSCOPE_PROBE_PICK_MS=N` — show the chooser, then after N ms activate row 0 and let the + run continue into the real application. A full `xvfb-run` run with **no `--server`** must + therefore reach the scope and emit every existing dump line unchanged — that is the + end-to-end assertion, and it costs no new dump format. + +- `UDPSCOPE_PROBE_PICK_MS` with `UDPSCOPE_PROBE_NEWHOST=host:port` — drive `[New host…]` + instead: open the form, fill it, press `[Connect]`. The crash surface of a dialog built + and destroyed is as much the point as the state, the same way `UDPSCOPE_PROBE_FIELDS_MS` + is. + +- The multicast round trip, run serially: once with `--server`/`--multicast` to write the + file, then `grep multicast` on it, then once with no `--server` at all, checking the row + reads `multicast` and that `cfg` came back with the group. + +CLAUDE.md gains these under Verification plus an invariant for the TOML top-block rule and +the `--server`-absent condition; README.md gains the feature and why the list is filtered. + +## Files touched + +| File | Change | +| --- | --- | +| `src/config.hpp/.cpp` | `server_specified` flag | +| `src/layout_store.hpp/.cpp` | `KnownSource`, `known_sources()`, `forget_source()`, `multicast`/`interface` keys | +| `src/source_dialog.hpp/.cpp` | new: the chooser and the form | +| `src/main.cpp` | call the chooser when `--server` was absent | +| `src/ui.cpp` | carry multicast/interface on `App`, write them in `collect_layout()` | +| `CMakeLists.txt` | the new source | +| `CLAUDE.md`, `README.md` | the documentation this project requires for both |