Skip to content

Fix intermittent reconnect failures and missing charge point ID extraction#752

Open
rishabhvaish wants to merge 1 commit intomobilityhouse:masterfrom
rishabhvaish:fix/751-reconnect-charge-point-id
Open

Fix intermittent reconnect failures and missing charge point ID extraction#752
rishabhvaish wants to merge 1 commit intomobilityhouse:masterfrom
rishabhvaish:fix/751-reconnect-charge-point-id

Conversation

@rishabhvaish
Copy link

Summary

  • Add extract_charge_point_id() utility for robust WebSocket path parsing — handles nested paths, query strings, empty/malformed paths
  • Make ChargePoint.start() catch connection exceptions gracefully instead of crashing, with informative log output
  • Update v16 and v201 examples to validate charge point ID before creating instances, and handle ConnectionClosed cleanly

Problem

When chargers intermittently disconnect and reconnect, two issues arise:

  1. Path parsing is fragile: The examples use websocket.request.path.strip("/"), which breaks for nested paths like /ocpp/CP001 (returns ocpp/CP001 instead of CP001) and silently produces empty strings for root paths
  2. No connection lifecycle handling: ChargePoint.start() calls await self._connection.recv() in a bare loop — if the WebSocket closes, the exception propagates unhandled, leaving no diagnostic information

Changes

ocpp/charge_point.py

  • New function extract_charge_point_id(path): Safely extracts the last path segment as the charge point ID, handling edge cases (empty paths, query strings, fragments, multiple slashes)
  • ChargePoint.start() resilience: Wraps recv() in a try/except that logs the disconnection reason and exits the loop cleanly

examples/v16/central_system.py and examples/v201/central_system.py

  • Use extract_charge_point_id() instead of path.strip("/")
  • Validate the extracted ID (close connection if None)
  • Log connect/disconnect events
  • Wrap cp.start() in a ConnectionClosed handler

tests/test_charge_point_connection.py (new)

  • 14 parametric tests for extract_charge_point_id() covering normal paths, nested paths, empty/None inputs, query strings, special characters
  • 4 async tests for ChargePoint.start() verifying clean exit on connection close, message processing before close, disconnection logging, and reconnection with new instances

Test plan

  • All 182 tests pass (164 existing + 18 new)
  • Manual test with a real charger simulator connecting/disconnecting
  • Verify log output shows charge point ID on connect and disconnect

@a-alak
Copy link
Contributor

a-alak commented Feb 13, 2026

I like your thoughts generally!

I just have couple of comments:

extract_charge_point_id() seems a bit out of scope for this lib. One could imagine various validation checks that consumers of this lib wanted on the charge point id. Of course those could be added independently, and extract_charge_point_id() is definitely in accordance with min requirements of OCPP spec. So could be a nice utility to make available.

The idea of better connection error handling I think is great. However there is couple of issues with the implementation. First of all a bit of bad practice catching all exceptions. Of course one could argue that it makes sense for logging, but I think this should be more specific.
Logging every error as info I will say is semantically incorrect. As minimum this should be a warning, and probably people want to inspect the exception, so I think logging.exception is more fitting. Of course we could do both, to inform a disconnection, but I think it should be warn when the disconnection is due to an exception.
In the example you show a very relevant handling of the ConnectionClosed exception for the consumer. However the error handling in charge_point.start() will silence this error unfortunately. It is never re-raised. And since this is both a central system and a charge point implementation we have to account for the consumer wanting to handle stuff like re-connection logic, so they should be able to catch these errors. (also easy to imagine a central system implementing different handling of different errors, like creating a notification or what ever).

Even if we improve on this points I think we might end up creating some limitations. This library is currently coupled with the websockets library interface, which is fine. However I imagine people wrapping other websocket interfaces in compatible objects to be able to use FastAPI or AIOHTTP websockets with this library. These websockets will raise different errors, so behaviour will be different. I don't think that it should be even more coupled with websockets.

Just my quick thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants