-
Notifications
You must be signed in to change notification settings - Fork 100
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
Feature/test data and fixes #276
Feature/test data and fixes #276
Conversation
8f9f8bd
to
3c37f43
Compare
await self.evse_controller.set_status_secc_session( | ||
ServiceStatus.STARTING | ||
) |
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.
We currently use ServiceStatus to indicate starting and stopping of the entire iso15118_service in our pro version. I see the intention here is to indicate starting and stopping of charging sessions instead. Could you add a new enum SessionStatus along the lines of SESSION_STARTED, SESSION_ENDED for this, please?
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.
I've checked if this status is relevant in our code. But it is not. We can remove the whole function.
@@ -315,6 +318,7 @@ async def end_current_session( | |||
) | |||
await cancel_task(self.tcp_server_handler) | |||
await cancel_task(self.comm_sessions[peer_ip_address][1]) | |||
await self.evse_controller.set_status_secc_session(ServiceStatus.STOPPING) |
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.
Same comment as above - SessionStatus.SESSION_ENDED here?
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.
Thank you for you contribution. :)
@@ -188,6 +195,13 @@ async def set_status(self, status: ServiceStatus) -> None: | |||
""" | |||
raise NotImplementedError | |||
|
|||
@abstractmethod | |||
async def set_status_secc_session(self, status: ServiceStatus) -> None: |
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.
The type here would also need updating to the new enum.
async def set_status_secc_session(self, status: ServiceStatus) -> None: | ||
pass | ||
|
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.
Same comment as above.
There are EVs that sometimes send values below zero | ||
(e.g. Skoda Enyaq). | ||
""" | ||
|
||
_min_limit: int = -10 | ||
unit: Literal[UnitSymbol.AMPERE] = Field(None, alias="Unit") |
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.
Out of curiosity - when does this usually happen?
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.
This can happen when charging starts. I don't know why but some times they request a negative current as first current demand request.
… range definition)
(because Skoda Enyaq sends in the beginning of CurrentDemand a value of -2)
a977800
to
eeb271c
Compare
@shalinnijel2 I've rebased everything and removed the |
This PR replaces #232
Everything is rebased and changes requested by @tropxy are implemented