New EVCC config isAliveAfterSession introduced #361
Conversation
* Whether or not the EVCC keeps running after a charging session is finished * Default 'True' should not affect EVCC behaviour * When set it to 'False', EVCC terminates by StopNotification on 'TCP connection closed'
| @@ -306,7 +306,7 @@ async def __init__. Therefore, we need to create a separate async | |||
|
|
|||
| logger.info("Communication session handler started") | |||
|
|
|||
| await wait_for_tasks(self.list_of_tasks) | |||
| await wait_for_tasks(self.list_of_tasks, asyncio.ALL_COMPLETED) | |||
There was a problem hiding this comment.
Wouldn't this stop the session from ending if say for instance the udp server fails to start? As self.get_from_rcv_queue() wouldn't have exited.
There was a problem hiding this comment.
I think you are right, asyncio.ALL_COMPLETED is not necessary. wait_for_tasks() default argument asyncio.FIRST_EXCEPTION should suffice.
"FIRST_EXCEPTION | The function will return when any future finishes by raising an exception. If no future raises an exception then it is equivalent to ALL_COMPLETED." - https://docs.python.org/3/library/asyncio-task.html#waiting-primitives
Confirmed by testing without it.
README.md
Outdated
| @@ -173,6 +173,7 @@ Supported settings in EVCC configuration are given below: | |||
| | sdpRetryCycles | `1` | Indicates how often shall SDP (SECC Discovery Protocol) retries happen before reverting to using nominal duty cycle PWM-based charging | | |||
| | maxContractCerts | `3` | Maximum amount of contract certificates the EV stores. | | |||
| | maxSupportingPoints | `1024` | Indicates the maximum number of entries the EVCC supports within the sub-elements of a ScheduleTuple | | |||
| | isAliveAfterSession | `True` | Whether or not the EVCC keeps running after a charging session is finished | | |||
There was a problem hiding this comment.
Apologies I didn't clarify this under issues. As the user can't proceed until the session is killed anyway, I don't see much value in making this a configurable field. The behaviour you have added to gracefully exit is the only change required. Do you have a use case in mind where keeping the session alive would be useful?
There was a problem hiding this comment.
No, I don't have any use case in mind for keeping the session alive. I wanted to keep the session by default and stop the session when the configuration is used. However, it seems like the new configuration key is not necessary as stopping session can be the default behavior👍. I personally think, evcc should be terminated as a default after session, because evcc is not starting a new session again.
| if notification.successful: | ||
| if not self.config.is_alive_after_session: | ||
| break |
There was a problem hiding this comment.
Following my suggestion from above, I think this would suffice. What do you think?
| if notification.successful: | |
| if not self.config.is_alive_after_session: | |
| break | |
| if notification.successful: | |
| break |
There was a problem hiding this comment.
It looks good to me. Added another commit to fix all together.
* The isAliveAfterSession is not required anymore * EVCC calls wait_for_tasks with default return_when=FIRST_EXCEPTION because wait_for_tasks returns both when 1)Exception on any task in the list or 2)Completing all tasks
|
Hi @touchlinux . All good. I'm merging the changes to master. Hope you don't mind. Thank you for your contribution. :). |
Whether or not the EVCC keeps running after a charging session is finished
Default 'True' should not affect EVCC behaviour
When set it to 'False', EVCC terminates by StopNotification on 'TCP connection closed'
Tested with modified
evcc_config_eim_dc.jsonto verify EVCC is terminated gracefully.