Add select platform to Vistapool#172547
Open
fdebrus wants to merge 7 commits into
Open
Conversation
Adds the select platform so users can choose pool-controller modes
from Home Assistant. Five entities exposed (all EntityCategory.CONFIG):
- Pump mode (filtration.mode) - manual / auto / heat / smart / intel
- Pump speed (filtration.manVel) - slow / medium / high
- Filtration timer speed 1-3 (filtration.timerVel{1,2,3}) - slow /
medium / high each
VistapoolSelectEntityDescription extends SelectEntityDescription with
value_path and exists_path; the inherited options list serves as both
the HA-facing option set and the integer-to-name mapping. current_option
indexes into options using a _to_index helper that tolerates string-int
values from the API. async_select_option writes back via
AquariteClient.set_value with the option's index; AquariteError becomes
the existing translated set_failed HomeAssistantError.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a select platform to the Vistapool integration (pump mode/speed and timer speeds), including entity translations and new tests/snapshots.
Changes:
- Implement Vistapool
SelectEntitysupport with multiple select entity descriptions and write-back via the API. - Add translations for new select entities and a new translated error message for set failures.
- Add test coverage + snapshots for select entities, and enable the select platform in the integration.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/vistapool/test_select.py | New tests validating select entity creation, option mapping, service calls, and error handling. |
| tests/components/vistapool/snapshots/test_select.ambr | New snapshots for registry entries and states for the added select entities. |
| homeassistant/components/vistapool/strings.json | Adds select entity translations and a set_failed error translation key. |
| homeassistant/components/vistapool/select.py | New select platform implementation including option mapping and API write-back. |
| homeassistant/components/vistapool/quality_scale.yaml | Updates quality scale rule annotations related to actions. |
| homeassistant/components/vistapool/init.py | Enables the select platform alongside sensor. |
8 tasks
Confirms the HomeAssistantError raised by the select_option handler is the expected set_failed one rather than an unrelated HomeAssistantError.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c45432bc4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The case passes filtration.mode=None (present-but-null), not an absent
key, so the label is now accurate. Both null and missing-key routes
flow through coordinator.get_value("filtration.mode") == None and
land in the same "unknown" state, so coverage is unchanged.
0.5.1 wraps aiohttp.ClientError and asyncio.TimeoutError from both the REST send_command path and the upstream auth client refresh in ConnectionError (an AquariteError subclass), so transport failures surface as the translated set_failed HomeAssistantError via the existing except AquariteError clause.
Comment on lines
+71
to
+78
| if description.exists_path is not None: | ||
| required = ( | ||
| (description.exists_path,) | ||
| if isinstance(description.exists_path, str) | ||
| else description.exists_path | ||
| ) | ||
| if not all(coordinator.get_value(path) for path in required): | ||
| continue |
Comment on lines
+131
to
+136
| try: | ||
| await self.coordinator.api.set_value( | ||
| self.coordinator.pool_id, | ||
| self.entity_description.value_path, | ||
| index, | ||
| ) |
Comment on lines
+123
to
+130
| try: | ||
| index = options.index(option) | ||
| except ValueError as err: | ||
| raise HomeAssistantError( | ||
| translation_domain=DOMAIN, | ||
| translation_key="set_failed", | ||
| translation_placeholders={"entity": self.entity_id}, | ||
| ) from err |
Comment on lines
+137
to
+142
| except AquariteError as err: | ||
| raise HomeAssistantError( | ||
| translation_domain=DOMAIN, | ||
| translation_key="set_failed", | ||
| translation_placeholders={"entity": self.entity_id}, | ||
| ) from err |
| assert await hass.config_entries.async_setup(mock_config_entry.entry_id) | ||
| await hass.async_block_till_done() | ||
|
|
||
| assert hass.states.get("select.my_pool_pump_mode").state == "unknown" |
This reverts commit 6dbf274.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Adds the select platform to the Vistapool integration. Lets users pick controller modes from Home Assistant.
Selects exposed (per pool, all
EntityCategory.CONFIG):filtration.mode. Options:manual,auto,heat,smart,intel.filtration.manVel. Options:slow,medium,high.filtration.timerVel{1,2,3}. Same speed options.Implementation notes
Same patterns as the merged
sensor/ openbinary_sensor/switch/number:VistapoolSelectEntityDescriptionextendsSelectEntityDescriptionwithvalue_pathandexists_path(single path or tuple). The inheritedoptions: list[str] | Noneserves both as the HA-facing option set and the integer-to-name mapping for read/write.*(VistapoolSelectEntityDescription(...) for i in (1, 2, 3))instead of three near-identical literals.current_optionindexes intooptionsusing a_to_indexhelper that tolerates string-int values from the API (int("3")→3) and returnsNonefor missing / unparseable / out-of-range raw values.async_select_optionwrites the option's index back viaAquariteClient.set_value.AquariteErrorbecomes a translatedHomeAssistantErrorusing the existingset_failedtranslation key (introduced in the switch PR).quality_scale.yamlaction-exceptionsmoves fromexempttodone.Tests
tests/components/vistapool/test_select.pycovers:test_all_entitieswithsnapshot_platform(pins every entity's registry entry + state).current_optionhappy path across int indices (0, 2, 4) and a stringified-int ("3").current_optionunknown across missing (None), non-numeric string, and out-of-range index.select_optionacrosspump_mode,pump_speed, and afiltration_timer_speed_*to pin the round-trip and the API path each entity writes to.AquariteErrorfrom the library is re-raised asHomeAssistantError.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: