From 045375da58747f1b6cbe117614ba3212d150923d Mon Sep 17 00:00:00 2001 From: "Michael \"Chishm\" Chisholm" Date: Wed, 6 Apr 2022 22:08:12 +1000 Subject: [PATCH 1/4] Fix incorrect types of test data structures --- tests/components/dlna_dmr/test_config_flow.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/components/dlna_dmr/test_config_flow.py b/tests/components/dlna_dmr/test_config_flow.py index 44ab4ea313fa4..c1388bf173058 100644 --- a/tests/components/dlna_dmr/test_config_flow.py +++ b/tests/components/dlna_dmr/test_config_flow.py @@ -74,7 +74,7 @@ ] }, }, - x_homeassistant_matching_domains=(DLNA_DOMAIN,), + x_homeassistant_matching_domains={DLNA_DOMAIN}, ) @@ -390,7 +390,7 @@ async def test_ssdp_missing_services(hass: HomeAssistant) -> None: """Test SSDP ignores devices that are missing required services.""" # No services defined at all discovery = dataclasses.replace(MOCK_DISCOVERY) - discovery.upnp = discovery.upnp.copy() + discovery.upnp = dict(discovery.upnp) del discovery.upnp[ssdp.ATTR_UPNP_SERVICE_LIST] result = await hass.config_entries.flow.async_init( DLNA_DOMAIN, @@ -402,7 +402,7 @@ async def test_ssdp_missing_services(hass: HomeAssistant) -> None: # AVTransport service is missing discovery = dataclasses.replace(MOCK_DISCOVERY) - discovery.upnp = discovery.upnp.copy() + discovery.upnp = dict(discovery.upnp) discovery.upnp[ssdp.ATTR_UPNP_SERVICE_LIST] = { "service": [ service @@ -431,7 +431,7 @@ async def test_ssdp_ignore_device(hass: HomeAssistant) -> None: assert result["reason"] == "alternative_integration" discovery = dataclasses.replace(MOCK_DISCOVERY) - discovery.upnp = discovery.upnp.copy() + discovery.upnp = dict(discovery.upnp) discovery.upnp[ ssdp.ATTR_UPNP_DEVICE_TYPE ] = "urn:schemas-upnp-org:device:ZonePlayer:1" @@ -450,7 +450,7 @@ async def test_ssdp_ignore_device(hass: HomeAssistant) -> None: ("Royal Philips Electronics", "Philips TV DMR"), ]: discovery = dataclasses.replace(MOCK_DISCOVERY) - discovery.upnp = discovery.upnp.copy() + discovery.upnp = dict(discovery.upnp) discovery.upnp[ssdp.ATTR_UPNP_MANUFACTURER] = manufacturer discovery.upnp[ssdp.ATTR_UPNP_MODEL_NAME] = model result = await hass.config_entries.flow.async_init( From 16927b3abdb76d076742924a871f081a6a8ca61e Mon Sep 17 00:00:00 2001 From: "Michael \"Chishm\" Chisholm" Date: Wed, 6 Apr 2022 22:09:59 +1000 Subject: [PATCH 2/4] Loosen MIME-type filtering for async_browse_media --- .../components/dlna_dmr/media_player.py | 27 +++++++++++++------ .../components/dlna_dmr/test_media_player.py | 20 ++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/dlna_dmr/media_player.py b/homeassistant/components/dlna_dmr/media_player.py index 101b59c712506..41fb9470585b4 100644 --- a/homeassistant/components/dlna_dmr/media_player.py +++ b/homeassistant/components/dlna_dmr/media_player.py @@ -767,7 +767,11 @@ async def async_browse_media( ) def _get_content_filter(self) -> Callable[[BrowseMedia], bool]: - """Return a function that filters media based on what the renderer can play.""" + """Return a function that filters media based on what the renderer can play. + + The filtering is pretty loose; it's better to show something that can't + be played than hide something that can. + """ if not self._device or not self._device.sink_protocol_info: # Nothing is specified by the renderer, so show everything _LOGGER.debug("Get content filter with no device or sink protocol info") @@ -778,18 +782,25 @@ def _get_content_filter(self) -> Callable[[BrowseMedia], bool]: # Renderer claims it can handle everything, so show everything return lambda _: True - # Convert list of things like "http-get:*:audio/mpeg:*" to just "audio/mpeg" - content_types: list[str] = [] + # Convert list of things like "http-get:*:audio/mpeg;codecs=mp3:*" + # to just "audio/mpeg" + content_types = set[str]() for protocol_info in self._device.sink_protocol_info: protocol, _, content_format, _ = protocol_info.split(":", 3) + # Transform content_format for better generic matching + content_format = content_format.lower().replace("/x-", "/", 1) + content_format = content_format.partition(";")[0] + if protocol in STREAMABLE_PROTOCOLS: - content_types.append(content_format) + content_types.add(content_format) - def _content_type_filter(item: BrowseMedia) -> bool: - """Filter media items by their content_type.""" - return item.media_content_type in content_types + def _content_filter(item: BrowseMedia) -> bool: + """Filter media items by their media_content_type.""" + content_type = item.media_content_type + content_type = content_type.lower().replace("/x-", "/", 1).partition(";")[0] + return content_type in content_types - return _content_type_filter + return _content_filter @property def media_title(self) -> str | None: diff --git a/tests/components/dlna_dmr/test_media_player.py b/tests/components/dlna_dmr/test_media_player.py index bed89f3db9d42..4741f2e501817 100644 --- a/tests/components/dlna_dmr/test_media_player.py +++ b/tests/components/dlna_dmr/test_media_player.py @@ -997,6 +997,26 @@ async def test_browse_media( # Audio file should appear assert expected_child_audio in response["result"]["children"] + # Device specifies extra parameters in MIME type, uses non-standard "x-" + # prefix, and capitilizes things, all of which should be ignored + dmr_device_mock.sink_protocol_info = [ + "http-get:*:audio/X-MPEG;codecs=mp3:*", + ] + client = await hass_ws_client() + await client.send_json( + { + "id": 1, + "type": "media_player/browse_media", + "entity_id": mock_entity_id, + } + ) + response = await client.receive_json() + assert response["success"] + # Video file should not be shown + assert expected_child_video not in response["result"]["children"] + # Audio file should appear + assert expected_child_audio in response["result"]["children"] + # Device does not specify what it can play dmr_device_mock.sink_protocol_info = [] client = await hass_ws_client() From eeafb89647e0cf5683d5eb44342a56a22258fd16 Mon Sep 17 00:00:00 2001 From: "Michael \"Chishm\" Chisholm" Date: Wed, 6 Apr 2022 22:15:28 +1000 Subject: [PATCH 3/4] Add option to not filter results when browsing media Some devices do not report all that they support, and in this case filtering will hide media that's actually playable. Most devices are OK, though, and it's better to hide what they can't play. Add an option, off by default, to show all media. --- .../components/dlna_dmr/config_flow.py | 23 +++--- homeassistant/components/dlna_dmr/const.py | 1 + .../components/dlna_dmr/media_player.py | 12 ++- .../components/dlna_dmr/strings.json | 3 +- .../components/dlna_dmr/translations/en.json | 1 + tests/components/dlna_dmr/test_config_flow.py | 4 + .../components/dlna_dmr/test_media_player.py | 82 +++++++++++++++++++ 7 files changed, 114 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/dlna_dmr/config_flow.py b/homeassistant/components/dlna_dmr/config_flow.py index e8999c3ebb61b..546be8d268beb 100644 --- a/homeassistant/components/dlna_dmr/config_flow.py +++ b/homeassistant/components/dlna_dmr/config_flow.py @@ -4,7 +4,7 @@ from collections.abc import Callable, Mapping import logging from pprint import pformat -from typing import Any, Optional, cast +from typing import Any, Optional, Type, cast from urllib.parse import urlparse from async_upnp_client.client import UpnpError @@ -21,6 +21,7 @@ import homeassistant.helpers.config_validation as cv from .const import ( + CONF_BROWSE_UNFILTERED, CONF_CALLBACK_URL_OVERRIDE, CONF_LISTEN_PORT, CONF_POLL_AVAILABILITY, @@ -322,6 +323,7 @@ async def async_step_init( options[CONF_LISTEN_PORT] = listen_port options[CONF_CALLBACK_URL_OVERRIDE] = callback_url_override options[CONF_POLL_AVAILABILITY] = user_input[CONF_POLL_AVAILABILITY] + options[CONF_BROWSE_UNFILTERED] = user_input[CONF_BROWSE_UNFILTERED] # Save if there's no errors, else fall through and show the form again if not errors: @@ -329,9 +331,14 @@ async def async_step_init( fields = {} - def _add_with_suggestion(key: str, validator: Callable) -> None: - """Add a field to with a suggested, not default, value.""" - if (suggested_value := options.get(key)) is None: + def _add_with_suggestion(key: str, validator: Callable | Type[bool]) -> None: + """Add a field to with a suggested value. + + For bools, use the existing value as default, or fallback to False. + """ + if validator is bool: + fields[vol.Required(key, default=options.get(key, False))] = validator + elif (suggested_value := options.get(key)) is None: fields[vol.Optional(key)] = validator else: fields[ @@ -341,12 +348,8 @@ def _add_with_suggestion(key: str, validator: Callable) -> None: # listen_port can be blank or 0 for "bind any free port" _add_with_suggestion(CONF_LISTEN_PORT, cv.port) _add_with_suggestion(CONF_CALLBACK_URL_OVERRIDE, str) - fields[ - vol.Required( - CONF_POLL_AVAILABILITY, - default=options.get(CONF_POLL_AVAILABILITY, False), - ) - ] = bool + _add_with_suggestion(CONF_POLL_AVAILABILITY, bool) + _add_with_suggestion(CONF_BROWSE_UNFILTERED, bool) return self.async_show_form( step_id="init", diff --git a/homeassistant/components/dlna_dmr/const.py b/homeassistant/components/dlna_dmr/const.py index a4118a0ce78f6..4f9982061fbda 100644 --- a/homeassistant/components/dlna_dmr/const.py +++ b/homeassistant/components/dlna_dmr/const.py @@ -16,6 +16,7 @@ CONF_LISTEN_PORT: Final = "listen_port" CONF_CALLBACK_URL_OVERRIDE: Final = "callback_url_override" CONF_POLL_AVAILABILITY: Final = "poll_availability" +CONF_BROWSE_UNFILTERED: Final = "browse_unfiltered" DEFAULT_NAME: Final = "DLNA Digital Media Renderer" diff --git a/homeassistant/components/dlna_dmr/media_player.py b/homeassistant/components/dlna_dmr/media_player.py index 41fb9470585b4..c59dd9bb27433 100644 --- a/homeassistant/components/dlna_dmr/media_player.py +++ b/homeassistant/components/dlna_dmr/media_player.py @@ -45,6 +45,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from .const import ( + CONF_BROWSE_UNFILTERED, CONF_CALLBACK_URL_OVERRIDE, CONF_LISTEN_PORT, CONF_POLL_AVAILABILITY, @@ -106,6 +107,7 @@ async def async_setup_entry( event_callback_url=entry.options.get(CONF_CALLBACK_URL_OVERRIDE), poll_availability=entry.options.get(CONF_POLL_AVAILABILITY, False), location=entry.data[CONF_URL], + browse_unfiltered=entry.options.get(CONF_BROWSE_UNFILTERED, False), ) async_add_entities([entity]) @@ -122,6 +124,8 @@ class DlnaDmrEntity(MediaPlayerEntity): # Last known URL for the device, used when adding this entity to hass to try # to connect before SSDP has rediscovered it, or when SSDP discovery fails. location: str + # Should the async_browse_media function *not* filter out incompatible media? + browse_unfiltered: bool _device_lock: asyncio.Lock # Held when connecting or disconnecting the device _device: DmrDevice | None = None @@ -144,6 +148,7 @@ def __init__( event_callback_url: str | None, poll_availability: bool, location: str, + browse_unfiltered: bool, ) -> None: """Initialize DLNA DMR entity.""" self.udn = udn @@ -152,6 +157,7 @@ def __init__( self._event_addr = EventListenAddr(None, event_port, event_callback_url) self.poll_availability = poll_availability self.location = location + self.browse_unfiltered = browse_unfiltered self._device_lock = asyncio.Lock() async def async_added_to_hass(self) -> None: @@ -273,6 +279,7 @@ async def async_config_update_listener( ) self.location = entry.data[CONF_URL] self.poll_availability = entry.options.get(CONF_POLL_AVAILABILITY, False) + self.browse_unfiltered = entry.options.get(CONF_BROWSE_UNFILTERED, False) new_port = entry.options.get(CONF_LISTEN_PORT) or 0 new_callback_url = entry.options.get(CONF_CALLBACK_URL_OVERRIDE) @@ -760,7 +767,10 @@ async def async_browse_media( # media_content_type is ignored; it's the content_type of the current # media_content_id, not the desired content_type of whomever is calling. - content_filter = self._get_content_filter() + if self.browse_unfiltered: + content_filter = None + else: + content_filter = self._get_content_filter() return await media_source.async_browse_media( self.hass, media_content_id, content_filter=content_filter diff --git a/homeassistant/components/dlna_dmr/strings.json b/homeassistant/components/dlna_dmr/strings.json index ac77009e0cb61..d646f20f7a102 100644 --- a/homeassistant/components/dlna_dmr/strings.json +++ b/homeassistant/components/dlna_dmr/strings.json @@ -44,7 +44,8 @@ "data": { "listen_port": "Event listener port (random if not set)", "callback_url_override": "Event listener callback URL", - "poll_availability": "Poll for device availability" + "poll_availability": "Poll for device availability", + "browse_unfiltered": "Show incompatible media when browsing" } } }, diff --git a/homeassistant/components/dlna_dmr/translations/en.json b/homeassistant/components/dlna_dmr/translations/en.json index 512dfe7f11c48..51cbd875211f4 100644 --- a/homeassistant/components/dlna_dmr/translations/en.json +++ b/homeassistant/components/dlna_dmr/translations/en.json @@ -47,6 +47,7 @@ "step": { "init": { "data": { + "browse_unfiltered": "Show incompatible media when browsing", "callback_url_override": "Event listener callback URL", "listen_port": "Event listener port (random if not set)", "poll_availability": "Poll for device availability" diff --git a/tests/components/dlna_dmr/test_config_flow.py b/tests/components/dlna_dmr/test_config_flow.py index c1388bf173058..5c6df1a5bfdf9 100644 --- a/tests/components/dlna_dmr/test_config_flow.py +++ b/tests/components/dlna_dmr/test_config_flow.py @@ -11,6 +11,7 @@ from homeassistant import config_entries, data_entry_flow from homeassistant.components import ssdp from homeassistant.components.dlna_dmr.const import ( + CONF_BROWSE_UNFILTERED, CONF_CALLBACK_URL_OVERRIDE, CONF_LISTEN_PORT, CONF_POLL_AVAILABILITY, @@ -558,6 +559,7 @@ async def test_options_flow( user_input={ CONF_CALLBACK_URL_OVERRIDE: "Bad url", CONF_POLL_AVAILABILITY: False, + CONF_BROWSE_UNFILTERED: False, }, ) @@ -572,6 +574,7 @@ async def test_options_flow( CONF_LISTEN_PORT: 2222, CONF_CALLBACK_URL_OVERRIDE: "http://override/callback", CONF_POLL_AVAILABILITY: True, + CONF_BROWSE_UNFILTERED: True, }, ) @@ -580,4 +583,5 @@ async def test_options_flow( CONF_LISTEN_PORT: 2222, CONF_CALLBACK_URL_OVERRIDE: "http://override/callback", CONF_POLL_AVAILABILITY: True, + CONF_BROWSE_UNFILTERED: True, } diff --git a/tests/components/dlna_dmr/test_media_player.py b/tests/components/dlna_dmr/test_media_player.py index 4741f2e501817..eef5d93639696 100644 --- a/tests/components/dlna_dmr/test_media_player.py +++ b/tests/components/dlna_dmr/test_media_player.py @@ -23,6 +23,7 @@ from homeassistant.components import ssdp from homeassistant.components.dlna_dmr import media_player from homeassistant.components.dlna_dmr.const import ( + CONF_BROWSE_UNFILTERED, CONF_CALLBACK_URL_OVERRIDE, CONF_LISTEN_PORT, CONF_POLL_AVAILABILITY, @@ -1034,6 +1035,87 @@ async def test_browse_media( assert expected_child_audio in response["result"]["children"] +async def test_browse_media_unfiltered( + hass: HomeAssistant, + hass_ws_client, + config_entry_mock: MockConfigEntry, + dmr_device_mock: Mock, + mock_entity_id: str, +) -> None: + """Test the async_browse_media method with filtering turned off and on.""" + # Based on cast's test_entity_browse_media + await async_setup_component(hass, MS_DOMAIN, {MS_DOMAIN: {}}) + await hass.async_block_till_done() + + expected_child_video = { + "title": "Epic Sax Guy 10 Hours.mp4", + "media_class": "video", + "media_content_type": "video/mp4", + "media_content_id": "media-source://media_source/local/Epic Sax Guy 10 Hours.mp4", + "can_play": True, + "can_expand": False, + "thumbnail": None, + "children_media_class": None, + } + expected_child_audio = { + "title": "test.mp3", + "media_class": "music", + "media_content_type": "audio/mpeg", + "media_content_id": "media-source://media_source/local/test.mp3", + "can_play": True, + "can_expand": False, + "thumbnail": None, + "children_media_class": None, + } + + # Device can only play MIME type audio/mpeg and audio/vorbis + dmr_device_mock.sink_protocol_info = [ + "http-get:*:audio/mpeg:*", + "http-get:*:audio/vorbis:*", + ] + + # Filtering turned on by default + assert CONF_BROWSE_UNFILTERED not in config_entry_mock.options + + client = await hass_ws_client() + await client.send_json( + { + "id": 1, + "type": "media_player/browse_media", + "entity_id": mock_entity_id, + } + ) + response = await client.receive_json() + assert response["success"] + # Video file should not be shown + assert expected_child_video not in response["result"]["children"] + # Audio file should appear + assert expected_child_audio in response["result"]["children"] + + # Filtering turned off via config entry + hass.config_entries.async_update_entry( + config_entry_mock, + options={ + CONF_BROWSE_UNFILTERED: True, + }, + ) + await hass.async_block_till_done() + + client = await hass_ws_client() + await client.send_json( + { + "id": 1, + "type": "media_player/browse_media", + "entity_id": mock_entity_id, + } + ) + response = await client.receive_json() + assert response["success"] + # All files should be returned + assert expected_child_video in response["result"]["children"] + assert expected_child_audio in response["result"]["children"] + + async def test_playback_update_state( hass: HomeAssistant, dmr_device_mock: Mock, mock_entity_id: str ) -> None: From 13292d7eb58c223b5eaa614f7aa7842072d43552 Mon Sep 17 00:00:00 2001 From: "Michael \"Chishm\" Chisholm" Date: Thu, 7 Apr 2022 22:39:02 +1000 Subject: [PATCH 4/4] Fix linting issues --- homeassistant/components/dlna_dmr/config_flow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/dlna_dmr/config_flow.py b/homeassistant/components/dlna_dmr/config_flow.py index 546be8d268beb..24f2e0363d50d 100644 --- a/homeassistant/components/dlna_dmr/config_flow.py +++ b/homeassistant/components/dlna_dmr/config_flow.py @@ -4,7 +4,7 @@ from collections.abc import Callable, Mapping import logging from pprint import pformat -from typing import Any, Optional, Type, cast +from typing import Any, Optional, cast from urllib.parse import urlparse from async_upnp_client.client import UpnpError @@ -331,7 +331,7 @@ async def async_step_init( fields = {} - def _add_with_suggestion(key: str, validator: Callable | Type[bool]) -> None: + def _add_with_suggestion(key: str, validator: Callable | type[bool]) -> None: """Add a field to with a suggested value. For bools, use the existing value as default, or fallback to False.