Add Homevolt battery integration #160416
Add Homevolt battery integration #160416
Conversation
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR adds a new integration for Homevolt battery systems, enabling local network monitoring and control of Homevolt battery devices. The integration implements Bronze-level quality scale requirements with support for config flow-based setup, sensor entities, and a data update coordinator for polling device status.
Key changes:
- Implements config flow with connection validation and error handling for authentication and connectivity issues
- Adds sensor platform supporting multiple sensor types (power, energy, voltage, temperature, battery percentage, etc.)
- Uses DataUpdateCoordinator for efficient 15-second polling updates with proper error handling
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/homevolt/init.py | Entry point implementing async setup/unload with coordinator initialization and connection testing |
| homeassistant/components/homevolt/config_flow.py | Config flow with user input validation, unique ID enforcement, and exception handling |
| homeassistant/components/homevolt/coordinator.py | Data update coordinator managing device polling with authentication and connection error handling |
| homeassistant/components/homevolt/sensor.py | Sensor platform creating entities for various device measurements with device info and unique IDs |
| homeassistant/components/homevolt/const.py | Domain constants and type definitions including custom ConfigEntry type |
| homeassistant/components/homevolt/manifest.json | Integration metadata defining requirements, IoT class, and quality scale level |
| homeassistant/components/homevolt/strings.json | Localization strings for config flow UI elements |
| homeassistant/components/homevolt/quality_scale.yaml | Quality scale rule tracking showing Bronze completion and Silver/Gold progress |
| tests/components/homevolt/test_config_flow.py | Comprehensive config flow tests covering success, error recovery, and duplicate prevention |
| tests/components/homevolt/conftest.py | Test fixtures mocking async_setup_entry for config flow tests |
| requirements_all.txt | Adds homevolt==0.2.3 dependency |
| requirements_test_all.txt | Adds homevolt==0.2.3 test dependency |
| homeassistant/generated/config_flows.py | Registers homevolt in config flow list |
| homeassistant/generated/integrations.json | Adds integration metadata to generated registry |
| self._attr_device_info = DeviceInfo( | ||
| identifiers={(DOMAIN, f"{device_id}_{sensor_device_id}")}, | ||
| configuration_url=coordinator.client.hostname, | ||
| manufacturer=MANUFACTURER, | ||
| model=device_metadata.model, | ||
| name=device_metadata.name, |
There was a problem hiding this comment.
The device_metadata retrieved using .get() could return None if the sensor_device_id is not found in the device_metadata dictionary. Accessing .model and .name attributes on a None value will raise an AttributeError. Consider adding validation to handle this case, either by checking if device_metadata is not None before accessing its attributes, or by using a default value.
| self._attr_device_info = DeviceInfo( | |
| identifiers={(DOMAIN, f"{device_id}_{sensor_device_id}")}, | |
| configuration_url=coordinator.client.hostname, | |
| manufacturer=MANUFACTURER, | |
| model=device_metadata.model, | |
| name=device_metadata.name, | |
| model = device_metadata.model if device_metadata is not None else None | |
| name = device_metadata.name if device_metadata is not None else description.name | |
| self._attr_device_info = DeviceInfo( | |
| identifiers={(DOMAIN, f"{device_id}_{sensor_device_id}")}, | |
| configuration_url=coordinator.client.hostname, | |
| manufacturer=MANUFACTURER, | |
| model=model, | |
| name=name, |
| HomevoltSensor( | ||
| SensorEntityDescription( | ||
| key=sensor_name, | ||
| name=sensor_name, |
There was a problem hiding this comment.
The entity is using the raw sensor_name directly as the name instead of using translation keys. According to the coding guidelines, entities with has_entity_name set to True should use translation keys via the _attr_translation_key attribute. This would make the entity names translatable and consistent with Home Assistant best practices. Consider setting _attr_translation_key in the SensorEntityDescription instead of using name=sensor_name.
| async def async_setup_entry(hass: HomeAssistant, entry: HomevoltConfigEntry) -> bool: | ||
| """Set up Homevolt from a config entry.""" | ||
| host: str = entry.data[CONF_HOST] | ||
| password: str | None = entry.data.get(CONF_PASSWORD) | ||
|
|
||
| websession = async_get_clientsession(hass) | ||
| client = Homevolt(host, password, websession=websession) | ||
|
|
||
| try: | ||
| await client.update_info() | ||
| except HomevoltConnectionError as err: | ||
| raise ConfigEntryNotReady( | ||
| f"Unable to connect to Homevolt battery: {err}" | ||
| ) from err | ||
|
|
||
| coordinator = HomevoltDataUpdateCoordinator(hass, entry, client) | ||
| await coordinator.async_config_entry_first_refresh() | ||
|
|
||
| entry.runtime_data = coordinator | ||
|
|
||
| await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) | ||
|
|
||
| return True |
There was a problem hiding this comment.
The async_setup_entry function in the init module lacks test coverage. Tests should verify successful setup, error handling for connection failures (ConfigEntryNotReady), and that the coordinator is properly initialized with first refresh. Similar integrations in this repository include test_init.py files with comprehensive coverage of setup scenarios.
| STEP_USER_DATA_SCHEMA = vol.Schema( | ||
| { | ||
| vol.Required(CONF_HOST): str, | ||
| vol.Optional(CONF_PASSWORD): str, |
There was a problem hiding this comment.
What do we use the password for? When does a device have one?
| parallel-updates: | ||
| status: exempt | ||
| comment: Coordinator handles updates, no explicit parallel updates needed. |
There was a problem hiding this comment.
We can still mention it explicitly
| @dataclass | ||
| class Description: | ||
| """Sensor metadata description.""" | ||
|
|
||
| device_class: SensorDeviceClass | None | ||
| state_class: SensorStateClass | None | ||
| native_unit_of_measurement: str | None | ||
|
|
||
|
|
||
| SENSOR_META: dict[SensorType, Description] = { | ||
| SensorType.COUNT: Description( | ||
| None, |
There was a problem hiding this comment.
Why don''t you use SensorEntityDescriptions for this?
| sensor_device_id = sensor.device_identifier | ||
| device_metadata = coordinator.data.device_metadata.get(sensor_device_id) | ||
| self._attr_device_info = DeviceInfo( | ||
| identifiers={(DOMAIN, f"{device_id}_{sensor_device_id}")}, |
| @property | ||
| def native_value(self) -> StateType: | ||
| """Return the native value of the sensor.""" | ||
| return self.coordinator.data.sensors[self.entity_description.key].value |
There was a problem hiding this comment.
what if a sensor doesn't expose everything?
Breaking change
Proposed change
Add Homevolt battery integration
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: