Add MPRIS support via go-music-players/mpris library#44
Add MPRIS support via go-music-players/mpris library#44stashluk wants to merge 2 commits intoshinokada:mainfrom
Conversation
✅ Deploy Preview for terminal-radio ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request adds MPRIS2 (Media Player Remote Interfacing Specification) support to the radio player application. A new MPRISAdapter translates the existing MPVPlayer to the MPRIS interface, enabling external media control. CLI support includes a Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / main()
participant MPRISAdapter as MPRISAdapter
participant MPRISServer as MPRIS Server
participant Player as MPVPlayer
participant UI as Bubble Tea UI
CLI->>CLI: Parse -mpris flag
CLI->>MPRISAdapter: NewMPRISAdapter(app.QuickFavPlayer)
activate MPRISAdapter
MPRISAdapter->>Player: Store reference
deactivate MPRISAdapter
alt MPRIS enabled
CLI->>MPRISServer: NewServer(adapter)
activate MPRISServer
MPRISServer->>MPRISServer: Register MPRIS interface
deactivate MPRISServer
Note over CLI: On failure: log warning, continue
Note over CLI: On success: defer server.Close()
end
CLI->>UI: NewApp() + tea.NewProgram()
activate UI
UI->>UI: Start Bubble Tea event loop
deactivate UI
Note over MPRISServer,Player: External MPRIS clients can now control playback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/tera/main.go (1)
54-80:⚠️ Potential issue | 🟠 Major
--mprisis never set when it's the first argument.In the
case "--mpris":branch (line 73), you parseos.Args[2:], which drops the flag itself, soenableMprisremains false andtera --mprisdoes nothing.Proposed fix
case "--mpris": // Parse remaining flags + *enableMpris = true flag.CommandLine.Parse(os.Args[2:])
🤖 Fix all issues with AI agents
In `@internal/player/mpris_adapter.go`:
- Around line 19-33: Play() currently no-ops; implement it to resume the current
station by calling the player's GetCurrentStation() and, if non-nil/valid, call
Play(station) and return its error; if no current station, return nil. Also
update CanPause() to return a.player.IsPlaying() (true only when actively
playing) instead of always true so the MPRIS state reflects actual pause
capability. Ensure you use the existing methods a.player.GetCurrentStation(),
a.player.Play(station), and a.player.IsPlaying() and preserve error propagation
from Play(station).
stashluk
force-pushed
the
integrate-go-music-players-mpris
branch
from
957428d to
6277040
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/ui/app.go (1)
392-454:⚠️ Potential issue | 🔴 CriticalFix remaining
quickFavPlayerreferences (compile error).After the rename, the volume-control block still uses
a.quickFavPlayer(Line 392/397/413/429). That field no longer exists, so the file won’t compile. Replace those witha.QuickFavPlayer.🛠️ Proposed fix
- if a.playingFromMain && a.quickFavPlayer != nil { + if a.playingFromMain && a.QuickFavPlayer != nil { switch msg.String() { case "/": // Decrease volume - newVol := a.quickFavPlayer.DecreaseVolume(5) + newVol := a.QuickFavPlayer.DecreaseVolume(5) @@ case "*": // Increase volume - newVol := a.quickFavPlayer.IncreaseVolume(5) + newVol := a.QuickFavPlayer.IncreaseVolume(5) @@ case "m": // Toggle mute - muted, vol := a.quickFavPlayer.ToggleMute() + muted, vol := a.QuickFavPlayer.ToggleMute()cmd/tera/main.go (1)
53-80:⚠️ Potential issue | 🟠 Major
--mprisflag is never set when passed as first argument.When
os.Args[1] == "--mpris", the code matches this in the switch statement and callsflag.CommandLine.Parse(os.Args[2:]), which skips the--mprisargument entirely. The flag parser never sees it, leavingenableMprisfalse and preventing MPRIS from being enabled.🛠️ Proposed fix
case "--mpris": // Parse remaining flags + *enableMpris = true flag.CommandLine.Parse(os.Args[2:])
Add optional MPRIS2 (Media Player Remote Interfacing Specification) support to enable desktop media controls, system keyboard shortcuts, and Now Playing widgets on GNOME/KDE environments. Changes: - Add MPRISAdapter bridging MPVPlayer to MPRIS interface - Implement Play() to resume current station if available - Add --mpris CLI flag to enable MPRIS server - Export App.QuickFavPlayer for MPRIS adapter access - Add go-music-players/mpris v0.1.0 dependency - CanPause() returns IsPlaying() to reflect actual pause capability Since tera is a radio player, Next/Previous and Seek operations are disabled as they don't apply to live radio streams. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
stashluk
force-pushed
the
integrate-go-music-players-mpris
branch
from
b1e6376 to
dcf10cf
Compare
1 participant
This PR adds optional MPRIS2 support for desktop integration with media players and system controls.
Summary by CodeRabbit
Release Notes
--mpriscommand-line flag, enabling control through external media applications, system interfaces, and third-party media control tools--versionflag for quick version information display✏️ Tip: You can customize this high-level summary in your review settings.