Add set-default action #8

Merged
lucas merged 2 commits from agent/issue-2-set-default into main 2026-11-12 22:22:18 +01:00
Collaborator

Adds a set-default action that makes its target node the PipeWire default sink or source, plus the listener-unregister fix it builds on.

  • fix(audio): UnregisterNodeListener / UnregisterLinkListener looped with Itr != val.rbegin(), so they never removed anything. Replaced with std::erase.
  • feat(actions): AudioService now also holds default-nodes-api, exposing SetDefaultNode / IsDefaultNode and a DefaultNodeListeners fan-out driven by the plugin's changed signal. Action_SetDefault is set-only (second press is a no-op), derives sink vs. source from the node's media.class (prefix match, */Internal rejected), and writes its LED only from the reported active default, so it also tracks wpctl/GUI changes. README documents the new action type.

Two deviations from the plan:

  • SPA_ID_INVALID is spelled G_MAXUINT32 (the plan's stated fallback). The build only gets wireplumber-0.5's include dirs — the existing code already has to spell PipeWire headers as pipewire-0.3/pipewire/keys.h — so <spa/utils/defs.h> is not reliably on the include path. Upstream treats the two as equal (wpctl.c: "currently SPA_ID_INVALID == G_MAXUINT32").
  • In Init, the branch where default-nodes-api is already present also delivers the initial state, and disconnects a previous handler before connecting: Init runs again after a WirePlumber reconnect and would otherwise stack handlers on the same plugin instance.

Verification

No WirePlumber/GLib/PipeWire development headers are available in this session (no root, no package manager), and this Forgejo instance has no Actions runner, so neither a real build nor the behavioural steps from the issue could be run here. What was checked instead:

  • The API usage was verified against WirePlumber 0.5.10 sources: modules/module-default-nodes-api.c declares get-default-node (G_TYPE_UINT (const gchar* media_class), returning SPA_ID_INVALID when unset), set-default-configured-node-name (G_TYPE_BOOLEAN (const gchar*, const gchar*)) and a zero-argument changed signal that is emitted only for the active default keys (default.audio.sink / default.audio.source), not the configured ones — which is exactly the LED semantics the issue asks for. src/tools/wpctl.c loads the module the same way this code does (wp_core_load_component(..., "module", ...), no explicit activation).
  • All touched and new translation units pass g++ -std=c++2c -fsyntax-only against a throwaway stub of the GLib/WirePlumber surface (discarded afterwards, not committed). That covers C++ syntax, the new virtual override signatures and the std::erase rewrite — it does not cover the real headers.

The behavioural checklist from the issue (steps 1–7) still needs a machine with PipeWire and the device.

Closes: #2

Adds a `set-default` action that makes its `target` node the PipeWire default sink or source, plus the listener-unregister fix it builds on. - **`fix(audio)`**: `UnregisterNodeListener` / `UnregisterLinkListener` looped with `Itr != val.rbegin()`, so they never removed anything. Replaced with `std::erase`. - **`feat(actions)`**: `AudioService` now also holds `default-nodes-api`, exposing `SetDefaultNode` / `IsDefaultNode` and a `DefaultNodeListeners` fan-out driven by the plugin's `changed` signal. `Action_SetDefault` is set-only (second press is a no-op), derives sink vs. source from the node's `media.class` (prefix match, `*/Internal` rejected), and writes its LED only from the reported active default, so it also tracks `wpctl`/GUI changes. README documents the new action type. Two deviations from the plan: - `SPA_ID_INVALID` is spelled `G_MAXUINT32` (the plan's stated fallback). The build only gets `wireplumber-0.5`'s include dirs — the existing code already has to spell PipeWire headers as `pipewire-0.3/pipewire/keys.h` — so `<spa/utils/defs.h>` is not reliably on the include path. Upstream treats the two as equal (`wpctl.c`: *"currently SPA_ID_INVALID == G_MAXUINT32"*). - In `Init`, the branch where `default-nodes-api` is already present also delivers the initial state, and disconnects a previous handler before connecting: `Init` runs again after a WirePlumber reconnect and would otherwise stack handlers on the same plugin instance. ### Verification No WirePlumber/GLib/PipeWire development headers are available in this session (no root, no package manager), and this Forgejo instance has no Actions runner, so neither a real build nor the behavioural steps from the issue could be run here. What was checked instead: - The API usage was verified against WirePlumber 0.5.10 sources: `modules/module-default-nodes-api.c` declares `get-default-node` (`G_TYPE_UINT (const gchar* media_class)`, returning `SPA_ID_INVALID` when unset), `set-default-configured-node-name` (`G_TYPE_BOOLEAN (const gchar*, const gchar*)`) and a zero-argument `changed` signal that is emitted only for the *active* default keys (`default.audio.sink` / `default.audio.source`), not the configured ones — which is exactly the LED semantics the issue asks for. `src/tools/wpctl.c` loads the module the same way this code does (`wp_core_load_component(..., "module", ...)`, no explicit activation). - All touched and new translation units pass `g++ -std=c++2c -fsyntax-only` against a throwaway stub of the GLib/WirePlumber surface (discarded afterwards, not committed). That covers C++ syntax, the new virtual override signatures and the `std::erase` rewrite — it does not cover the real headers. The behavioural checklist from the issue (steps 1–7) still needs a machine with PipeWire and the device. Closes: #2
Both unregister helpers looped with `Itr != val.rbegin()`, a condition
that is false on the first iteration, so a destroyed action was never
removed from the listener vectors and left a dangling pointer behind.

Co-Authored-By: Claude Opus 5 via omp-deck
A button mapped to `set-default` makes its target node the PipeWire
default; the sink/source distinction comes from the node's media.class,
so nothing extra can be misconfigured. It is set-only: the daemon never
claims or clears the default on its own, since that is global state
other tools write too.

The LED tracks the active default via WirePlumber's default-nodes-api
"changed" signal, so it also follows changes made with wpctl or a mixer
GUI.

Co-Authored-By: Claude Opus 5 via omp-deck
omp-deck changed title from WIP: Add set-default action to Add set-default action 2026-11-12 21:56:59 +01:00
Owner

Rebase please, that should trigger CI

Rebase please, that should trigger CI
omp-deck force-pushed agent/issue-2-set-default from 903fad3f10 to 9268e56ba1
All checks were successful
Build / build (push) Successful in 1m57s
Build / build (pull_request) Successful in 4m28s
2026-11-12 22:17:39 +01:00
Compare
Author
Collaborator

Rebased onto main (now 659be1e, no overlapping files) and force-pushed: head is 9268e56.

CI did trigger and both checks are green — Build / build (push) and Build / build (pull_request). So the compile against WirePlumber 0.5.14 is confirmed, including the G_MAXUINT32 spelling for SPA_ID_INVALID.

Still untested here: the behavioural steps 1–7 from #2, which need a running PipeWire session and the device.

Rebased onto `main` (now `659be1e`, no overlapping files) and force-pushed: head is `9268e56`. CI did trigger and both checks are green — `Build / build (push)` and `Build / build (pull_request)`. So the compile against WirePlumber 0.5.14 is confirmed, including the `G_MAXUINT32` spelling for `SPA_ID_INVALID`. Still untested here: the behavioural steps 1–7 from #2, which need a running PipeWire session and the device.
lucas merged commit 5e5762613c into main 2026-11-12 22:22:18 +01:00
lucas deleted branch agent/issue-2-set-default 2026-11-12 22:22:18 +01:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
lucas/nanoKontroller2!8
No description provided.