Support media control buttons #1

Closed
opened 2026-11-12 19:38:51 +01:00 by lucas · 0 comments
Owner

Problem

Play, pause, etc. The button names are already there to map, so we need actions to connect them to.

includes/NanoKontrol2.hpp already defines every transport control (Play, Stop, Rewind, FastForward, PreviousTrack, NextTrack, Record, Cycle, Set, markers), so they can be named as keys in config.json. What is missing is an action type to bind them to: the four existing action types (Mute, volume, set-default, connection) are all PipeWire node actions, and a grep for mpris|dbus|gio-2.0|playerctl across the repository returns no hits. Mapping Play today builds nothing.

The daemon must therefore gain media-player control over MPRIS (org.mpris.MediaPlayer2.* on the session bus), reachable from config through a new action type, with LED feedback on the transport buttons that have LEDs.

Agreed behaviour:

  • Commands: play-pause, stop, next, previous, seek-backward, seek-forward.
  • A press acts on exactly one player — the top-ranked one — not on every match.
  • Optional target narrows the candidate players; absent target means every player.
  • Play LED: solid while the selected player is playing, blinking while paused, off while stopped or when no player matches. Stop LED: lit while the selected player is stopped, off otherwise and when no player matches. PreviousTrack/NextTrack never touch an LED.
  • Rewind/FastForward light their LED while held (press feedback, not player state), seek once on press and then every 200 ms after a 400 ms hold.
  • Record and Cycle stay unmapped: MPRIS models no recording.

Plan

Context

Add one new action type media that drives MPRIS players over GDBus, backed by a new MediaService that tracks the players on the session bus, plus the LED support that type needs (blinking in LedManager, two missing members in the LED enum). gio-2.0 ships with glib, which is already a dependency of both the build (mise.toml installs libglib2.0-dev) and the packages (PKGBUILD depends on glib2), so this adds a CMake line and no new runtime or bootstrap package. End state: {"Play": {"type": "media", "params": {"command": "play-pause"}}} in config.json makes the Play button toggle playback of the most relevant player and makes its LED tell the truth about that player's state.

Approach

Steps are ordered so the tree builds and nanoKontroller2_tests passes after each one. Steps 1–3 are independent of each other; step 4 depends on 1–3; step 5 depends on 4.

1. Pure MPRIS player bookkeeping (no D-Bus, unit-testable)

New includes/MprisPlayerRegistry.h / src/MprisPlayerRegistry.cpp. This file must include nothing from glib/gio — it is compiled into the test target, which links neither (CMakeLists.txt:69-74). It reuses NodeMatcher (includes/NodeMatcher.h) for pattern matching; NodeMatcher is plain anchored-regex-over-strings and needs no change.

enum class PlaybackStatus : std::uint8_t { Stopped = 0, Paused = 1, Playing = 2 };

struct MprisPlayer {
    // Well-known bus name with any trailing `.instance_<n>` segment removed.
    // This is the only bus-derived string that is ever matched.
    std::string MatchName;
    // `Identity` from `org.mpris.MediaPlayer2`; empty until the property read lands.
    std::string Identity;
    PlaybackStatus Status{PlaybackStatus::Stopped};
    // Value of the caller's counter when this player last entered Playing; 0 if never.
    std::uint64_t LastPlayingSeq{0};
    // Value of the caller's counter when the player appeared.
    std::uint64_t AppearedSeq{0};
};

Free functions (exported for tests):

// Drops a trailing dot-segment matching `instance_?[0-9]+`; returns busName otherwise.
std::string StripInstanceSuffix(std::string_view busName);

// Every dot-segment-aligned suffix of name, longest first:
// "org.mpris.MediaPlayer2.spotify" -> {"org.mpris.MediaPlayer2.spotify",
//   "mpris.MediaPlayer2.spotify", "MediaPlayer2.spotify", "spotify"}.
std::vector<std::string> BusNameSuffixes(std::string_view name);

// Lowest matching entry index of the matcher over the player's bus-name
// suffixes and its Identity; nullopt when nothing matches. An empty matcher
// (no `target` configured) matches every player and yields 0.
std::optional<std::size_t> MatchEntry(const MprisPlayer& player, const NodeMatcher& target);

MatchEntry uses NodeMatcher::FirstMatchingEntry (includes/NodeMatcher.h:22) per candidate string and takes the minimum; the candidates are BusNameSuffixes(player.MatchName) plus player.Identity when non-empty.

Registry class, keyed by the well-known bus name (the caller's handle; MatchName is derived from it on Add):

class MprisPlayerRegistry {
public:
    void Add(const std::string& busName, std::uint64_t seq);
    void Remove(const std::string& busName);
    // Bumps LastPlayingSeq to seq when status transitions into Playing.
    void SetStatus(const std::string& busName, PlaybackStatus status, std::uint64_t seq);
    void SetIdentity(const std::string& busName, const std::string& identity);

    // Top-ranked candidate, or nullopt when no player matches. Returns the
    // bus name so the caller can look its proxy up.
    [[nodiscard]] std::optional<std::string> Select(const NodeMatcher& target) const;
    [[nodiscard]] const MprisPlayer* Find(const std::string& busName) const;
};

Select ranks matching players by, in order: higher Status; then higher LastPlayingSeq; then lower MatchEntry value; then lower AppearedSeq. (Status band first keeps a playing player winning over a long-idle one; recency before entry order is the agreed tiebreak.) Ranking is a total order, so selection is deterministic.

Add on an already-known bus name updates nothing and keeps the original AppearedSeq; Remove/SetStatus/SetIdentity on an unknown bus name are silent no-ops (a vanish/signal race must not crash the daemon).

2. Blinking LEDs

includes/LedManager.h / src/LedManager.cpp. LedManager currently exposes only Set/Toggle/ResetState/GetCachedState over a state_ cache that keeps redundant MIDI writes off the wire. Give it the blink timer so no action owns one:

void SetBlinking(NanoKontrol2::LED control, bool blinking, std::uint8_t channel = 0);
~LedManager();
  • Private bool Write(NanoKontrol2::LED, bool on, std::uint8_t channel) holds the current body of Set (update state_, m_midiManager.sendControlChange).
  • Set first erases the control from the blink set (so a solid state always wins over a blink), then calls Write.
  • SetBlinking(c, true, ch) inserts {c, ch} into std::unordered_map<NanoKontrol2::LED, std::uint8_t> m_Blinking and, if m_BlinkSourceId == 0, starts m_BlinkSourceId = g_timeout_add(500, &LedManager::OnBlinkTick, this). SetBlinking(c, false, ch) erases it and, when the map becomes empty, g_source_remove(m_BlinkSourceId) and m_BlinkSourceId = 0. Turning blinking off leaves the LED in whatever phase it was in; callers that care call Set instead (the media action always does).
  • OnBlinkTick flips m_BlinkPhase and calls Write for every entry, returning G_SOURCE_CONTINUE. It must not call Set, which would clear the blink set.
  • ResetState clears m_Blinking and the source before its existing loop that writes every LED off.
  • The destructor removes the source if non-zero. LedManager is a member of AppState (src/main.cpp:31), destroyed after g_main_loop_run returns, so the source can still be alive at that point.

src/LedManager.cpp gains #include <glib.h>.

3. Rewind/FastForward LED members

includes/NanoKontrol2.hpp: add to the LED enum, in the transport block next to Cycle (line 87):

Rewind      = static_cast<uint8_t>(Control::Rewind),
FastForward = static_cast<uint8_t>(Control::FastForward),

This makes LedManager::IsLedCapable true for CC 43/44 and makes ResetState() clear them at startup — a visible device change, intended.

The comment on line 32, // --- Transport buttons (no LED) ---, is already stale (Play, Stop, Record, Cycle appear in the LED enum below it). Replace it with // --- Transport buttons (Play, Stop, Record, Cycle, Rewind, FastForward have LEDs) ---.

4. MediaService: session bus, player tracking, method calls

New includes/MediaService.h / src/MediaService.cpp, owning a MprisPlayerRegistry and the GDBus plumbing. All callbacks land on the GLib main context the daemon already runs (src/main.cpp:115,154), the same thread actions run on, so no extra marshalling is needed.

struct SelectedPlayer {
    std::string BusName;
    PlaybackStatus Status{PlaybackStatus::Stopped};
    bool bCanSeek{false};
};

class MediaService {
public:
    void Init();
    [[nodiscard]] bool IsAvailable() const;

    void RegisterPlayerListener(IActionBase* listener);
    void UnregisterPlayerListener(const IActionBase* listener);

    [[nodiscard]] std::optional<SelectedPlayer> Select(const NodeMatcher& target) const;
    // method is one of "PlayPause", "Stop", "Next", "Previous".
    bool Call(const std::string& busName, const char* method);
    bool Seek(const std::string& busName, std::int64_t offsetMicroseconds);
};
  • Init calls g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &Error). On failure it prints Warning: no D-Bus session bus (<error message>) – media actions will be inactive. on stderr, leaves the connection null, and returns. IsAvailable() is false from then on; every Select returns nullopt and Call/Seek return false. There is no reconnect loop (unlike the WirePlumber one in src/main.cpp:50-69): a session bus that is gone means the session is gone.
  • Enumerate existing players once: call ListNames on org.freedesktop.DBus and Add every name starting with org.mpris.MediaPlayer2..
  • Track arrivals/departures with g_dbus_connection_signal_subscribe on NameOwnerChanged using G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE with arg0 org.mpris.MediaPlayer2: a non-empty new owner means Add + proxy creation, an empty new owner means Remove + proxy teardown.
  • Per player create two proxies via g_dbus_proxy_new (async) on object path /org/mpris/MediaPlayer2: one on org.mpris.MediaPlayer2.Player (kept, used for calls, PlaybackStatus and CanSeek), one on org.mpris.MediaPlayer2 used only to read the cached Identity into SetIdentity and then released. GDBusProxy caches only its own interface's properties, which is why Identity needs the second proxy.
  • Connect g-properties-changed on the player proxy: map PlaybackStatus strings "Playing"/"Paused"/"Stopped" to the enum (anything else → Stopped) and call SetStatus with the next value of a monotonic std::uint64_t m_Seq member, incremented on every Add/status change.
  • After any change (player added, removed, status changed, identity resolved), call on_players_changed() on every registered listener. Add that virtual to IActionBase next to the existing on_default_nodes_changed (includes/Actions/IAction.h:51), with an empty default body.
  • Select asks the registry, then reads CanSeek from the player proxy's cached properties (absent → treat as true, since a player that does not advertise the property may still seek) and returns SelectedPlayer.
  • Call/Seek look the proxy up by bus name; a missing proxy (the player vanished between selection and call) prints Media command "<method>" failed: player "<busName>" is gone. on stderr and returns false. The call itself is g_dbus_proxy_call(..., G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, nullptr, callback, this) — asynchronous, never blocking the main loop — with a callback that calls g_dbus_proxy_call_finish and prints Media command "<method>" on "<busName>" failed: <error> on stderr when it errors. Seek passes g_variant_new("(x)", offsetMicroseconds).
  • The destructor unsubscribes the signal subscription and unrefs every proxy and the connection.

AppState (src/main.cpp:22-35) gains a MediaService m_MediaService; member, and main calls State.m_MediaService.Init(); immediately before the action-building loop at src/main.cpp:136, so actions can register and query during construction.

5. The media action, and threading MediaService through action construction

Plumbing (clean cutover, no overloads kept):

  • includes/Actions/ActionFactoryRegistry.h:14 — FactoryFunction becomes std::function<std::unique_ptr<IActionBase>(const ActionParams&, LedManager&, AudioService&, MediaService&, const NanoKontrol2::Control)>; BuildAction gains a MediaService& mediaService parameter after audioService. Add a class MediaService; forward declaration.
  • src/Actions/ActionFactoryRegistry.cpp:5-19 — matching signature, forwarding the new argument.
  • includes/Actions/IAction.h — IActionBase::Init gains MediaService& mediaService after audioService (line 26-29); add MediaService* m_mediaService{nullptr}; next to m_audioService (line 64) and a class MediaService; forward declaration (line 17-18); the Registrator lambda (line 94-98) takes and forwards the new parameter.
  • src/Actions/IAction.cpp:10-19 — assign m_mediaService; ~IActionBase (line 4-8) additionally calls m_mediaService->UnregisterPlayerListener(this);.
  • src/main.cpp:142 — pass State.m_MediaService between State.m_AudioService and Enum.value().

These five are the complete callsite set: grep -rn "BuildAction\|IActionBase::Init\|FactoryFunction" src includes returns exactly them.

New includes/Actions/Action_Media.h / src/Actions/Action_Media.cpp:

class Action_Media final : public IAction<Action_Media, "media"> {
public:
    ~Action_Media() override;
    void on_midi(int value) override;
    void on_players_changed() override;
protected:
    void Init_Internal(const ActionParams& params) override;
private:
    enum class Command : std::uint8_t { PlayPause, Stop, Next, Previous, SeekBackward, SeekForward };
    void RefreshLed();
    void StopSeekRepeat();
    ...
};

Init_Internal:

  • Read command with GetScalarParam(params, "command") (includes/AppSettings.h:22). Accepted literals, mapped exactly: "play-pause" → Command::PlayPause, "stop" → Stop, "next" → Next, "previous" → Previous, "seek-backward" → SeekBackward, "seek-forward" → SeekForward. Comparison is case-sensitive, like connection's default-state.
  • Missing command: Warning: Media action has no "command" parameter – action will be inactive. on stderr, set m_bInactive = true, return. Unknown value: Warning: Unknown media command "<value>" – action will be inactive., same handling. An inactive action's on_midi returns immediately and it registers no listener.
  • seconds (seek commands only; silently ignored for the others): GetScalarParam(params, "seconds") parsed with std::stod; non-numeric or <= 0 prints Warning: Invalid "seconds" value "<value>" for media action – using default 5. and keeps 5.0. m_SeekMicroseconds = static_cast<std::int64_t>(m_Seconds * 1'000'000.0), negated for SeekBackward.
  • target needs no parsing: IActionBase::Init (src/Actions/IAction.cpp:14-16) already builds m_Target from it, and ExpandFriendlyNames (src/AppSettings.cpp:42-58) has already resolved aliases. Distinguish absent from broken: if params.contains("target") and m_Target.IsEmpty() (every pattern failed to compile), print Warning: Media action's "target" has no usable pattern – action will be inactive. and go inactive. An absent target leaves the matcher empty, which MatchEntry treats as "every player".
  • Register with m_mediaService->RegisterPlayerListener(this) and call RefreshLed() once, so the LED is correct before any press.

on_midi, following the existing press convention (src/Actions/Action_Mute.cpp:27-57): act on value == 127 guarded by m_bWaitingForLow, clear the latch on value == 0.

  • PlayPause/Stop/Next/Previous: on press, m_mediaService->Select(m_Target); nullopt → LOG_DEBUG("Media action: no matching player for press.\n") (macro from includes/DebugLog.h) and return; otherwise Call(Selected->BusName, Method) with the method literal for the command. The LED is never written optimistically here — it refreshes when PropertiesChanged arrives, the same rule Action_SetDefault keeps (src/Actions/Action_SetDefault.cpp:37-39).
  • SeekBackward/SeekForward: on press, light the LED first when LedManager::IsLedCapable(m_Control) (m_ledManager->Set(static_cast<NanoKontrol2::LED>(m_Control), true)) — it is press feedback and must be visible even without a player. Then Select; nullopt → nothing more. If bCanSeek is false, print Media action: player "<busName>" cannot seek. on stderr only when busName != m_SeekUnsupportedLoggedFor (then store it), and start no timer. Otherwise perform one Seek and arm m_SeekDelaySourceId = g_timeout_add(400, ...); that callback performs one Seek, sets m_SeekRepeatSourceId = g_timeout_add(200, ...), clears m_SeekDelaySourceId and returns G_SOURCE_REMOVE. The 200 ms callback re-selects (the top-ranked player may have changed), seeks, and returns G_SOURCE_CONTINUE; if selection yields nullopt it calls StopSeekRepeat() and returns G_SOURCE_REMOVE.
  • On value == 0 for the seek commands: StopSeekRepeat() and m_ledManager->Set(led, false) when LED-capable.
  • StopSeekRepeat() g_source_removes both ids if non-zero and zeroes them. It is called on release, from the destructor, and from the repeat callback when no player remains — so a release event lost to an unplugged device cannot leave an unattended scrub running past the action's lifetime.

RefreshLed() (called from Init_Internal and on_players_changed), only when LedManager::IsLedCapable(m_Control); it returns immediately for Next, Previous, SeekBackward and SeekForward, whose LED meaning is press feedback or nothing:

  • PlayPause: Select → Playing ⇒ Set(led, true) (which also cancels blinking); Paused ⇒ SetBlinking(led, true); Stopped or nullopt ⇒ Set(led, false).
  • Stop: Stopped ⇒ Set(led, true); Playing, Paused or nullopt ⇒ Set(led, false).

The destructor calls StopSeekRepeat(); listener unregistration happens in ~IActionBase.

Build wiring in CMakeLists.txt: add pkg_check_modules(GIO REQUIRED gio-2.0) after the GOBJECT line (17-19), and ${GIO_INCLUDE_DIRS} / ${GIO_LIBRARIES} / ${GIO_CFLAGS_OTHER} to the three lists at lines 27-52. src/*.cpp is globbed (line 23), so the new sources need no listing. No packaging change: gio-2.0 comes from the same glib2 / libglib2.0-dev package already required.

packaging/nanokontroller2.service: append dbus.socket to the existing After= line (line 4) so the user unit starts after the session bus is up.

6. Unit tests for selection and matching

New tests/MprisPlayerRegistry_tests.cpp, in the same dependency-free style as tests/NodeMatcher_tests.cpp (its own CHECK macro, failure counter, main() calling each test function; no test framework). Add tests/MprisPlayerRegistry_tests.cpp and src/MprisPlayerRegistry.cpp to the nanoKontroller2_tests target (CMakeLists.txt:69-74) — it is a second translation unit with its own main, so either put it in the existing executable by merging the file's checks into tests/NodeMatcher_tests.cpp's main(), or add a second add_executable/add_test pair named nanoKontroller2_mpris_tests. Use the second executable; the existing test file stays untouched.

Checks to cover:

  • StripInstanceSuffix: org.mpris.MediaPlayer2.firefox.instance_1234 → org.mpris.MediaPlayer2.firefox; …firefox.instance1234 → same; …spotify unchanged; …instance_abc unchanged (non-numeric tail is a real player name).
  • BusNameSuffixes: exact list and longest-first order for org.mpris.MediaPlayer2.spotify.
  • MatchEntry: spotify matches that player; otify does not (suffixes start at dot boundaries); MediaPlayer2\.spotify matches; an Identity of Mozilla Firefox matches a Mozilla Firefox entry; a default-constructed NodeMatcher matches every player with index 0; with entries {"vlc", "spotify"} the spotify player yields index 1.
  • Select ranking: a Playing player beats a Paused one that played more recently; between two Paused players the higher LastPlayingSeq wins; with equal LastPlayingSeq the lower entry index wins; with those equal too the lower AppearedSeq wins; an empty registry and a registry where nothing matches both yield nullopt.
  • Remove/SetStatus on an unknown bus name do not throw and leave selection unchanged.

Critical files & anchors

  • includes/Actions/IAction.h — IActionBase::Init (26-29), m_audioService (64), Registrator lambda (94-98): the three places MediaService& must be threaded through; on_default_nodes_changed (51) is the pattern for the new on_players_changed virtual.
  • src/Actions/Action_SetDefault.cpp — on_midi press latch and the "never write the LED optimistically" rule the media action copies; Refresh_LED_State (58-65) is the shape of RefreshLed.
  • src/LedManager.cpp — Set (14-18) is split into Write + blink cancellation; ResetState (8-12) must also clear the blink set and timer.
  • includes/NanoKontrol2.hpp — Control (11-75) vs LED (82-118): LED values are a strict subset checked by magic_enum::enum_contains in IsLedCapable, so adding Rewind/FastForward is what makes their LEDs addressable at all.
  • tests/NodeMatcher_tests.cpp — the hand-rolled CHECK/counter/main harness the new test file must mirror.

Verification

Working directory is the repository root; the build uses the project's mise toolchain.

  1. Build with tests:
    mise exec -- cmake -B build -DCMAKE_BUILD_TYPE=Debug -DNANOKONTROLLER2_BUILD_TESTS=ON && mise exec -- cmake --build build -j$(nproc)
  2. Unit tests: ctest --test-dir build --output-on-failure → both test executables print All checks passed. and ctest reports 100% passed. This covers step 1 and step 6 (selection ranking, instance stripping, suffix matching).
  3. Session-bus end-to-end, with the nanoKONTROL2 connected and at least one MPRIS player running (confirm with busctl --user list | grep org.mpris). Write /tmp/media-test.json:
    {
      "Actions": {
        "Play":         { "type": "media", "params": { "command": "play-pause" } },
        "Stop":         { "type": "media", "params": { "command": "stop" } },
        "NextTrack":    { "type": "media", "params": { "command": "next" } },
        "PreviousTrack":{ "type": "media", "params": { "command": "previous" } },
        "FastForward":  { "type": "media", "params": { "command": "seek-forward" } },
        "Rewind":       { "type": "media", "params": { "command": "seek-backward" } }
      }
    }
    
    Run ./build/nanoKontroller2 -c /tmp/media-test.json and check, with the player paused:
    • Play LED blinks at roughly one flash per second (step 2 + step 5 LED rule).
    • Pressing Play resumes playback and the Play LED goes solid; pressing it again pauses and the LED returns to blinking.
    • Pausing the player from its own UI makes the Play LED blink without any button press (the PropertiesChanged path of step 4).
    • Pressing Stop stops the player: Stop LED solid, Play LED off.
    • Pressing NextTrack/PreviousTrack changes track and no LED changes.
    • Tapping FastForward advances the position by ~5 s; holding it for ~1.4 s advances by ~30 s (one on press, one at 400 ms, then ~5 more at 200 ms); the FastForward LED is lit for exactly as long as the button is held.
  4. Degraded bus: DBUS_SESSION_BUS_ADDRESS=unix:path=/nonexistent ./build/nanoKontroller2 -c /tmp/media-test.json → exactly one Warning: no D-Bus session bus … line on stderr, the daemon keeps running, all transport LEDs stay off, and pressing Play does nothing and prints nothing further (step 4's inert path).
  5. Regression on the existing actions: run the daemon with the repository's config.json and confirm a fader still changes volume and a Mute button still toggles mute with its LED — this exercises the changed Init/BuildAction signatures from step 5.

Assumptions & contingencies

  • Rewind/FastForward LEDs exist on the device. They are absent from the current LED enum, and the stale line-32 comment claims transport buttons have no LEDs, so the enum is being corrected from a hardware report rather than from code. If pressing Rewind/FastForward produces no light after step 3, drop the two new LED members and the LED writes in the seek branch of on_midi, and keep the seek commands as LED-less actions like next/previous.
  • Players emit PropertiesChanged for PlaybackStatus. Required by the MPRIS spec but not universally honoured. If a specific player's LED lags reality, that is accepted: do not add polling.
  • Default seek step is 5 s. Configurable per action via seconds.
## Problem Play, pause, etc. The button names are already there to map, so we need actions to connect them to. `includes/NanoKontrol2.hpp` already defines every transport control (`Play`, `Stop`, `Rewind`, `FastForward`, `PreviousTrack`, `NextTrack`, `Record`, `Cycle`, `Set`, markers), so they can be named as keys in `config.json`. What is missing is an action type to bind them to: the four existing action types (`Mute`, `volume`, `set-default`, `connection`) are all PipeWire node actions, and a grep for `mpris|dbus|gio-2.0|playerctl` across the repository returns no hits. Mapping `Play` today builds nothing. The daemon must therefore gain media-player control over MPRIS (`org.mpris.MediaPlayer2.*` on the session bus), reachable from config through a new action type, with LED feedback on the transport buttons that have LEDs. Agreed behaviour: - Commands: `play-pause`, `stop`, `next`, `previous`, `seek-backward`, `seek-forward`. - A press acts on exactly one player — the top-ranked one — not on every match. - Optional `target` narrows the candidate players; absent `target` means every player. - `Play` LED: solid while the selected player is playing, blinking while paused, off while stopped or when no player matches. `Stop` LED: lit while the selected player is stopped, off otherwise and when no player matches. `PreviousTrack`/`NextTrack` never touch an LED. - `Rewind`/`FastForward` light their LED while held (press feedback, not player state), seek once on press and then every 200 ms after a 400 ms hold. - `Record` and `Cycle` stay unmapped: MPRIS models no recording. ## Plan ### Context Add one new action type `media` that drives MPRIS players over GDBus, backed by a new `MediaService` that tracks the players on the session bus, plus the LED support that type needs (blinking in `LedManager`, two missing members in the `LED` enum). `gio-2.0` ships with glib, which is already a dependency of both the build (`mise.toml` installs `libglib2.0-dev`) and the packages (`PKGBUILD` depends on `glib2`), so this adds a CMake line and no new runtime or bootstrap package. End state: `{"Play": {"type": "media", "params": {"command": "play-pause"}}}` in `config.json` makes the Play button toggle playback of the most relevant player and makes its LED tell the truth about that player's state. ### Approach Steps are ordered so the tree builds and `nanoKontroller2_tests` passes after each one. Steps 1–3 are independent of each other; step 4 depends on 1–3; step 5 depends on 4. #### 1. Pure MPRIS player bookkeeping (no D-Bus, unit-testable) New `includes/MprisPlayerRegistry.h` / `src/MprisPlayerRegistry.cpp`. This file must include nothing from glib/gio — it is compiled into the test target, which links neither (`CMakeLists.txt:69-74`). It reuses `NodeMatcher` (`includes/NodeMatcher.h`) for pattern matching; `NodeMatcher` is plain anchored-regex-over-strings and needs no change. ```cpp enum class PlaybackStatus : std::uint8_t { Stopped = 0, Paused = 1, Playing = 2 }; struct MprisPlayer { // Well-known bus name with any trailing `.instance_<n>` segment removed. // This is the only bus-derived string that is ever matched. std::string MatchName; // `Identity` from `org.mpris.MediaPlayer2`; empty until the property read lands. std::string Identity; PlaybackStatus Status{PlaybackStatus::Stopped}; // Value of the caller's counter when this player last entered Playing; 0 if never. std::uint64_t LastPlayingSeq{0}; // Value of the caller's counter when the player appeared. std::uint64_t AppearedSeq{0}; }; ``` Free functions (exported for tests): ```cpp // Drops a trailing dot-segment matching `instance_?[0-9]+`; returns busName otherwise. std::string StripInstanceSuffix(std::string_view busName); // Every dot-segment-aligned suffix of name, longest first: // "org.mpris.MediaPlayer2.spotify" -> {"org.mpris.MediaPlayer2.spotify", // "mpris.MediaPlayer2.spotify", "MediaPlayer2.spotify", "spotify"}. std::vector<std::string> BusNameSuffixes(std::string_view name); // Lowest matching entry index of the matcher over the player's bus-name // suffixes and its Identity; nullopt when nothing matches. An empty matcher // (no `target` configured) matches every player and yields 0. std::optional<std::size_t> MatchEntry(const MprisPlayer& player, const NodeMatcher& target); ``` `MatchEntry` uses `NodeMatcher::FirstMatchingEntry` (`includes/NodeMatcher.h:22`) per candidate string and takes the minimum; the candidates are `BusNameSuffixes(player.MatchName)` plus `player.Identity` when non-empty. Registry class, keyed by the well-known bus name (the caller's handle; `MatchName` is derived from it on `Add`): ```cpp class MprisPlayerRegistry { public: void Add(const std::string& busName, std::uint64_t seq); void Remove(const std::string& busName); // Bumps LastPlayingSeq to seq when status transitions into Playing. void SetStatus(const std::string& busName, PlaybackStatus status, std::uint64_t seq); void SetIdentity(const std::string& busName, const std::string& identity); // Top-ranked candidate, or nullopt when no player matches. Returns the // bus name so the caller can look its proxy up. [[nodiscard]] std::optional<std::string> Select(const NodeMatcher& target) const; [[nodiscard]] const MprisPlayer* Find(const std::string& busName) const; }; ``` `Select` ranks matching players by, in order: higher `Status`; then higher `LastPlayingSeq`; then lower `MatchEntry` value; then lower `AppearedSeq`. (Status band first keeps a playing player winning over a long-idle one; recency before entry order is the agreed tiebreak.) Ranking is a total order, so selection is deterministic. `Add` on an already-known bus name updates nothing and keeps the original `AppearedSeq`; `Remove`/`SetStatus`/`SetIdentity` on an unknown bus name are silent no-ops (a vanish/signal race must not crash the daemon). #### 2. Blinking LEDs `includes/LedManager.h` / `src/LedManager.cpp`. `LedManager` currently exposes only `Set`/`Toggle`/`ResetState`/`GetCachedState` over a `state_` cache that keeps redundant MIDI writes off the wire. Give it the blink timer so no action owns one: ```cpp void SetBlinking(NanoKontrol2::LED control, bool blinking, std::uint8_t channel = 0); ~LedManager(); ``` - Private `bool Write(NanoKontrol2::LED, bool on, std::uint8_t channel)` holds the current body of `Set` (update `state_`, `m_midiManager.sendControlChange`). - `Set` first erases the control from the blink set (so a solid state always wins over a blink), then calls `Write`. - `SetBlinking(c, true, ch)` inserts `{c, ch}` into `std::unordered_map<NanoKontrol2::LED, std::uint8_t> m_Blinking` and, if `m_BlinkSourceId == 0`, starts `m_BlinkSourceId = g_timeout_add(500, &LedManager::OnBlinkTick, this)`. `SetBlinking(c, false, ch)` erases it and, when the map becomes empty, `g_source_remove(m_BlinkSourceId)` and `m_BlinkSourceId = 0`. Turning blinking off leaves the LED in whatever phase it was in; callers that care call `Set` instead (the media action always does). - `OnBlinkTick` flips `m_BlinkPhase` and calls `Write` for every entry, returning `G_SOURCE_CONTINUE`. It must not call `Set`, which would clear the blink set. - `ResetState` clears `m_Blinking` and the source before its existing loop that writes every LED off. - The destructor removes the source if non-zero. `LedManager` is a member of `AppState` (`src/main.cpp:31`), destroyed after `g_main_loop_run` returns, so the source can still be alive at that point. `src/LedManager.cpp` gains `#include <glib.h>`. #### 3. `Rewind`/`FastForward` LED members `includes/NanoKontrol2.hpp`: add to the `LED` enum, in the transport block next to `Cycle` (line 87): ```cpp Rewind = static_cast<uint8_t>(Control::Rewind), FastForward = static_cast<uint8_t>(Control::FastForward), ``` This makes `LedManager::IsLedCapable` true for CC 43/44 and makes `ResetState()` clear them at startup — a visible device change, intended. The comment on line 32, `// --- Transport buttons (no LED) ---`, is already stale (`Play`, `Stop`, `Record`, `Cycle` appear in the `LED` enum below it). Replace it with `// --- Transport buttons (Play, Stop, Record, Cycle, Rewind, FastForward have LEDs) ---`. #### 4. `MediaService`: session bus, player tracking, method calls New `includes/MediaService.h` / `src/MediaService.cpp`, owning a `MprisPlayerRegistry` and the GDBus plumbing. All callbacks land on the GLib main context the daemon already runs (`src/main.cpp:115,154`), the same thread actions run on, so no extra marshalling is needed. ```cpp struct SelectedPlayer { std::string BusName; PlaybackStatus Status{PlaybackStatus::Stopped}; bool bCanSeek{false}; }; class MediaService { public: void Init(); [[nodiscard]] bool IsAvailable() const; void RegisterPlayerListener(IActionBase* listener); void UnregisterPlayerListener(const IActionBase* listener); [[nodiscard]] std::optional<SelectedPlayer> Select(const NodeMatcher& target) const; // method is one of "PlayPause", "Stop", "Next", "Previous". bool Call(const std::string& busName, const char* method); bool Seek(const std::string& busName, std::int64_t offsetMicroseconds); }; ``` - `Init` calls `g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &Error)`. On failure it prints `Warning: no D-Bus session bus (<error message>) – media actions will be inactive.` on stderr, leaves the connection null, and returns. `IsAvailable()` is false from then on; every `Select` returns `nullopt` and `Call`/`Seek` return false. There is no reconnect loop (unlike the WirePlumber one in `src/main.cpp:50-69`): a session bus that is gone means the session is gone. - Enumerate existing players once: call `ListNames` on `org.freedesktop.DBus` and `Add` every name starting with `org.mpris.MediaPlayer2.`. - Track arrivals/departures with `g_dbus_connection_signal_subscribe` on `NameOwnerChanged` using `G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE` with arg0 `org.mpris.MediaPlayer2`: a non-empty new owner means `Add` + proxy creation, an empty new owner means `Remove` + proxy teardown. - Per player create two proxies via `g_dbus_proxy_new` (async) on object path `/org/mpris/MediaPlayer2`: one on `org.mpris.MediaPlayer2.Player` (kept, used for calls, `PlaybackStatus` and `CanSeek`), one on `org.mpris.MediaPlayer2` used only to read the cached `Identity` into `SetIdentity` and then released. `GDBusProxy` caches only its own interface's properties, which is why `Identity` needs the second proxy. - Connect `g-properties-changed` on the player proxy: map `PlaybackStatus` strings `"Playing"`/`"Paused"`/`"Stopped"` to the enum (anything else → `Stopped`) and call `SetStatus` with the next value of a monotonic `std::uint64_t m_Seq` member, incremented on every `Add`/status change. - After any change (player added, removed, status changed, identity resolved), call `on_players_changed()` on every registered listener. Add that virtual to `IActionBase` next to the existing `on_default_nodes_changed` (`includes/Actions/IAction.h:51`), with an empty default body. - `Select` asks the registry, then reads `CanSeek` from the player proxy's cached properties (absent → treat as `true`, since a player that does not advertise the property may still seek) and returns `SelectedPlayer`. - `Call`/`Seek` look the proxy up by bus name; a missing proxy (the player vanished between selection and call) prints `Media command "<method>" failed: player "<busName>" is gone.` on stderr and returns false. The call itself is `g_dbus_proxy_call(..., G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, nullptr, callback, this)` — asynchronous, never blocking the main loop — with a callback that calls `g_dbus_proxy_call_finish` and prints `Media command "<method>" on "<busName>" failed: <error>` on stderr when it errors. `Seek` passes `g_variant_new("(x)", offsetMicroseconds)`. - The destructor unsubscribes the signal subscription and unrefs every proxy and the connection. `AppState` (`src/main.cpp:22-35`) gains a `MediaService m_MediaService;` member, and `main` calls `State.m_MediaService.Init();` immediately before the action-building loop at `src/main.cpp:136`, so actions can register and query during construction. #### 5. The `media` action, and threading `MediaService` through action construction Plumbing (clean cutover, no overloads kept): - `includes/Actions/ActionFactoryRegistry.h:14` — `FactoryFunction` becomes `std::function<std::unique_ptr<IActionBase>(const ActionParams&, LedManager&, AudioService&, MediaService&, const NanoKontrol2::Control)>`; `BuildAction` gains a `MediaService& mediaService` parameter after `audioService`. Add a `class MediaService;` forward declaration. - `src/Actions/ActionFactoryRegistry.cpp:5-19` — matching signature, forwarding the new argument. - `includes/Actions/IAction.h` — `IActionBase::Init` gains `MediaService& mediaService` after `audioService` (line 26-29); add `MediaService* m_mediaService{nullptr};` next to `m_audioService` (line 64) and a `class MediaService;` forward declaration (line 17-18); the `Registrator` lambda (line 94-98) takes and forwards the new parameter. - `src/Actions/IAction.cpp:10-19` — assign `m_mediaService`; `~IActionBase` (line 4-8) additionally calls `m_mediaService->UnregisterPlayerListener(this);`. - `src/main.cpp:142` — pass `State.m_MediaService` between `State.m_AudioService` and `Enum.value()`. These five are the complete callsite set: `grep -rn "BuildAction\|IActionBase::Init\|FactoryFunction" src includes` returns exactly them. New `includes/Actions/Action_Media.h` / `src/Actions/Action_Media.cpp`: ```cpp class Action_Media final : public IAction<Action_Media, "media"> { public: ~Action_Media() override; void on_midi(int value) override; void on_players_changed() override; protected: void Init_Internal(const ActionParams& params) override; private: enum class Command : std::uint8_t { PlayPause, Stop, Next, Previous, SeekBackward, SeekForward }; void RefreshLed(); void StopSeekRepeat(); ... }; ``` `Init_Internal`: - Read `command` with `GetScalarParam(params, "command")` (`includes/AppSettings.h:22`). Accepted literals, mapped exactly: `"play-pause"` → `Command::PlayPause`, `"stop"` → `Stop`, `"next"` → `Next`, `"previous"` → `Previous`, `"seek-backward"` → `SeekBackward`, `"seek-forward"` → `SeekForward`. Comparison is case-sensitive, like `connection`'s `default-state`. - Missing `command`: `Warning: Media action has no "command" parameter – action will be inactive.` on stderr, set `m_bInactive = true`, return. Unknown value: `Warning: Unknown media command "<value>" – action will be inactive.`, same handling. An inactive action's `on_midi` returns immediately and it registers no listener. - `seconds` (seek commands only; silently ignored for the others): `GetScalarParam(params, "seconds")` parsed with `std::stod`; non-numeric or `<= 0` prints `Warning: Invalid "seconds" value "<value>" for media action – using default 5.` and keeps `5.0`. `m_SeekMicroseconds = static_cast<std::int64_t>(m_Seconds * 1'000'000.0)`, negated for `SeekBackward`. - `target` needs no parsing: `IActionBase::Init` (`src/Actions/IAction.cpp:14-16`) already builds `m_Target` from it, and `ExpandFriendlyNames` (`src/AppSettings.cpp:42-58`) has already resolved aliases. Distinguish absent from broken: if `params.contains("target")` and `m_Target.IsEmpty()` (every pattern failed to compile), print `Warning: Media action's "target" has no usable pattern – action will be inactive.` and go inactive. An absent `target` leaves the matcher empty, which `MatchEntry` treats as "every player". - Register with `m_mediaService->RegisterPlayerListener(this)` and call `RefreshLed()` once, so the LED is correct before any press. `on_midi`, following the existing press convention (`src/Actions/Action_Mute.cpp:27-57`): act on `value == 127` guarded by `m_bWaitingForLow`, clear the latch on `value == 0`. - `PlayPause`/`Stop`/`Next`/`Previous`: on press, `m_mediaService->Select(m_Target)`; `nullopt` → `LOG_DEBUG("Media action: no matching player for press.\n")` (macro from `includes/DebugLog.h`) and return; otherwise `Call(Selected->BusName, Method)` with the method literal for the command. The LED is never written optimistically here — it refreshes when `PropertiesChanged` arrives, the same rule `Action_SetDefault` keeps (`src/Actions/Action_SetDefault.cpp:37-39`). - `SeekBackward`/`SeekForward`: on press, light the LED first when `LedManager::IsLedCapable(m_Control)` (`m_ledManager->Set(static_cast<NanoKontrol2::LED>(m_Control), true)`) — it is press feedback and must be visible even without a player. Then `Select`; `nullopt` → nothing more. If `bCanSeek` is false, print `Media action: player "<busName>" cannot seek.` on stderr only when `busName != m_SeekUnsupportedLoggedFor` (then store it), and start no timer. Otherwise perform one `Seek` and arm `m_SeekDelaySourceId = g_timeout_add(400, ...)`; that callback performs one `Seek`, sets `m_SeekRepeatSourceId = g_timeout_add(200, ...)`, clears `m_SeekDelaySourceId` and returns `G_SOURCE_REMOVE`. The 200 ms callback re-selects (the top-ranked player may have changed), seeks, and returns `G_SOURCE_CONTINUE`; if selection yields `nullopt` it calls `StopSeekRepeat()` and returns `G_SOURCE_REMOVE`. - On `value == 0` for the seek commands: `StopSeekRepeat()` and `m_ledManager->Set(led, false)` when LED-capable. - `StopSeekRepeat()` `g_source_remove`s both ids if non-zero and zeroes them. It is called on release, from the destructor, and from the repeat callback when no player remains — so a release event lost to an unplugged device cannot leave an unattended scrub running past the action's lifetime. `RefreshLed()` (called from `Init_Internal` and `on_players_changed`), only when `LedManager::IsLedCapable(m_Control)`; it returns immediately for `Next`, `Previous`, `SeekBackward` and `SeekForward`, whose LED meaning is press feedback or nothing: - `PlayPause`: `Select` → `Playing` ⇒ `Set(led, true)` (which also cancels blinking); `Paused` ⇒ `SetBlinking(led, true)`; `Stopped` or `nullopt` ⇒ `Set(led, false)`. - `Stop`: `Stopped` ⇒ `Set(led, true)`; `Playing`, `Paused` or `nullopt` ⇒ `Set(led, false)`. The destructor calls `StopSeekRepeat()`; listener unregistration happens in `~IActionBase`. Build wiring in `CMakeLists.txt`: add `pkg_check_modules(GIO REQUIRED gio-2.0)` after the `GOBJECT` line (17-19), and `${GIO_INCLUDE_DIRS}` / `${GIO_LIBRARIES}` / `${GIO_CFLAGS_OTHER}` to the three lists at lines 27-52. `src/*.cpp` is globbed (line 23), so the new sources need no listing. No packaging change: `gio-2.0` comes from the same glib2 / `libglib2.0-dev` package already required. `packaging/nanokontroller2.service`: append ` dbus.socket` to the existing `After=` line (line 4) so the user unit starts after the session bus is up. #### 6. Unit tests for selection and matching New `tests/MprisPlayerRegistry_tests.cpp`, in the same dependency-free style as `tests/NodeMatcher_tests.cpp` (its own `CHECK` macro, failure counter, `main()` calling each test function; no test framework). Add `tests/MprisPlayerRegistry_tests.cpp` and `src/MprisPlayerRegistry.cpp` to the `nanoKontroller2_tests` target (`CMakeLists.txt:69-74`) — it is a second translation unit with its own `main`, so either put it in the existing executable by merging the file's checks into `tests/NodeMatcher_tests.cpp`'s `main()`, or add a second `add_executable`/`add_test` pair named `nanoKontroller2_mpris_tests`. Use the second executable; the existing test file stays untouched. Checks to cover: - `StripInstanceSuffix`: `org.mpris.MediaPlayer2.firefox.instance_1234` → `org.mpris.MediaPlayer2.firefox`; `…firefox.instance1234` → same; `…spotify` unchanged; `…instance_abc` unchanged (non-numeric tail is a real player name). - `BusNameSuffixes`: exact list and longest-first order for `org.mpris.MediaPlayer2.spotify`. - `MatchEntry`: `spotify` matches that player; `otify` does not (suffixes start at dot boundaries); `MediaPlayer2\.spotify` matches; an `Identity` of `Mozilla Firefox` matches a `Mozilla Firefox` entry; a default-constructed `NodeMatcher` matches every player with index 0; with entries `{"vlc", "spotify"}` the spotify player yields index 1. - `Select` ranking: a `Playing` player beats a `Paused` one that played more recently; between two `Paused` players the higher `LastPlayingSeq` wins; with equal `LastPlayingSeq` the lower entry index wins; with those equal too the lower `AppearedSeq` wins; an empty registry and a registry where nothing matches both yield `nullopt`. - `Remove`/`SetStatus` on an unknown bus name do not throw and leave selection unchanged. ### Critical files & anchors - `includes/Actions/IAction.h` — `IActionBase::Init` (26-29), `m_audioService` (64), `Registrator` lambda (94-98): the three places `MediaService&` must be threaded through; `on_default_nodes_changed` (51) is the pattern for the new `on_players_changed` virtual. - `src/Actions/Action_SetDefault.cpp` — `on_midi` press latch and the "never write the LED optimistically" rule the media action copies; `Refresh_LED_State` (58-65) is the shape of `RefreshLed`. - `src/LedManager.cpp` — `Set` (14-18) is split into `Write` + blink cancellation; `ResetState` (8-12) must also clear the blink set and timer. - `includes/NanoKontrol2.hpp` — `Control` (11-75) vs `LED` (82-118): LED values are a strict subset checked by `magic_enum::enum_contains` in `IsLedCapable`, so adding `Rewind`/`FastForward` is what makes their LEDs addressable at all. - `tests/NodeMatcher_tests.cpp` — the hand-rolled `CHECK`/counter/`main` harness the new test file must mirror. ### Verification Working directory is the repository root; the build uses the project's mise toolchain. 1. Build with tests: `mise exec -- cmake -B build -DCMAKE_BUILD_TYPE=Debug -DNANOKONTROLLER2_BUILD_TESTS=ON && mise exec -- cmake --build build -j$(nproc)` 2. Unit tests: `ctest --test-dir build --output-on-failure` → both test executables print `All checks passed.` and ctest reports 100% passed. This covers step 1 and step 6 (selection ranking, instance stripping, suffix matching). 3. Session-bus end-to-end, with the nanoKONTROL2 connected and at least one MPRIS player running (confirm with `busctl --user list | grep org.mpris`). Write `/tmp/media-test.json`: ```json { "Actions": { "Play": { "type": "media", "params": { "command": "play-pause" } }, "Stop": { "type": "media", "params": { "command": "stop" } }, "NextTrack": { "type": "media", "params": { "command": "next" } }, "PreviousTrack":{ "type": "media", "params": { "command": "previous" } }, "FastForward": { "type": "media", "params": { "command": "seek-forward" } }, "Rewind": { "type": "media", "params": { "command": "seek-backward" } } } } ``` Run `./build/nanoKontroller2 -c /tmp/media-test.json` and check, with the player paused: - Play LED blinks at roughly one flash per second (step 2 + step 5 LED rule). - Pressing Play resumes playback and the Play LED goes solid; pressing it again pauses and the LED returns to blinking. - Pausing the player from its own UI makes the Play LED blink without any button press (the `PropertiesChanged` path of step 4). - Pressing Stop stops the player: Stop LED solid, Play LED off. - Pressing NextTrack/PreviousTrack changes track and no LED changes. - Tapping FastForward advances the position by ~5 s; holding it for ~1.4 s advances by ~30 s (one on press, one at 400 ms, then ~5 more at 200 ms); the FastForward LED is lit for exactly as long as the button is held. 4. Degraded bus: `DBUS_SESSION_BUS_ADDRESS=unix:path=/nonexistent ./build/nanoKontroller2 -c /tmp/media-test.json` → exactly one `Warning: no D-Bus session bus …` line on stderr, the daemon keeps running, all transport LEDs stay off, and pressing Play does nothing and prints nothing further (step 4's inert path). 5. Regression on the existing actions: run the daemon with the repository's `config.json` and confirm a fader still changes volume and a Mute button still toggles mute with its LED — this exercises the changed `Init`/`BuildAction` signatures from step 5. ### Assumptions & contingencies - **Rewind/FastForward LEDs exist on the device.** They are absent from the current `LED` enum, and the stale line-32 comment claims transport buttons have no LEDs, so the enum is being corrected from a hardware report rather than from code. If pressing Rewind/FastForward produces no light after step 3, drop the two new `LED` members and the LED writes in the seek branch of `on_midi`, and keep the seek commands as LED-less actions like `next`/`previous`. - **Players emit `PropertiesChanged` for `PlaybackStatus`.** Required by the MPRIS spec but not universally honoured. If a specific player's LED lags reality, that is accepted: do not add polling. - **Default seek step is 5 s.** Configurable per action via `seconds`.
lucas closed this issue 2026-11-13 14:27:45 +01:00
Sign in to join this conversation.
No milestone
No assignees
1 participant
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#1
No description provided.