media_player_all_features #1092
Conversation
for more information, see https://pre-commit.ci
WalkthroughThis pull request introduces enhancements to the media player functionality within the API by adding new states, commands, and detailed attributes. The changes include updates to the protobuf definitions, the API client method, and internal models, allowing for additional capabilities such as enqueueing media, grouping, and advanced playback controls. New optional parameters in the API client facilitate these enhanced command requests, while updated enums and message fields provide richer representations of media player states and commands. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
CodSpeed Performance ReportMerging #1092 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1092 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 2742 2778 +36
=========================================
+ Hits 2742 2778 +36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
aioesphomeapi/api.proto(4 hunks)aioesphomeapi/client.py(2 hunks)aioesphomeapi/model.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: - Do not generate or add any sequence diagrams
**: - Do not generate or add any sequence diagrams
aioesphomeapi/client.pyaioesphomeapi/api.protoaioesphomeapi/model.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: py 3.12 on windows-latest (skip_cython)
🔇 Additional comments (8)
aioesphomeapi/client.py (1)
1232-1260: New media player command parameters look good.The additions of
enqueueandgroup_membersparameters to themedia_player_commandmethod are well-implemented, with proper optional typing and conditional flag setting. The implementation follows the same pattern as the existing parameters.aioesphomeapi/api.proto (4)
1127-1159: Media player state and command enums expanded appropriately.The new states and commands enhance the media player functionality as described in the PR objectives, adding support for announcements, power control, track navigation and playback controls.
1190-1193: Support flags for new media player capabilities.The new boolean capability flags correctly implement the planned functionality additions: turning media players on/off, grouping players together, and navigating between tracks.
1203-1210: Enhanced media player state attributes.The additional fields for song metadata and playback state provide much more detailed information about currently playing media, which will greatly improve the user experience.
1231-1236: Command request fields for new functionality.The enqueue and group_members fields enable the new features and follow the same pattern as existing command parameters with appropriate has_* flags.
aioesphomeapi/model.py (3)
813-816: Media player state enum values correctly added.The new state enum values match the protocol definitions and enable the new functionality.
872-875: Media player info capability flags properly added.These flags correctly correspond to the new fields in the ListEntitiesMediaPlayerResponse message.
891-899: Media player state information significantly enhanced.The new attributes in MediaPlayerEntityState provide rich metadata about media playback, matching the protocol definition.
aioesphomeapi/model.py
Outdated
| TOGGLE = 5 | ||
| VOLUME_UP = 6 | ||
| VOLUME_DOWN = 7 | ||
| ENQUEUE = (8,) | ||
| REPEAT_ONE = (9,) | ||
| REPEAT_OFF = (10,) | ||
| CLEAR_PLAYLIST = (11,) | ||
| NEXT_TRACK = (12,) | ||
| PREVIOUS_TRACK = (13,) | ||
| TURN_ON = (14,) | ||
| TURN_OFF = (15,) | ||
| SHUFFLE = (16,) | ||
| UNSHUFFLE = (17,) | ||
| REPEAT_ALL = (18,) | ||
| JOIN = (19,) | ||
| UNJOIN = (20,) |
There was a problem hiding this comment.
Fix tuple syntax in MediaPlayerCommand enum values.
There's a syntax error in how the new commands are defined - they're incorrectly using tuple syntax with trailing commas instead of simple integer values.
Apply this diff to fix the syntax:
TOGGLE = 5
VOLUME_UP = 6
VOLUME_DOWN = 7
- ENQUEUE = (8,)
- REPEAT_ONE = (9,)
- REPEAT_OFF = (10,)
- CLEAR_PLAYLIST = (11,)
- NEXT_TRACK = (12,)
- PREVIOUS_TRACK = (13,)
- TURN_ON = (14,)
- TURN_OFF = (15,)
- SHUFFLE = (16,)
- UNSHUFFLE = (17,)
- REPEAT_ALL = (18,)
- JOIN = (19,)
- UNJOIN = (20,)
+ ENQUEUE = 8
+ REPEAT_ONE = 9
+ REPEAT_OFF = 10
+ CLEAR_PLAYLIST = 11
+ NEXT_TRACK = 12
+ PREVIOUS_TRACK = 13
+ TURN_ON = 14
+ TURN_OFF = 15
+ SHUFFLE = 16
+ UNSHUFFLE = 17
+ REPEAT_ALL = 18
+ JOIN = 19
+ UNJOIN = 20The current implementation with tuple values would cause runtime errors when converting between Python enums and protocol enum values.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TOGGLE = 5 | |
| VOLUME_UP = 6 | |
| VOLUME_DOWN = 7 | |
| ENQUEUE = (8,) | |
| REPEAT_ONE = (9,) | |
| REPEAT_OFF = (10,) | |
| CLEAR_PLAYLIST = (11,) | |
| NEXT_TRACK = (12,) | |
| PREVIOUS_TRACK = (13,) | |
| TURN_ON = (14,) | |
| TURN_OFF = (15,) | |
| SHUFFLE = (16,) | |
| UNSHUFFLE = (17,) | |
| REPEAT_ALL = (18,) | |
| JOIN = (19,) | |
| UNJOIN = (20,) | |
| TOGGLE = 5 | |
| VOLUME_UP = 6 | |
| VOLUME_DOWN = 7 | |
| ENQUEUE = 8 | |
| REPEAT_ONE = 9 | |
| REPEAT_OFF = 10 | |
| CLEAR_PLAYLIST = 11 | |
| NEXT_TRACK = 12 | |
| PREVIOUS_TRACK = 13 | |
| TURN_ON = 14 | |
| TURN_OFF = 15 | |
| SHUFFLE = 16 | |
| UNSHUFFLE = 17 | |
| REPEAT_ALL = 18 | |
| JOIN = 19 | |
| UNJOIN = 20 |
…lle/aioesphomeapi into media_player_all_features
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
aioesphomeapi/model.py (2)
873-875: Consider renaming for clarity.
supports_turn_off_onmight be more consistent if namedsupports_turn_on_offorsupports_power_toggleto match reading order and be more descriptive. This is a minor naming nitpick but can improve clarity.
891-899: Consider using strong types for repeat/shuffle/timing fields.Storing
repeatas a string andduration/positionas integers can work, but using dedicated enums (for repeat/shuffle) or floats (for more precise timing) can improve type safety and clarity. Otherwise, please ensure robust validation for string-basedrepeatvalues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aioesphomeapi/model.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: - Do not generate or add any sequence diagrams
**: - Do not generate or add any sequence diagrams
aioesphomeapi/model.py
🔇 Additional comments (2)
aioesphomeapi/model.py (2)
814-816: Check for feature usage of newly introduced states.These new states (
OFF,ON, andANNOUNCING) appear valid. Please ensure they are handled throughout the codebase wherever a media player state might be set or interpreted, so that existing code that expects onlyIDLE,PLAYING, orPAUSEDdoes not encounter unexpected behavior.
825-840: All new commands are well-structured and consistent.Thank you for fixing the previous tuple-related syntax. These commands integrate well with the existing
MediaPlayerCommandpattern.
bdraco
added
the
waiting-for-esphome
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
aioesphomeapi/client.py (1)
1231-1262: Consider adding documentation for the new parametersThe method lacks docstring documentation explaining what the new parameters do. Adding documentation would improve developer experience and code maintenance.
def media_player_command( self, key: int, *, command: MediaPlayerCommand | None = None, volume: float | None = None, media_url: str | None = None, announcement: bool | None = None, enqueue: str | None = None, group_members: str | None = None, ) -> None: + """Send a command to a media player. + + Args: + key: The key identifying the media player. + command: The command to send to the media player. + volume: The volume to set on the media player. + media_url: The URL of media to play. + announcement: Whether this is an announcement. + enqueue: The enqueue option for the media player. + group_members: The group members for the media player when operating in group mode. + """ req = MediaPlayerCommandRequest(key=key)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aioesphomeapi/client.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: - Do not generate or add any sequence diagrams
**: - Do not generate or add any sequence diagrams
aioesphomeapi/client.py
🔇 Additional comments (2)
aioesphomeapi/client.py (2)
1239-1240: Method signature enhancement for extended media player functionalityThe addition of
enqueueandgroup_membersparameters aligns well with the PR objectives, enabling support for the media player enqueueing functionality and grouping capabilities.
1255-1260: Implementation follows consistent parameter handling patternThe implementation for the new parameters follows the established pattern in the codebase, properly checking for
Nonevalues before setting the request attributes and their corresponding flags.
Extends the media_player to additional features. supports_turn_off_on, supports_grouping, supports_next_prevous_track, State extended to include song attributes.