Add NRGkick integration and tests #159995
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
There was a problem hiding this comment.
Pull request overview
This PR introduces the NRGkick EV charger integration to Home Assistant Core, bringing it from HACS. The integration supports reading device state via the official local REST API using the nrgkick-api PyPI library.
Key changes:
- New integration with sensor platform for monitoring EV charger state, power flow, temperatures, and energy metrics
- Support for zeroconf discovery with optional BasicAuth authentication
- Comprehensive config flow with reauth and reconfigure support
- Data update coordinator for efficient polling (30-second interval)
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/nrgkick/__init__.py |
Integration entry point with setup and unload logic |
homeassistant/components/nrgkick/api.py |
API wrapper translating library exceptions to HA exceptions |
homeassistant/components/nrgkick/config_flow.py |
Config flow with user, zeroconf, reauth, and reconfigure steps |
homeassistant/components/nrgkick/coordinator.py |
Data update coordinator with command verification methods |
homeassistant/components/nrgkick/const.py |
Constants and mapping dictionaries for status/error codes |
homeassistant/components/nrgkick/entity.py |
Base entity class with device info and suggested_object_id |
homeassistant/components/nrgkick/sensor.py |
60+ sensor entities for power, energy, temperatures, and diagnostics |
homeassistant/components/nrgkick/diagnostics.py |
Diagnostics data collection |
homeassistant/components/nrgkick/manifest.json |
Integration metadata declaring bronze quality scale |
homeassistant/components/nrgkick/strings.json |
Translations for config flow, entities, and exceptions |
homeassistant/components/nrgkick/icons.json |
Custom icon mappings for sensors |
homeassistant/components/nrgkick/quality_scale.yaml |
Quality scale compliance tracking |
tests/components/nrgkick/* |
Comprehensive test suite with 100% config flow coverage |
| Generated files | Updates to requirements, zeroconf, config_flows, mypy, CODEOWNERS |
There was a problem hiding this comment.
Have you considered using a decorator on the methods where you call the api? That might be a less invasive option of wrapping the calls
There was a problem hiding this comment.
Good idea, makes it more elegant than the previous wrapping method - changed that!
There was a problem hiding this comment.
My comment was more trying to aim like a solution as we have in spotify and I believe in peblar. As in, just call the library normally, and then just have a way to retry that. This way you don't have to modify this file when you add new calls or whatnot and you are just more flexible
| """ | ||
| return self._key | ||
|
|
||
| def _setup_device_info(self) -> None: |
There was a problem hiding this comment.
I think this can be moved to the constructor
There was a problem hiding this comment.
Not sure if this referred to the method that returned self._key (which is already gone now, which kept the entity id in English), or the _setup_device_info(). I'd prefer to keep that in its own function to keep the constructor short, and execute the few bits of logic in an own function. Making the code inline wouldn't make the code much shorter, so I'd prefer the additional function for more clarity if that's ok.
There was a problem hiding this comment.
It's more that the constructor is generally used for setting up the base things of the entity, like for example the unique id, so when you look at this file at first glance, I'd come to the conclusion that there is no unique id set in the base entity, while it is. So I'd prefer it to have it in the constructor for increased visbility
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
…ronze status for first pull request
…re HA ui correctly serializes schema parsing info
…move remaining code for charging control
|
Thanks for the thorough review - I've implemented all the changes, adapted the automated tests and re-tested with the physical device (also resulted in an updated pypi library). Also rebased the pull request. Ready for review again! |
|
To keep the pull request up-to-date while waiting for the review of the finished changes, I've updated with the current base branch, re-tested with the device and also used this to improve test coverage especially for the JSON API disabled use-case that is now automatically handled in the user flow thanks to the feedback. |
There was a problem hiding this comment.
My comment was more trying to aim like a solution as we have in spotify and I believe in peblar. As in, just call the library normally, and then just have a way to retry that. This way you don't have to modify this file when you add new calls or whatnot and you are just more flexible
| try: | ||
| host = _normalize_host(user_input[CONF_HOST]) | ||
| except vol.Invalid: | ||
| errors["base"] = "cannot_connect" |
There was a problem hiding this comment.
So we currently only raise Invalid when the input is invalid. We can move this kind of validation towards the schema. This way it will be validated in front of the step. The transformation from URL -> host:port can be done inside of the step, but then we don't have to catch Invalid anymore
| NRGkickSensorEntityDescription( | ||
| key="charge_count", | ||
| translation_key="charge_count", | ||
| state_class=SensorStateClass.TOTAL_INCREASING, | ||
| value_path=("values", "general", "charge_count"), | ||
| ), |
There was a problem hiding this comment.
Can add translated unit of measurement
| NRGkickSensorEntityDescription( | ||
| key="rcd_trigger", | ||
| translation_key="rcd_trigger", | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_path=("values", "general", "rcd_trigger"), | ||
| value_fn=lambda value: _map_code_to_translation_key(value, RCD_TRIGGER_MAP), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="warning_code", | ||
| translation_key="warning_code", | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_path=("values", "general", "warning_code"), | ||
| value_fn=lambda value: _map_code_to_translation_key(value, WARNING_CODE_MAP), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="error_code", | ||
| translation_key="error_code", | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_path=("values", "general", "error_code"), | ||
| value_fn=lambda value: _map_code_to_translation_key(value, ERROR_CODE_MAP), | ||
| ), |
| NRGkickSensorEntityDescription( | ||
| key="housing_temperature", | ||
| translation_key="housing_temperature", | ||
| device_class=SensorDeviceClass.TEMPERATURE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| native_unit_of_measurement=UnitOfTemperature.CELSIUS, | ||
| value_path=("values", "temperatures", "housing"), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="connector_l1_temperature", | ||
| translation_key="connector_l1_temperature", | ||
| device_class=SensorDeviceClass.TEMPERATURE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| native_unit_of_measurement=UnitOfTemperature.CELSIUS, | ||
| value_path=("values", "temperatures", "connector_l1"), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="connector_l2_temperature", | ||
| translation_key="connector_l2_temperature", | ||
| device_class=SensorDeviceClass.TEMPERATURE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| native_unit_of_measurement=UnitOfTemperature.CELSIUS, | ||
| value_path=("values", "temperatures", "connector_l2"), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="connector_l3_temperature", | ||
| translation_key="connector_l3_temperature", | ||
| device_class=SensorDeviceClass.TEMPERATURE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| native_unit_of_measurement=UnitOfTemperature.CELSIUS, | ||
| value_path=("values", "temperatures", "connector_l3"), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="domestic_plug_1_temperature", | ||
| translation_key="domestic_plug_1_temperature", | ||
| device_class=SensorDeviceClass.TEMPERATURE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| native_unit_of_measurement=UnitOfTemperature.CELSIUS, | ||
| value_path=("values", "temperatures", "domestic_plug_1"), | ||
| ), | ||
| NRGkickSensorEntityDescription( | ||
| key="domestic_plug_2_temperature", | ||
| translation_key="domestic_plug_2_temperature", | ||
| device_class=SensorDeviceClass.TEMPERATURE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| native_unit_of_measurement=UnitOfTemperature.CELSIUS, | ||
| value_path=("values", "temperatures", "domestic_plug_2"), | ||
| ), |
There was a problem hiding this comment.
It's not a thermometer, so I'd consider this diagnostic
| data: Any = self.coordinator.data | ||
| for index, key in enumerate(self.entity_description.value_path): | ||
| if data is None: | ||
| return None | ||
|
|
||
| # Coordinator returns a NRGkickData container; the first path segment | ||
| # is always one of: info/control/values. | ||
| if index == 0 and isinstance(data, NRGkickData): | ||
| data = getattr(data, key, None) | ||
| continue | ||
|
|
||
| if not isinstance(data, dict): | ||
| return None | ||
|
|
||
| data = data.get(key) |
There was a problem hiding this comment.
- The coordinator data can't be None. If the coordinator has no data, UpdateFailed should've been raised, and then entities would present as unavailable and this wouldn't be resolved.
- Instead of using getattr, can we go to a value_fn approach of just putting the full data into the value_fn and make it like
value_fn=lambda data: data.values.temperatures.domestic_plug_2.get("domestic_plug_2_temperature"). This way it's more typesafe and less prone to errors
| "relay_state": { | ||
| "name": "Relay state", | ||
| "state": { | ||
| "l1": "L1", | ||
| "l1_l2": "L1, L2", | ||
| "l1_l2_l3": "L1, L2, L3", | ||
| "l1_l3": "L1, L3", | ||
| "l2": "L2", | ||
| "l2_l3": "L2, L3", | ||
| "l3": "L3", | ||
| "n": "N", | ||
| "n_l1": "N, L1", | ||
| "n_l1_l2": "N, L1, L2", | ||
| "n_l1_l2_l3": "N, L1, L2, L3", | ||
| "n_l1_l3": "N, L1, L3", | ||
| "n_l2": "N, L2", | ||
| "n_l2_l3": "N, L2, L3", | ||
| "n_l3": "N, L3", | ||
| "no_relay": "No relay", | ||
| "unknown": "Unknown" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Wouldn't it make sense to have some kind of binary sensors for this instead as that's way easier to automate with? I am not sure what a relay state is, but if I wanted to automate based on that the relay is on on N, then I'd have to check within the state, while if we had a binary sensor that said Neutral relay state then it'd be just an on/off and it'd be easier to automate with
|
Oh and btw, if you have questions, feel free to reach out on Discord |
…nd instead assert if set
…default to home assistants own None - unknown state
…ice api, but set that to home assistant's None
…d response error instead. Also remove test cases for that scenario
…st name in config flow and the host is wrong
2 participants
Proposed change
This PR brings the NRGkick mobile EV charger (wallbox) integration from HACS ( GitHub repo ) to HA Core. It supports reading the state using the official local REST API (accessed via PyPi library).
To comply with best practices, I've trimmed down my existing community integration to just the sensor platform and will add charging control back in future updates to the HA core 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: