Replace custom actions for sleep timer with buttons in bluesound integration #133604
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
LouisChrist
force-pushed
the
bluesound_buttons_for_sleep_timer
branch
from
c0e68d0 to
912529b
Compare
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
I have created a new PR #135125 for the data update coordinator. I will update this one after the other one has been merged. |
| config_entry.data[CONF_PORT], | ||
| ), | ||
| ], | ||
| update_before_add=True, |
There was a problem hiding this comment.
| update_before_add=True, |
They don't have state
| ) | ||
|
|
||
|
|
||
| class SetSleepTimerButton(CoordinatorEntity[BluesoundCoordinator], ButtonEntity): |
There was a problem hiding this comment.
Would it maybe make sense to create a base entity that will be inherited in both the media player and the buttons? It would help deduplicate the device info logic
There was a problem hiding this comment.
I would reommend doing this in a separate PR
| async def async_press(self) -> None: | ||
| """Set the sleep timer.""" | ||
| await self._player.sleep_timer() | ||
|
|
||
|
|
||
| class ClearSleepTimerButton(CoordinatorEntity[BluesoundCoordinator], ButtonEntity): | ||
| """Representation of a sleep timer button.""" | ||
|
|
||
| def __init__( | ||
| self, coordinator: BluesoundCoordinator, player: Player, port: int | ||
| ) -> None: | ||
| """Initialize the Bluesound button.""" | ||
| super().__init__(coordinator) | ||
| sync_status = coordinator.data.sync_status | ||
|
|
||
| self._player = player | ||
| self._attr_unique_id = ( | ||
| f"clear-sleep-timer-{format_unique_id(sync_status.mac, port)}" | ||
| ) | ||
| self._attr_name = f"{sync_status.name} Clear Sleep Timer" | ||
| self._attr_device_info = generate_device_info(sync_status, port) | ||
| self._attr_entity_registry_enabled_default = False | ||
|
|
||
| async def async_press(self) -> None: | ||
| """Clear the sleep timer.""" | ||
| sleep = -1 | ||
| while sleep != 0: | ||
| sleep = await self._player.sleep_timer() |
There was a problem hiding this comment.
Instead of having 2 classes, create a generic class that takes in an ButtonEntityDescription, which you can extend for Bluesound to add a press_fn which you execute when someone presses the button. This way we can deduplicate even less
| @pytest.fixture | ||
| async def setup_config_entry_buttons_enabled( | ||
| hass: HomeAssistant, | ||
| config_entry: MockConfigEntry, | ||
| player_mocks: PlayerMocks, | ||
| entity_registry: er.EntityRegistry, | ||
| ) -> None: | ||
| """Set up the platform.""" | ||
| player_name = player_mocks.player_data.sync_status_long_polling_mock.get().name | ||
|
|
||
| entity_registry.async_get_or_create( | ||
| domain=BUTTON_DOMAIN, | ||
| platform=DOMAIN, | ||
| unique_id=f"set-sleep-timer-{config_entry.unique_id}", | ||
| suggested_object_id=f"{player_name}_set_sleep_timer", | ||
| disabled_by=None, | ||
| ) | ||
|
|
||
| entity_registry.async_get_or_create( | ||
| domain=BUTTON_DOMAIN, | ||
| platform=DOMAIN, | ||
| unique_id=f"clear-sleep-timer-{config_entry.unique_id}", | ||
| suggested_object_id=f"{player_name}_clear_sleep_timer", | ||
| disabled_by=None, | ||
| ) | ||
|
|
||
| config_entry.add_to_hass(hass) | ||
| assert await hass.config_entries.async_setup(config_entry.entry_id) | ||
| await hass.async_block_till_done() |
There was a problem hiding this comment.
We have a fixture for this
@pytest.mark.usefixtures("entity_registry_enabled_by_default")
| buttons = [ | ||
| BluesoundButton( | ||
| config_entry.runtime_data.coordinator, | ||
| config_entry.runtime_data.player, | ||
| config_entry.data[CONF_PORT], | ||
| description, | ||
| ) | ||
| for description in BUTTON_DESCRIPTIONS | ||
| ] | ||
|
|
||
| async_add_entities(buttons) |
There was a problem hiding this comment.
| buttons = [ | |
| BluesoundButton( | |
| config_entry.runtime_data.coordinator, | |
| config_entry.runtime_data.player, | |
| config_entry.data[CONF_PORT], | |
| description, | |
| ) | |
| for description in BUTTON_DESCRIPTIONS | |
| ] | |
| async_add_entities(buttons) | |
| async_add_entities( | |
| BluesoundButton( | |
| config_entry.runtime_data.coordinator, | |
| config_entry.runtime_data.player, | |
| config_entry.data[CONF_PORT], | |
| description, | |
| ) | |
| for description in BUTTON_DESCRIPTIONS | |
| ) |
| async def set_sleep_timer(player: Player) -> None: | ||
| """Set the sleep timer.""" | ||
| await player.sleep_timer() | ||
|
|
||
|
|
||
| BUTTON_DESCRIPTIONS = [ | ||
| BluesoundButtonEntityDescription( | ||
| key="set_sleep_timer", | ||
| name="Set Sleep Timer", | ||
| press_fn=set_sleep_timer, | ||
| ), |
There was a problem hiding this comment.
| async def set_sleep_timer(player: Player) -> None: | |
| """Set the sleep timer.""" | |
| await player.sleep_timer() | |
| BUTTON_DESCRIPTIONS = [ | |
| BluesoundButtonEntityDescription( | |
| key="set_sleep_timer", | |
| name="Set Sleep Timer", | |
| press_fn=set_sleep_timer, | |
| ), | |
| BUTTON_DESCRIPTIONS = [ | |
| BluesoundButtonEntityDescription( | |
| key="set_sleep_timer", | |
| name="Set Sleep Timer", | |
| press_fn=lambda player: player.sleep_timer, | |
| ), |
There was a problem hiding this comment.
The typing is not working out. press_fn returns None, but player.sleep_timer returns an int.
| ), | ||
| BluesoundButtonEntityDescription( | ||
| key="clear_sleep_timer", | ||
| name="Clear Sleep Timer", |
There was a problem hiding this comment.
Let's use entity translations for this. Assign a translation_key and make a reference in strings.json.
(You can also use that translation key to add a nice icon in icons.json)
|
|
||
| self.entity_description = description | ||
| self._player = player | ||
| self._attr_entity_registry_enabled_default = False |
There was a problem hiding this comment.
Let's set the enabled default in the entity descriptions
| self._player = player | ||
| self._attr_entity_registry_enabled_default = False | ||
| self._attr_unique_id = ( | ||
| f"{self.entity_description.key}-{format_unique_id(sync_status.mac, port)}" |
There was a problem hiding this comment.
| f"{self.entity_description.key}-{format_unique_id(sync_status.mac, port)}" | |
| f"{description.key}-{format_unique_id(sync_status.mac, port)}" |
| self._attr_unique_id = ( | ||
| f"{self.entity_description.key}-{format_unique_id(sync_status.mac, port)}" | ||
| ) | ||
| self._attr_name = f"{sync_status.name} {description.name}" |
There was a problem hiding this comment.
Please set _attr_has_entity_name = True. and if you do the translation thing theres nothing more you have to do
There was a problem hiding this comment.
I have set has_entity_name in the EntityDescription
| self.hass, | ||
| DOMAIN, | ||
| f"deprecated_service_{SERVICE_SET_TIMER}", | ||
| breaks_in_ha_version="2025.7.0", |
There was a problem hiding this comment.
| breaks_in_ha_version="2025.7.0", | |
| breaks_in_ha_version="2025.8.0", |
| self.hass, | ||
| DOMAIN, | ||
| f"deprecated_service_{SERVICE_CLEAR_TIMER}", | ||
| breaks_in_ha_version="2025.7.0", |
There was a problem hiding this comment.
| breaks_in_ha_version="2025.7.0", | |
| breaks_in_ha_version="2025.8.0", |
LouisChrist
force-pushed
the
bluesound_buttons_for_sleep_timer
branch
from
72b1972 to
f0eadf3
Compare
Proposed change
This replaces the custom actions
bluesound.set_sleep_timerandbluesound.clear_sleep_timerwith two corresponding buttons.The actions are deprecated, but still there. The version I defined for deprecation is a guess. I am not sure what the policies regarding deprecation are.
The newly added buttons are disabled by default because I think they are more of an edge case.
Type of change
Additional information
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.To help with the load of incoming pull requests: