feat/service_status/ab#2950 #148
Conversation
| @@ -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.
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?
| if tls: | ||
| self.tls_ready_event.set() | ||
| else: | ||
| self.tcp_ready_event.set() | ||
|
|
There was a problem hiding this comment.
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.
I thought it could be used in the future to notify when a server failed. It's not needed for now.
| @@ -72,6 +73,14 @@ class EVDataContext: | |||
| soc: Optional[int] = None # 0-100 | |||
|
|
|||
|
|
|||
| class EVSEServiceStatus(str, Enum): | |||
There was a problem hiding this comment.
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?
| async def check_ready_status(self) -> None: | ||
| # Wait until all flags are set | ||
| while self.check_events() is False: | ||
| await asyncio.sleep(0.1) |
There was a problem hiding this comment.
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?
This PR allows setting and logging service status based on the servers' statuses: