Gate charging-release switches on Modbus release mode - #1
Open
jensteblick wants to merge 1 commit into
Open
Conversation
The Charging enabled (X300) and Available (X304) switches were exposed unconditionally, but the CHARX controller only honours those registers when the charging release mode (X120) is Modbus (5). Veton chargers ship with release mode = OCPP (4), where OCPP owns authorisation and start/stop and the supported EMS control is the max-charging-current register X301. On a normal charger both switches therefore wrote to a register the controller silently ignored, presenting a control that did nothing and contradicting the public EMS integration guide. - switch.py: shared base gates `available` on X120 == 5, exposes the release mode in the entity attributes, and warns once per entity (not per 5 s refresh) explaining why the switch is inactive and what to use instead. Unique IDs unchanged, so existing entities are not re-registered. - const.py: RELEASE_MODE_MODBUS constant instead of a bare literal. - dashboard.py: Controls card leads with Max Current; the two switch rows become conditional rows that hide while the entity is unavailable. - modbus_client.py: docstrings only — no behaviour change. - README: correct the Controls bullet, the switch table row, and the smart charging section that told users to toggle Charging enabled from automations; add a section on what is actually controllable. - tests: 18 new tests covering the gating, the warn-once behaviour and the conditional dashboard rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR Summary by QodoGate release switches by charger Modbus mode
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1. Partial read disables switches
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+67
to
+72
| mode = self._release_mode | ||
| if mode is None: | ||
| return False | ||
| if mode != RELEASE_MODE_MODBUS: | ||
| self._warn_release_mode(mode) | ||
| return False |
There was a problem hiding this comment.
1. Partial read disables switches 🐞 Bug ☼ Reliability
If the X100–X120 config read fails, release_mode remains its default 0 while the coordinator publishes the partial update as successful, so both switches become unavailable even when the charger is still in Modbus mode. A persistent config-block failure disables both controls indefinitely, while a transient failure also emits misleading warn-once messages.
Agent Prompt
## Issue description
A failed X100–X120 read leaves `release_mode` at the dataclass default `0`. Because the exception is swallowed and the coordinator publishes the partial snapshot, the new availability gate treats the charger as non-Modbus and disables both switches.
## Issue Context
The release mode now controls entity availability, so a missing register value must not be indistinguishable from the real Dashboard mode. Retain the last validated release mode, represent an unread value as `None`, or fail the coordinator update so its previous snapshot remains active. Add coverage for a config-only read failure while the previous mode is Modbus.
## Fix Focus Areas
- custom_components/veton/modbus_client.py[34-43]
- custom_components/veton/modbus_client.py[192-207]
- custom_components/veton/coordinator.py[47-66]
- custom_components/veton/switch.py[54-72]
- tests/test_switch.py[57-79]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+27
to
+30
| _INACTIVE_NOTE = ( | ||
| "Charging release is owned by OCPP on this charger; this switch only works " | ||
| "with release mode = Modbus (X120 = 5). Use the 'Max charging current' " | ||
| "number (X301) to steer charging from an EMS or automation." |
There was a problem hiding this comment.
2. Inactive reason misidentifies mode 🐞 Bug ◔ Observability
inactive_reason always says OCPP owns charging even when X120 reports Dashboard, Local whitelist, External control, Permanent release, or an unknown value. This exposes factually incorrect diagnostic information for every non-Modbus mode except OCPP.
Agent Prompt
## Issue description
The static inactive-reason text claims that OCPP owns charging for all non-Modbus release modes, although the integration recognizes several distinct modes.
## Issue Context
The availability warning already resolves the actual mode through `RELEASE_MODE`; the entity attribute should similarly use the current mode or use mode-neutral wording. Add attribute tests for modes 0–3 and unknown values, not only OCPP.
## Fix Focus Areas
- custom_components/veton/switch.py[27-30]
- custom_components/veton/switch.py[92-103]
- custom_components/veton/const.py[24-32]
- tests/test_switch.py[88-106]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Why
The Charging enabled (
X300) and Available (X304) switches were exposed unconditionally. The CHARX controller only honours those registers when the charging release mode (X120) is Modbus (5) — and Veton chargers ship with release mode = OCPP (4), where OCPP owns authorisation/start/stop and the supported EMS control is the max-charging-current registerX301.So on a normal Veton charger both switches wrote to a register the controller silently ignored: a control that did nothing, and one that contradicted the public EMS integration guide, which asks third-party systems not to take over charging release.
Found while answering an external integrator who was pointed at both repos.
What changed
switch.py— shared base gatesavailableonX120 == 5(still deferring tosuper().available), exposes the release mode in entity attributes, and logs a warning once per entity — not on every 5 s refresh — naming the actual mode and pointing at the max-current number. Unique IDs are unchanged, so existing entities are not re-registered.const.py—RELEASE_MODE_MODBUSconstant instead of a bare5.dashboard.py— the Controls card now leads with Max Current; the two switch rows became conditional rows that hide while the entity is unavailable, instead of rendering permanently greyed out.modbus_client.py— docstrings only, no behaviour change.README.md— corrects the Controls bullet and theswitchtable row, adds a "Release mode — what you can actually control" section, and fixes the smart-charging section that told users to toggle Charging enabled from their automations.The switches are kept, not removed: they are correct for a standalone, non-OCPP charger configured with release mode = Modbus.
Tests
42 passed(39 before). 18 new tests: gating across modes 0–5,data is None,last_update_success = False, attribute contents, warn-once across repeated checks, and the conditional dashboard rows.Note
Tests are mock-based — this has not been exercised against a running Home Assistant or a real charger. Worth a spot check on a live unit before release.
manifest.jsonis still at1.1.0; no version bump, since that is a release decision.🤖 Generated with Claude Code