Skip to content

Fix id_tag sensor not updated for local authorization (#1871)#1905

Open
rishabhvaish wants to merge 3 commits intolbbrhzn:mainfrom
rishabhvaish:fix/id-tag-not-updated-start-transaction-1871
Open

Fix id_tag sensor not updated for local authorization (#1871)#1905
rishabhvaish wants to merge 3 commits intolbbrhzn:mainfrom
rishabhvaish:fix/id-tag-not-updated-start-transaction-1871

Conversation

@rishabhvaish
Copy link
Copy Markdown

@rishabhvaish rishabhvaish commented Mar 3, 2026

Summary

  • sensor.<cpid>_id_tag stays stale when charger performs local OCPP 1.6 authorization (skips Authorize, sends StartTransaction directly)
  • Root cause: on_start_transaction sets connector-level id_tag but not charger-level (connector 0)
  • Fix: also update self._metrics[0][cstat.id_tag.value] in on_start_transaction
  • Also clear charger-level id_tag in on_stop_transaction for symmetry
  • OCPP 2.0.1 handler already does this correctly

Files changed

  • custom_components/ocpp/ocppv16.py — update charger-level id_tag in on_start_transaction and clear it in on_stop_transaction

Test plan

  • Use RFID card authorized in charger's local list (no Authorize message sent)
  • Verify sensor.<cpid>_id_tag updates on StartTransaction
  • Verify sensor.<cpid>_id_tag still updates on normal Authorize -> StartTransaction flow
  • Verify id_tag clears on StopTransaction

Fixes #1871

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Starting a connector session now also updates the station-level session ID. Stopping a connector clears or adjusts the station-level ID only when appropriate, preventing stale or conflicting session identifiers across multiple connectors.
  • Tests

    • Added a test that verifies stopping one connector in a multi-connector scenario preserves or updates the station-level ID correctly and clears the stopped connector's ID.

…lbbrhzn#1871)

When an OCPP 1.6 charger performs local authorization (e.g., RFID card in
local list), it skips the Authorize message and sends StartTransaction directly.
The on_start_transaction handler only set the connector-level id_tag but not
the charger-level (connector 0) metric, so sensor.<cpid>_id_tag remained stale.

Also clear the charger-level id_tag in on_stop_transaction for symmetry.

The OCPP 2.0.1 handler (on_transaction_event) already handles this correctly.

Fixes lbbrhzn#1871

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Rishabh Vaish <rishabhvaish.904@gmail.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f185011-75ce-4586-b278-b3703fdfe903

📥 Commits

Reviewing files that changed from the base of the PR and between 8df158f and 984a577.

📒 Files selected for processing (1)
  • tests/test_additional_charge_point_v16.py

📝 Walkthrough

Walkthrough

Synchronize charger-level (connector 0) and per-connector id_tag metrics during transaction lifecycle: set connector 0 id_tag on StartTransaction; on StopTransaction, clear connector 0 only if no other connector is active, otherwise set it to an active connector's id_tag. (33 words)

Changes

Cohort / File(s) Summary
Transaction id_tag sync
custom_components/ocpp/ocppv16.py
On StartTransaction, set the charger-level (connector 0) Id.Tag to the starting connector's id_tag. On StopTransaction, when clearing a connector's id_tag, clear connector 0 only if no other connectors remain active; otherwise set connector 0 to a remaining active connector's id_tag.
Test: multi-connector stop behavior
tests/test_additional_charge_point_v16.py
Add test_stop_transaction_multi_connector_keeps_station_id_tag to verify stopping one of multiple active connectors clears that connector's id_tag and updates charger-level id_tag to a remaining active connector's id_tag.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • drc38

Poem

🐰 I hopped through code with whiskers keen,
Set tag at start, kept station clean,
Stop one connector — others stay,
The station tag hops the right way,
A carrot nods: “Sync—nice and neat!” 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing the id_tag sensor not being updated for local OCPP 1.6 authorization, which is the primary objective of this PR.
Linked Issues check ✅ Passed The PR directly addresses issue #1871 by updating charger-level id_tag in on_start_transaction and on_stop_transaction, matching the proposed solution and ensuring the sensor reflects locally authorized transactions.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the id_tag sensor bug: modifications to on_start_transaction and on_stop_transaction handlers, plus a test for multi-connector scenarios, which is in scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd4a8ee and c612959.

📒 Files selected for processing (1)
  • custom_components/ocpp/ocppv16.py

…tion

When stopping a transaction on one connector, the station-level id_tag
(connector 0) was unconditionally cleared, blanking the charger-wide
sensor even when another connector still had an active transaction.

Now checks _active_tx for remaining active connectors before clearing.
If another connector is still active, connector 0's id_tag is updated
to that connector's id_tag instead of being blanked.

Addresses CodeRabbit review feedback on PR lbbrhzn#1905.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rishabhvaish rishabhvaish temporarily deployed to continuous-integration March 17, 2026 16:13 — with GitHub Actions Inactive
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.86%. Comparing base (bd4a8ee) to head (8df158f).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
custom_components/ocpp/ocppv16.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1905      +/-   ##
==========================================
- Coverage   94.92%   94.86%   -0.06%     
==========================================
  Files          12       12              
  Lines        2954     2960       +6     
==========================================
+ Hits         2804     2808       +4     
- Misses        150      152       +2     

☔ View full report in Codecov by Sentry.
📢 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.

Add test for the case where stopping a transaction on one connector
while another connector still has an active transaction preserves
the station-level id_tag (connector 0) with the remaining active
connector's id_tag instead of clearing it.

This covers the 2 missing lines flagged by codecov patch check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

sensor.<cpid>_id_tag not updated with single connector ocppv1.6 charger

1 participant