From b31041698f31cb807d01166ee9c5355701e86330 Mon Sep 17 00:00:00 2001 From: Christopher Bailey Date: Wed, 29 Dec 2021 13:39:45 -0500 Subject: [PATCH] Feedback from previous PR (#63022) --- .../components/unifiprotect/media_player.py | 24 +++------- .../unifiprotect/test_media_player.py | 47 ++++++++++--------- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/homeassistant/components/unifiprotect/media_player.py b/homeassistant/components/unifiprotect/media_player.py index 63490a80d3c..cd5ac9992ca 100644 --- a/homeassistant/components/unifiprotect/media_player.py +++ b/homeassistant/components/unifiprotect/media_player.py @@ -1,7 +1,6 @@ """Support for Ubiquiti's UniFi Protect NVR.""" from __future__ import annotations -from collections.abc import Callable, Sequence import logging from typing import Any @@ -16,7 +15,6 @@ from homeassistant.components.media_player import ( from homeassistant.components.media_player.const import ( MEDIA_TYPE_MUSIC, SUPPORT_PLAY_MEDIA, - SUPPORT_SELECT_SOURCE, SUPPORT_STOP, SUPPORT_VOLUME_SET, SUPPORT_VOLUME_STEP, @@ -24,7 +22,8 @@ from homeassistant.components.media_player.const import ( from homeassistant.config_entries import ConfigEntry from homeassistant.const import STATE_IDLE, STATE_PLAYING from homeassistant.core import HomeAssistant, callback -from homeassistant.helpers.entity import Entity +from homeassistant.exceptions import HomeAssistantError +from homeassistant.helpers.entity_platform import AddEntitiesCallback from .const import DOMAIN from .data import ProtectData @@ -36,7 +35,7 @@ _LOGGER = logging.getLogger(__name__) async def async_setup_entry( hass: HomeAssistant, entry: ConfigEntry, - async_add_entities: Callable[[Sequence[Entity]], None], + async_add_entities: AddEntitiesCallback, ) -> None: """Discover cameras with speakers on a UniFi Protect NVR.""" data: ProtectData = hass.data[DOMAIN][entry.entry_id] @@ -71,11 +70,7 @@ class ProtectMediaPlayer(ProtectDeviceEntity, MediaPlayerEntity): self._attr_name = f"{self.device.name} Speaker" self._attr_supported_features = ( - SUPPORT_PLAY_MEDIA - | SUPPORT_VOLUME_SET - | SUPPORT_VOLUME_STEP - | SUPPORT_STOP - | SUPPORT_SELECT_SOURCE + SUPPORT_PLAY_MEDIA | SUPPORT_VOLUME_SET | SUPPORT_VOLUME_STEP | SUPPORT_STOP ) self._attr_media_content_type = MEDIA_TYPE_MUSIC @@ -92,7 +87,6 @@ class ProtectMediaPlayer(ProtectDeviceEntity, MediaPlayerEntity): else: self._attr_state = STATE_IDLE - @callback async def async_set_volume_level(self, volume: float) -> None: """Set volume level, range 0..1.""" @@ -116,20 +110,14 @@ class ProtectMediaPlayer(ProtectDeviceEntity, MediaPlayerEntity): """Play a piece of media.""" if media_type != MEDIA_TYPE_MUSIC: - _LOGGER.warning( - "%s: Cannot play media type of %s, only `%s` supported", - self.device.name, - media_type, - MEDIA_TYPE_MUSIC, - ) - return + raise ValueError("Only music media type is supported") _LOGGER.debug("Playing Media %s for %s Speaker", media_id, self.device.name) await self.async_media_stop() try: await self.device.play_audio(media_id, blocking=False) except StreamError as err: - _LOGGER.error("Error while playing media: %s", err) + raise HomeAssistantError from err else: # update state after starting player self._async_updated_event() diff --git a/tests/components/unifiprotect/test_media_player.py b/tests/components/unifiprotect/test_media_player.py index fc6429eec96..fc39b5ec805 100644 --- a/tests/components/unifiprotect/test_media_player.py +++ b/tests/components/unifiprotect/test_media_player.py @@ -1,4 +1,4 @@ -"""Test the UniFi Protect button platform.""" +"""Test the UniFi Protect media_player platform.""" # pylint: disable=protected-access from __future__ import annotations @@ -23,6 +23,7 @@ from homeassistant.const import ( Platform, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_registry as er from .conftest import MockEntityFixture @@ -85,7 +86,7 @@ async def test_media_player_setup( assert state assert state.state == STATE_IDLE assert state.attributes[ATTR_ATTRIBUTION] == DEFAULT_ATTRIBUTION - assert state.attributes[ATTR_SUPPORTED_FEATURES] == 7684 + assert state.attributes[ATTR_SUPPORTED_FEATURES] == 5636 assert state.attributes[ATTR_MEDIA_CONTENT_TYPE] == "music" assert state.attributes[ATTR_MEDIA_VOLUME_LEVEL] == expected_volume @@ -201,16 +202,17 @@ async def test_media_player_play_invalid( camera[0].__fields__["play_audio"] = Mock() camera[0].play_audio = AsyncMock() - await hass.services.async_call( - "media_player", - "play_media", - { - ATTR_ENTITY_ID: camera[1], - "media_content_id": "/test.png", - "media_content_type": "image", - }, - blocking=True, - ) + with pytest.raises(ValueError): + await hass.services.async_call( + "media_player", + "play_media", + { + ATTR_ENTITY_ID: camera[1], + "media_content_id": "/test.png", + "media_content_type": "image", + }, + blocking=True, + ) assert not camera[0].play_audio.called @@ -226,16 +228,17 @@ async def test_media_player_play_error( camera[0].play_audio = AsyncMock(side_effect=StreamError) camera[0].wait_until_audio_completes = AsyncMock() - await hass.services.async_call( - "media_player", - "play_media", - { - ATTR_ENTITY_ID: camera[1], - "media_content_id": "/test.mp3", - "media_content_type": "music", - }, - blocking=True, - ) + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + "media_player", + "play_media", + { + ATTR_ENTITY_ID: camera[1], + "media_content_id": "/test.mp3", + "media_content_type": "music", + }, + blocking=True, + ) assert camera[0].play_audio.called assert not camera[0].wait_until_audio_completes.called