From 9fd6db4b5ff7e3999f8aaf0c0e0d75d7d661600b Mon Sep 17 00:00:00 2001 From: uvjustin <46082645+uvjustin@users.noreply.github.com> Date: Fri, 15 May 2020 01:42:00 +0800 Subject: [PATCH] Clean up forked_daapd volume saving/setting in async_play_media (#35584) * Clean up volume saving/setting in async_play_media * Set source to pipe when queued externally * Add server version requirement to error string --- .../components/forked_daapd/media_player.py | 42 +++++++++++-------- .../components/forked_daapd/strings.json | 2 +- .../forked_daapd/translations/en.json | 2 +- .../forked_daapd/test_media_player.py | 35 ++++++++++++++-- 4 files changed, 59 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/forked_daapd/media_player.py b/homeassistant/components/forked_daapd/media_player.py index 8224ea68914..1b52d454def 100644 --- a/homeassistant/components/forked_daapd/media_player.py +++ b/homeassistant/components/forked_daapd/media_player.py @@ -237,7 +237,7 @@ class ForkedDaapdMaster(MediaPlayerDevice): self._track_info = defaultdict( str ) # _track info is found by matching _player data with _queue data - self._last_outputs = None # used for device on/off + self._last_outputs = [] # used for device on/off self._last_volume = DEFAULT_UNMUTE_VOLUME self._player_last_updated = None self._pipe_control_api = {} @@ -349,6 +349,13 @@ class ForkedDaapdMaster(MediaPlayerDevice): ): self._tts_requested = False self._tts_queued = True + + if ( + self._queue["count"] >= 1 + and self._queue["items"][0]["data_kind"] == "pipe" + and self._queue["items"][0]["title"] in KNOWN_PIPES + ): # if we're playing a pipe, set the source automatically so we can forward controls + self._source = f"{self._queue['items'][0]['title']} (pipe)" self._update_track_info() event.set() @@ -407,6 +414,7 @@ class ForkedDaapdMaster(MediaPlayerDevice): async def async_turn_on(self): """Restore the last on outputs state.""" # restore state + await self._api.set_volume(volume=self._last_volume * 100) if self._last_outputs: futures = [] for output in self._last_outputs: @@ -418,19 +426,16 @@ class ForkedDaapdMaster(MediaPlayerDevice): ) ) await asyncio.wait(futures) - else: - selected = [] - for output in self._outputs: - selected.append(output["id"]) - await self._api.set_enabled_outputs(selected) + else: # enable all outputs + await self._api.set_enabled_outputs( + [output["id"] for output in self._outputs] + ) async def async_turn_off(self): """Pause player and store outputs state.""" await self.async_media_pause() - if any( - [output["selected"] for output in self._outputs] - ): # only store output state if some output is selected - self._last_outputs = self._outputs + self._last_outputs = self._outputs + if any([output["selected"] for output in self._outputs]): await self._api.set_enabled_outputs([]) async def async_toggle(self): @@ -613,8 +618,12 @@ class ForkedDaapdMaster(MediaPlayerDevice): url = self._api.full_url(url) return url - async def _set_tts_volumes(self): + async def _save_and_set_tts_volumes(self): + if self.volume_level: # save master volume + self._last_volume = self.volume_level + self._last_outputs = self._outputs if self._outputs: + await self._api.set_volume(volume=self._tts_volume * 100) futures = [] for output in self._outputs: futures.append( @@ -623,7 +632,6 @@ class ForkedDaapdMaster(MediaPlayerDevice): ) ) await asyncio.wait(futures) - await self._api.set_volume(volume=self._tts_volume * 100) async def _pause_and_wait_for_callback(self): """Send pause and wait for the pause callback to be received.""" @@ -641,14 +649,12 @@ class ForkedDaapdMaster(MediaPlayerDevice): """Play a URI.""" if media_type == MEDIA_TYPE_MUSIC: saved_state = self.state # save play state - if any([output["selected"] for output in self._outputs]): # save outputs - self._last_outputs = self._outputs - await self._api.set_enabled_outputs([]) # turn off outputs + saved_mute = self.is_volume_muted sleep_future = asyncio.create_task( asyncio.sleep(self._tts_pause_time) ) # start timing now, but not exact because of fd buffer + tts latency await self._pause_and_wait_for_callback() - await self._set_tts_volumes() + await self._save_and_set_tts_volumes() # save position saved_song_position = self._player["item_progress_ms"] saved_queue = ( @@ -678,7 +684,9 @@ class ForkedDaapdMaster(MediaPlayerDevice): _LOGGER.warning("TTS request timed out") self._tts_playing_event.clear() # TTS done, return to normal - await self.async_turn_on() # restores outputs + await self.async_turn_on() # restore outputs and volumes + if saved_mute: # mute if we were muted + await self.async_mute_volume(True) if self._use_pipe_control(): # resume pipe await self._api.add_to_queue( uris=self._sources_uris[self._source], clear=True diff --git a/homeassistant/components/forked_daapd/strings.json b/homeassistant/components/forked_daapd/strings.json index ea822a5559c..33f9c7b91aa 100644 --- a/homeassistant/components/forked_daapd/strings.json +++ b/homeassistant/components/forked_daapd/strings.json @@ -16,7 +16,7 @@ "websocket_not_enabled": "forked-daapd server websocket not enabled.", "wrong_host_or_port": "Unable to connect. Please check host and port.", "wrong_password": "Incorrect password.", - "wrong_server_type": "Not a forked-daapd server.", + "wrong_server_type": "The forked-daapd integration requires a forked-daapd server with version >= 27.0.", "unknown_error": "Unknown error." }, "abort": { diff --git a/homeassistant/components/forked_daapd/translations/en.json b/homeassistant/components/forked_daapd/translations/en.json index 6fe1b006984..0c87c6624ff 100644 --- a/homeassistant/components/forked_daapd/translations/en.json +++ b/homeassistant/components/forked_daapd/translations/en.json @@ -9,7 +9,7 @@ "websocket_not_enabled": "forked-daapd server websocket not enabled.", "wrong_host_or_port": "Unable to connect. Please check host and port.", "wrong_password": "Incorrect password.", - "wrong_server_type": "Not a forked-daapd server." + "wrong_server_type": "The forked-daapd integration requires a forked-daapd server with version >= 27.0." }, "flow_title": "forked-daapd server: {name} ({host})", "step": { diff --git a/tests/components/forked_daapd/test_media_player.py b/tests/components/forked_daapd/test_media_player.py index b0bed5aba3b..8f3a6c2c139 100644 --- a/tests/components/forked_daapd/test_media_player.py +++ b/tests/components/forked_daapd/test_media_player.py @@ -111,7 +111,7 @@ SAMPLE_PLAYER_STOPPED = { "item_progress_ms": 5, } -SAMPLE_TTS_QUEUE = { +SAMPLE_QUEUE_TTS = { "version": 833, "count": 1, "items": [ @@ -127,11 +127,31 @@ SAMPLE_TTS_QUEUE = { "length_ms": 0, "track_number": 1, "media_kind": "music", + "data_kind": "url", "uri": "tts_proxy_somefile.mp3", } ], } +SAMPLE_QUEUE_PIPE = { + "version": 833, + "count": 1, + "items": [ + { + "id": 12322, + "title": "librespot-java", + "artist": "some artist", + "album": "some album", + "album_artist": "The xx", + "length_ms": 0, + "track_number": 1, + "media_kind": "music", + "data_kind": "pipe", + "uri": "pipeuri", + } + ], +} + SAMPLE_CONFIG = { "websocket_port": 3688, "version": "25.0", @@ -272,7 +292,7 @@ async def get_request_return_values_fixture(): "config": SAMPLE_CONFIG, "outputs": SAMPLE_OUTPUTS_ON, "player": SAMPLE_PLAYER_PAUSED, - "queue": SAMPLE_TTS_QUEUE, + "queue": SAMPLE_QUEUE_TTS, } @@ -630,7 +650,9 @@ async def pipe_control_api_object_fixture( return pipe_control_api.return_value -async def test_librespot_java_stuff(hass, pipe_control_api_object): +async def test_librespot_java_stuff( + hass, get_request_return_values, mock_api_object, pipe_control_api_object +): """Test options update and librespot-java stuff.""" state = hass.states.get(TEST_MASTER_ENTITY_NAME) assert state.attributes[ATTR_INPUT_SOURCE] == "librespot-java (pipe)" @@ -652,6 +674,13 @@ async def test_librespot_java_stuff(hass, pipe_control_api_object): ) state = hass.states.get(TEST_MASTER_ENTITY_NAME) assert state.attributes[ATTR_INPUT_SOURCE] == SOURCE_NAME_DEFAULT + # test pipe getting queued externally changes source + get_request_return_values["queue"] = SAMPLE_QUEUE_PIPE + updater_update = mock_api_object.start_websocket_handler.call_args[0][2] + await updater_update(["queue"]) + await hass.async_block_till_done() + state = hass.states.get(TEST_MASTER_ENTITY_NAME) + assert state.attributes[ATTR_INPUT_SOURCE] == "librespot-java (pipe)" async def test_librespot_java_play_media(hass, pipe_control_api_object):