-
Notifications
You must be signed in to change notification settings - Fork 0
Detect silent failure when Preparing returns to Available #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,17 @@ | |
| from ..engine import Engine | ||
| from typing import cast | ||
|
|
||
| def get_error_code(sn: call.StatusNotification) -> str: | ||
| return ( | ||
| sn.error_code | ||
| if sn.error_code != ChargePointErrorCode.other_error | ||
| else sn.vendor_error_code or "Unknown Error" | ||
| ) | ||
|
|
||
|
|
||
| def or_now(ts: Optional[str]) -> str: | ||
| return ts or datetime.now(timezone.utc).isoformat() | ||
|
|
||
|
|
||
| class ChargePoint16(cp): | ||
| def __init__(self, ftp, engine, *args, **kwargs): | ||
|
|
@@ -55,7 +66,7 @@ def on_heartbeat(self, **kwargs): | |
|
|
||
| @on(Action.authorize) | ||
| def on_authorize(self, **kwargs): | ||
| id_tag_info = IdTagInfo(status=AuthorizationStatusEnumType.accepted) # type: ignore | ||
| id_tag_info = IdTagInfo(status=AuthorizationStatusEnumType.accepted) | ||
| return call_result.Authorize(id_tag_info=id_tag_info) | ||
|
|
||
| @on(Action.meter_values) | ||
|
|
@@ -176,13 +187,13 @@ async def set_charging_profile_req(self, payload: call.SetChargingProfile): | |
| async def get_composite_schedule( | ||
| self, payload: call.GetCompositeSchedule | ||
| ) -> call_result.GetCompositeSchedule: | ||
| return await self.call(payload) # type: ignore | ||
| return await self.call(payload) | ||
|
|
||
| async def get_composite_schedule_req( | ||
| self, **kwargs | ||
| ) -> call_result.GetCompositeSchedule: | ||
| payload = call.GetCompositeSchedule(**kwargs) | ||
| return await self.call(payload) # type: ignore | ||
| return await self.call(payload) | ||
|
|
||
| async def clear_charging_profile_req(self, **kwargs): | ||
| payload = call.ClearChargingProfile(**kwargs) | ||
|
|
@@ -234,27 +245,47 @@ async def get_diagnostic(self, start_time: str) -> str: | |
| ) | ||
| ), | ||
| ) | ||
|
|
||
| await self._diagnostics_upload_future | ||
| if result.file_name: | ||
| return self._ftp.get_pathname(result.file_name) | ||
| raise Exception("No diagnostic file name received") | ||
|
|
||
| async def handle_status_notification(self, sn: call.StatusNotification): | ||
| if sn.status == ChargePointStatus.available: | ||
| session = self._sessions.get(sn.connector_id) | ||
|
|
||
| if session and not session.has_entered_charging(): | ||
| session.ended_silent( | ||
| or_now(sn.timestamp), | ||
|
|
||
| "PREPARING_ABORTED_BEFORE_CHARGING", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this string? Indeed it makes sense for session to hold an enum indicating why session was considered as failed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Introducing an enum for failure reasons makes sense and is planned as a follow-up (together with automated tests), but I wanted to keep this change minimal and avoid expanding the scope of this PR. |
||
| ) | ||
| await self._engine.handle_failed_session( | ||
| cast(Session, session), | ||
| ) | ||
| self._sessions.pop(sn.connector_id, None) | ||
|
|
||
| if sn.status == ChargePointStatus.preparing: | ||
| self._sessions[sn.connector_id] = Session( | ||
| self, | ||
| or_now(sn.timestamp), | ||
|
|
||
| ) | ||
| if sn.status == ChargePointStatus.charging: | ||
| session = self._sessions.get(sn.connector_id) | ||
| if session: | ||
| session.mark_entered_charging() | ||
|
|
||
| if sn.status == ChargePointStatus.faulted: | ||
| session = self._sessions.get(sn.connector_id) | ||
| if not session: | ||
| session = Session(self, or_now(sn.timestamp)) | ||
| self._sessions[sn.connector_id] = session | ||
| session.ended(or_now(sn.timestamp), get_error_code(sn)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we call get_error_code by function was removed? Does it work?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You’re right — the helper was missing after refactoring. |
||
| await self._engine.handle_failed_session( | ||
| cast(Session, session), | ||
| cast(Session, session) | ||
| ) | ||
| self._sessions.pop(sn.connector_id, None) | ||
|
|
||
|
|
||
| class Session: | ||
|
|
@@ -263,11 +294,22 @@ def __init__(self, charger: ChargePoint16, timestamp: str): | |
| self._end_timestamp = "" | ||
| self._start_timestamp = timestamp | ||
| self._error_code = "" | ||
| self._has_entered_charging = False | ||
|
|
||
| def mark_entered_charging(self): | ||
| self._has_entered_charging = True | ||
|
|
||
| def has_entered_charging(self) -> bool: | ||
| return self._has_entered_charging | ||
|
|
||
| def ended(self, timestamp: str, error_code: str): | ||
| self._end_timestamp = timestamp | ||
| self._error_code = error_code | ||
|
|
||
| def ended_silent(self, timestamp: str, reason: str): | ||
| self._end_timestamp = timestamp | ||
| self._error_code = reason | ||
|
|
||
| async def get_diagnostic(self) -> str: | ||
| pathname = await self._charger.get_diagnostic(self._start_timestamp) | ||
| return read_diagnostic_file(pathname) | ||
|
|
@@ -280,15 +322,3 @@ def timestamp(self) -> str: | |
|
|
||
| def error_code(self) -> str: | ||
| return self._error_code | ||
|
|
||
|
|
||
| def or_now(ts: Optional[str]) -> str: | ||
| return ts or datetime.now(timezone.utc).isoformat() | ||
|
|
||
|
|
||
| def get_error_code(sn: call.StatusNotification) -> str: | ||
| return ( | ||
| sn.error_code | ||
| if sn.error_code != ChargePointErrorCode.other_error | ||
| else sn.vendor_error_code or "Unknown Error" | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to move functions from bottom to top?