-
Notifications
You must be signed in to change notification settings - Fork 98
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
feat/service_status/ab#2950 #148
Changes from 5 commits
d49649d
26eb08f
59cf518
d5e9fc6
16f9b0e
b2f8970
d19bdbf
5a0b981
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 |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
""" | ||
from abc import ABC, abstractmethod | ||
from dataclasses import dataclass | ||
from enum import Enum | ||
from typing import Dict, List, Optional, Union | ||
|
||
from iso15118.shared.messages.datatypes import ( | ||
|
@@ -72,6 +73,14 @@ class EVDataContext: | |
soc: Optional[int] = None # 0-100 | ||
|
||
|
||
class EVSEServiceStatus(str, Enum): | ||
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. As the intention of the enum would be to indicate the status of the entire 15118 service and not just the one EVSE (even though that is what it is being used for at the moment but would change soon), I think it might be better to reflect that - calling it ServiceStatus should do? 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. Done |
||
READY = "ready" | ||
STARTING = "starting" | ||
STOPPING = "stopping" | ||
ERROR = "error" | ||
BUSY = "busy" | ||
|
||
|
||
@dataclass | ||
class EVChargeParamsLimits: | ||
ev_max_voltage: Optional[Union[PVEVMaxVoltageLimit, PVEVMaxVoltage]] = None | ||
|
@@ -91,6 +100,13 @@ def reset_ev_data_context(self): | |
# | COMMON FUNCTIONS (FOR ALL ENERGY TRANSFER MODES) | | ||
# ============================================================================ | ||
|
||
@abstractmethod | ||
async def set_status(self, status: EVSEServiceStatus) -> None: | ||
""" | ||
Sets the new status for the EVSE Controller | ||
""" | ||
raise NotImplementedError | ||
|
||
@abstractmethod | ||
async def get_evse_id(self, protocol: Protocol) -> str: | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,8 @@ def __init__(self, session_handler_queue: asyncio.Queue, iface: str) -> None: | |
self.port_no_tls = get_tcp_port() | ||
self.port_tls = get_tcp_port() | ||
self.iface = iface | ||
self.tls_ready_event: asyncio.Event = asyncio.Event() | ||
self.tcp_ready_event: asyncio.Event = asyncio.Event() | ||
|
||
# Making sure the TCP and TLS port are definitely different | ||
while self.port_no_tls == self.port_tls: | ||
|
@@ -112,6 +114,11 @@ async def server_factory(self, tls: bool) -> None: | |
f"port {port}" | ||
) | ||
|
||
if tls: | ||
self.tls_ready_event.set() | ||
else: | ||
self.tcp_ready_event.set() | ||
|
||
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. Same comment regarding udp server. Wouldn't it suffice if we pass in the ready_event Event through the start_tls() or start_no_tls() method - as the state is not required after this - so making it a member variable would not be required. What do you think? 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. I thought it could be used in the future to notify when a server failed. It's not needed for now. |
||
try: | ||
# Shield the task so we can handle the cancellation | ||
# closing the opening connections | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ class UDPServer(asyncio.DatagramProtocol): | |
def __init__(self, session_handler_queue: asyncio.Queue, iface: str): | ||
self.started: bool = False | ||
self.iface = iface | ||
self.ready_event: asyncio.Event = asyncio.Event() | ||
self._session_handler_queue: asyncio.Queue = session_handler_queue | ||
self._rcv_queue: asyncio.Queue = asyncio.Queue() | ||
self._transport: Optional[DatagramTransport] = None | ||
|
@@ -99,6 +100,7 @@ async def start(self): | |
f"{SDP_MULTICAST_GROUP}%{self.iface} " | ||
f"and port {SDP_SERVER_PORT}" | ||
) | ||
self.ready_event.set() | ||
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. I'm not sure if there is much value in making read_event a member variable as the only thing time it is used for is to indicate when the udp server is setup? Wouldn't it suffice to pass it through the start() method? 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. Done. |
||
tasks = [self.rcv_task()] | ||
await wait_for_tasks(tasks) | ||
|
||
|
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 think 100 ms maybe too liberal for this - as servers could potentially be ready in just a few ms - modify this to be 10ms maybe?
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.
Sure.