Skip to content

Handle station-level StatusNotification in OCPP 2.0.1 - #2020

Merged
drc38 merged 2 commits into
lbbrhzn:mainfrom
KingHavok:fix/v201-station-status-guard
Jul 30, 2026
Merged

Handle station-level StatusNotification in OCPP 2.0.1#2020
drc38 merged 2 commits into
lbbrhzn:mainfrom
KingHavok:fix/v201-station-status-guard

Conversation

@KingHavok

@KingHavok KingHavok commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Part of the OCPP 2.0.1 work from #2008 (follows #2012 and #2013). One more PR — a connector-count fallback for chargers whose inventory reports no connectors — builds on this and will follow separately.

The problem

Some chargers send a station-level StatusNotification with evseId=0 / connectorId=0 — the FoxESS A-series sends one on every boot, mirroring OCPP 1.6's connectorId=0 convention for the charge point itself.

_apply_status_notification feeds it straight into the per-connector bookkeeping, where evse_id - 1 == -1:

  • on the first such notification, while _connector_status is still empty, self._connector_status[-1] raises IndexError, which propagates out of the status-notification flush and aborts post_connect — the charger never finishes setting up;
  • once _connector_status is populated, Python's negative indexing makes it silently write the station's status into the LAST EVSE's slot.

The fix

Station-level notifications are recorded as the charger-level Status metric — the same key the OCPP 1.6 handler uses for connectorId=0 and that the availability switch reads — and never enter the per-connector path.

Deliberately not written to (0, Status.Connector): that key is owned by the EVSE aggregation in _report_evse_status, and a station-level Available written there would mask a faulted connector through the flattened sensor's fallback chain.

Testing

  • Regression test added to the main v201 flow: a station-level Unavailable arrives before the connector's Available, and the test asserts no crash, the charger-level Status metric receives it, and the connector's own status is not clobbered. Mutation-verified: with the guard removed, the flush aborts and connector status never lands.
  • Validated on live hardware (FoxESS A022KP1, HA 2026.7.4): the charger's boot-time (0,0) notification routes to the charger Status sensor on every 2.0.1 connection, with normal connector handling unchanged.
  • Full suite and pre-commit run --all-files pass.

Summary by CodeRabbit

  • Bug Fixes
    • Improved OCPP 2.0.1/2.1 station-level StatusNotification handling to apply correctly for the valid (0,0) case.
    • Prevented misattribution of status when notifications contain malformed or degenerate EVSE/connector identifiers; such updates are now dropped safely.
    • Ensured station-level availability is tracked independently without overwriting EVSE-level aggregated connector status.
  • Tests
    • Added unit coverage for station-level status behavior, including immediate handling without a connector map and validation that malformed identifiers don’t update metrics.

OCPP 2.0.1 permits a StatusNotification for the charging station itself
(evseId=0/connectorId=0) and some chargers send one on every boot - the
FoxESS A-series does.

_apply_status_notification fed those straight into the per-connector
bookkeeping, where evse_id - 1 == -1 either raised IndexError, on the
first such notification while _connector_status was still empty, or
silently wrote the station's status into the LAST EVSE's slot once it
was not. The IndexError propagates out of the status-notification flush
and aborts post_connect, so the charger never finishes setting up.

Record station-level notifications as the charger-level Status metric
instead - the same key the OCPP 1.6 handler uses for connectorId=0 and
that the availability switch reads. (0, Status.Connector) deliberately
stays owned by the EVSE aggregation in _report_evse_status: a
station-level 'Available' written there would mask a faulted connector
through the flattened sensor's fallback chain.
@KingHavok
KingHavok temporarily deployed to continuous-integration July 30, 2026 04:28 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 06c74f12-a6e5-4eb1-834b-9876a99499f5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

StatusNotification handling now distinguishes valid station-level (0,0) messages from malformed identifier combinations, applying only the former to charger-level metrics. Boot and unit tests cover metric ownership, buffering, and dropped malformed notifications.

Changes

Station-Level Status Handling

Layer / File(s) Summary
Handle station-level status notifications
custom_components/ocpp/ocppv201.py, tests/test_charge_point_v201.py
(0,0) notifications update only the charger-level status metric and bypass buffering; boot-time assertions verify connector aggregation is not overwritten.
Validate malformed identifier handling
tests/test_v201_station_status.py
Tests verify station-level notifications apply without a connector map and malformed identifier combinations are dropped without pending or connector state changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: handling station-level StatusNotification messages in OCPP 2.0.1.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/ocppv201.py`:
- Around line 180-184: Update the station-level branch in the StatusNotification
handling around the evse_id and connector_id check to match only when both
identifiers equal zero. Ensure mixed-zero or negative combinations do not enter
this branch and continue through the EVSE/connector handling path.
- Around line 168-185: Update on_status_notification so the exact station-level
tuple (evse_id, connector_id) == (0, 0) bypasses _ensure_connector_map() and
reaches the charger-level metrics branch immediately. Preserve connector-map
buffering for all other notifications, and add a regression covering empty or
unavailable inventory that verifies charger-level Status is set rather than
queued.
🪄 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 Plus

Run ID: 14e57293-5867-4bd0-a16c-b76c5cc67fe5

📥 Commits

Reviewing files that changed from the base of the PR and between b194222 and e529529.

📒 Files selected for processing (2)
  • custom_components/ocpp/ocppv201.py
  • tests/test_charge_point_v201.py

Comment thread custom_components/ocpp/ocppv201.py
Comment thread custom_components/ocpp/ocppv201.py Outdated
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.05%. Comparing base (b194222) to head (c5d0b97).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2020   +/-   ##
=======================================
  Coverage   95.04%   95.05%           
=======================================
  Files          12       12           
  Lines        3005     3011    +6     
=======================================
+ Hits         2856     2862    +6     
  Misses        149      149           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Review findings on the guard:

A station-level StatusNotification never routes through the connector
map, so holding it behind _ensure_connector_map stranded it forever on a
charger whose inventory yields no map - and the chargers that send
station-level statuses are exactly the ones with such inventories. It is
now applied immediately, so the charger-level Status works regardless of
what the inventory turns out to be.

The station-level predicate is tightened to exactly (0, 0). Other
degenerate ids - (1, 0), (0, 1), negatives - are neither station-level
nor a real connector: fed to the per-connector bookkeeping they would
index with -1 (the IndexError / silent last-slot overwrite this guard
exists to prevent), and recorded as station-level they would
misattribute another entity's state to the charger. They are logged at
debug level and dropped.
@KingHavok
KingHavok temporarily deployed to continuous-integration July 30, 2026 04:43 — with GitHub Actions Inactive
@KingHavok

Copy link
Copy Markdown
Contributor Author

Both findings addressed in c5d0b97. On the predicate: took the intent but not the committable suggestion — restricting to == (0, 0) alone would route (1, 0)/(0, 1) into the per-connector path, where connector_id - 1 == -1 indexes negatively: the same IndexError/last-slot-overwrite class this PR fixes (applying the suggestion verbatim fails the new parametrized test with IndexError). Instead: exact (0, 0) → charger-level Status; other degenerate ids are debug-logged and dropped (neither station-level nor a real connector — recording them as station status would misattribute); real pairs unchanged. Sub-1 ids are also exempt from map buffering per the first comment, with regression tests covering the no-map path.

@KingHavok

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@drc38
drc38 merged commit 49170ac into lbbrhzn:main Jul 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants