Devialet browse and play media #134168

fwestenberg · 2024-12-28T20:38:01Z

Breaking change

Proposed change

Add play media and browse media support for Devialet.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue:
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

homeassistant/components/devialet/coordinator.py

joostlek · 2025-01-17T16:19:49Z

homeassistant/components/devialet/coordinator.py

+        if await self.client.async_search_allowed():
+            self.entry.async_create_background_task(
+                hass=self.hass,
+                target=self.client.async_discover_upnp_device(),
+                name=f"{DOMAIN}_UPnP_Discovery",
+            )


Why do we do this?

At every restart of the speaker, the UPnP port will change. This method finds the UPnP port and initiates the playback capability.

homeassistant/components/devialet/manifest.json

tests/components/devialet/__init__.py

home-assistant · 2025-01-17T16:20:49Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

tests/components/devialet/test_init.py

tests/components/devialet/test_media_player.py

joostlek · 2025-01-31T11:21:13Z

homeassistant/components/devialet/strings.json

+  },
+  "exceptions": {
+    "upnp_error": {
+      "message": "Unable to play media. UPnP setup not ready for Devialet device."


What can the user do about this?

The idea is, when you select a source (radio browser for example) and playback is not possible, you get noticed instead of waiting and nothing happens...

But usually when UPnP is not available, browsing media is also not supported. This should (almost) never happen.

Yes, but what does UPnP setup not ready mean for the user? What do they need to do to fix it

There is no known solution, beside maybe restart the speaker.

joostlek · 2025-01-31T11:23:14Z

tests/components/devialet/test_init.py

@@ -13,18 +17,19 @@ async def test_load_unload_config_entry(
    hass: HomeAssistant, aioclient_mock: AiohttpClientMocker
 ) -> None:
    """Test the Devialet configuration entry loading and unloading."""
-    entry = await setup_integration(hass, aioclient_mock)
+    with patch.object(DevialetApi, "upnp_available", return_value=True):


Suggested change

with patch.object(DevialetApi, "upnp_available", return_value=True):

with patch("homeassistant.components.devialet.DevialetApi.upnp_available", return_value=True):

The integration would be a lot nicer if we could patch the library instead of the HTTP calls

Why do you prefer the patch above the patch.object here?

tests/components/devialet/test_media_player.py

joostlek · 2025-01-31T11:23:55Z

tests/components/devialet/test_media_player.py

+        assert entry.state is ConfigEntryState.LOADED
+
+        freezer.tick(10)
+        async_fire_time_changed(hass)


Suggested change

async_fire_time_changed(hass)

async_fire_time_changed(hass)

await hass.async_block_till_done()

Why do we need to skip time here in the first place? The integration already has state at this point right?

If this part is removed, the update has not taken place yet and the attributes are not set.

So without the tick: KeyError: 'volume_level'

I think what Joost means is to explain why the integration is not in a stable state after calling await setup_integration(hass, aioclient_mock), considering that function does:

await hass.config_entries.async_setup(entry.entry_id) await hass.async_block_till_done()

And the config entry setup does an initial refresh of the coordinator:

core/homeassistant/components/devialet/__init__.py

Lines 21 to 22 in 47811e1

await coordinator.async_config_entry_first_refresh()

I think the problem is that the media player entity only sets its state in DevialetMediaPlayerEntity._handle_coordinator_update, which is not called when the entity is added

core/homeassistant/components/devialet/media_player.py

Lines 69 to 74 in 47811e1

@callback

def _handle_coordinator_update(self) -> None:

"""Handle updated data from the coordinator."""

if not self.coordinator.client.is_available:

self.async_write_ha_state()

return

tests/components/devialet/test_media_player.py

joostlek · 2025-01-31T11:26:26Z

tests/components/devialet/test_media_player.py

+        with patch.object(DevialetApi, "equalizer", new_callable=PropertyMock) as mock:
+            mock.return_value = None
+
+            with patch.object(
+                DevialetApi, "night_mode", new_callable=PropertyMock
+            ) as mock:
+                mock.return_value = True
+


Seeing this I would really recommend doing proper mocks, because this isn't ideal. Like, either use library mocks or use http mocks. Don't use both as this becomes hard to maintain

I can improve testing in another PR. It requires a lot of time to me and this is already an existing part.

Yes but the tests themselves can be a lot cleaner than having multiple nested with patches, you can consider patching the full object or merging the patch statements with patch(), patch():

So I merged the patch statements at all locations. It's not perfect, but already a lot cleaner I think. Thanks for the suggestion.

joostlek · 2025-01-31T11:26:44Z

tests/components/devialet/test_media_player.py

+        patch(
+            "homeassistant.components.media_source.async_browse_media",
+            return_value=True,
+        ) as mock_browse_media,


Why do we patch this internal?

What would be another solution to test the async browse media function?

I don't know, I did work on the Spotify one, but I am more wondering why we do this so I can see how we would solve this otherwise, hence its just a question rather than a "this should be changed" :)

I tried to patch the first function after the Devialet integration's call. So that's why I've patched this one. Not sure if it's the best way to test it but it seems to work.

github-actions · 2025-05-20T13:15:01Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

emontnemery · 2025-06-25T12:48:03Z

@fwestenberg if you think the PR is ready for another round of review, or if you expect some feedback, please click the "Ready for review" button.

fwestenberg · 2025-06-25T12:50:10Z

Great idea. I think only the tests are left.

emontnemery · 2025-06-30T13:29:59Z

@fwestenberg Did you see this comment #134168 (comment) ?

fwestenberg · 2025-06-30T13:53:46Z

@fwestenberg Did you see this comment #134168 (comment) ?

I've seen it, but currently I don't have the time to improve/fix this. Beside: the Devialet speaker seems to not play all different stream types from the Radio Browser for example, so it is not very reliable. A very nice solution is to install Music Assistant within Home Assistant and stream using Airplay.

emontnemery · 2025-07-01T11:39:32Z

I've seen it, but currently I don't have the time to improve/fix this

OK, should this PR be moved back to draft state then until you have time to address that change?

home-assistant bot added bugfix cla-signed dependency has-tests integration: devialet integration: stookwijzer new-feature by-code-owner Quality Scale: No score labels Dec 28, 2024

fwestenberg marked this pull request as draft December 28, 2024 20:38

fwestenberg closed this Dec 28, 2024

fwestenberg force-pushed the Devialet-browse-and-play-media branch from e6bb28c to adb1fbb Compare December 28, 2024 21:04

Devialet add play and browse media support

2ad9173

fwestenberg reopened this Dec 28, 2024

fwestenberg marked this pull request as ready for review December 28, 2024 21:43

fwestenberg mentioned this pull request Dec 28, 2024

TTS possible? fwestenberg/devialet#8

Closed

fwestenberg marked this pull request as draft December 28, 2024 21:49

fwestenberg marked this pull request as ready for review December 28, 2024 21:49

fwestenberg marked this pull request as draft December 28, 2024 23:16

fwestenberg added 2 commits December 29, 2024 13:10

Adapt Devialet tests to firmware 2.17.6

f983845

Merge branch 'dev' into Devialet-browse-and-play-media

7d76970

fwestenberg marked this pull request as ready for review December 29, 2024 14:22

Remove unused constant

14bbf7c

fwestenberg marked this pull request as draft December 29, 2024 23:15

Add tests for Devialet

9494f41

fwestenberg marked this pull request as ready for review January 6, 2025 09:10

joostlek requested changes Jan 17, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 17, 2025 16:20

Remove unnecessary properties

100b220

joostlek requested changes Jan 21, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 21, 2025 15:24

fwestenberg added 6 commits January 22, 2025 07:42

Change to patch.object

038ccc5

Merge branch 'dev' into Devialet-browse-and-play-media

2b8252d

Remove unused import

e6bea0a

Remove duplicate import

981bf03

Replace update service with tick

8b7608e

Add browse media test

c64ec32

fwestenberg marked this pull request as ready for review January 29, 2025 22:00

home-assistant bot requested a review from joostlek January 29, 2025 22:00

fwestenberg added 4 commits January 29, 2025 22:33

Improve browse media test

339f012

Add tests for play_media exceptions

0b7f959

Change comment

d60f555

Merge branch 'dev' into Devialet-browse-and-play-media

6770eb6

joostlek requested changes Jan 31, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 31, 2025 11:26

fwestenberg added 3 commits January 31, 2025 13:19

Cleanup tests

ab6d3ea

Test available operations and source

b5da495

Merge branch 'dev' into Devialet-browse-and-play-media

8236a01

github-actions bot added the stale label May 20, 2025

Merge branch 'dev' into Devialet-browse-and-play-media

b25a86d

github-actions bot removed the stale label May 20, 2025

fwestenberg marked this pull request as ready for review June 25, 2025 12:49

home-assistant bot requested a review from joostlek June 25, 2025 12:49

	with patch.object(DevialetApi, "upnp_available", return_value=True):
	with patch("homeassistant.components.devialet.DevialetApi.upnp_available", return_value=True):

	async_fire_time_changed(hass)
	async_fire_time_changed(hass)
	await hass.async_block_till_done()

	@callback
	def _handle_coordinator_update(self) -> None:
	"""Handle updated data from the coordinator."""
	if not self.coordinator.client.is_available:
	self.async_write_ha_state()
	return

Uh oh!

Devialet browse and play media #134168

Are you sure you want to change the base?

Devialet browse and play media #134168

Conversation

fwestenberg commented Dec 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking change

Proposed change

Type of change

Additional information

Checklist

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fwestenberg Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

home-assistant bot commented Jan 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emontnemery Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 20, 2025

Uh oh!

emontnemery commented Jun 25, 2025

Uh oh!

fwestenberg commented Jun 25, 2025

Uh oh!

emontnemery commented Jun 30, 2025

fwestenberg commented Dec 28, 2024 •

edited

Loading

fwestenberg Jan 17, 2025 •

edited

Loading

emontnemery Jun 25, 2025 •

edited

Loading