diff --git a/homeassistant/components/openhome/media_player.py b/homeassistant/components/openhome/media_player.py index 10ffa692b371..bcda02057621 100644 --- a/homeassistant/components/openhome/media_player.py +++ b/homeassistant/components/openhome/media_player.py @@ -9,6 +9,8 @@ from openhomedevice.exceptions import OpenhomeError from homeassistant.components import media_source from homeassistant.components.media_player import ( + SERVICE_PLAY_MEDIA, + SERVICE_SELECT_SOURCE, BrowseMedia, MediaPlayerEntity, MediaPlayerEntityFeature, @@ -16,12 +18,27 @@ from homeassistant.components.media_player import ( MediaType, async_process_play_media_url, ) +from homeassistant.const import ( + SERVICE_MEDIA_NEXT_TRACK, + SERVICE_MEDIA_PAUSE, + SERVICE_MEDIA_PLAY, + SERVICE_MEDIA_PREVIOUS_TRACK, + SERVICE_MEDIA_STOP, + SERVICE_TURN_OFF, + SERVICE_TURN_ON, + SERVICE_VOLUME_DOWN, + SERVICE_VOLUME_MUTE, + SERVICE_VOLUME_SET, + SERVICE_VOLUME_UP, +) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from . import OpenhomeConfigEntry from .const import DOMAIN +from .services import SERVICE_INVOKE_PIN SUPPORT_OPENHOME = ( MediaPlayerEntityFeature.SELECT_SOURCE @@ -50,14 +67,16 @@ async def async_setup_entry( type _FuncType[_T, **_P, _R] = Callable[Concatenate[_T, _P], Awaitable[_R]] type _ReturnFuncType[_T, **_P, _R] = Callable[ - Concatenate[_T, _P], Coroutine[Any, Any, _R | None] + Concatenate[_T, _P], Coroutine[Any, Any, _R] ] -def catch_request_errors[_OpenhomeDeviceT: OpenhomeDevice, **_P, _R]() -> Callable[ +def catch_request_errors[_OpenhomeDeviceT: OpenhomeDevice, **_P, _R]( + action: str, +) -> Callable[ [_FuncType[_OpenhomeDeviceT, _P, _R]], _ReturnFuncType[_OpenhomeDeviceT, _P, _R] ]: - """Catch OpenhomeError errors.""" + """Return decorator that catches errors and raises HomeAssistantError.""" def call_wrapper( func: _FuncType[_OpenhomeDeviceT, _P, _R], @@ -67,13 +86,15 @@ def catch_request_errors[_OpenhomeDeviceT: OpenhomeDevice, **_P, _R]() -> Callab @functools.wraps(func) async def wrapper( self: _OpenhomeDeviceT, *args: _P.args, **kwargs: _P.kwargs - ) -> _R | None: + ) -> _R: """Catch OpenhomeError errors.""" try: return await func(self, *args, **kwargs) except OpenhomeError as err: - _LOGGER.error("Error during call %s: %s", func.__name__, err) - return None + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key=action, + ) from err return wrapper @@ -173,22 +194,19 @@ class OpenhomeDevice(MediaPlayerEntity): _LOGGER.warning("Error updating %s: %s", self.entity_id, err) self._attr_available = False - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_TURN_ON) @override async def async_turn_on(self) -> None: """Bring device out of standby.""" await self._device.set_standby(False) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_TURN_OFF) @override async def async_turn_off(self) -> None: """Put device in standby.""" await self._device.set_standby(True) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_PLAY_MEDIA) @override async def async_play_media( self, media_type: MediaType | str, media_id: str, **kwargs: Any @@ -214,82 +232,71 @@ class OpenhomeDevice(MediaPlayerEntity): track_details = {"title": "Home Assistant", "uri": media_id} await self._device.play_media(track_details) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_MEDIA_PAUSE) @override async def async_media_pause(self) -> None: """Send pause command.""" await self._device.pause() - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_MEDIA_STOP) @override async def async_media_stop(self) -> None: """Send stop command.""" await self._device.stop() - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_MEDIA_PLAY) @override async def async_media_play(self) -> None: """Send play command.""" await self._device.play() - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_MEDIA_NEXT_TRACK) @override async def async_media_next_track(self) -> None: """Send next track command.""" await self._device.skip(1) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_MEDIA_PREVIOUS_TRACK) @override async def async_media_previous_track(self) -> None: """Send previous track command.""" await self._device.skip(-1) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_SELECT_SOURCE) @override async def async_select_source(self, source: str) -> None: """Select input source.""" await self._device.set_source(self._source_index[source]) - @catch_request_errors() + @catch_request_errors(SERVICE_INVOKE_PIN) async def async_invoke_pin(self, pin): """Invoke pin.""" - try: - if self._device.pins_enabled: - await self._device.invoke_pin(pin) - else: - _LOGGER.error("Pins service not supported") - except OpenhomeError as err: - _LOGGER.error("Error invoking pin %s: %s", pin, err) + if not self._device.pins_enabled: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="pins_not_supported", + ) + await self._device.invoke_pin(pin) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_VOLUME_UP) @override async def async_volume_up(self) -> None: """Volume up media player.""" await self._device.increase_volume() - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_VOLUME_DOWN) @override async def async_volume_down(self) -> None: """Volume down media player.""" await self._device.decrease_volume() - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_VOLUME_SET) @override async def async_set_volume_level(self, volume: float) -> None: """Set volume level, range 0..1.""" await self._device.set_volume(int(volume * 100)) - # pylint: disable-next=home-assistant-action-swallowed-exception - @catch_request_errors() + @catch_request_errors(SERVICE_VOLUME_MUTE) @override async def async_mute_volume(self, mute: bool) -> None: """Mute (true) or unmute (false) media player.""" diff --git a/homeassistant/components/openhome/strings.json b/homeassistant/components/openhome/strings.json index 38e3de28605e..956577a4710b 100644 --- a/homeassistant/components/openhome/strings.json +++ b/homeassistant/components/openhome/strings.json @@ -1,4 +1,51 @@ { + "exceptions": { + "invoke_pin": { + "message": "Failed to invoke the pin" + }, + "media_next_track": { + "message": "Failed to move to the next track" + }, + "media_pause": { + "message": "Failed to pause" + }, + "media_play": { + "message": "Failed to play" + }, + "media_previous_track": { + "message": "Failed to move to the previous track" + }, + "media_stop": { + "message": "Failed to stop" + }, + "pins_not_supported": { + "message": "This device does not support pins" + }, + "play_media": { + "message": "Failed to play media" + }, + "select_source": { + "message": "Failed to select the source" + }, + "turn_off": { + "message": "Failed to turn off" + }, + "turn_on": { + "message": "Failed to turn on" + }, + "volume_down": { + "message": "Failed to turn down the volume" + }, + "volume_mute": { + "message": "Failed to set the mute state" + }, + "volume_set": { + "message": "Failed to set the volume" + }, + "volume_up": { + "message": "Failed to turn up the volume" + } + }, "services": { "invoke_pin": { "description": "Starts playing content pinned on the specified device.", diff --git a/tests/components/openhome/test_media_player.py b/tests/components/openhome/test_media_player.py new file mode 100644 index 000000000000..43c6465b4bb0 --- /dev/null +++ b/tests/components/openhome/test_media_player.py @@ -0,0 +1,286 @@ +"""Tests for the Openhome media player platform.""" + +from collections.abc import Callable, Generator +from datetime import timedelta +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +from openhomedevice.device import Device +from openhomedevice.exceptions import OpenhomeConnectionError +import pytest + +from homeassistant.components.media_player import ( + ATTR_INPUT_SOURCE, + ATTR_MEDIA_CONTENT_ID, + ATTR_MEDIA_CONTENT_TYPE, + ATTR_MEDIA_VOLUME_LEVEL, + ATTR_MEDIA_VOLUME_MUTED, + DOMAIN as MEDIA_PLAYER_DOMAIN, + SERVICE_PLAY_MEDIA, + SERVICE_SELECT_SOURCE, + MediaType, +) +from homeassistant.components.openhome.const import DOMAIN +from homeassistant.components.openhome.services import ( + ATTR_PIN_INDEX, + SERVICE_INVOKE_PIN, +) +from homeassistant.const import ( + ATTR_ENTITY_ID, + CONF_HOST, + SERVICE_MEDIA_NEXT_TRACK, + SERVICE_MEDIA_PAUSE, + SERVICE_MEDIA_PLAY, + SERVICE_MEDIA_PREVIOUS_TRACK, + SERVICE_MEDIA_STOP, + SERVICE_TURN_OFF, + SERVICE_TURN_ON, + SERVICE_VOLUME_DOWN, + SERVICE_VOLUME_MUTE, + SERVICE_VOLUME_SET, + SERVICE_VOLUME_UP, + Platform, +) +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError +from homeassistant.util import dt as dt_util + +from tests.common import MockConfigEntry, async_fire_time_changed + +ENTITY_ID = "media_player.friendly_name" + +SOURCES = [ + {"index": 0, "name": "Playlist", "type": "Playlist"}, + {"index": 1, "name": "Radio", "type": "Radio"}, +] + +# The device coroutines the actions drive, referenced from the library so a +# rename upstream fails the test rather than silently skipping an action. +ACTION_METHODS = ( + Device.set_standby, + Device.play, + Device.pause, + Device.stop, + Device.skip, + Device.increase_volume, + Device.decrease_volume, + Device.set_volume, + Device.set_mute, + Device.set_source, + Device.play_media, + Device.invoke_pin, +) + +# Each action, the coroutine it drives, and a source type that exposes the +# supported feature it is gated behind. +ACTIONS = [ + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_TURN_ON, + {}, + Device.set_standby, + "Playlist", + id="turn_on", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_TURN_OFF, + {}, + Device.set_standby, + "Playlist", + id="turn_off", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_MEDIA_PLAY, + {}, + Device.play, + "Playlist", + id="media_play", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_MEDIA_PAUSE, + {}, + Device.pause, + "Playlist", + id="media_pause", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_MEDIA_STOP, + {}, + Device.stop, + "Radio", + id="media_stop", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_MEDIA_NEXT_TRACK, + {}, + Device.skip, + "Playlist", + id="media_next_track", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_MEDIA_PREVIOUS_TRACK, + {}, + Device.skip, + "Playlist", + id="media_previous_track", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_VOLUME_UP, + {}, + Device.increase_volume, + "Playlist", + id="volume_up", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_VOLUME_DOWN, + {}, + Device.decrease_volume, + "Playlist", + id="volume_down", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_VOLUME_SET, + {ATTR_MEDIA_VOLUME_LEVEL: 0.5}, + Device.set_volume, + "Playlist", + id="volume_set", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_VOLUME_MUTE, + {ATTR_MEDIA_VOLUME_MUTED: True}, + Device.set_mute, + "Playlist", + id="volume_mute", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_SELECT_SOURCE, + {ATTR_INPUT_SOURCE: "Playlist"}, + Device.set_source, + "Playlist", + id="select_source", + ), + pytest.param( + MEDIA_PLAYER_DOMAIN, + SERVICE_PLAY_MEDIA, + { + ATTR_MEDIA_CONTENT_TYPE: MediaType.MUSIC, + ATTR_MEDIA_CONTENT_ID: "http://localhost/track.flac", + }, + Device.play_media, + "Playlist", + id="play_media", + ), + pytest.param( + DOMAIN, + SERVICE_INVOKE_PIN, + {ATTR_PIN_INDEX: 1}, + Device.invoke_pin, + "Playlist", + id="invoke_pin", + ), +] + + +@pytest.fixture +def mock_device() -> Generator[MagicMock]: + """Return a mocked Openhome device that polls successfully.""" + with patch("homeassistant.components.openhome.Device", MagicMock()) as mock_class: + device = mock_class.return_value + device.init = AsyncMock() + device.uuid = MagicMock(return_value="uuid") + device.manufacturer = MagicMock(return_value="manufacturer") + device.model_name = MagicMock(return_value="model_name") + device.friendly_name = MagicMock(return_value="friendly_name") + device.volume_enabled = True + device.pins_enabled = True + device.room = AsyncMock(return_value="room") + device.track_info = AsyncMock(return_value={}) + device.volume = AsyncMock(return_value=50) + device.is_muted = AsyncMock(return_value=False) + device.sources = AsyncMock(return_value=SOURCES) + device.is_in_standby = AsyncMock(return_value=False) + device.transport_state = AsyncMock(return_value="Playing") + for method in ACTION_METHODS: + setattr(device, method.__name__, AsyncMock()) + yield device + + +async def setup_platform( + hass: HomeAssistant, mock_device: MagicMock, source_type: str +) -> None: + """Load the media player platform and poll once to populate features.""" + mock_device.source = AsyncMock( + return_value={"index": 0, "name": source_type, "type": source_type} + ) + entry = MockConfigEntry( + domain=DOMAIN, data={CONF_HOST: "http://localhost"}, unique_id="uuid" + ) + entry.add_to_hass(hass) + + with patch("homeassistant.components.openhome.PLATFORMS", [Platform.MEDIA_PLAYER]): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + # Supported features are only set once the device has been polled. + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=30)) + await hass.async_block_till_done() + + +@pytest.mark.parametrize( + ("domain", "service", "data", "method", "source_type"), ACTIONS +) +async def test_action_error_is_raised( + hass: HomeAssistant, + mock_device: MagicMock, + domain: str, + service: str, + data: dict[str, Any], + method: Callable[..., Any], + source_type: str, +) -> None: + """Test every action raises when the device rejects the request.""" + await setup_platform(hass, mock_device, source_type) + + mocked = getattr(mock_device, method.__name__) + mocked.side_effect = OpenhomeConnectionError("no route to host") + + with pytest.raises(HomeAssistantError) as err: + await hass.services.async_call( + domain, service, {ATTR_ENTITY_ID: ENTITY_ID, **data}, blocking=True + ) + + # The message is keyed on the service name, so each action reports its own. + assert err.value.translation_domain == DOMAIN + assert err.value.translation_key == service + mocked.assert_awaited() + + +async def test_invoke_pin_without_pin_support( + hass: HomeAssistant, mock_device: MagicMock +) -> None: + """Test invoking a pin on a device without pin support raises.""" + mock_device.pins_enabled = False + await setup_platform(hass, mock_device, "Playlist") + + with pytest.raises(HomeAssistantError) as err: + await hass.services.async_call( + DOMAIN, + SERVICE_INVOKE_PIN, + {ATTR_ENTITY_ID: ENTITY_ID, ATTR_PIN_INDEX: 1}, + blocking=True, + ) + + assert err.value.translation_domain == DOMAIN + assert err.value.translation_key == "pins_not_supported" + mock_device.invoke_pin.assert_not_awaited()