From a066d48a3a21ed862e7ffabdfe00127aacb7c43e Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Sun, 4 Oct 2026 00:00:42 +0200 Subject: [PATCH] plan: source chooser at startup Seven tasks, each ending in a probe assertion rather than a unit test -- this project's regression net is the headless probes, not a suite. The spec's multicast round trip is split across three tasks so nothing in the plan needs a group this machine can join. --- .../plans/2026-10-03-source-chooser.md | 1540 +++++++++++++++++ 1 file changed, 1540 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-03-source-chooser.md diff --git a/docs/superpowers/plans/2026-10-03-source-chooser.md b/docs/superpowers/plans/2026-10-03-source-chooser.md new file mode 100644 index 0000000..7ba6b90 --- /dev/null +++ b/docs/superpowers/plans/2026-10-03-source-chooser.md @@ -0,0 +1,1540 @@ + + + +# Source Chooser Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Launched with no `--server`, udpscope opens a dialog listing the sources it has a stored session for, plus a form for a new one, instead of silently connecting to `127.0.0.1:8000`. + +**Architecture:** A pre-flight dialog. `main()` calls `choose_source(cfg)` before `ScopeModel` exists, so a cancel costs no buffer and no socket, and `ScopeModel`, `Acquisition`, `RowSet` and `run_ui()` are untouched. The list is built from the session files `layout_store` already owns, filtered to those naming at least one signal; the session format gains `multicast`/`interface` keys so a row can reconnect exactly. + +**Tech Stack:** C++20, GTK4 (`gtk_init()` + a nested `GMainLoop`), CMake + Ninja. No new dependencies. + +**Spec:** `docs/superpowers/specs/2026-10-03-source-chooser-design.md` — read it before Task 1. + +**One deliberate departure from the spec's Verification section.** The spec asks for the multicast +round trip as a single serial run pair (`--server`/`--multicast` to write the file, then no +`--server` to read it back). That needs a group this machine can actually join, which a headless +run cannot be relied on to have, so the same property is proved in three cheaper pieces instead: +Task 1's one-off proves the two keys round-trip through the file *and* land in the top block, Task 3 +proves the running app writes what it was launched with (unicast plus `--interface 127.0.0.1`, which +needs no join), and Task 5 proves a stored multicast row reconnects — from a session file written by +hand. No multicast networking is required anywhere in this plan. + +## Global Constraints + +- **Every source file carries an SPDX MIT header**, exactly: `// SPDX-License-Identifier: MIT` then `// SPDX-FileCopyrightText: 2026 Martino Ferrari`. +- **The build must stay warning-clean** under `-Wall -Wextra -Wpedantic` (set for this target in `CMakeLists.txt`). Pre-existing warnings in `src/ui.cpp` (`unused variable 'output_built'` at ~6044, two `-Wmissing-field-initializers` for `MathNode::source`, two `bugprone-integer-division`) are **not yours** — do not fix them in this work. +- **There is no unit test suite.** The project's regression net is the headless probes in `src/ui.cpp`, driven by env vars under `xvfb-run`. New checks extend that net; for code that runs before the GTK app exists, a one-off `/tmp/check_*.cpp` linked against the sources is this project's established practice (see `/tmp/check_math.cpp`, `/tmp/check_icons.c` in CLAUDE.md). +- **Run one `xvfb-run -a` at a time.** Two concurrent instances land on the same display and the second gets no GL context — it dumps `layout over 0 frames` and `valid=-1`, which looks like a pipeline failure and is not one. +- **Kill the mock after every run that started one**, in its own command, with a bracketed pattern: `pkill -f 'mock_udpstream[e]r'` then confirm with `pgrep -f 'mock_udpstream[e]r'`. `tools/mock_udpstreamer.py` outlives its `--duration` and leftovers hold ports. +- **Comment style:** this codebase's comments say *why*, and name the defect a rule exists to prevent. Match it. Do not add comments to code you did not change. +- **`gtk_window_destroy()` does not emit `close-request`.** Any dialog whose state is cleared by a `close-request` handler must be closed with `gtk_window_close()`. This cost this project a use-after-free in the math dialog already. +- **Do not reformat, refactor or "improve" code outside the lines each task names.** + +--- + +### Task 1: The session file stores how the source was reached + +A session file is named `_.toml`, so its name cannot say whether that source was multicast or which interface it joined on. Without these keys a chooser row cannot reconnect to a multicast source at all. + +**Files:** +- Modify: `src/layout_store.hpp:38-48` (add two fields to `LayoutState`) +- Modify: `src/layout_store.cpp:227-245` (parse them), `src/layout_store.cpp:325-338` (write them) +- Test: `/tmp/check_layout_rt.cpp` (one-off, not added to the build) + +**Interfaces:** +- Consumes: nothing. +- Produces: `LayoutState::multicast` and `LayoutState::interface_addr`, both `std::string`, empty meaning unicast and default-route respectively. Tasks 2 and 3 read and write them. + +- [ ] **Step 1: Write the failing test** + +Create `/tmp/check_layout_rt.cpp`: + +```cpp +// Round-trips a LayoutState through save_layout()/load_layout() and checks where the +// connection keys land in the file. One-off; not part of the build. +#include "layout_store.hpp" + +#include +#include +#include +#include + +static int failures = 0; + +static void check(bool ok, const char* what) { + std::printf("%-58s %s\n", what, ok ? "ok" : "FAIL"); + if (!ok) ++failures; +} + +static std::string slurp(const char* path) { + std::ifstream in(path); + std::ostringstream o; + o << in.rdbuf(); + return o.str(); +} + +int main() { + const char* path = "/tmp/check_layout_rt.toml"; + + udpscope::LayoutState st; + st.window_s = 2.0; + st.rows.push_back({{"sine", "", "", ""}}); + st.enabled.push_back("sine"); + st.calibration.push_back({"sine", 2.0, 0.5}); + st.multicast = "239.1.2.3"; + st.interface_addr = "10.0.0.9"; + check(udpscope::save_layout(path, st), "save_layout"); + + udpscope::LayoutState back; + check(udpscope::load_layout(path, back), "load_layout"); + check(back.multicast == "239.1.2.3", "multicast survives the round trip"); + check(back.interface_addr == "10.0.0.9", "interface survives the round trip"); + check(back.window_s == 2.0, "window_s still survives"); + check(back.enabled.size() == 1, "enabled still survives"); + check(back.calibration.size() == 1, "calibration still survives"); + + // The rule that cost this project the `enabled` key once: in TOML a bare key written after a + // table header belongs to *that table*, so a connection key emitted after `[[plot]]` would be + // parsed into the plot and silently lost. Assert the position, not just the value -- a reader + // and a writer that agree with each other can still both be wrong. + const std::string text = slurp(path); + const auto first_table = text.find("[["); + const auto mc = text.find("multicast = "); + const auto ifc = text.find("interface = "); + check(mc != std::string::npos && mc < first_table, "multicast is in the top block"); + check(ifc != std::string::npos && ifc < first_table, "interface is in the top block"); + + // A file written by an older build has neither key and must load as unicast on the default + // route rather than as something unset. + { + std::ofstream out("/tmp/check_layout_old.toml"); + out << "window_s = 1\nplots = 1\n\n[[plot]]\nsignals = [\"sine\", \"\", \"\", \"\"]\n"; + } + udpscope::LayoutState old; + check(udpscope::load_layout("/tmp/check_layout_old.toml", old), "load an older file"); + check(old.multicast.empty() && old.interface_addr.empty(), "an older file reads as unicast"); + + std::printf("%s\n", failures == 0 ? "all ok" : "FAILURES"); + return failures == 0 ? 0 : 1; +} +``` + +- [ ] **Step 2: Run it to make sure it fails** + +```bash +g++ -std=c++20 -Isrc -Ideps/plotter/include /tmp/check_layout_rt.cpp \ + src/layout_store.cpp src/math_channel.cpp src/measure.cpp -o /tmp/check_layout_rt +``` + +Expected: **compile error** — `'struct udpscope::LayoutState' has no member named 'multicast'`. + +- [ ] **Step 3: Add the fields** + +In `src/layout_store.hpp`, inside `struct LayoutState`, after the `enabled` member and before `math`: + +```cpp + /// How this source was reached, so the startup chooser can reconnect to it exactly. The file is + /// named `_.toml` and that name can say neither of these. Empty `multicast` is + /// unicast; empty `interface_addr` is the default route. + std::string multicast; + std::string interface_addr; +``` + +- [ ] **Step 4: Write them** + +In `src/layout_store.cpp`, in `save_layout()`, immediately after the `o << "plots = " ...` line and **before** the `enabled` block: + +```cpp + // Top level, before any table, for the same reason as `enabled` below. + if (!state.multicast.empty()) o << "multicast = " << toml_escape(state.multicast) << "\n"; + if (!state.interface_addr.empty()) { + o << "interface = " << toml_escape(state.interface_addr) << "\n"; + } +``` + +- [ ] **Step 5: Parse them** + +In `src/layout_store.cpp`, in the `if (section == Section::Top)` chain, after the `plots` branch: + +```cpp + } else if (key == "multicast") { + st.multicast = strip_quote(value); + } else if (key == "interface") { + st.interface_addr = strip_quote(value); +``` + +- [ ] **Step 6: Run the test to verify it passes** + +```bash +g++ -std=c++20 -Isrc -Ideps/plotter/include /tmp/check_layout_rt.cpp \ + src/layout_store.cpp src/math_channel.cpp src/measure.cpp -o /tmp/check_layout_rt \ + && /tmp/check_layout_rt +``` + +Expected: every line `ok`, last line `all ok`, exit 0. + +- [ ] **Step 7: Verify the whole app still builds** + +```bash +cmake --build build +``` + +Expected: no new warnings, no errors. + +- [ ] **Step 8: Commit** + +```bash +git add src/layout_store.hpp src/layout_store.cpp +git commit -m "session file records how the source was reached + +A session is named _.toml, which cannot say whether that source +was multicast or which interface it joined on, so a chooser row could not +reconnect to one. Both go in the top block: a bare key after a [[table]] +header belongs to that table, which is how the enabled key was lost once." +``` + +--- + +### Task 2: `known_sources()` and `forget_source()` + +The chooser needs the list of sources with a stored session. It belongs 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. + +**Files:** +- Modify: `src/layout_store.hpp` (add `KnownSource`, two functions, before `layout_path()`) +- Modify: `src/layout_store.cpp` (includes at the top; implementations after `layout_path()`) +- Test: `/tmp/check_sources.cpp` (one-off) + +**Interfaces:** +- Consumes: `LayoutState::multicast`, `LayoutState::interface_addr` (Task 1). +- Produces: + - `struct KnownSource { std::string host; std::uint16_t port; std::string multicast; std::string interface_addr; int rows; int signals; std::int64_t mtime; };` + - `std::vector known_sources();` + - `bool forget_source(const KnownSource& src);` + + Tasks 4, 5 and 6 use all three. + +- [ ] **Step 1: Write the failing test** + +Create `/tmp/check_sources.cpp`: + +```cpp +// known_sources() against a throwaway config directory: the filter, the ordering, the +// filename split and forget_source(). One-off; not part of the build. +#include "layout_store.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include + +static int failures = 0; + +static void check(bool ok, const char* what) { + std::printf("%-58s %s\n", what, ok ? "ok" : "FAIL"); + if (!ok) ++failures; +} + +/// Writes one session file and backdates it, so the ordering assertion does not depend on how +/// fast this program runs -- three files written in one millisecond have one mtime. +static void write_source(const std::string& host, std::uint16_t port, const char* signal, + const char* multicast, int age_s) { + udpscope::LayoutState st; + st.window_s = 1.0; + st.rows.push_back({{signal, "", "", ""}}); + if (multicast != nullptr) st.multicast = multicast; + const std::string path = udpscope::layout_path(host, port); + udpscope::save_layout(path, st); + const ::time_t now = ::time(nullptr); + const ::utimbuf t{now - age_s, now - age_s}; + ::utime(path.c_str(), &t); +} + +int main() { + // Its own directory, so the developer's real ~/.config/udpscope is neither read nor written. + ::setenv("XDG_CONFIG_HOME", "/tmp/check-sources-home", 1); + std::system("rm -rf /tmp/check-sources-home"); + + write_source("127.0.0.1", 8000, "sine", nullptr, 300); // oldest + write_source("10.0.0.5", 8001, "square", "239.1.2.3", 60); + write_source("10.0.0.6", 8002, "saw", nullptr, 1); // newest + write_source("127.0.0.1", 9561, "", nullptr, 30); // a probe leftover: no named signal + + std::vector got = udpscope::known_sources(); + for (const auto& s : got) { + std::printf(" %s:%u rows=%d signals=%d multicast=%s\n", s.host.c_str(), s.port, s.rows, + s.signals, s.multicast.c_str()); + } + + check(got.size() == 3, "a session with no named signal is not listed"); + if (got.size() == 3) { + check(got[0].host == "10.0.0.6" && got[0].port == 8002, "newest first"); + check(got[2].host == "127.0.0.1" && got[2].port == 8000, "oldest last"); + check(got[1].multicast == "239.1.2.3", "the stored group comes back"); + check(got[1].signals == 1 && got[1].rows == 1, "the row and signal counts are the file's"); + } + + // The host part may itself hold '_', because `sanitize()` turns anything unsafe into one, so + // the split must be at the *last* '_' or a host comes back cut in half. + write_source("a_b.example", 7000, "sine", nullptr, 10); + got = udpscope::known_sources(); + bool split_ok = false; + for (const auto& s : got) { + if (s.host == "a_b.example" && s.port == 7000) split_ok = true; + } + check(split_ok, "the filename splits at the last underscore"); + + // Forgetting one removes its file and nothing else. + const std::size_t before = got.size(); + udpscope::KnownSource doomed; + doomed.host = "10.0.0.5"; + doomed.port = 8001; + check(udpscope::forget_source(doomed), "forget_source reports success"); + check(udpscope::known_sources().size() == before - 1, "forget_source removes exactly one"); + + std::printf("%s\n", failures == 0 ? "all ok" : "FAILURES"); + return failures == 0 ? 0 : 1; +} +``` + +- [ ] **Step 2: Run it to make sure it fails** + +```bash +g++ -std=c++20 -Isrc -Ideps/plotter/include /tmp/check_sources.cpp \ + src/layout_store.cpp src/math_channel.cpp src/measure.cpp -o /tmp/check_sources +``` + +Expected: **compile error** — `'known_sources' is not a member of 'udpscope'`. + +- [ ] **Step 3: Declare the interface** + +In `src/layout_store.hpp`, immediately **before** the `layout_path()` declaration: + +```cpp +/// A data source udpscope has a stored session for: one row of the startup chooser. +struct KnownSource { + std::string host; ///< the CONFIG server, even for a multicast session + std::uint16_t port = 0; + std::string multicast; ///< empty = unicast + std::string interface_addr; ///< empty = the default route + int rows = 0; ///< plots the session had + int signals = 0; ///< named slots across those plots + std::int64_t mtime = 0; ///< orders the list; never shown +}; + +/// Every session file that names at least one signal, newest first. +/// +/// 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 when this was written -- and every one holds +/// `signals = ["", "", "", ""]`. A listed source is one somebody actually configured. +std::vector known_sources(); + +/// Deletes one source's session file; the chooser's `x`. False means the file was already gone or +/// could not be removed, which costs the caller nothing -- there is no state to roll back. +bool forget_source(const KnownSource& src); +``` + +- [ ] **Step 4: Implement them** + +In `src/layout_store.cpp`, add to the includes at the top (keep them sorted as the file has them): + +```cpp +#include +#include +#include +``` + +Then, immediately **after** the `layout_path()` definition: + +```cpp +std::vector known_sources() { + std::vector out; + const std::string dir = config_dir(); + DIR* d = ::opendir(dir.c_str()); + // No directory yet is an empty list, not an error: a first run has never saved anything. + if (d == nullptr) return out; + while (const dirent* e = ::readdir(d)) { + const std::string name = e->d_name; + if (name.size() <= 5 || name.compare(name.size() - 5, 5, ".toml") != 0) continue; + const std::string stem = name.substr(0, name.size() - 5); + // The *last* '_': `sanitize()` replaces anything unsafe in a host with one, so the host part + // may hold them too and a first-match split would return a host cut in half. + const std::size_t cut = stem.rfind('_'); + if (cut == std::string::npos || cut + 1 == stem.size()) continue; + const long port = std::strtol(stem.c_str() + cut + 1, nullptr, 10); + if (port <= 0 || port > 65535) continue; + + LayoutState st; + const std::string path = dir + "/" + name; + if (!load_layout(path, st)) continue; + + KnownSource s; + s.host = stem.substr(0, cut); + s.port = static_cast(port); + s.multicast = st.multicast; + s.interface_addr = st.interface_addr; + s.rows = static_cast(st.rows.size()); + for (const LayoutState::Row& row : st.rows) { + for (const std::string& sig : row.signals) { + if (!sig.empty()) ++s.signals; + } + } + if (s.signals == 0) continue; // see the header: a session nobody configured + + struct stat sb {}; + s.mtime = (::stat(path.c_str(), &sb) == 0) ? static_cast(sb.st_mtime) : 0; + out.push_back(std::move(s)); + } + ::closedir(d); + // Stable, so two files sharing an mtime keep the order the directory gave them rather than + // swapping between runs. + std::stable_sort(out.begin(), out.end(), + [](const KnownSource& a, const KnownSource& b) { return a.mtime > b.mtime; }); + return out; +} + +bool forget_source(const KnownSource& src) { + // Through `layout_path()` rather than by rebuilding the name here: one expression of the + // scheme. `src.host` came back out of a filename, so it is already sanitized, and `sanitize()` + // keeps alphanumerics, '.', '-' and '_' -- it is idempotent, so the round trip is exact. + return std::remove(layout_path(src.host, src.port).c_str()) == 0; +} +``` + +- [ ] **Step 5: Run the test to verify it passes** + +```bash +g++ -std=c++20 -Isrc -Ideps/plotter/include /tmp/check_sources.cpp \ + src/layout_store.cpp src/math_channel.cpp src/measure.cpp -o /tmp/check_sources \ + && /tmp/check_sources +``` + +Expected: every line `ok`, last line `all ok`, exit 0. `utimbuf` comes from the test's own +``; do not add that include to `layout_store.cpp`, which has no business backdating a file. + +- [ ] **Step 6: Confirm the developer's own config directory was untouched** + +```bash +ls -1 ~/.config/udpscope/ | wc -l +``` + +Expected: unchanged from before the task (the test sets `XDG_CONFIG_HOME`). If this dropped, `forget_source()` is deleting outside the configured directory — stop and fix it. + +- [ ] **Step 7: Verify the app still builds** + +```bash +cmake --build build +``` + +Expected: no new warnings, no errors. + +- [ ] **Step 8: Commit** + +```bash +git add src/layout_store.hpp src/layout_store.cpp +git commit -m "list the sources with a stored session + +known_sources() reads the session files the layout store already owns, keeping +only those that name a signal: a development machine accumulates one file per +probe run on a throwaway port, and all 140 of them here hold empty slots." +``` + +--- + +### Task 3: The app writes its own connection into the session file + +Task 1 taught the file to carry the connection; nothing fills it in yet. `collect_layout()` is where a session's state is read back out of the running app. + +**Files:** +- Modify: `src/ui.cpp:267-268` (two new `App` members), `src/ui.cpp:558-561` (in `collect_layout()`), `src/ui.cpp:7160-7161` (in `run_ui()`) + +**Interfaces:** +- Consumes: `LayoutState::multicast`, `LayoutState::interface_addr` (Task 1). +- Produces: `App::source_multicast`, `App::source_interface` (both `std::string`). Nothing later depends on them; this closes Task 1's loop. + +- [ ] **Step 1: Write the failing test** + +There is no unit harness for `App`; the check is a real run. Record the expectation first by running it against the current build: + +```bash +rm -f ~/.config/udpscope/127.0.0.1_9999.toml +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=4000 UDPSCOPE_PROBE_WINDOW_MS=1500 \ + ./build/udpscope --server 127.0.0.1 --port 9999 --interface 127.0.0.1 >/dev/null 2>&1 +grep -nE '^(multicast|interface) =' ~/.config/udpscope/127.0.0.1_9999.toml; echo "grep=$?" +``` + +Expected: `grep=1` — no such key. (No mock and no listener on 9999 on purpose: nothing needs to connect. `UDPSCOPE_PROBE_WINDOW_MS` drives the window control, which schedules a layout save at `src/ui.cpp:1798`, and the `destroy` handler at `src/ui.cpp:5587` flushes it — that is what writes the file with no data ever arriving.) + +- [ ] **Step 2: Add the `App` members** + +In `src/ui.cpp`, after `std::uint16_t source_port = 0;` (line 268): + +```cpp + /// How that source was reached. Unused at runtime -- `Acquisition` has its own copy -- but + /// written into the session file, because `_.toml` can say neither, and the startup + /// chooser needs both to reconnect to a multicast source from a stored row. + std::string source_multicast; + std::string source_interface; +``` + +- [ ] **Step 3: Collect them** + +In `collect_layout()`, replace the two lines at `src/ui.cpp:558-560`: + +```cpp + // The app's own lists are the pane's source of truth for both of these. + st.math = app.math; + st.measures = app.measures; +``` + +with: + +```cpp + // The app's own lists are the pane's source of truth for both of these. + st.math = app.math; + st.measures = app.measures; + // The connection is the session's too: the file's name carries only host and port. + st.multicast = app.source_multicast; + st.interface_addr = app.source_interface; +``` + +- [ ] **Step 4: Fill them in** + +In `run_ui()`, after `app.source_port = cfg.port;` (line 7161): + +```cpp + app.source_multicast = cfg.multicast; + app.source_interface = cfg.interface_addr; +``` + +- [ ] **Step 5: Build** + +```bash +cmake --build build +``` + +Expected: no new warnings, no errors. + +- [ ] **Step 6: Run the test to verify it passes** + +```bash +rm -f ~/.config/udpscope/127.0.0.1_9999.toml +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=4000 UDPSCOPE_PROBE_WINDOW_MS=1500 \ + ./build/udpscope --server 127.0.0.1 --port 9999 --interface 127.0.0.1 >/dev/null 2>&1 +grep -nE '^(interface|multicast) =|^\[\[' ~/.config/udpscope/127.0.0.1_9999.toml +``` + +Expected: `interface = "127.0.0.1"` on a **lower line number** than any `[[` table, and no `multicast` key (this run was unicast, and an empty value is not written). + +- [ ] **Step 7: Clean up the throwaway session file** + +```bash +rm -f ~/.config/udpscope/127.0.0.1_9999.toml +``` + +- [ ] **Step 8: Commit** + +```bash +git add src/ui.cpp +git commit -m "save the connection with the session + +collect_layout() now reports the multicast group and interface the app was +launched with, so a stored session says how to reach its source and not just +which host and port it was." +``` + +--- + +### Task 4: `--server` absent runs the chooser, and the chooser can list without a display + +The smallest end-to-end slice: the flag, the new translation unit, the wiring in `main()`, and the list-and-exit probe. No widgets yet — the probe returns before `gtk_init()`, so it needs no display and proves the discovery path from inside the real binary. + +**Files:** +- Modify: `src/config.hpp:13-33` (add `server_specified`), `src/config.cpp:68-70` (set it) +- Create: `src/source_dialog.hpp`, `src/source_dialog.cpp` +- Modify: `src/main.cpp:13-22` (call the chooser) +- Modify: `CMakeLists.txt:55-69` (add the source) + +**Interfaces:** +- Consumes: `known_sources()`, `KnownSource` (Task 2). +- Produces: + - `Config::server_specified` (`bool`, default `false`). + - `bool udpscope::choose_source(Config& cfg);` in `src/source_dialog.hpp` — `true` means `cfg` now names the source to open, `false` means the user declined and the caller should exit 0. + - `void apply_source(Config& cfg, const KnownSource& src);` and `void report_sources(const std::vector& known);` — both in `source_dialog.cpp`'s **anonymous namespace**, never in the header: nothing outside this translation unit calls either, and Tasks 5 and 6 add their callers to the same file. + +- [ ] **Step 1: Write the failing test** + +```bash +UDPSCOPE_PROBE_SOURCES=1 ./build/udpscope; echo "exit=$?" +``` + +Expected: the app **connects to 127.0.0.1:8000 and opens a window** (or fails on the display) — it ignores the variable entirely, because nothing reads it. That is the current behaviour this task replaces. + +- [ ] **Step 2: Add the flag** + +In `src/config.hpp`, inside `struct Config`, after the `interface_addr` member: + +```cpp + /// Whether `--server` was given. Absent, the application asks which source to open rather + /// than silently taking the default above -- so this is "did the user choose", not "is + /// `server` valid". The default stays a usable value for every other path. + bool server_specified = false; +``` + +In `src/config.cpp`, in the `--server` branch: + +```cpp + } else if (a == "--server" || a.rfind("--server=", 0) == 0) { + c.server = value_of(argc, argv, i, "--server"); + c.server_specified = true; +``` + +- [ ] **Step 3: Create the header** + +`src/source_dialog.hpp`: + +```cpp +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Martino Ferrari + +#pragma once + +#include "config.hpp" + +namespace udpscope { + +/// Asks which data source to open: a list of the sources with a stored session, and a form for a +/// new one. Returns true with `cfg`'s connection fields filled in, or false if the user declined +/// -- in which case `cfg` is untouched and the caller should exit. +/// +/// Runs *before* `ScopeModel` exists, so declining costs no buffer and no socket. It drives its +/// own `gtk_init()` and nested `GMainLoop`; `run_ui()`'s `GtkApplication` comes afterwards in the +/// same process, which works because `gtk_init()` is idempotent. +bool choose_source(Config& cfg); + +} // namespace udpscope +``` + +- [ ] **Step 4: Create the implementation** + +`src/source_dialog.cpp`: + +```cpp +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Martino Ferrari + +#include "source_dialog.hpp" + +#include "layout_store.hpp" + +#include + +#include +#include +#include + +namespace udpscope { +namespace { + +/// A chooser row's source becomes the connection to open. The interface comes along with the +/// group: it is the local address the join happens on, so storing one without the other would +/// reconnect to the right group on the wrong path. +void apply_source(Config& cfg, const KnownSource& src) { + cfg.server = src.host; + cfg.port = src.port; + cfg.multicast = src.multicast; + cfg.interface_addr = src.interface_addr; +} + +/// One line per known source on stderr, in the style of the probes in `ui.cpp`. +void report_sources(const std::vector& known) { + std::fprintf(stderr, "[sources] %zu known:", known.size()); + for (const KnownSource& s : known) { + std::fprintf(stderr, " %s:%u plots=%d signals=%d", s.host.c_str(), s.port, s.rows, s.signals); + if (!s.multicast.empty()) std::fprintf(stderr, " multicast=%s", s.multicast.c_str()); + if (!s.interface_addr.empty()) { + std::fprintf(stderr, " interface=%s", s.interface_addr.c_str()); + } + std::fprintf(stderr, " |"); + } + std::fprintf(stderr, "\n"); + std::fflush(stderr); +} + +} // namespace + +bool choose_source(Config& cfg) { + const std::vector known = known_sources(); + + // The discovery half, checkable with no display at all: this returns before `gtk_init()`, so + // it asserts what the list found without needing an X server or a GL context. False means the + // caller exits 0, which is what "list and exit" wants. + if (g_getenv("UDPSCOPE_PROBE_SOURCES") != nullptr) { + report_sources(known); + return false; + } + + gtk_init(); + (void)cfg; + return false; +} + +} // namespace udpscope +``` + +- [ ] **Step 5: Wire it into `main()`** + +In `src/main.cpp`, add the include beside the others: + +```cpp +#include "source_dialog.hpp" +``` + +and replace the `parse_args` block's closing brace through the `ScopeModel` line so it reads: + +```cpp + } catch (const std::exception& e) { + std::fprintf(stderr, "udpscope: %s\n", e.what()); + return 2; + } + + // No `--server` means the user has not said which source to open, so ask. Before the model: + // declining must cost no buffer and no socket, and nothing below has to cope with "no source". + if (!cfg.server_specified && !choose_source(cfg)) return 0; + + udpscope::ScopeModel model(cfg); +``` + +- [ ] **Step 6: Add the source to the build** + +In `CMakeLists.txt`, in `add_executable(udpscope ...)`, after `src/layout_store.cpp`: + +```cmake + src/source_dialog.cpp +``` + +- [ ] **Step 7: Build** + +```bash +cmake --build build +``` + +Expected: no new warnings, no errors. + +- [ ] **Step 8: Run the test to verify it passes** + +```bash +UDPSCOPE_PROBE_SOURCES=1 ./build/udpscope; echo "exit=$?" +``` + +Expected: one `[sources] N known: ...` line, `exit=0`, **no window and no X server needed**. The listed entries must be sources with configured signals only — none of the `9561`-style probe ports. + +- [ ] **Step 9: Verify `--server` still bypasses the chooser** + +```bash +python3 tools/mock_udpstreamer.py --port 9701 --duration 20 >/dev/null 2>&1 & +sleep 1 +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=6000 UDPSCOPE_ENABLE_ALL_MS=1000 \ + ./build/udpscope --server 127.0.0.1 --port 9701 2>&1 | grep -E 'frames=|gaps=|x-desync' +``` + +Expected: the usual dump, `gaps=0`, `x-desync 0` — a run with `--server` must not notice this feature exists. + +- [ ] **Step 10: Kill the mock** + +```bash +pkill -f 'mock_udpstream[e]r'; sleep 1; pgrep -f 'mock_udpstream[e]r' || echo "stopped" +``` + +Expected: `stopped`. + +- [ ] **Step 11: Commit** + +```bash +git add src/config.hpp src/config.cpp src/source_dialog.hpp src/source_dialog.cpp \ + src/main.cpp CMakeLists.txt +git commit -m "ask which source to open when --server is absent + +The chooser runs before ScopeModel exists, so declining costs no buffer and no +socket. UDPSCOPE_PROBE_SOURCES=1 prints what the discovery found and exits, +before gtk_init(), so the list is checkable with no display." +``` + +--- + +### Task 5: The chooser window + +**Files:** +- Modify: `src/source_dialog.cpp` (replace the `gtk_init(); (void)cfg; return false;` stub) +- Test: a hand-written session file plus a run with no `--server` + +**Interfaces:** +- Consumes: `apply_source()`, `report_sources()`, `known_sources()`, `forget_source()`. +- Produces: `struct Chooser` and `void chooser_populate(Chooser&)`, both in the anonymous namespace. Task 6 adds the form's widgets to `Chooser` and calls `chooser_finish(c, true)` from it. + +- [ ] **Step 1: Write the failing test** + +Create a known source by hand — a stored multicast session, which is the case no live network is needed to check: + +```bash +mkdir -p ~/.config/udpscope +cat > ~/.config/udpscope/10.0.0.5_8001.toml <<'EOF' +# udpscope session layout (written by the app; safe to edit) +window_s = 2 +plots = 1 +multicast = "239.1.2.3" +interface = "10.0.0.9" + +[[plot]] +signals = ["sine", "", "", ""] +EOF +xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=800 ./build/udpscope 2>&1 | grep '\[sources\]' +echo "exit=$?" +``` + +Expected: **no `[sources] picked` line** — nothing reads `UDPSCOPE_PROBE_PICK_MS`, the stub returns false and the process exits 0 having shown nothing. + +- [ ] **Step 2: Replace the stub with the chooser** + +In `src/source_dialog.cpp`, inside the anonymous namespace, after `report_sources()`: + +```cpp +/// The chooser's own state for the life of its nested loop. A plain local in `choose_source()`: +/// the loop runs inside that call, so nothing here outlives the stack frame and there is nothing +/// to free on a handler path. +struct Chooser { + Config* cfg = nullptr; + GMainLoop* loop = nullptr; + bool chosen = false; + GtkWidget* window = nullptr; + GtkWidget* list = nullptr; + std::vector known; +}; + +void chooser_finish(Chooser& c, bool chosen) { + c.chosen = chosen; + if (c.window != nullptr) { + GtkWidget* w = c.window; + c.window = nullptr; // before the destroy: `close-request` must not re-enter this + gtk_window_destroy(GTK_WINDOW(w)); + } + if (c.loop != nullptr) g_main_loop_quit(c.loop); +} + +void chooser_populate(Chooser& c); + +void on_row_activated(GtkListBox*, GtkListBoxRow* row, gpointer user) { + Chooser& c = *static_cast(user); + const int idx = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(row), "source-index")); + if (idx < 0 || static_cast(idx) >= c.known.size()) return; + apply_source(*c.cfg, c.known[static_cast(idx)]); + chooser_finish(c, true); +} + +void on_forget_clicked(GtkButton* button, gpointer user) { + Chooser& c = *static_cast(user); + const int idx = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(button), "source-index")); + if (idx < 0 || static_cast(idx) >= c.known.size()) return; + forget_source(c.known[static_cast(idx)]); + // Repopulate rather than remove one row: the rows carry their index into `known`, and dropping + // an entry renumbers every later one. Re-reading is also the honest answer to "did it go". + chooser_populate(c); +} + +/// Fills the list box from `known_sources()`. Called again after a forget. +void chooser_populate(Chooser& c) { + while (GtkWidget* child = gtk_widget_get_first_child(c.list)) { + gtk_list_box_remove(GTK_LIST_BOX(c.list), child); + } + c.known = known_sources(); + + if (c.known.empty()) { + GtkWidget* empty = gtk_label_new("No saved sources yet."); + gtk_widget_add_css_class(empty, "dim-label"); + gtk_widget_set_margin_top(empty, 24); + gtk_widget_set_margin_bottom(empty, 24); + // Not a `GtkListBoxRow`, so it cannot be activated into a source that does not exist. + gtk_list_box_append(GTK_LIST_BOX(c.list), empty); + gtk_list_box_set_selection_mode(GTK_LIST_BOX(c.list), GTK_SELECTION_NONE); + return; + } + gtk_list_box_set_selection_mode(GTK_LIST_BOX(c.list), GTK_SELECTION_SINGLE); + + for (std::size_t i = 0; i < c.known.size(); ++i) { + const KnownSource& s = c.known[i]; + GtkWidget* row = gtk_list_box_row_new(); + g_object_set_data(G_OBJECT(row), "source-index", GINT_TO_POINTER(static_cast(i))); + + GtkWidget* line = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12); + gtk_widget_set_margin_start(line, 12); + gtk_widget_set_margin_end(line, 6); + gtk_widget_set_margin_top(line, 6); + gtk_widget_set_margin_bottom(line, 6); + + GtkWidget* text = gtk_box_new(GTK_ORIENTATION_VERTICAL, 2); + gtk_widget_set_hexpand(text, TRUE); + + // The *server*, even for a multicast session: that is the host the file is named after and + // the one CONFIG is fetched from. Titling the row by the group would name it after something + // that is not its identity. + const std::string title = s.host + ":" + std::to_string(s.port); + GtkWidget* title_label = gtk_label_new(title.c_str()); + gtk_widget_set_halign(title_label, GTK_ALIGN_START); + gtk_box_append(GTK_BOX(text), title_label); + + std::string sub = std::to_string(s.rows) + (s.rows == 1 ? " plot, " : " plots, ") + + std::to_string(s.signals) + (s.signals == 1 ? " signal" : " signals"); + if (!s.multicast.empty()) sub += ", multicast " + s.multicast; + GtkWidget* sub_label = gtk_label_new(sub.c_str()); + gtk_widget_add_css_class(sub_label, "dim-label"); + gtk_widget_set_halign(sub_label, GTK_ALIGN_START); + gtk_box_append(GTK_BOX(text), sub_label); + gtk_box_append(GTK_BOX(line), text); + + GtkWidget* forget = gtk_button_new_from_icon_name("window-close-symbolic"); + gtk_widget_set_tooltip_text(forget, "Forget this source (deletes its saved session)"); + gtk_widget_add_css_class(forget, "flat"); + gtk_widget_set_valign(forget, GTK_ALIGN_CENTER); + g_object_set_data(G_OBJECT(forget), "source-index", GINT_TO_POINTER(static_cast(i))); + g_signal_connect(forget, "clicked", G_CALLBACK(on_forget_clicked), &c); + gtk_box_append(GTK_BOX(line), forget); + + gtk_list_box_row_set_child(GTK_LIST_BOX_ROW(row), line); + gtk_list_box_append(GTK_LIST_BOX(c.list), row); + } +} + +gboolean on_chooser_close(GtkWindow*, gpointer user) { + Chooser& c = *static_cast(user); + c.window = nullptr; // GTK destroys it for us on the default handler + chooser_finish(c, false); + return FALSE; +} + +gboolean on_chooser_key(GtkEventControllerKey*, guint keyval, guint, GdkModifierType, + gpointer user) { + if (keyval != GDK_KEY_Escape) return FALSE; + chooser_finish(*static_cast(user), false); + return TRUE; +} + +/// `UDPSCOPE_PROBE_PICK_MS`: activate the first row, the way a click does. +gboolean pick_probe(gpointer user) { + Chooser& c = *static_cast(user); + GtkListBoxRow* row = gtk_list_box_get_row_at_index(GTK_LIST_BOX(c.list), 0); + if (row == nullptr) { + std::fprintf(stderr, "[sources] picked nothing: the list is empty\n"); + std::fflush(stderr); + chooser_finish(c, false); + return G_SOURCE_REMOVE; + } + // `row-activated`, not `gtk_widget_activate()`: this project has already learned that + // activating a widget takes a path a real click does not (a `GtkButton` holds for its 250 ms + // ACTIVATE_TIMEOUT and then emits into a widget a rebuild has destroyed). + g_signal_emit_by_name(c.list, "row-activated", row); + std::fprintf(stderr, "[sources] picked %s:%u multicast=%s interface=%s\n", + c.cfg->server.c_str(), c.cfg->port, c.cfg->multicast.c_str(), + c.cfg->interface_addr.c_str()); + std::fflush(stderr); + return G_SOURCE_REMOVE; +} +``` + +- [ ] **Step 3: Build the window in `choose_source()`** + +Replace `gtk_init(); (void)cfg; return false;` with: + +```cpp + gtk_init(); + + Chooser c; + c.cfg = &cfg; + c.window = gtk_window_new(); + gtk_window_set_title(GTK_WINDOW(c.window), "udpscope \xe2\x80\x94 choose a source"); + gtk_window_set_default_size(GTK_WINDOW(c.window), 420, 320); + + GtkWidget* box = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0); + gtk_window_set_child(GTK_WINDOW(c.window), box); + + GtkWidget* scroller = gtk_scrolled_window_new(); + gtk_widget_set_vexpand(scroller, TRUE); + gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(scroller), GTK_POLICY_NEVER, + GTK_POLICY_AUTOMATIC); + c.list = gtk_list_box_new(); + gtk_list_box_set_activate_on_single_click(GTK_LIST_BOX(c.list), TRUE); + g_signal_connect(c.list, "row-activated", G_CALLBACK(on_row_activated), &c); + gtk_scrolled_window_set_child(GTK_SCROLLED_WINDOW(scroller), c.list); + gtk_box_append(GTK_BOX(box), scroller); + + GtkWidget* actions = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 6); + gtk_widget_set_margin_start(actions, 12); + gtk_widget_set_margin_end(actions, 12); + gtk_widget_set_margin_top(actions, 12); + gtk_widget_set_margin_bottom(actions, 12); + gtk_widget_set_halign(actions, GTK_ALIGN_END); + GtkWidget* new_host = gtk_button_new_with_label("New host\xe2\x80\xa6"); + GtkWidget* cancel = gtk_button_new_with_label("Cancel"); + g_signal_connect(cancel, "clicked", G_CALLBACK(+[](GtkButton*, gpointer user) { + chooser_finish(*static_cast(user), false); + }), + &c); + gtk_box_append(GTK_BOX(actions), new_host); + gtk_box_append(GTK_BOX(actions), cancel); + gtk_box_append(GTK_BOX(box), actions); + + chooser_populate(c); + if (c.known.empty()) gtk_widget_grab_focus(new_host); + + g_signal_connect(c.window, "close-request", G_CALLBACK(on_chooser_close), &c); + GtkEventController* keys = gtk_event_controller_key_new(); + g_signal_connect(keys, "key-pressed", G_CALLBACK(on_chooser_key), &c); + gtk_widget_add_controller(c.window, keys); + + gtk_window_present(GTK_WINDOW(c.window)); + + if (const char* env = g_getenv("UDPSCOPE_PROBE_PICK_MS")) { + g_timeout_add(static_cast(std::atoi(env)), pick_probe, &c); + } + + c.loop = g_main_loop_new(nullptr, FALSE); + g_main_loop_run(c.loop); + g_main_loop_unref(c.loop); + c.loop = nullptr; + return c.chosen; +``` + +Add `#include ` to the includes for `std::atoi`. + +- [ ] **Step 4: Build** + +```bash +cmake --build build +``` + +Expected: no new warnings, no errors. If `+[](GtkButton*, gpointer)` does not convert, the lambda has captured something — it must not. + +- [ ] **Step 5: Run the test to verify it passes** + +```bash +xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=800 UDPSCOPE_EXIT_AFTER_MS=4000 \ + ./build/udpscope 2>&1 | grep -E '\[sources\]|layout loaded' +``` + +Expected: `[sources] picked 10.0.0.5:8001 multicast=239.1.2.3 interface=10.0.0.9` — the stored group and interface came back, which is the assertion that a multicast row reconnects without any multicast networking in the test. The run then continues into the real application (it will not receive data; that is fine and not what this checks). + +- [ ] **Step 6: Check the empty-list path** + +```bash +XDG_CONFIG_HOME=/tmp/no-such-config xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=600 \ + ./build/udpscope 2>&1 | grep '\[sources\]'; echo "exit=$?" +``` + +Expected: `[sources] picked nothing: the list is empty`, and the process exits 0 — an empty list must not crash on `gtk_list_box_get_row_at_index(.., 0)` returning null. + +- [ ] **Step 7: Check the forget path** + +```bash +ls ~/.config/udpscope/10.0.0.5_8001.toml && \ +UDPSCOPE_PROBE_SOURCES=1 ./build/udpscope 2>&1 | grep -c '10.0.0.5:8001' +``` + +Expected: the file exists and the count is `1`. Then confirm `forget_source()` is reachable and correct by deleting through it in the one-off from Task 2 (`/tmp/check_sources`), which already asserts it; **do not** add a probe that deletes a real session file. + +- [ ] **Step 8: Verify a `--server` run is still untouched** + +```bash +python3 tools/mock_udpstreamer.py --port 9702 --duration 20 >/dev/null 2>&1 & +sleep 1 +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=6000 UDPSCOPE_ENABLE_ALL_MS=1000 \ + ./build/udpscope --server 127.0.0.1 --port 9702 2>&1 \ + | grep -E 'frames=|gaps=|x-desync|trace ink' +``` + +Expected: the usual dump, `gaps=0`, `x-desync 0`, `0 collapsed, 0 staircase`. + +- [ ] **Step 9: Kill the mock** + +```bash +pkill -f 'mock_udpstream[e]r'; sleep 1; pgrep -f 'mock_udpstream[e]r' || echo "stopped" +``` + +Expected: `stopped`. + +- [ ] **Step 10: Commit** + +```bash +git add src/source_dialog.cpp +git commit -m "the source chooser's list + +One row per stored session, titled by its server -- the host the file is named +after and the one CONFIG comes from -- with the group as subtitle. A forget +repopulates rather than removing one row, because the rows carry their index +into the list and dropping one renumbers the rest." +``` + +--- + +### Task 6: The connection form + +**Files:** +- Modify: `src/source_dialog.cpp` (fields on `Chooser`, the form, the `[New host…]` handler) +- Test: a run with no `--server` driving the form instead of a row + +**Interfaces:** +- Consumes: `Chooser`, `chooser_finish()` (Task 5). +- Produces: nothing used elsewhere. This completes the feature. + +- [ ] **Step 1: Write the failing test** + +```bash +xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=800 UDPSCOPE_PROBE_NEWHOST=10.0.0.7:8123 \ + ./build/udpscope 2>&1 | grep '\[sources\]' +``` + +Expected: `[sources] picked 10.0.0.5:8001 ...` — the variable is ignored and row 0 is taken instead. + +- [ ] **Step 2: Add the form's widgets to `Chooser`** + +In `struct Chooser`, after `std::vector known;`: + +```cpp + /// The connection form, created on demand. Members of the chooser rather than a heap struct: + /// the chooser outlives the form by construction, so there is nothing to free on a close path + /// -- and a dialog whose context is freed by a handler is how this project earned a use-after + /// -free in the math editor. + GtkWidget* form = nullptr; + GtkWidget* form_server = nullptr; + GtkWidget* form_port = nullptr; + GtkWidget* form_group = nullptr; + GtkWidget* form_iface = nullptr; + GtkWidget* form_connect = nullptr; +``` + +- [ ] **Step 3: Add the form's handlers** + +In the anonymous namespace, after `chooser_populate()`: + +```cpp +void form_close(Chooser& c) { + if (c.form == nullptr) return; + GtkWidget* w = c.form; + c.form = nullptr; // before the close, so the handler cannot re-enter + gtk_window_close(GTK_WINDOW(w)); +} + +void on_form_server_changed(GtkEditable* entry, gpointer user) { + Chooser& c = *static_cast(user); + if (c.form_connect == nullptr) return; + // The only validation here. A *malformed* address is the acquisition layer's to report -- it + // already does, and the starvation notice covers a source that takes the socket and says + // nothing -- so checking it twice would be two expressions of one rule. + gtk_widget_set_sensitive(c.form_connect, gtk_entry_buffer_get_length( + gtk_entry_get_buffer(GTK_ENTRY(entry))) > 0); +} + +void on_form_connect(GtkButton*, gpointer user) { + Chooser& c = *static_cast(user); + const char* server = gtk_editable_get_text(GTK_EDITABLE(c.form_server)); + if (server == nullptr || server[0] == '\0') return; + c.cfg->server = server; + c.cfg->port = static_cast( + gtk_spin_button_get_value_as_int(GTK_SPIN_BUTTON(c.form_port))); + c.cfg->multicast = gtk_editable_get_text(GTK_EDITABLE(c.form_group)); + c.cfg->interface_addr = gtk_editable_get_text(GTK_EDITABLE(c.form_iface)); + form_close(c); + chooser_finish(c, true); +} + +/// A labelled row for the form's grid. +GtkWidget* form_field(GtkWidget* grid, int row, const char* label, GtkWidget* widget) { + GtkWidget* l = gtk_label_new(label); + gtk_widget_set_halign(l, GTK_ALIGN_END); + gtk_grid_attach(GTK_GRID(grid), l, 0, row, 1, 1); + gtk_widget_set_hexpand(widget, TRUE); + gtk_grid_attach(GTK_GRID(grid), widget, 1, row, 1, 1); + return widget; +} + +void open_form(Chooser& c) { + if (c.form != nullptr) { + gtk_window_present(GTK_WINDOW(c.form)); + return; + } + c.form = gtk_window_new(); + gtk_window_set_title(GTK_WINDOW(c.form), "New host"); + gtk_window_set_transient_for(GTK_WINDOW(c.form), GTK_WINDOW(c.window)); + gtk_window_set_modal(GTK_WINDOW(c.form), TRUE); + + GtkWidget* box = gtk_box_new(GTK_ORIENTATION_VERTICAL, 12); + gtk_widget_set_margin_start(box, 12); + gtk_widget_set_margin_end(box, 12); + gtk_widget_set_margin_top(box, 12); + gtk_widget_set_margin_bottom(box, 12); + gtk_window_set_child(GTK_WINDOW(c.form), box); + + GtkWidget* grid = gtk_grid_new(); + gtk_grid_set_row_spacing(GTK_GRID(grid), 6); + gtk_grid_set_column_spacing(GTK_GRID(grid), 12); + gtk_box_append(GTK_BOX(box), grid); + + c.form_server = form_field(grid, 0, "Server", gtk_entry_new()); + gtk_editable_set_text(GTK_EDITABLE(c.form_server), "127.0.0.1"); + // A spin button holds no decimals by default, which is right for a port and wrong for every + // other numeric field in this project -- see the math dialog's fields. + c.form_port = form_field(grid, 1, "Port", gtk_spin_button_new_with_range(1, 65535, 1)); + gtk_spin_button_set_value(GTK_SPIN_BUTTON(c.form_port), 8000); + c.form_group = form_field(grid, 2, "Multicast group", gtk_entry_new()); + gtk_entry_set_placeholder_text(GTK_ENTRY(c.form_group), "empty = unicast"); + c.form_iface = form_field(grid, 3, "Interface", gtk_entry_new()); + gtk_entry_set_placeholder_text(GTK_ENTRY(c.form_iface), "empty = default route"); + + GtkWidget* actions = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 6); + gtk_widget_set_halign(actions, GTK_ALIGN_END); + GtkWidget* cancel = gtk_button_new_with_label("Cancel"); + g_signal_connect(cancel, "clicked", G_CALLBACK(+[](GtkButton*, gpointer user) { + form_close(*static_cast(user)); + }), + &c); + c.form_connect = gtk_button_new_with_label("Connect"); + g_signal_connect(c.form_connect, "clicked", G_CALLBACK(on_form_connect), &c); + gtk_box_append(GTK_BOX(actions), cancel); + gtk_box_append(GTK_BOX(actions), c.form_connect); + gtk_box_append(GTK_BOX(box), actions); + + g_signal_connect(c.form_server, "changed", G_CALLBACK(on_form_server_changed), &c); + // `close-request` clears the pointer, and only a *close* emits it: `gtk_window_destroy()` does + // not, which is how the math dialog once kept presenting a finalized window. + g_signal_connect(c.form, "close-request", G_CALLBACK(+[](GtkWindow*, gpointer user) { + static_cast(user)->form = nullptr; + return FALSE; + }), + &c); + + gtk_window_present(GTK_WINDOW(c.form)); + gtk_widget_grab_focus(c.form_server); +} +``` + +- [ ] **Step 4: Replace the `pick_probe` with one that can drive either path** + +Replace the whole `pick_probe()` body's `row == nullptr` branch and add the new-host branch, so the function reads: + +```cpp +gboolean pick_probe(gpointer user) { + Chooser& c = *static_cast(user); + + // `UDPSCOPE_PROBE_NEWHOST=host:port` drives the form instead of a row. The crash surface of a + // dialog built, filled and destroyed is as much the point as the values, the same way + // `UDPSCOPE_PROBE_FIELDS_MS` is in `ui.cpp`. + if (const char* spec = g_getenv("UDPSCOPE_PROBE_NEWHOST")) { + open_form(c); + const std::string s = spec; + const std::size_t colon = s.rfind(':'); + gtk_editable_set_text(GTK_EDITABLE(c.form_server), + colon == std::string::npos ? s.c_str() : s.substr(0, colon).c_str()); + if (colon != std::string::npos) { + gtk_spin_button_set_value(GTK_SPIN_BUTTON(c.form_port), std::atof(s.c_str() + colon + 1)); + } + std::fprintf(stderr, "[sources] form open, connect_sensitive=%d\n", + gtk_widget_get_sensitive(c.form_connect) ? 1 : 0); + g_signal_emit_by_name(c.form_connect, "clicked"); + std::fprintf(stderr, "[sources] picked %s:%u multicast=%s interface=%s\n", + c.cfg->server.c_str(), c.cfg->port, c.cfg->multicast.c_str(), + c.cfg->interface_addr.c_str()); + std::fflush(stderr); + return G_SOURCE_REMOVE; + } + + GtkListBoxRow* row = gtk_list_box_get_row_at_index(GTK_LIST_BOX(c.list), 0); + if (row == nullptr) { + std::fprintf(stderr, "[sources] picked nothing: the list is empty\n"); + std::fflush(stderr); + chooser_finish(c, false); + return G_SOURCE_REMOVE; + } + g_signal_emit_by_name(c.list, "row-activated", row); + std::fprintf(stderr, "[sources] picked %s:%u multicast=%s interface=%s\n", + c.cfg->server.c_str(), c.cfg->port, c.cfg->multicast.c_str(), + c.cfg->interface_addr.c_str()); + std::fflush(stderr); + return G_SOURCE_REMOVE; +} +``` + +`pick_probe()` now calls `open_form()`, so `open_form()` must be defined above it — it is, if you added Step 3's block after `chooser_populate()` and `pick_probe()` stays last. + +- [ ] **Step 5: Wire `[New host…]`** + +In `choose_source()`, after creating `new_host` and before appending it: + +```cpp + g_signal_connect(new_host, "clicked", G_CALLBACK(+[](GtkButton*, gpointer user) { + open_form(*static_cast(user)); + }), + &c); +``` + +- [ ] **Step 6: Build** + +```bash +cmake --build build +``` + +Expected: no new warnings, no errors. + +- [ ] **Step 7: Run the test to verify it passes** + +```bash +xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=800 UDPSCOPE_PROBE_NEWHOST=10.0.0.7:8123 \ + ./build/udpscope 2>&1 | grep '\[sources\]' +``` + +Expected: + +``` +[sources] form open, connect_sensitive=1 +[sources] picked 10.0.0.7:8123 multicast= interface= +``` + +and no SIGSEGV. `connect_sensitive=1` is the assertion that a non-empty server enables `[Connect]`. + +- [ ] **Step 8: Check the empty-server refusal** + +```bash +timeout 10 xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=800 UDPSCOPE_PROBE_NEWHOST= \ + ./build/udpscope 2>&1 | grep '\[sources\]' +``` + +Expected: + +``` +[sources] form open, connect_sensitive=0 +[sources] picked 127.0.0.1:8000 multicast= interface= +``` + +`UDPSCOPE_PROBE_NEWHOST=` is an empty string, which `g_getenv` still returns, so the form opens with the server cleared — `connect_sensitive=0`, and the `picked` line reports the **default** `127.0.0.1:8000` unchanged because `on_form_connect()` returned early on an empty server. Nothing chose a source, so the chooser stays up and `timeout 10` is what ends the run; that is the expected outcome here, not a hang. Confirm nothing survived it: + +```bash +pgrep -f 'build/udpscop[e]' || echo "stopped" +``` + +- [ ] **Step 9: Verify a `--server` run is still untouched** + +```bash +python3 tools/mock_udpstreamer.py --port 9703 --duration 20 >/dev/null 2>&1 & +sleep 1 +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=6000 UDPSCOPE_ENABLE_ALL_MS=1000 \ + ./build/udpscope --server 127.0.0.1 --port 9703 2>&1 \ + | grep -E 'frames=|gaps=|x-desync|trace ink|toolbar icons' +``` + +Expected: the usual dump, `gaps=0`, `x-desync 0`, `0 collapsed, 0 staircase`, every icon `(ok)`. + +- [ ] **Step 10: Kill the mock** + +```bash +pkill -f 'mock_udpstream[e]r'; sleep 1; pgrep -f 'mock_udpstream[e]r' || echo "stopped" +``` + +Expected: `stopped`. + +- [ ] **Step 11: Commit** + +```bash +git add src/source_dialog.cpp +git commit -m "the new-host connection form + +Server, port, multicast group and interface, and nothing else: --buffer-mb and +friends are tuning rather than which source, and a form that asks for them is +one a user has to read first. The form's widgets live on the chooser, which +outlives them, so no close path frees a live context." +``` + +--- + +### Task 7: Documentation + +This project documents a feature in **both** `README.md` (what was built and why) and `CLAUDE.md` (the verification entry and the invariant). A change that is not in both is not finished here. + +**Files:** +- Modify: `CLAUDE.md` (the Verification list, the Invariants list, the Build-and-run options line) +- Modify: `README.md` (a new passage near the session-persistence one at ~line 450) + +**Interfaces:** none. + +- [ ] **Step 1: Add the verification entry** + +In `CLAUDE.md`, in the Verification list, immediately after the `toolbar icons:` entry: + +```markdown +- `[sources] N known: HOST:PORT plots=P signals=S [multicast=G] [interface=I] | ...` (stderr, + `UDPSCOPE_PROBE_SOURCES=1`) — the startup chooser's discovery, printed and exited **before + `gtk_init()`**, so this one needs no display and no `xvfb-run`. **Only sources with a named + signal may appear**: a development machine accumulates one session file per probe run on a + throwaway port (140 of them when this was written), and the filter in `known_sources()` is + what keeps the list the sources a user configured. Order is newest first by file mtime. +- `[sources] picked HOST:PORT multicast=G interface=I` (stderr, `UDPSCOPE_PROBE_PICK_MS`) — + shows the chooser, then activates row 0 the way a click does and continues into the real + application, so a full run with **no `--server`** must reach the scope and emit every other + dump line unchanged. `multicast=`/`interface=` coming back non-empty from a stored row is the + assertion that a multicast session reconnects to its group, and it needs **no multicast + networking**: write the session file by hand and read the line. With + `UDPSCOPE_PROBE_NEWHOST=host:port` it drives the form instead — `[sources] form open, + connect_sensitive=1` then a `picked` line, with no SIGSEGV, which is as much a crash test of a + dialog built and destroyed as it is a state check. An empty list must print `picked nothing` + rather than dereferencing a null row. +``` + +- [ ] **Step 2: Add the invariant** + +In `CLAUDE.md`, in the Invariants list, immediately **before** the `Every source file carries an SPDX MIT header` bullet: + +```markdown +- **The chooser runs before `ScopeModel` exists, and `--server` is what decides.** `Config` carries + `server_specified`, set only by the `--server` branch, and `main()` asks only when it is false — + so declining costs no buffer and no socket, and `ScopeModel`, `Acquisition`, `RowSet` and + `run_ui()` need no notion of "no source yet". It is `--server` alone and not "no arguments": + `--multicast` still needs a server to fetch CONFIG from. **Every documented probe passes + `--server`**, which is what makes the feature invisible to the regression net — a probe added + without it would reach the chooser and hang. `gtk_init()` plus a nested `GMainLoop`, and then + `run_ui()`'s `GtkApplication` afterwards in the same process, is a verified combination + (`gtk_init()` is idempotent; running your own loop is the documented GTK4 replacement for the + removed `gtk_dialog_run()`). +- **A session file's name cannot say how its source was reached, so the file says it.** + `_.toml` carries neither the multicast group nor the interface, so a chooser row + could not reconnect to a multicast source at all. `LayoutState::multicast`/`interface_addr` are + written in the **top block, before the first `[[table]]`** — a bare key after a table header is + that table's key in TOML, which is exactly how `enabled` was parsed into the last + `[[calibration]]` record and lost without a word. A file from an older build has neither key and + loads as unicast on the default route. The chooser's list filters to sessions that **name at + least one signal**, because a probe run leaves a file per throwaway port and all of them hold + `signals = ["", "", "", ""]`; `known_sources()` splits the filename at the **last** `_`, since + `sanitize()` turns anything unsafe in a host into one. +``` + +- [ ] **Step 3: Note it in Build and run** + +In `CLAUDE.md`, in the Build-and-run section, after the `Options:` paragraph: + +```markdown +Run with no `--server` and udpscope asks which source to open: the sessions it has stored, plus a +form for a new host. Every probe below passes `--server`, so none of them sees it. +``` + +- [ ] **Step 4: Write the README passage** + +In `README.md`, immediately **after** the paragraph beginning `**The session is saved with the layout**`, add: + +```markdown +**Launched with no `--server`, udpscope asks which source to open.** It used to connect to +`127.0.0.1:8000` because that is the default in `Config` — fine for one streamer, useless for +several, and there was no way to see which sources the app already knew about. The chooser lists +one row per stored session: the server and port it was named after, how many plots and signals it +held, and its multicast group when it had one, newest first. `New host…` is a four-field form — +server, port, multicast group, interface — and nothing more: `--buffer-mb` and the rest are tuning +rather than "which source", and a form that asks for them is a form you have to read before using +the app. Cancel exits before anything is allocated, because declining to choose a source is not a +failure. + +It runs **before the model exists**, which is the whole reason it is cheap: no buffer, no socket +and no window until a source is known, and `ScopeModel`, `Acquisition` and `run_ui()` are +untouched by the feature. Every headless probe passes `--server`, so none of them reaches it. + +Two things the list had to get right. A session file is named `_.toml` and that name +cannot say whether the source was multicast or which interface it joined on, so the file now +records both — in the top block, before the first table, because a bare key written after a +`[[table]]` header belongs to that table in TOML, which is how the `enabled` key was silently lost +once already. And the list is **filtered to sessions that name a signal**: a development machine +accumulates one file per probe run on a throwaway port — 140 of them here, every one with +`signals = ["", "", "", ""]` — so an unfiltered chooser opens as a wall of ports nobody chose. A +row's `×` forgets a source by deleting its session file, with no confirmation: the gesture is the +confirmation, and what it destroys is one layout the app rewrites on the next clean exit. +``` + +- [ ] **Step 5: Check both files read correctly** + +```bash +grep -n 'sources\]' CLAUDE.md | head +grep -n 'no .--server., udpscope asks' README.md +``` + +Expected: the two Verification entries and the README passage are present. + +- [ ] **Step 6: Commit** + +```bash +git add CLAUDE.md README.md +git commit -m "document the source chooser + +Both files, per this project's rule: the probe entries and the two invariants +in CLAUDE.md, the feature and why the list is filtered in README." +``` + +--- + +## Final verification + +Run these in order, one at a time, after Task 7. + +- [ ] **A clean build from scratch** + +```bash +cmake -S . -B /tmp/udpscope-clean -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo \ + && cmake --build /tmp/udpscope-clean 2>&1 | grep -iE 'warning|error' | grep -v 'output_built\|missing-field-initializers' ; echo "new warnings above (none expected)" +``` + +- [ ] **The two one-offs still pass** + +```bash +/tmp/check_layout_rt && /tmp/check_sources +``` + +Expected: `all ok` from both. + +- [ ] **The discovery probe** + +```bash +UDPSCOPE_PROBE_SOURCES=1 ./build/udpscope +``` + +Expected: one `[sources] N known:` line, no probe-port entries, exit 0. + +- [ ] **The full pick-and-run** + +```bash +xvfb-run -a env UDPSCOPE_PROBE_PICK_MS=800 UDPSCOPE_EXIT_AFTER_MS=6000 \ + ./build/udpscope 2>&1 | grep -E '\[sources\]|layout loaded|frames=' +``` + +- [ ] **A `--server` regression run, which must be identical to before this work** + +```bash +python3 tools/mock_udpstreamer.py --rate 1300000 --batch 2000 --duration 30 --port 9704 >/dev/null 2>&1 & +sleep 1 +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=12000 UDPSCOPE_ENABLE_ALL_MS=1000 \ + UDPSCOPE_PROBE_MEASURES_MS=2000 UDPSCOPE_PROBE_CURSORS_MS=3500 \ + ./build/udpscope --server 127.0.0.1 --port 9704 --buffer-mb 64 2>&1 \ + | grep -E 'frames=|gaps=|x-desync|trace ink|toolbar icons|badge frames' +``` + +Expected: `gaps=0`, `x-desync 0`, `0 collapsed, 0 staircase`, every icon `(ok)`, both `lit` and `dim` non-zero. + +- [ ] **Kill the mock** + +```bash +pkill -f 'mock_udpstream[e]r'; sleep 1; pgrep -f 'mock_udpstream[e]r' || echo "stopped" +``` + +- [ ] **Remove the task's throwaway session files** + +```bash +rm -f ~/.config/udpscope/10.0.0.5_8001.toml ~/.config/udpscope/127.0.0.1_970*.toml \ + ~/.config/udpscope/127.0.0.1_9999.toml +rm -rf /tmp/udpscope-clean /tmp/check-sources-home +```