Add Wyoming satellite #104759
Add Wyoming satellite #104759
Conversation
home-assistant
bot
added
cla-signed
has-tests
integration: wyoming
new-feature
new-platform
labels
|
Hey there @balloob, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| ) | ||
| platforms = item.service.platforms | ||
| if item.satellite is not None: | ||
| platforms += SATELLITE_PLATFORMS |
There was a problem hiding this comment.
| platforms += SATELLITE_PLATFORMS | |
| platforms = platforms + SATELLITE_PLATFORMS |
There was a problem hiding this comment.
Honestly, I think this suggestion isn't an improvement...
There was a problem hiding this comment.
The intent is to not change the original variable anymore
There was a problem hiding this comment.
The new platforms only seem to calculate state themselves without state input from a device or service. The binary sensor is borderline as it interprets pipeline events as state, if I understand correctly. The select and switch entities look like virtual config entities. We don't allow this normally.
| return WyomingSatellite(hass, service, satellite_device) | ||
|
|
||
|
|
||
| async def update_listener(hass: HomeAssistant, entry: ConfigEntry): |
There was a problem hiding this comment.
Missing return value typing.
|
|
||
| # Use zeroconf name + service name as unique id. | ||
| # The satellite will use its own MAC as the zeroconf name by default. | ||
| unique_id = f"{discovery_info.name}_{self._name}" |
There was a problem hiding this comment.
Use the local variable name instead of the attribute.
| ) | ||
|
|
||
| return self.async_create_entry( | ||
| title=self._name, |
There was a problem hiding this comment.
Define a local variable when accessing an attribute more than once.
| def __init__(self, device: SatelliteDevice) -> None: | ||
| """Initialize entity.""" | ||
| self._device = device | ||
| self._attr_unique_id = f"{device.satellite_id}-{self.entity_description.key}" |
There was a problem hiding this comment.
There's no entity description attribute set. It needs to be set here in the parent if we need to use it here.
| """Run when run() has fully stopped.""" | ||
| _LOGGER.debug("Satellite task stopped") | ||
|
|
||
| # ------------------------------------------------------------------------- |
| """Turn on.""" | ||
| self._attr_is_on = True | ||
| self.async_write_ha_state() | ||
| self._device.set_is_enabled(True) |
There was a problem hiding this comment.
Why do we change the device after updating the entity state and not before? Normally the entity should reflect the device feature state and not the other way around.
| item.satellite = _make_satellite(hass, entry, service) | ||
|
|
||
| # Set up satellite sensors, switches, etc. | ||
| await hass.config_entries.async_forward_entry_setups(entry, SATELLITE_PLATFORMS) |
There was a problem hiding this comment.
Note that it would be an error if we tried to forward the config entry more than once to a platform. There's no overlap at the moment, and I guess it would be caught in tests.
| hass: HomeAssistant, init_satellite, satellite_config_entry: ConfigEntry | ||
| ) -> SatelliteDevice: | ||
| """Get a satellite device fixture.""" | ||
| return hass.data[DOMAIN][satellite_config_entry.entry_id].satellite.device |
There was a problem hiding this comment.
Don't access hass.data in tests.
| state = hass.states.get(assist_in_progress_id) | ||
| assert state is not None | ||
| assert state.state == STATE_OFF | ||
| assert not satellite_device.is_active |
There was a problem hiding this comment.
Don't assert the device. It's a detail of the integration.
| assert hass.states.get(pipeline_entity_id) is not None | ||
|
|
||
| # Remove | ||
| device_registry.async_remove_device(satellite_device.device_id) |
There was a problem hiding this comment.
Removing a device and checking that the entity state is gone is more testing the device registry than testing this integration.
* First draft of Wyoming satellite * Set up homeassistant in tests * Move satellite * Add devices with binary sensor and select * Add more events * Add satellite enabled switch * Fix mistake * Only set up necessary platforms for satellites * Lots of fixes * Add tests * Use config entry id as satellite id * Initial satellite test * Add satellite pipeline test * More tests * More satellite tests * Only support single device per config entry * Address comments * Make a copy of platforms
Breaking change
Proposed change
NOTE: Tests still need to be added.
Raspberry Pi voice satellites currently use the websocket API, which does not allow them to be proper HA devices with an area and HA controls/sensors like ESPHome and VoIP.
This PR adds a "Wyoming Satellite", which uses recent additions to the Wyoming protocol to include:
Two satellite modes are supported (controlled by the satellite):
A reference implementation of the satellite client is available in the wyoming-satellite project.
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..coveragerc.To help with the load of incoming pull requests: