Fix intermittent reconnect failures and missing charge point ID extraction#752
Fix intermittent reconnect failures and missing charge point ID extraction#752rishabhvaish wants to merge 2 commits intomobilityhouse:masterfrom
Conversation
|
I like your thoughts generally! I just have couple of comments:
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. Even if we improve on this points I think we might end up creating some limitations. This library is currently coupled with the Just my quick thoughts. |
- Remove bare `except Exception` in ChargePoint.start() so exceptions propagate to consumers, enabling custom reconnection logic and error-specific handling without coupling the library to websockets - Use `logging.warning` instead of `logging.info` for disconnections in examples, and log the exception details - Update tests to verify exceptions propagate rather than being silenced Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the thoughtful review, @a-alak! All great points. I've pushed a commit addressing each one: On On exception handling in
On logging levels — Updated the examples to use The examples still demonstrate All 181 tests pass. |
|
Great, I think the PR looks good now! |
|
Thank you for the thorough review and feedback, @a-alak! I really appreciate you taking the time to help improve the implementation. Your suggestions made the PR much cleaner and more focused. 🙏 |
|
Hi! Just checking in — this PR was approved by @a-alak on Feb 17. Is there anything else needed before it can be merged? Happy to address any additional feedback. Thanks! |
|
Thanks for the thorough review @a-alak! Glad the PR looks good now. Let me know if there's anything else needed for merging. |
|
I am not a maintainer, just a consumer (and have minimal contributions). I can't approve or merge the PR. |
Summary
extract_charge_point_id()utility for robust WebSocket path parsing — handles nested paths, query strings, empty/malformed pathsChargePoint.start()catch connection exceptions gracefully instead of crashing, with informative log outputConnectionClosedcleanlyProblem
When chargers intermittently disconnect and reconnect, two issues arise:
websocket.request.path.strip("/"), which breaks for nested paths like/ocpp/CP001(returnsocpp/CP001instead ofCP001) and silently produces empty strings for root pathsChargePoint.start()callsawait self._connection.recv()in a bare loop — if the WebSocket closes, the exception propagates unhandled, leaving no diagnostic informationChanges
ocpp/charge_point.pyextract_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: Wrapsrecv()in a try/except that logs the disconnection reason and exits the loop cleanlyexamples/v16/central_system.pyandexamples/v201/central_system.pyextract_charge_point_id()instead ofpath.strip("/")None)cp.start()in aConnectionClosedhandlertests/test_charge_point_connection.py(new)extract_charge_point_id()covering normal paths, nested paths, empty/None inputs, query strings, special charactersChargePoint.start()verifying clean exit on connection close, message processing before close, disconnection logging, and reconnection with new instancesTest plan