Add solarman integration#152525
Conversation
home-assistant
Bot
added
cla-needed
has-tests
new-integration
integration: solarman
labels
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @solarmanpv
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
Bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
- The title mentions Solarman, but there are 2 integrations being added? What's the difference between them?
- We should only add one platform at a time
- Please use a proper development environment
- Device or service specific data should be put in a library hosted on PyPI
|
Thanks for the review and feedback!
This PR should be solely for the Solarman integration. The Solarman and Indevolt are for different devices and have different sensors. I have now removed all Indevolt-related code and has submit it in a separate PR.
OK, I will the switch platform from this PR to keep it focused on the sensor platform and will create a new PR for the switch once this one is merged.
Could you please provide more details on this point? Thanks!
I'm working on moving the device logic to a PyPI library and will integrate it here once it's ready. |
https://developers.home-assistant.io/docs/development_environment |
There was a problem hiding this comment.
Hi @xiaozhouhhh
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
There was a problem hiding this comment.
Pull request overview
Adds a new solarman integration to Home Assistant Core, including config flow, sensor entities, and test coverage with fixtures/snapshots.
Changes:
- Introduce the
solarmanintegration with config flow (user + zeroconf), coordinator, base entity, and sensor platform. - Add tests (config flow, init load/unload, sensors + snapshots) and device fixtures for multiple models.
- Register dependency + generated metadata updates (requirements, zeroconf, config flows, integrations index, CODEOWNERS).
Reviewed changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/solarman/test_sensor.py | Adds sensor + availability tests for multiple Solarman device fixtures. |
| tests/components/solarman/test_init.py | Adds config entry setup/unload and setup failure tests. |
| tests/components/solarman/test_config_flow.py | Adds user and zeroconf config flow tests (success, errors, duplicates, IP update). |
| tests/components/solarman/snapshots/test_sensor.ambr | Stores snapshot baselines for created sensor entities/states. |
| tests/components/solarman/fixtures/gl meter/data.json | Adds fixture payload for “gl meter” device telemetry. |
| tests/components/solarman/fixtures/gl meter/config.json | Adds fixture payload for “gl meter” device config response. |
| tests/components/solarman/fixtures/SP-2W-EU/data.json | Adds fixture payload for smart plug telemetry. |
| tests/components/solarman/fixtures/SP-2W-EU/config.json | Adds fixture payload for smart plug config response. |
| tests/components/solarman/fixtures/P1-2W/data.json | Adds fixture payload for P1 meter telemetry. |
| tests/components/solarman/fixtures/P1-2W/config.json | Adds fixture payload for P1 meter config response. |
| tests/components/solarman/conftest.py | Provides shared fixtures: config entry + mocked Solarman client. |
| tests/components/solarman/init.py | Adds helper to set up the integration in tests. |
| requirements_test_all.txt | Adds solarman-opendata to test requirements bundle. |
| requirements_all.txt | Adds solarman-opendata to full requirements bundle. |
| homeassistant/generated/zeroconf.py | Registers _solarman._tcp.local. to map to the solarman domain. |
| homeassistant/generated/integrations.json | Registers integration metadata (name/type/config_flow/iot_class). |
| homeassistant/generated/config_flows.py | Adds solarman to config flow domain list. |
| homeassistant/components/solarman/strings.json | Adds config flow strings, entity translations, and exception translation key. |
| homeassistant/components/solarman/sensor.py | Implements sensor platform with entity descriptions + coordinator-backed values. |
| homeassistant/components/solarman/quality_scale.yaml | Adds quality scale tracking file for the new integration. |
| homeassistant/components/solarman/manifest.json | Adds integration manifest: dependency, config flow, zeroconf, codeowners, docs URL. |
| homeassistant/components/solarman/entity.py | Adds base entity providing DeviceInfo (identifiers, manufacturer, model, etc.). |
| homeassistant/components/solarman/coordinator.py | Adds DataUpdateCoordinator wrapping the Solarman client polling. |
| homeassistant/components/solarman/const.py | Adds domain/constants/model mapping and platform list. |
| homeassistant/components/solarman/config_flow.py | Adds config flow for user and zeroconf discovery/confirmation. |
| homeassistant/components/solarman/init.py | Adds async_setup_entry/async_unload_entry wiring for coordinator + platforms. |
| CODEOWNERS | Adds code ownership for the new integration + its tests. |
You can also share your feedback on Copilot code review. Take the survey.
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="update_failed", | ||
| ) from e |
| SensorEntityDescription( | ||
| key="power factor", | ||
| translation_key="power_factor", | ||
| native_unit_of_measurement=PERCENTAGE, | ||
| device_class=SensorDeviceClass.POWER_FACTOR, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), |
| self.client = Solarman( | ||
| async_get_clientsession(self.hass), self.host, DEFAULT_PORT |
|
|
||
| mock_solarman.fetch_data.side_effect = ConnectionError | ||
| freezer.tick(UPDATE_INTERVAL) | ||
| async_fire_time_changed(hass) |
| config-entry-unloading: todo | ||
| docs-configuration-parameters: | ||
| status: exempt | ||
| comment: | | ||
| This integration does not have an options flow. | ||
| docs-installation-parameters: todo | ||
| entity-unavailable: todo | ||
| integration-owner: todo | ||
| log-when-unavailable: todo | ||
| parallel-updates: todo |
| config-entry-unloading: todo | ||
| docs-configuration-parameters: | ||
| status: exempt | ||
| comment: | | ||
| This integration does not have an options flow. | ||
| docs-installation-parameters: todo | ||
| entity-unavailable: todo | ||
| integration-owner: todo | ||
| log-when-unavailable: todo | ||
| parallel-updates: todo |
| @@ -0,0 +1,86 @@ | |||
| rules: | |||
| assert coordinator.config_entry | ||
| super().__init__(coordinator) | ||
| self.entity_description = description | ||
| self._attr_unique_id = f"{coordinator.config_entry.unique_id}_{description.key}" |
There was a problem hiding this comment.
Pull request overview
This PR adds a new solarman integration to Home Assistant Core, including configuration flow (user + zeroconf), a sensor platform backed by a DataUpdateCoordinator, and initial automated test coverage with fixtures/snapshots.
Changes:
- Add
homeassistant.components.solarmanintegration (manifest, config flow, coordinator, entities/sensors, strings, quality scale). - Register Zeroconf service mapping and generated metadata for config flows/integrations.
- Add tests/fixtures/snapshots and introduce the
solarman-opendatadependency.
Reviewed changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/solarman/test_sensor.py | Adds sensor + availability tests (snapshot-based and unavailable behavior). |
| tests/components/solarman/test_init.py | Adds config entry setup/unload and setup failure tests. |
| tests/components/solarman/test_config_flow.py | Adds user + zeroconf config flow tests (success/error/duplicate/ip change). |
| tests/components/solarman/snapshots/test_sensor.ambr | Captures snapshot expectations for created sensor entities/states. |
| tests/components/solarman/fixtures/gl meter/data.json | Adds device data fixture for “gl meter” model. |
| tests/components/solarman/fixtures/gl meter/config.json | Adds device config fixture for “gl meter” model. |
| tests/components/solarman/fixtures/SP-2W-EU/data.json | Adds device data fixture for “SP-2W-EU” model. |
| tests/components/solarman/fixtures/SP-2W-EU/config.json | Adds device config fixture for “SP-2W-EU” model. |
| tests/components/solarman/fixtures/P1-2W/data.json | Adds device data fixture for “P1-2W” model. |
| tests/components/solarman/fixtures/P1-2W/config.json | Adds device config fixture for “P1-2W” model. |
| tests/components/solarman/conftest.py | Adds shared fixtures for client mocking and config entries. |
| tests/components/solarman/init.py | Adds a helper to set up the integration in tests. |
| requirements_test_all.txt | Adds solarman-opendata to test requirements. |
| requirements_all.txt | Adds solarman-opendata to core requirements. |
| homeassistant/generated/zeroconf.py | Registers _solarman._tcp.local. zeroconf service to the integration domain. |
| homeassistant/generated/integrations.json | Adds generated integration metadata for solarman. |
| homeassistant/generated/config_flows.py | Adds solarman to generated config flow registry. |
| homeassistant/components/solarman/strings.json | Adds config flow + entity + exception translations for solarman. |
| homeassistant/components/solarman/sensor.py | Implements sensor platform/entity descriptions and coordinator data mapping. |
| homeassistant/components/solarman/quality_scale.yaml | Introduces quality scale tracking file for the integration. |
| homeassistant/components/solarman/manifest.json | Adds the integration manifest, dependency, and zeroconf declaration. |
| homeassistant/components/solarman/entity.py | Adds base entity with DeviceInfo (identifiers, model, manufacturer, etc.). |
| homeassistant/components/solarman/coordinator.py | Adds DataUpdateCoordinator fetching device data via solarman-opendata. |
| homeassistant/components/solarman/config_flow.py | Implements user + zeroconf config flows to create config entries. |
| homeassistant/components/solarman/init.py | Wires setup/unload and coordinator initialization + platform forwarding. |
| CODEOWNERS | Adds code ownership for integration and its tests. |
You can also share your feedback on Copilot code review. Take the survey.
| SensorEntityDescription( | ||
| key="power factor", | ||
| translation_key="power_factor", | ||
| native_unit_of_measurement=PERCENTAGE, | ||
| device_class=SensorDeviceClass.POWER_FACTOR, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), |
| async def _async_update_data(self) -> dict[str, Any]: | ||
| """Fetch and update device data.""" | ||
| try: | ||
| return await self.api.fetch_data() | ||
| except ConnectionError as e: | ||
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="update_failed", | ||
| ) from e |
| freezer.tick(UPDATE_INTERVAL) | ||
| async_fire_time_changed(hass) |
| SensorEntityDescription( | ||
| key="active power", | ||
| translation_key="active_power", | ||
| native_unit_of_measurement=UnitOfPower.WATT, | ||
| device_class=SensorDeviceClass.POWER, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), |
| "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", | ||
| "another_device": "The configured device is not the same found on this IP address" |
| status: exempt | ||
| comment: | | ||
| No authentication required. | ||
| test-coverage: todo |
There was a problem hiding this comment.
Pull request overview
Introduces a new solarman Home Assistant integration with config flow, zeroconf discovery, coordinator-based polling, and sensor entities, plus accompanying tests/fixtures and generated registry updates.
Changes:
- Added the
solarmanintegration (manifest, config flow, coordinator, base entity, sensor platform, translations, quality scale). - Added comprehensive tests for config flow, init, and sensors (including snapshots and fixtures).
- Registered zeroconf service and integration metadata; added dependency pins and codeowners.
Reviewed changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/solarman/* | New Solarman integration implementation (config flow, coordinator, entities, translations, manifest, quality scale). |
| homeassistant/generated/* | Registers Solarman for config flows, zeroconf discovery, and integrations index. |
| requirements_all.txt / requirements_test_all.txt | Adds solarman-opendata==0.0.2 dependency pins. |
| tests/components/solarman/* | New test suite, fixtures, and snapshots for the integration. |
| CODEOWNERS | Assigns ownership for new integration and its tests. |
| "quality_scale": "bronze", | ||
| "requirements": ["solarman-opendata==0.0.2"], | ||
| "zeroconf": ["_solarman._tcp.local."] |
There was a problem hiding this comment.
The PR title mentions submitting a “gold-level configuration file”, but the integration is declared as quality_scale: "bronze" and a full new integration (code/tests/generated files) is being added. Please align the PR title/description with the actual scope, or update the quality-scale intent/claims accordingly.
| SensorEntityDescription( | ||
| key="apparent power", | ||
| native_unit_of_measurement=UnitOfPower.WATT, | ||
| device_class=SensorDeviceClass.APPARENT_POWER, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), | ||
| SensorEntityDescription( | ||
| key="reactive power", | ||
| native_unit_of_measurement=UnitOfPower.WATT, | ||
| device_class=SensorDeviceClass.REACTIVE_POWER, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), |
There was a problem hiding this comment.
APPARENT_POWER and REACTIVE_POWER are using UnitOfPower.WATT, which is the wrong unit for these quantities. Use the appropriate units from homeassistant.const (VA for apparent power and var/VAr for reactive power) so the device classes and unit semantics match Home Assistant expectations.
| SensorEntityDescription( | ||
| key="power factor", | ||
| translation_key="power_factor", | ||
| native_unit_of_measurement=PERCENTAGE, | ||
| device_class=SensorDeviceClass.POWER_FACTOR, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), |
There was a problem hiding this comment.
The fixture/snapshot shows power factor values like 0.63 while the unit is %. That produces incorrect displayed units (0.63% instead of 63%). Either convert the incoming value to a 0–100 percentage (e.g., via a value_fn/processing in native_value) or change the unit to be unitless (and adjust the device class/unit combination accordingly).
| device_info = config_data.get(CONF_DEVICE, config_data) | ||
|
|
||
| self.device_sn = device_info[CONF_SN] | ||
| self.model = device_info[CONF_TYPE] | ||
| self.mac = dr.format_mac(device_info[CONF_MAC]) | ||
|
|
||
| await self.async_set_unique_id(self.device_sn) | ||
| self._abort_if_unique_id_configured() | ||
|
|
||
| if not errors: | ||
| return self.async_create_entry( | ||
| title=f"{MODEL_NAME_MAP[self.model]} ({self.host})", | ||
| data={ | ||
| CONF_HOST: self.host, | ||
| CONF_SN: self.device_sn, | ||
| CONF_MODEL: self.model, | ||
| CONF_MAC: self.mac, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
MODEL_NAME_MAP[self.model] will raise a KeyError if the device reports a model/type not present in the mapping (including unexpected capitalization/spacing). Since this value comes from the device, handle unknown models explicitly (e.g., abort with a clear “unsupported_device” reason or fall back to the raw model string for the title) to avoid crashing the config flow.
| def device_fixture(request: pytest.FixtureRequest) -> str | None: | ||
| """Return the device fixtures for a specific device.""" | ||
| return getattr(request, "param", None) |
There was a problem hiding this comment.
device_fixture is typed as str | None, but downstream fixtures (e.g., mock_config_entry) treat it as a guaranteed str and index MODEL_NAME_MAP[device_fixture]. Make device_fixture always return str (and require parametrization), or raise a clear error when the fixture is used without a parameter to avoid hard-to-debug KeyError/TypeError failures.
| def device_fixture(request: pytest.FixtureRequest) -> str | None: | |
| """Return the device fixtures for a specific device.""" | |
| return getattr(request, "param", None) | |
| def device_fixture(request: pytest.FixtureRequest) -> str: | |
| """Return the device fixtures for a specific device.""" | |
| if not hasattr(request, "param"): | |
| msg = "The 'device_fixture' fixture must be parametrized with a device name." | |
| raise RuntimeError(msg) | |
| return request.param |
davidrapan
left a comment
davidrapan
left a comment
There was a problem hiding this comment.
Please, address comments and change requests from #152525 (review) 😉
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
I made some fixes. I think the PR is fine to be merged, but the library is not being built and published in the CI. Please make sure that that is the case, make sure there's a new release and bump it in the PR and we can merge it!
| "domain": "solarman", | ||
| "name": "Solarman", |
There was a problem hiding this comment.
The PR description says “Add integration for Indevolt energy storage devices”, but the code adds the “Solarman” integration (domain solarman). Please update the PR description (and/or title) to match the actual integration being added so release notes and review context are accurate.
| "abort": { | ||
| "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", | ||
| "another_device": "The configured device is not the same found on this IP address", | ||
| "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", | ||
| "timeout": "Connection timed out while trying to connect to the device" | ||
| }, | ||
| "error": { | ||
| "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", | ||
| "timeout": "[%key:common::config_flow::error::timeout_connect%]", | ||
| "unknown": "[%key:common::config_flow::error::unknown%]" | ||
| }, |
There was a problem hiding this comment.
The config flow can async_abort(reason=\"unknown\") (e.g., zeroconf path), but config.abort has no unknown key. This will lead to an untranslated/missing abort message in the UI. Add unknown under config.abort (or change zeroconf to surface the error as a form error instead of an abort) so all abort reasons are translated.
| try: | ||
| return await self.api.fetch_data() | ||
| except ConnectionError as e: | ||
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="update_failed", | ||
| ) from e |
There was a problem hiding this comment.
Only ConnectionError is converted into a translated UpdateFailed. If the library raises TimeoutError (which the tests already simulate during setup) or other common network exceptions, they’ll bypass the translated failure path and may produce noisier logs/less consistent availability behavior. Consider catching TimeoutError (and any other expected transport exceptions from solarman-opendata) and raising the same translated UpdateFailed.
| entity-event-setup: | ||
| status: exempt | ||
| comment: | | ||
| Entities of this integration does not explicitly subscribe to events. |
There was a problem hiding this comment.
Grammar: replace 'does not' with 'do not' to match the plural subject 'Entities'.
| Entities of this integration does not explicitly subscribe to events. | |
| Entities of this integration do not explicitly subscribe to events. |
|
Thanks a lot for your suggestions and help! I’ve set up the library to be built and published in CI—let me know if anything should be changed :) |
|
I think this was it! Do you have Discord and are you able to send a message? Then we can add you to the right channels as a codeowner |
joostlek
changed the title
| "name": "Total actual energy" | ||
| }, | ||
| "total_act_energy_lt": { | ||
| "name": "Total actual energy low tariff" | ||
| }, | ||
| "total_act_energy_nt": { | ||
| "name": "Total actual energy normal tariff" | ||
| }, | ||
| "total_act_power": { | ||
| "name": "Total actual power" | ||
| }, | ||
| "total_act_ret_energy": { | ||
| "name": "Total actual returned energy" | ||
| }, | ||
| "total_act_ret_energy_lt": { | ||
| "name": "Total actual returned energy low tariff" | ||
| }, | ||
| "total_act_ret_energy_nt": { | ||
| "name": "Total actual returned energy normal tariff" | ||
| }, | ||
| "total_act_ret_power": { | ||
| "name": "Total actual returned power" |
There was a problem hiding this comment.
That "actual" in all these sensor values is confusing.
What does that tell the user compared to other sensor values?
Asking as a translator …
Breaking change
Proposed change
Add integration for Indevolt energy storage devices.
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: