Add reboot notification setting and integration settings flows for OCPP#1990
Add reboot notification setting and integration settings flows for OCPP#1990dominikandreas wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe integration adds a central system setting for enabling charger reboot notifications, exposes it through setup, reconfigure, and options flows, conditionally dispatches notifications, and updates translations, documentation, fixtures, and tests. ChangesReboot notification settings
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigFlow
participant OcppOptionsFlowHandler
participant ConfigEntry
participant ChargePoint
participant HomeAssistant
User->>ConfigFlow: Submit central system settings
ConfigFlow->>ConfigEntry: Create or update configuration
User->>OcppOptionsFlowHandler: Submit options
OcppOptionsFlowHandler->>ConfigEntry: Persist reboot notification preference
OcppOptionsFlowHandler->>HomeAssistant: Schedule integration reload
ChargePoint->>HomeAssistant: Schedule reboot notification when enabled
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_charge_point_core.py (1)
228-231: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEnsure safe closure of the mock coroutine.
Calling an
AsyncMockreturns an awaitable that may not possess aclose()method, which could lead to anAttributeErrorwhencoro.close()is invoked. Consider checking for thecloseattribute to make the fake task creator robust.♻️ Proposed fix
def fake_async_create_task(coro): scheduled.append(coro) - coro.close() + if hasattr(coro, "close"): + coro.close() return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_charge_point_core.py` around lines 228 - 231, Update fake_async_create_task to close the captured coroutine only when it provides a callable close attribute, while preserving the existing scheduling and return behavior.custom_components/ocpp/translations/i-default.json (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep translation files synchronized.
The
optionsblock was added toen.jsonbut seems to be missing fromi-default.json. Consider adding theoptions.step.initsection here as well to keep the base translation file consistent.♻️ Proposed fix
"abort": { "single_instance_allowed": "Only a single instance is allowed", "reauth_successful": "New charger configured" } - } + }, + "options": { + "step": { + "init": { + "title": "OCPP Central System Settings", + "description": "If you need help with the configuration have a look [here]({docs_url})", + "data": { + "host": "Central system host address", + "port": "Central system port number", + "ssl": "Secure connection", + "ssl_certfile_path": "Path to SSL certificate or (None)", + "ssl_keyfile_path": "Path to SSL key or (None)", + "csid": "Central system identity", + "enable_reboot_notifications": "Enable charger reboot notifications", + "websocket_close_timeout": "Websocket close timeout (seconds)", + "websocket_ping_tries": "Websocket successive times to try connection before closing", + "websocket_ping_interval": "Websocket ping interval (seconds)", + "websocket_ping_timeout": "Websocket ping timeout (seconds)" + } + } + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@custom_components/ocpp/translations/i-default.json` at line 14, Add the missing options.step.init translation section to i-default.json, matching the structure and keys already present in en.json while preserving the existing enable_reboot_notifications entry and translation-file formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@custom_components/ocpp/config_flow.py`:
- Around line 327-329: Replace the `_async_abort_entries_match` call in the
options-flow port-change handling with an explicit iteration over existing
config entries, aborting when another entry in the domain already uses the
requested port. Preserve the current behavior of allowing the unchanged port and
avoid relying on the unavailable OptionsFlow method.
---
Nitpick comments:
In `@custom_components/ocpp/translations/i-default.json`:
- Line 14: Add the missing options.step.init translation section to
i-default.json, matching the structure and keys already present in en.json while
preserving the existing enable_reboot_notifications entry and translation-file
formatting.
In `@tests/test_charge_point_core.py`:
- Around line 228-231: Update fake_async_create_task to close the captured
coroutine only when it provides a callable close attribute, while preserving the
existing scheduling and return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb6ec4ba-0e56-41d0-874f-141bbc23c121
📒 Files selected for processing (9)
custom_components/ocpp/chargepoint.pycustom_components/ocpp/config_flow.pycustom_components/ocpp/const.pycustom_components/ocpp/translations/en.jsoncustom_components/ocpp/translations/i-default.jsondocs/support.mdtests/const.pytests/test_charge_point_core.pytests/test_config_flow.py
| if user_input is not None: | ||
| if user_input[CONF_PORT] != self._config_entry.data[CONF_PORT]: | ||
| self._async_abort_entries_match({CONF_PORT: user_input[CONF_PORT]}) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Replace _async_abort_entries_match to prevent AttributeError.
Unlike ConfigFlow, OptionsFlow subclasses do not inherit the _async_abort_entries_match method in Home Assistant. If a user changes the port in the options flow, executing this line will raise an AttributeError and crash the flow.
You can manually check for port conflicts by iterating over existing config entries in the domain.
🐛 Proposed fix
if user_input is not None:
if user_input[CONF_PORT] != self._config_entry.data[CONF_PORT]:
- self._async_abort_entries_match({CONF_PORT: user_input[CONF_PORT]})
+ for entry in self.hass.config_entries.async_entries(self._config_entry.domain):
+ if (
+ entry.entry_id != self._config_entry.entry_id
+ and entry.data.get(CONF_PORT) == user_input[CONF_PORT]
+ ):
+ return self.async_abort(reason="already_configured")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if user_input is not None: | |
| if user_input[CONF_PORT] != self._config_entry.data[CONF_PORT]: | |
| self._async_abort_entries_match({CONF_PORT: user_input[CONF_PORT]}) | |
| if user_input is not None: | |
| if user_input[CONF_PORT] != self._config_entry.data[CONF_PORT]: | |
| for entry in self.hass.config_entries.async_entries(self._config_entry.domain): | |
| if ( | |
| entry.entry_id != self._config_entry.entry_id | |
| and entry.data.get(CONF_PORT) == user_input[CONF_PORT] | |
| ): | |
| return self.async_abort(reason="already_configured") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@custom_components/ocpp/config_flow.py` around lines 327 - 329, Replace the
`_async_abort_entries_match` call in the options-flow port-change handling with
an explicit iteration over existing config entries, aborting when another entry
in the domain already uses the requested port. Preserve the current behavior of
allowing the unchanged port and avoid relying on the unavailable OptionsFlow
method.
This PR adds two new features:
OptionsFlowhandler to configure settings from Home Assistant’s Integrations page after initial setupThe new reboot notification setting is now included in the create flow, reconfigure flow, and integration options flow. Alongside that, this PR introduces support for reconfiguring an existing OCPP entry and adds an options flow so users can update settings such as host, port, SSL, central system ID, websocket timing values, and the new reboot notification behavior without recreating the integration.
Screenshot of the new feature for changing the integration settings:
The changes were implemented with copilot and GPT 5.4. I've only reviewed and tested them in my own installation.
Related issue: #1425
Summary by CodeRabbit
New Features
Bug Fixes
Documentation