Support media control buttons #1
Labels
No labels
Agent
Ignore
Agent
In Progress
Agent
Planned
Agent
Planning
Agent
Ready
Agent
Triage
Compat
Breaking
Kind
Bug
Kind
Documentation
Kind
Enhancement
Kind
Feature
Kind
Security
Kind
Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
lucas/nanoKontroller2#1
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Play, pause, etc. The button names are already there to map, so we need actions to connect them to.
includes/NanoKontrol2.hppalready defines every transport control (Play,Stop,Rewind,FastForward,PreviousTrack,NextTrack,Record,Cycle,Set, markers), so they can be named as keys inconfig.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 formpris|dbus|gio-2.0|playerctlacross the repository returns no hits. MappingPlaytoday 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:
play-pause,stop,next,previous,seek-backward,seek-forward.targetnarrows the candidate players; absenttargetmeans every player.PlayLED: solid while the selected player is playing, blinking while paused, off while stopped or when no player matches.StopLED: lit while the selected player is stopped, off otherwise and when no player matches.PreviousTrack/NextTracknever touch an LED.Rewind/FastForwardlight their LED while held (press feedback, not player state), seek once on press and then every 200 ms after a 400 ms hold.RecordandCyclestay unmapped: MPRIS models no recording.Plan
Context
Add one new action type
mediathat drives MPRIS players over GDBus, backed by a newMediaServicethat tracks the players on the session bus, plus the LED support that type needs (blinking inLedManager, two missing members in theLEDenum).gio-2.0ships with glib, which is already a dependency of both the build (mise.tomlinstallslibglib2.0-dev) and the packages (PKGBUILDdepends onglib2), so this adds a CMake line and no new runtime or bootstrap package. End state:{"Play": {"type": "media", "params": {"command": "play-pause"}}}inconfig.jsonmakes 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_testspasses 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 reusesNodeMatcher(includes/NodeMatcher.h) for pattern matching;NodeMatcheris plain anchored-regex-over-strings and needs no change.Free functions (exported for tests):
MatchEntryusesNodeMatcher::FirstMatchingEntry(includes/NodeMatcher.h:22) per candidate string and takes the minimum; the candidates areBusNameSuffixes(player.MatchName)plusplayer.Identitywhen non-empty.Registry class, keyed by the well-known bus name (the caller's handle;
MatchNameis derived from it onAdd):Selectranks matching players by, in order: higherStatus; then higherLastPlayingSeq; then lowerMatchEntryvalue; then lowerAppearedSeq. (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.Addon an already-known bus name updates nothing and keeps the originalAppearedSeq;Remove/SetStatus/SetIdentityon 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.LedManagercurrently exposes onlySet/Toggle/ResetState/GetCachedStateover astate_cache that keeps redundant MIDI writes off the wire. Give it the blink timer so no action owns one:bool Write(NanoKontrol2::LED, bool on, std::uint8_t channel)holds the current body ofSet(updatestate_,m_midiManager.sendControlChange).Setfirst erases the control from the blink set (so a solid state always wins over a blink), then callsWrite.SetBlinking(c, true, ch)inserts{c, ch}intostd::unordered_map<NanoKontrol2::LED, std::uint8_t> m_Blinkingand, ifm_BlinkSourceId == 0, startsm_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)andm_BlinkSourceId = 0. Turning blinking off leaves the LED in whatever phase it was in; callers that care callSetinstead (the media action always does).OnBlinkTickflipsm_BlinkPhaseand callsWritefor every entry, returningG_SOURCE_CONTINUE. It must not callSet, which would clear the blink set.ResetStateclearsm_Blinkingand the source before its existing loop that writes every LED off.LedManageris a member ofAppState(src/main.cpp:31), destroyed afterg_main_loop_runreturns, so the source can still be alive at that point.src/LedManager.cppgains#include <glib.h>.3.
Rewind/FastForwardLED membersincludes/NanoKontrol2.hpp: add to theLEDenum, in the transport block next toCycle(line 87):This makes
LedManager::IsLedCapabletrue for CC 43/44 and makesResetState()clear them at startup — a visible device change, intended.The comment on line 32,
// --- Transport buttons (no LED) ---, is already stale (Play,Stop,Record,Cycleappear in theLEDenum below it). Replace it with// --- Transport buttons (Play, Stop, Record, Cycle, Rewind, FastForward have LEDs) ---.4.
MediaService: session bus, player tracking, method callsNew
includes/MediaService.h/src/MediaService.cpp, owning aMprisPlayerRegistryand 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.Initcallsg_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &Error). On failure it printsWarning: 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; everySelectreturnsnulloptandCall/Seekreturn false. There is no reconnect loop (unlike the WirePlumber one insrc/main.cpp:50-69): a session bus that is gone means the session is gone.ListNamesonorg.freedesktop.DBusandAddevery name starting withorg.mpris.MediaPlayer2..g_dbus_connection_signal_subscribeonNameOwnerChangedusingG_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACEwith arg0org.mpris.MediaPlayer2: a non-empty new owner meansAdd+ proxy creation, an empty new owner meansRemove+ proxy teardown.g_dbus_proxy_new(async) on object path/org/mpris/MediaPlayer2: one onorg.mpris.MediaPlayer2.Player(kept, used for calls,PlaybackStatusandCanSeek), one onorg.mpris.MediaPlayer2used only to read the cachedIdentityintoSetIdentityand then released.GDBusProxycaches only its own interface's properties, which is whyIdentityneeds the second proxy.g-properties-changedon the player proxy: mapPlaybackStatusstrings"Playing"/"Paused"/"Stopped"to the enum (anything else →Stopped) and callSetStatuswith the next value of a monotonicstd::uint64_t m_Seqmember, incremented on everyAdd/status change.on_players_changed()on every registered listener. Add that virtual toIActionBasenext to the existingon_default_nodes_changed(includes/Actions/IAction.h:51), with an empty default body.Selectasks the registry, then readsCanSeekfrom the player proxy's cached properties (absent → treat astrue, since a player that does not advertise the property may still seek) and returnsSelectedPlayer.Call/Seeklook the proxy up by bus name; a missing proxy (the player vanished between selection and call) printsMedia command "<method>" failed: player "<busName>" is gone.on stderr and returns false. The call itself isg_dbus_proxy_call(..., G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, nullptr, callback, this)— asynchronous, never blocking the main loop — with a callback that callsg_dbus_proxy_call_finishand printsMedia command "<method>" on "<busName>" failed: <error>on stderr when it errors.Seekpassesg_variant_new("(x)", offsetMicroseconds).AppState(src/main.cpp:22-35) gains aMediaService m_MediaService;member, andmaincallsState.m_MediaService.Init();immediately before the action-building loop atsrc/main.cpp:136, so actions can register and query during construction.5. The
mediaaction, and threadingMediaServicethrough action constructionPlumbing (clean cutover, no overloads kept):
includes/Actions/ActionFactoryRegistry.h:14—FactoryFunctionbecomesstd::function<std::unique_ptr<IActionBase>(const ActionParams&, LedManager&, AudioService&, MediaService&, const NanoKontrol2::Control)>;BuildActiongains aMediaService& mediaServiceparameter afteraudioService. Add aclass MediaService;forward declaration.src/Actions/ActionFactoryRegistry.cpp:5-19— matching signature, forwarding the new argument.includes/Actions/IAction.h—IActionBase::InitgainsMediaService& mediaServiceafteraudioService(line 26-29); addMediaService* m_mediaService{nullptr};next tom_audioService(line 64) and aclass MediaService;forward declaration (line 17-18); theRegistratorlambda (line 94-98) takes and forwards the new parameter.src/Actions/IAction.cpp:10-19— assignm_mediaService;~IActionBase(line 4-8) additionally callsm_mediaService->UnregisterPlayerListener(this);.src/main.cpp:142— passState.m_MediaServicebetweenState.m_AudioServiceandEnum.value().These five are the complete callsite set:
grep -rn "BuildAction\|IActionBase::Init\|FactoryFunction" src includesreturns exactly them.New
includes/Actions/Action_Media.h/src/Actions/Action_Media.cpp:Init_Internal:commandwithGetScalarParam(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, likeconnection'sdefault-state.command:Warning: Media action has no "command" parameter – action will be inactive.on stderr, setm_bInactive = true, return. Unknown value:Warning: Unknown media command "<value>" – action will be inactive., same handling. An inactive action'son_midireturns immediately and it registers no listener.seconds(seek commands only; silently ignored for the others):GetScalarParam(params, "seconds")parsed withstd::stod; non-numeric or<= 0printsWarning: Invalid "seconds" value "<value>" for media action – using default 5.and keeps5.0.m_SeekMicroseconds = static_cast<std::int64_t>(m_Seconds * 1'000'000.0), negated forSeekBackward.targetneeds no parsing:IActionBase::Init(src/Actions/IAction.cpp:14-16) already buildsm_Targetfrom it, andExpandFriendlyNames(src/AppSettings.cpp:42-58) has already resolved aliases. Distinguish absent from broken: ifparams.contains("target")andm_Target.IsEmpty()(every pattern failed to compile), printWarning: Media action's "target" has no usable pattern – action will be inactive.and go inactive. An absenttargetleaves the matcher empty, whichMatchEntrytreats as "every player".m_mediaService->RegisterPlayerListener(this)and callRefreshLed()once, so the LED is correct before any press.on_midi, following the existing press convention (src/Actions/Action_Mute.cpp:27-57): act onvalue == 127guarded bym_bWaitingForLow, clear the latch onvalue == 0.PlayPause/Stop/Next/Previous: on press,m_mediaService->Select(m_Target);nullopt→LOG_DEBUG("Media action: no matching player for press.\n")(macro fromincludes/DebugLog.h) and return; otherwiseCall(Selected->BusName, Method)with the method literal for the command. The LED is never written optimistically here — it refreshes whenPropertiesChangedarrives, the same ruleAction_SetDefaultkeeps (src/Actions/Action_SetDefault.cpp:37-39).SeekBackward/SeekForward: on press, light the LED first whenLedManager::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. ThenSelect;nullopt→ nothing more. IfbCanSeekis false, printMedia action: player "<busName>" cannot seek.on stderr only whenbusName != m_SeekUnsupportedLoggedFor(then store it), and start no timer. Otherwise perform oneSeekand armm_SeekDelaySourceId = g_timeout_add(400, ...); that callback performs oneSeek, setsm_SeekRepeatSourceId = g_timeout_add(200, ...), clearsm_SeekDelaySourceIdand returnsG_SOURCE_REMOVE. The 200 ms callback re-selects (the top-ranked player may have changed), seeks, and returnsG_SOURCE_CONTINUE; if selection yieldsnulloptit callsStopSeekRepeat()and returnsG_SOURCE_REMOVE.value == 0for the seek commands:StopSeekRepeat()andm_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 fromInit_Internalandon_players_changed), only whenLedManager::IsLedCapable(m_Control); it returns immediately forNext,Previous,SeekBackwardandSeekForward, whose LED meaning is press feedback or nothing:PlayPause:Select→Playing⇒Set(led, true)(which also cancels blinking);Paused⇒SetBlinking(led, true);Stoppedornullopt⇒Set(led, false).Stop:Stopped⇒Set(led, true);Playing,Pausedornullopt⇒Set(led, false).The destructor calls
StopSeekRepeat(); listener unregistration happens in~IActionBase.Build wiring in
CMakeLists.txt: addpkg_check_modules(GIO REQUIRED gio-2.0)after theGOBJECTline (17-19), and${GIO_INCLUDE_DIRS}/${GIO_LIBRARIES}/${GIO_CFLAGS_OTHER}to the three lists at lines 27-52.src/*.cppis globbed (line 23), so the new sources need no listing. No packaging change:gio-2.0comes from the same glib2 /libglib2.0-devpackage already required.packaging/nanokontroller2.service: appenddbus.socketto the existingAfter=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 astests/NodeMatcher_tests.cpp(its ownCHECKmacro, failure counter,main()calling each test function; no test framework). Addtests/MprisPlayerRegistry_tests.cppandsrc/MprisPlayerRegistry.cppto thenanoKontroller2_teststarget (CMakeLists.txt:69-74) — it is a second translation unit with its ownmain, so either put it in the existing executable by merging the file's checks intotests/NodeMatcher_tests.cpp'smain(), or add a secondadd_executable/add_testpair namednanoKontroller2_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;…spotifyunchanged;…instance_abcunchanged (non-numeric tail is a real player name).BusNameSuffixes: exact list and longest-first order fororg.mpris.MediaPlayer2.spotify.MatchEntry:spotifymatches that player;otifydoes not (suffixes start at dot boundaries);MediaPlayer2\.spotifymatches; anIdentityofMozilla Firefoxmatches aMozilla Firefoxentry; a default-constructedNodeMatchermatches every player with index 0; with entries{"vlc", "spotify"}the spotify player yields index 1.Selectranking: aPlayingplayer beats aPausedone that played more recently; between twoPausedplayers the higherLastPlayingSeqwins; with equalLastPlayingSeqthe lower entry index wins; with those equal too the lowerAppearedSeqwins; an empty registry and a registry where nothing matches both yieldnullopt.Remove/SetStatuson 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),Registratorlambda (94-98): the three placesMediaService&must be threaded through;on_default_nodes_changed(51) is the pattern for the newon_players_changedvirtual.src/Actions/Action_SetDefault.cpp—on_midipress latch and the "never write the LED optimistically" rule the media action copies;Refresh_LED_State(58-65) is the shape ofRefreshLed.src/LedManager.cpp—Set(14-18) is split intoWrite+ blink cancellation;ResetState(8-12) must also clear the blink set and timer.includes/NanoKontrol2.hpp—Control(11-75) vsLED(82-118): LED values are a strict subset checked bymagic_enum::enum_containsinIsLedCapable, so addingRewind/FastForwardis what makes their LEDs addressable at all.tests/NodeMatcher_tests.cpp— the hand-rolledCHECK/counter/mainharness the new test file must mirror.Verification
Working directory is the repository root; the build uses the project's mise toolchain.
mise exec -- cmake -B build -DCMAKE_BUILD_TYPE=Debug -DNANOKONTROLLER2_BUILD_TESTS=ON && mise exec -- cmake --build build -j$(nproc)ctest --test-dir build --output-on-failure→ both test executables printAll checks passed.and ctest reports 100% passed. This covers step 1 and step 6 (selection ranking, instance stripping, suffix matching).busctl --user list | grep org.mpris). Write/tmp/media-test.json: Run./build/nanoKontroller2 -c /tmp/media-test.jsonand check, with the player paused:PropertiesChangedpath of step 4).DBUS_SESSION_BUS_ADDRESS=unix:path=/nonexistent ./build/nanoKontroller2 -c /tmp/media-test.json→ exactly oneWarning: 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).config.jsonand confirm a fader still changes volume and a Mute button still toggles mute with its LED — this exercises the changedInit/BuildActionsignatures from step 5.Assumptions & contingencies
LEDenum, 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 newLEDmembers and the LED writes in the seek branch ofon_midi, and keep the seek commands as LED-less actions likenext/previous.PropertiesChangedforPlaybackStatus. Required by the MPRIS spec but not universally honoured. If a specific player's LED lags reality, that is accepted: do not add polling.seconds.