improve reconnect and trigger boot notification

[lbbrhzn]

after adding trigger_boot_notification to post_connect, we could get into an infinite loop because the boot notification handler would create a new post_connect. This makes sense in case of a spontaneous reboot, but not if the boot notification was triggered. So we check for this, and only only re-do post connect if the boot notification was not triggered.

[drc38]

Should there be a finally: before line 821?

[drc38]

I think you also need to set return_exceptions=True on gather and then cancel the gather on exception to stop the monitor_connection task in the start and reconnect try blocks.

[lbbrhzn]

yes, something like that is needed. But with return_exceptions is True gather will wait until all awaitables have been completed, which is not what we want. If any awaitable raises an exception, all should be cancelled.

Once the exception is raised, gather completes and cancelling gather will not cancel the subtasks. There's two possible solutions:

1. Wrap the awaitables as tasks, and cancel them all in case there is an exception.
   https://python.tutorialink.com/how-to-cancel-all-remaining-tasks-in-gather-if-one-fails/

2. In the exception handler of each the subtask/awaitable, cancel the gather, without raising an exception, i.e.

try:
  ....
except Exception:
   self.gather.cancel()

I prefer option 2.

[drc38]

My understanding is if return_exceptions is True then cancelling gather will cancel the other tasks, but I haven't confirmed with a piece of test code. The difference being you have to test the result for an Exception. https://hynek.me/articles/waiting-in-asyncio/

[drc38]

My understanding is if return_exceptions is True then cancelling gather will cancel the other tasks, but I haven't confirmed with a piece of test code. The difference being you have to test the result for an Exception. https://hynek.me/articles/waiting-in-asyncio/

Ignore this, you are right it will only work for finite tasks as the result from gather is only returned after all tasks have completed successfully or thrown an exception (which may happen but there is no guarantee given we are running two indefinite loops). I agree option 2 looks cleaner.

[drc38]

Another option worth considering is removing the closing from the monitor_connection function, and simply propagate the error back to the calling try block (start and reconnect), after which the connection will close in the finally statement. The logging and closing can then be handled there eg

In monitor_connection:

except Exception as e:
    raise e

In start/reconnect add:

except asyncio.TimeoutError:
            _LOGGER.debug(f"Timeout in connection '{self.id}'")
except websockets.exceptions.ConnectionClosed as connection_closed_exception:
            _LOGGER.debug(
                f"Connection closed to '{self.id}': {connection_closed_exception}"
            )
except Exception as other_exception:
            _LOGGER.error(
                f"Unexpected exception in connection to '{self.id}': {other_exception}",
                exc_info=True,
            )

The while connection.open: in the monitor_connection should take care of exiting the loop. I had set self._connection = None in the start/reconnect functions which which could be used instead, my reasoning was it is unclear when websockets will set connection.open to false and so it is better to control the connection state in our code. I was hoping the PR would be merged upstream to clear the exception this sometimes caused mobilityhouse/ocpp#248

[lbbrhzn]

got something that seems to work now, please review.

[drc38]

I'm getting the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/websockets/legacy/server.py", line 293, in handler
    await self.ws_handler(self, path)
  File "/config/custom_components/ocpp/api.py", line 203, in on_connect
    f"Charger {cp_id} disconnected from {self.host}:{self.port} websocket={websocket.id}."
AttributeError: 'WebSocketServerProtocol' object has no attribute 'id'

[lbbrhzn]

Strange, works for me. What is your websockets version?
Should be there, see https://websockets.readthedocs.io/en/stable/reference/server.html#websockets.server.WebSocketServerProtocol

[lbbrhzn]

i'll release as beta now, so others can check if it works

[drc38]

Strange, works for me. What is your websockets version? Should be there, see https://websockets.readthedocs.io/en/stable/reference/server.html#websockets.server.WebSocketServerProtocol

That's odd somehow HA reverted to websockets 9.1 although I have been on 10.1 for sometime... I had to go into bash and upgrade to get 10.1 back again. This may be the reason:

volvooncall 0.9.1 requires websockets<=8.1, but you'll have websockets 10.1 which is incompatible.
vallox-websocket-api 2.8.1 requires websockets<10.0,>=9.1, but you'll have websockets 10.1 which is incompatible.