Design of iteration 2: MQTT client, HA device, app commands

libmosquitto's own thread (D42), app commands in the platform for --ha-remove (D43), committed TLS test certificates (D44). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-06 14:40:42 +02:00
parent cd62e57183
commit 7810db5cbe
2 changed files with 168 additions and 10 deletions
@@ -1,7 +1,8 @@
 # Design

-Status: **in progress**, iteration 1, phase 2 (planning and design). High-level architecture
-and MVP details done; next: platform changes and the implementation plan. Requirements and their decisions (D1–D15):
+Status: **in progress**. High-level architecture and MVP details done; iteration 1
+implemented. Iteration 2 (MQTT, HA device, UI-1): design approved 2026-10-06 ("Iteration 2:
+MQTT and HA device"), phase 3 (implementation). Requirements and their decisions (D1–D15):
 [REQUIREMENTS.md](REQUIREMENTS.md). Design decisions start at D16.

 ## Process and threads
@@ -14,7 +15,7 @@ and MVP details done; next: platform changes and the implementation plan. Requir
  | Thread | Runs |
  |---|---|
  | main | Qt event loop, QML, off-screen rendering of all surfaces, SteamVR event polling and texture submission, **all OpenVR calls** (I/O wrappers), HA device and entities, managers |
-  | MQTT | the MQTT client (libmosquitto): DNS lookup, TCP and TLS connect block for seconds when the broker is unreachable |
+  | MQTT | libmosquitto's network loop (`mosquitto_loop_start()`, a thread libmosquitto creates and manages, D42): DNS lookup, TCP and TLS connect block for seconds when the broker is unreachable |

  - OpenVR stays in the main thread: it isn't documented as thread-safe, and all events come
    through one queue.
@@ -27,8 +28,9 @@ and MVP details done; next: platform changes and the implementation plan. Requir
    HA device ◀── connectionChanged(state, error) ── MQTT client
    ```

-  - The MQTT client lives in its thread via `moveToThread()`; it creates its socket notifiers
-    and timers there, after the move.
+  - The MQTT client object lives in the main thread; libmosquitto calls its callbacks in its
+    own thread, and they only emit these signals with copied values (D42, details in
+    "Iteration 2: MQTT and HA device").
  - Not needed as own threads: WebEngine (own processes), Qt Multimedia and QtNetwork
    (asynchronous), later D-Bus and `QProcess` for network/audio (asynchronous). Rendering stays
    in the main thread as long as NFR-3 holds.
@@ -184,7 +186,7 @@ Notes for later:

 ## Open

-1. Detailed plan for implementation iteration 1 (see [PROGRESS.md](PROGRESS.md), "Next steps").
+1. Nothing open for iteration 2; points to verify are listed in its section.

 ## HA MQTT interface

@@ -260,6 +262,154 @@ Layout follows common practice (e.g. Zigbee2MQTT's `…/<device>/set`); one topi
 - Display time: one value from the configuration, default 10 s (Appendix A). Per-notification
  durations come with the JSON command.

+## Iteration 2: MQTT and HA device
+
+Implements the iteration 2 requirements ([PROGRESS.md](PROGRESS.md), "Next steps"; D41).
+
+### Components
+
+```
+main thread                                             libmosquitto thread
+Dashboard (app)
+ ├─ ConnectionStatus (QML singleton, UI-1) ◀─ signals ─┐
+ ├─ HaDevice ── publish/subscribe ──▶ MqttClient ◀─────┼── callbacks (connect, message, disconnect)
+ │   └─ HaSensor "uptime" ◀── QTimer 10 s (app)        │
+ └─ Config (immutable value, D19)                      │
+```
+
+MQTT and HA classes live in `frameassistant_lib` (tests link them); the platform stays free of
+MQTT (D17).
+
+| Class | File | Role |
+|---|---|---|
+| `MqttClient` | `src/mqtt/MqttClient.h` | **Interface** (QObject): `connectToBroker()`, `publish(topic, payload, retain)`, `subscribe(topic)`; signals `connectionChanged(state, error)`, `messageReceived(topic, payload)`. Tests use a `FakeMqttClient` |
+| `MosquittoClient` | `src/mqtt/MosquittoClient.*` | Implementation on libmosquitto: login, TLS, last will, reconnect with backoff |
+| `HaDevice` | `src/ha/HaDevice.*` | One HA device (D18): discovery, availability, republish on reconnect and HA birth, `removeFromHa()`. Owns its entities, uses an `MqttClient*` |
+| `HaEntity`, `HaSensor` | `src/ha/HaEntity.*`, `HaSensor.*` | Base class (D23): description (id, name, device class, unit, category, throttle interval), discovery component, state and availability topics, throttling (SEN-14). `HaSensor::setValue()` / `clearValue()` (unavailable, SEN-15) |
+| `ConnectionStatus` | `src/ConnectionStatus.*` (app, QML module) | QML singleton for UI-1: device name, broker, state (`Disconnected`/`Connecting`/`Connected`), last error. Member of `Dashboard` |
+
+### MQTT client (D42)
+
+- **Thread:** `mosquitto_connect_async()` plus `mosquitto_loop_start()`. libmosquitto creates
+  and manages its own thread (a pthread, not a `QThread`, no Qt event loop) for connect,
+  network traffic, keep-alive and reconnect, and calls our callbacks there.
+- **Callbacks only emit signals** with copied values. The `MosquittoClient` object lives in the
+  main thread, so Qt delivers them queued (D29's rules hold).
+- **Calls from the main thread** (`mosquitto_publish()`, `mosquitto_subscribe()`) are safe
+  while the loop runs: libmosquitto guards its state with mutexes. Needs a libmosquitto with
+  thread support (Debian, Arch); otherwise `mosquitto_loop_start()` returns
+  `MOSQ_ERR_NOT_SUPPORTED`, which is logged as an error.
+- **Client id** `frameassistant_<device id>`: two instances with the same id knock each other
+  off the broker (expected, D22). MQTT 3.1.1, QoS 1 for everything.
+- **Last will:** `offline`, retained, on `frameassistant/<id>/availability`. A killed process
+  (also `pkill` from `install.sh`) closes its TCP connection, so the broker sends the will at
+  once.
+- **Normal exit:** publish `offline`, wait up to 2 s for its acknowledgement,
+  `mosquitto_disconnect()`, `mosquitto_loop_stop()` (joins the thread), `mosquitto_destroy()`.
+  After that no callback can arrive.
+- **Reconnect (CON-4):** `mosquitto_reconnect_delay_set(1 s, 60 s, exponential)`. Nothing is
+  buffered.
+- **TLS:** `mosquitto_tls_set()` with `ca_file`; with an empty `ca_file`,
+  `MOSQ_OPT_TLS_USE_OS_CERTS` (libmosquitto ≥ 2.0). Client certificate if configured.
+- **Logging:** every connection state change and error with its reason (`qInfo`/`qWarning`, so
+  also in the log file).
+- To verify when implementing: whether the loop also retries when the *first*
+  `connect_async` fails (e.g. DNS error); if not, `MosquittoClient` retries that case itself.
+
+### HA device
+
+- **After each connect:** subscribe to `homeassistant/status`, then publish discovery, device
+  `online`, and every entity's state and availability.
+- **HA birth message** (`homeassistant/status` = `online`): republish after a random 1–5 s
+  delay (D32).
+- **Dummy sensor (D41):** `HaSensor` "uptime", seconds since start, fed by a 10 s `QTimer` in
+  the app.
+- **Discovery payload** (example; manufacturer and model come from OpenVR in iteration 3):
+
+  ```json
+  {
+    "dev": {"ids": ["frameassistant_wohnzimmer_frame"], "name": "Wohnzimmer Frame", "sw": "0.1.0"},
+    "o": {"name": "FrameAssistant", "sw": "0.1.0"},
+    "~": "frameassistant/wohnzimmer_frame",
+    "cmps": {
+      "uptime": {
+        "p": "sensor", "name": "Uptime",
+        "uniq_id": "frameassistant_wohnzimmer_frame_uptime",
+        "dev_cla": "duration", "unit_of_meas": "s", "ent_cat": "diagnostic",
+        "stat_t": "~/uptime/state",
+        "avty": [{"t": "~/availability"}, {"t": "~/uptime/availability"}], "avty_mode": "all"
+      }
+    },
+    "qos": 1
+  }
+  ```
+
+- To verify when implementing: that HA accepts the `~` base topic in device-based discovery.
+
+### Modes
+
+- The platform gets `DashboardMode::DesktopNoVr` (`--desktop-novr`). In iteration 2 it behaves
+  like `--desktop` (neither uses SteamVR yet; D22's optional SteamVR for `--desktop` comes in
+  iteration 3).
+- `Dashboard::prepare()` loads the settings for overlay, desktop and desktop-novr.
+  `HaDevice` is created in `loading()` and connects in `started()`.
+
+### App commands (D43)
+
+Today the platform hard-codes its manifest commands: the base constructor scans the raw
+`argv` for `--install-manifest`/`--uninstall-manifest`, creates a `QCoreApplication` instead of
+a `QGuiApplication` (no display needed, works over SSH), and `exec()` calls
+`runManifestCommand()`. This is opened up to the app:
+
+- The base constructor takes the app's **command options** in addition to its other options
+  (D38). The raw-`argv` scan includes their names, so they also get a `QCoreApplication`.
+- They are parsed with all other options: `--config`, `--log-file` work, `--help` lists them.
+- `exec()`: after the manifest check, an app command runs `prepare()` and then a new hook
+  `virtual int runCommand(const QString& name)`, whose result is the exit code. No engine,
+  surfaces or SteamVR.
+- Moving the manifest commands onto this mechanism is possible later, not part of iteration 2.
+
+### `--ha-remove`
+
+Against "ghost entities" (D32): retained discovery and states stay on the broker after an
+uninstall, so HA would show the device as unavailable forever (and recreate it after a manual
+delete).
+
+- An app command (D43): loads the settings, connects, publishes **empty retained payloads**
+  to discovery (HA then deletes the device and its entities), device availability, and each
+  entity's state and availability topics; disconnects.
+- Exit code 0 on success; ≠ 0 with a warning if the broker can't be reached within 10 s.
+- `uninstall.sh`: `pkill` → `sleep 1` → `--ha-remove` (a failure doesn't stop the uninstall) →
+  `--uninstall-manifest`.
+- Also usable by hand, e.g. after a device name change without `ha_unique_id`:
+  `--ha-remove --config <old settings>`.
+- It doesn't keep the device away: the next start publishes everything again.
+
+### Tests
+
+| Test | Content |
+|---|---|
+| `tst_haentity` | discovery component, topics, throttling (several values within an interval → one publish with the last value), unavailable without a value |
+| `tst_hadevice` (fake client) | discovery payload, `online` and states after connect, republish after reconnect and birth, will configuration, `removeFromHa()` |
+| `tst_mqtt` (integration; real `mosquitto` on a free port, QSKIP without it) | connect; login right and wrong; TLS with a test CA; reconnect after a broker restart; last will after `kill -9` of a small helper process |
+| `tst_surfaces` | `--desktop-novr`; app command: hook called, no surface created, exit code returned |
+| Manual | from the PC against the user's HA: device appears, online/offline, broker restart, two devices, `--ha-remove`, UI-1 in `--desktop`. On the Frame: the package with libmosquitto runs |
+
+- TLS test certificates (CA, server certificate and key, 100-year validity) are committed to
+  `tests/data/` (D44).
+
+### Implementation order
+
+Each step builds and has its tests green before the next:
+
+1. libmosquitto in CMake and the containers (D35)
+2. `MqttClient` + `MosquittoClient` + integration test
+3. `HaEntity`/`HaSensor`/`HaDevice` + unit tests
+4. App wiring, `--desktop-novr`, UI-1
+5. TLS
+6. App commands, `--ha-remove`, `uninstall.sh`
+7. Packaging check (`readelf -V`), docs
+
 ## Configuration file (D34)

 - Location: `~/.config/frameassistant/config.json` (`$XDG_CONFIG_HOME` respected); `--config
@@ -434,7 +584,7 @@ Collected 2026-10-05 for the notification design.
 | D25 | 2026-10-05 | Two overlay kinds in the platform: dashboard tabs and world overlays (MVP: one placement, not interactive). Set `VROverlayFlags_IsPremultiplied`. Runtime surfaces and interactive world overlays later | Notifications need world overlays; Qt Quick renders premultiplied alpha |
 | D26 | 2026-10-05 | One world overlay for all notifications; QML lays out the cards | No runtime surfaces, fewer resources; per-notification placement isn't required |
 | D27 | 2026-10-05 | MVP notification placement: spawn in front (about 1 m, slightly below the line of sight), then stay | Research favours non-head-locked placement; no continuous tracking needed |
-| D29 | 2026-10-05 | Threads: only the MQTT client runs in its own thread; everything else, incl. all OpenVR calls and the HA layer, in the main thread. New threads only for work that can block | Blocking connect must not stall rendering; OpenVR isn't documented as thread-safe; the narrowest possible thread boundary |
+| D29 | 2026-10-05 | *How the MQTT thread is created: see D42.* Threads: only the MQTT client runs in its own thread; everything else, incl. all OpenVR calls and the HA layer, in the main thread. New threads only for work that can block | Blocking connect must not stall rendering; OpenVR isn't documented as thread-safe; the narrowest possible thread boundary |
 | D30 | 2026-10-05 | Test strategy in four levels: unit tests with fakes at the boundaries, integration tests against a local mosquitto, UI/render tools, manual checklist (user's HA, the Frame) | Covers NFR-8; the narrow boundaries (D29, I/O wrappers) make fakes cheap; HA automation deferred |
 | D31 | 2026-10-05 | Device id from optional `ha_unique_id` in the configuration, else derived from the device name; entity ids `frameassistant_<device id>_<entity>` | Renaming shouldn't have to create a new HA device |
 | D32 | 2026-10-05 | One topic per entity under `frameassistant/<device id>/`, common-practice layout; discovery, availability and states retained; republish on HA's birth message with random delay; device + entity availability with `availability_mode: all`; clear retained topics on uninstall | HA prescribes no layout; the headset is often offline, so retained discovery keeps its entities defined; per-entity topics keep throttling and availability simple |
@@ -445,3 +595,6 @@ Collected 2026-10-05 for the notification design.
 | D37 | 2026-10-05 | The MVP is implemented in four iterations: 1 DSH-1 + configuration, 2 MQTT/HA device/UI-1, 3 sensors, 4 notifications | Small iterations (working rules); DSH-1 first because the user wants to see it early and it doesn't depend on MQTT |
 | D38 | 2026-10-05 | App-specific command-line options: the platform's base constructor takes an optional list of extra options, parses them with its own and exposes their values | The command line is parsed in the base constructor, before the derived app can contribute (`--config`); backward compatible, `--help` stays complete |
 | D39 | 2026-10-05 | *Superseded by D40 (REQUIREMENTS.md): tab and WebEngine removed.* DSH-1: `QtWebEngineQuick::initialize()` in `main.cpp` (no platform change); a named, persistent WebEngine profile under `~/.local/share/frameassistant/`; tab name "HA Dashboard" | WebEngine must be initialised before the `QGuiApplication`; the HA login must survive restarts |
+| D42 | 2026-10-06 | The MQTT client's blocking work runs in **libmosquitto's own thread** (`mosquitto_connect_async()` + `mosquitto_loop_start()`), incl. its reconnect with backoff; callbacks only emit Qt signals with values to the main thread. Changes how D29 is carried out, not its rules | Same effect as an own `QThread` with much less code; libmosquitto's publish/subscribe are thread-safe while its loop runs |
+| D43 | 2026-10-06 | **App commands** in the platform: the app passes command options to the base constructor; they get a `QCoreApplication` like the manifest commands, and `exec()` runs `prepare()` and then the hook `runCommand(name)` | `--ha-remove` must work without a display (`uninstall.sh` over SSH); reuses the existing manifest-command path; `--config`/`--log-file`/`--help` stay consistent |
+| D44 | 2026-10-06 | TLS test certificates are committed to `tests/data/` (100-year validity) | Deterministic tests; no openssl needed in the containers |