Use pyblu library in bluesound #117257
Use pyblu library in bluesound #117257
Conversation
|
Hey there @thrawnarn, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
LouisChrist
changed the title
| with suppress(TimeoutError): | ||
| await self.async_update_sync_status() | ||
| await self.async_update_presets() | ||
| await self.async_update_captures() |
There was a problem hiding this comment.
send_bluesound_command(which i replaced by the library) was called with raise_timeout=False which suppressed TimeoutError. I pulled that logic one level up and put it here.
| if self._sync_status is None: | ||
| return None |
There was a problem hiding this comment.
SyncStatus is one endpoint of the device. It starts with None and might be None if the device is not reachable.
There was a problem hiding this comment.
If the device is not reachable, I think we should raise an issue like PlatformNotReady I think
There was a problem hiding this comment.
I changed the return None to raise PlatformNotReady.
There was a problem hiding this comment.
No I mean, in the async_setup or async_setup_platform. If we can't fetch the state, that means the device doesn't respond and we can't get data. Right? In that case we should raise platform during setup
There was a problem hiding this comment.
Ah ok. I can do that.
There is one caveat: This integrations supports multiple devices. Throwing a PlatformNotReady would result in none of those working if one of the devices is not reachable.
There was a problem hiding this comment.
Right, let me ask some others how they did this back in the day
|
|
||
| async def async_media_stop(self) -> None: | ||
| """Send stop command.""" | ||
| if self.is_grouped and not self.is_master: | ||
| return | ||
|
|
||
| return await self.send_bluesound_command("Pause") | ||
| async with timeout(BluesoundPlayer.COMMAND_TIMEOUT): |
There was a problem hiding this comment.
Why don't we put this timeout in the library instead?
There was a problem hiding this comment.
The old code did not use the timeout functionality of aiohttp and used async with timeout(...) instead and i kept it that way.
There was a problem hiding this comment.
So it essentially does the same?
There was a problem hiding this comment.
Yes it should be equivalent. I had a quick look at aiohttp and it seems to use asyncio.timeout_at internally.
MartinHjelmare
changed the title
Breaking change
Sources behave slightly different. Radio station providers(e.g. TuneIn, Radio Paradise) are not part of source_list anymore as they are not selectable as source. Presets which play a specific radio station can be created in the BluOS app and are displayed as sources in Home Assistant. The currently selected source tries to match those presets and falls back to the radio station provider(e.g. TuneIn, Radio Paradise).
Proposed change
Adds pypi library for all device calls: pyblu
Type of change
Additional information
Requirement for other PR: PR: Add config_flow to bluesound
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: