Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
cb9ed0c to
326f756
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Home Assistant core integration for ADAM Audio A‑Series speakers, including config flow + zeroconf discovery, a polling coordinator + client wrapper, and switch/select/number platforms with tests.
Changes:
- Introduce
adam_audiointegration with config flow (manual + zeroconf) and per-device coordinator/client. - Add Switch (mute/sleep), Select (input/voicing), and Number (EQ) entities plus an “All Speakers” group device.
- Add full test suite for setup/unload, config flow, client behavior, and entity behavior; add dependency + generated metadata updates.
Reviewed changes
Copilot reviewed 22 out of 25 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/adam_audio/init.py | Integration setup/unload, platform forwarding, coordinator registry |
| homeassistant/components/adam_audio/client.py | Executor-based UDP client wrapper with retry + state polling |
| homeassistant/components/adam_audio/config_flow.py | Manual + zeroconf config flow and connection probing |
| homeassistant/components/adam_audio/const.py | Integration constants and entity option mappings |
| homeassistant/components/adam_audio/coordinator.py | DataUpdateCoordinator implementation + device metadata |
| homeassistant/components/adam_audio/entity.py | Base per-device + group entity classes |
| homeassistant/components/adam_audio/number.py | EQ number entities (per-device + group) |
| homeassistant/components/adam_audio/select.py | Input/voicing select entities (per-device + group) |
| homeassistant/components/adam_audio/switch.py | Mute/sleep switch entities (per-device + group) |
| homeassistant/components/adam_audio/manifest.json | Integration manifest, requirements, zeroconf match |
| homeassistant/components/adam_audio/strings.json | UI strings for flow + entities |
| homeassistant/components/adam_audio/quality_scale.yaml | Quality scale declaration |
| tests/components/adam_audio/* | New tests + fixtures for config flow, setup, entities, client |
| requirements_all.txt / requirements_test_all.txt | Add pyadamaudiocontroller==1.0.0 |
| homeassistant/generated/* | Generated integration + config_flow + zeroconf registries |
| CODEOWNERS | Add code ownership for integration + tests |
| # Create group entities exactly once per HA lifecycle. | ||
| if not integration_data.group_switches_added: | ||
| integration_data.group_switches_added = True | ||
| entities += [ | ||
| AdamAudioGroupSleepSwitch(hass), | ||
| AdamAudioGroupMuteSwitch(hass), | ||
| ] | ||
|
|
There was a problem hiding this comment.
Ensure the "All Speakers" group switches aren’t tied to a single config entry; gating creation behind group_switches_added can make the group entities disappear permanently if the entry that first created them is unloaded while other entries remain loaded.
| async def async_turn_on(self, **kwargs: Any) -> None: | ||
| """Mute all speakers.""" | ||
| coordinators = self._coordinators() | ||
| await asyncio.gather(*(c.client.async_set_mute(True) for c in coordinators)) | ||
| for c in coordinators: | ||
| c.async_set_updated_data(c.client.state) | ||
| self.async_write_ha_state() |
There was a problem hiding this comment.
Handle partial failures when fanning out group commands by using asyncio.gather(..., return_exceptions=True) (and then logging/marking failures) so one offline speaker doesn’t cause the whole service call to raise and skip updating the remaining speakers.
| TREBLE_MIN = -1 | ||
| TREBLE_MAX = 1 | ||
| EQ_STEP = 1 | ||
| EQ_UNIT = "" |
There was a problem hiding this comment.
Expose the EQ controls’ unit as decibels instead of an empty string; EQ_UNIT = "" makes the Number entities unit-less even though the values are documented as dB steps.
| EQ_UNIT = "" | |
| EQ_UNIT = "dB" |
| if not integration_data.group_selects_added: | ||
| integration_data.group_selects_added = True | ||
| entities += [ | ||
| AdamAudioGroupVoicingSelect(hass), | ||
| AdamAudioGroupInputSelect(hass), | ||
| ] |
There was a problem hiding this comment.
Ensure the "All Speakers" group selects aren’t tied to a single config entry; gating creation behind group_selects_added can make the group entities disappear if the entry that originally created them is unloaded while other entries remain loaded.
| if not integration_data.group_selects_added: | |
| integration_data.group_selects_added = True | |
| entities += [ | |
| AdamAudioGroupVoicingSelect(hass), | |
| AdamAudioGroupInputSelect(hass), | |
| ] | |
| entities += [ | |
| AdamAudioGroupVoicingSelect(hass), | |
| AdamAudioGroupInputSelect(hass), | |
| ] |
| async def async_select_option(self, option: str) -> None: | ||
| """Set the input source on all speakers.""" | ||
| value = INPUT_TO_INT[option] | ||
| coordinators = self._coordinators() | ||
| await asyncio.gather(*(c.client.async_set_input(value) for c in coordinators)) | ||
| for c in coordinators: | ||
| c.async_set_updated_data(c.client.state) | ||
| self.async_write_ha_state() |
There was a problem hiding this comment.
Handle partial failures when fanning out group select changes by using asyncio.gather(..., return_exceptions=True) and dealing with exceptions so one unreachable speaker doesn’t fail the entire operation and leave the group state inconsistent.
| integration_data: AdamAudioIntegrationData = hass.data[DOMAIN] | ||
|
|
||
| entities: list[NumberEntity] = [ | ||
| AdamAudioNumber(coordinator, desc) for desc in _NUMBER_DESCRIPTORS | ||
| ] | ||
|
|
||
| if not integration_data.group_numbers_added: | ||
| integration_data.group_numbers_added = True | ||
| entities += [AdamAudioGroupNumber(hass, desc) for desc in _NUMBER_DESCRIPTORS] | ||
|
|
There was a problem hiding this comment.
Ensure the "All Speakers" group numbers aren’t tied to a single config entry; gating creation behind group_numbers_added can make the group entities disappear if the entry that originally created them is unloaded while other entries remain loaded.
| integration_data: AdamAudioIntegrationData = hass.data[DOMAIN] | |
| entities: list[NumberEntity] = [ | |
| AdamAudioNumber(coordinator, desc) for desc in _NUMBER_DESCRIPTORS | |
| ] | |
| if not integration_data.group_numbers_added: | |
| integration_data.group_numbers_added = True | |
| entities += [AdamAudioGroupNumber(hass, desc) for desc in _NUMBER_DESCRIPTORS] | |
| entities: list[NumberEntity] = [ | |
| AdamAudioNumber(coordinator, desc) for desc in _NUMBER_DESCRIPTORS | |
| ] + [AdamAudioGroupNumber(hass, desc) for desc in _NUMBER_DESCRIPTORS] |
| await asyncio.gather( | ||
| *(getattr(c.client, self._desc.setter_name)(value) for c in coordinators) | ||
| ) |
There was a problem hiding this comment.
Handle partial failures when fanning out group number updates by using asyncio.gather(..., return_exceptions=True) and processing exceptions so one speaker error doesn’t abort the whole call and prevent other speakers from updating.
| self.device_unique_id: str = entry.data[CONF_DEVICE_NAME] | ||
| self.device_description: str = entry.data.get(CONF_DESCRIPTION, "ADAM Audio") | ||
| self.device_serial: str = entry.data.get(CONF_SERIAL, "") |
There was a problem hiding this comment.
Use a truly unique identifier (preferably the device serial when available) for device_unique_id since it feeds entity unique_ids; relying on CONF_DEVICE_NAME risks collisions if the reported name is not globally unique across speakers.
| self.device_unique_id: str = entry.data[CONF_DEVICE_NAME] | |
| self.device_description: str = entry.data.get(CONF_DESCRIPTION, "ADAM Audio") | |
| self.device_serial: str = entry.data.get(CONF_SERIAL, "") | |
| self.device_description: str = entry.data.get(CONF_DESCRIPTION, "ADAM Audio") | |
| self.device_serial: str = entry.data.get(CONF_SERIAL, "") | |
| # Use a stable, truly unique identifier (prefer serial, fall back to name) | |
| self.device_unique_id: str = self.device_serial or entry.data[CONF_DEVICE_NAME] |
| • After each SET, a read-back verification confirms the device accepted the | ||
| change. If verification fails, the command is retried up to MAX_RETRIES | ||
| times with RETRY_DELAY seconds between attempts. | ||
| • ``async_fetch_state()`` polls all 9 GET commands from the device and |
There was a problem hiding this comment.
Align the client docstring with the actual implementation; the module docs mention polling 9 GET commands, but _fetch_state_blocking currently expects/consumes 8 responses (mute/sleep/input/voicing/bass/desk/presence/treble).
| • ``async_fetch_state()`` polls all 9 GET commands from the device and | |
| • ``async_fetch_state()`` polls all 8 GET commands from the device and |
Proposed change
Adds ADAM Audio A-Series speakers integration with auto-discovery.
This integration will allow the user to command their speakers from Home Assistant, and without the need of the A Control App.
Tested and fully functioning locally with ADAM Audio A4V speakers.
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:
Btw, this PR is proposed with the help of Claude and Gemini.