Only route messages if connection is not None

[drc38]

We have an edge case when using a ChargePoint instance with Home Assistant (https://github.com/lbbrhzn/ocpp/) where the Charger does not close its websocket properly and keeps sending messages. In the integration we want to keep the ChargePoint instance and simply reconnect the websocket when it fails eg network, reboot etc. This can be achieved by setting the _connection to None when the server detects the websocket half-close (resulting in the Charger timing out and closing its half as well) but with the current ocpp code while True throws an AttributeError. Checking the connection is not none would solve this and is preferable to checking the websocket is still open in my view.

[OrangeTux]

Thanks for the PR @drc38.

In the integration we want to keep the ChargePoint instance and simply reconnect the websocket

This make sense. I've seen others wanting something similar. But I'm not entirely sure how your modification would help you. solving this problem. And whether your goal can be achieved in a different way. Can you provide a code snippet that shows the issue you're having?

[drc38]

The modification stops the ChargePoint instance from responding and replying to messages ocpp received from the charger after the websocket half-close, without generating an error. I will capture a debug log when the connection is left open and post.

[drc38]

Thanks for the PR @drc38.

In the integration we want to keep the ChargePoint instance and simply reconnect the websocket

This make sense. I've seen others wanting something similar. But I'm not entirely sure how your modification would help you. solving this problem. And whether your goal can be achieved in a different way. Can you provide a code snippet that shows the issue you're having?

Here is the debug log, note even though websockets closes there is still a ping/pong response afterwards

2021-09-15 19:40:00 DEBUG (MainThread) [custom_components.ocpp] Get Configuration for NumberOfConnectors: 1
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server > Frame(fin=True, opcode=<Opcode.PING: 9>, data=b'\xb0\x15>\xbd', rsv1=False, rsv2=False, rsv3=False)
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server - event = connection_lost([Errno 104] Connection reset by peer)
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server - state = CLOSED
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server x code = 1006, reason = [no reason]
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server - aborted pending pings: 808c3e54, b0153ebd
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server ! failing CLOSED WebSocket connection with code 1006
2021-09-15 19:40:04 ERROR (MainThread) [homeassistant] Error doing job: Future exception was never retrieved
websockets.exceptions.ConnectionClosedError: code = 1006 (connection closed abnormally [internal]), no reason
2021-09-15 19:40:04 DEBUG (MainThread) [websockets.protocol] server x half-closing TCP connection
2021-09-15 19:40:04 DEBUG (MainThread) [custom_components.ocpp] Websockets exception: code = 1006 (connection closed abnormally [internal]), no reason
2021-09-15 19:40:04 INFO (MainThread) [custom_components.ocpp] Charger ENX-AU21060165 disconnected from 0.0.0.0:9000.
2021-09-15 19:40:17 DEBUG (MainThread) [websockets.protocol] server > Frame(fin=True, opcode=<Opcode.PING: 9>, data=b'\x87W{\xad', rsv1=False, rsv2=False, rsv3=False)
2021-09-15 19:40:17 DEBUG (MainThread) [websockets.protocol] server - event = data_received(<10 bytes>)
2021-09-15 19:40:17 DEBUG (MainThread) [websockets.protocol] server < Frame(fin=True, opcode=<Opcode.PONG: 10>, data=b'\x87W{\xad', rsv1=False, rsv2=False, rsv3=False)
2021-09-15 19:40:17 DEBUG (MainThread) [websockets.protocol] server - received solicited pong: 87577bad

[OrangeTux]

The debug log doesn't help me much, unfortunately.

Assuming this PR gets merged, how you're planning to catch any exception related to connection issues? When you handle such exception, wouldn't it be an alternative to update the ChargePoint with a new connection and call ChargePoint.start() again?

I'm still having troubles understanding how this PR would make your live easier. A code snippet would help with that.

[drc38]

The debug log doesn't help me much, unfortunately.

Assuming this PR gets merged, how you're planning to catch any exception related to connection issues? When you handle such exception, wouldn't it be an alternative to update the ChargePoint with a new connection and call ChargePoint.start() again?

I'm still having troubles understanding how this PR would make your live easier. A code snippet would help with that.

Here is the code snippet for starting and reconnecting the charger from the ChargePoint instance

    async def start(self):
        """Start charge point."""
        try:
            await asyncio.gather(super().start(), self.post_connect())
        except websockets.exceptions.WebSocketException as e:
            _LOGGER.debug("Websockets exception: %s", e)
        finally:
            await self._connection.close()
            self._connection = None
            self.status = STATE_UNAVAILABLE

    async def reconnect(self, connection):
        """Reconnect charge point."""
        self._connection = connection
        self._metrics[cstat.reconnects.value].value += 1
        try:
            self.status = STATE_OK
            await super().start()
        except websockets.exceptions.WebSocketException as e:
            _LOGGER.debug("Websockets exception: %s", e)
        finally:
            await self._connection.close()
            self._connection = None
            self.status = STATE_UNAVAILABLE

Here is the relevant section from the server's on_connect method passed to websockets.serve. Let me know if you need more.

        try:
            if self.cpid not in self.charge_points:
                _LOGGER.info(f"Charger {cp_id} connected to {self.host}:{self.port}.")
                cp = ChargePoint(cp_id, websocket, self.hass, self.entry, self)
                self.charge_points[self.cpid] = cp
                await self.charge_points[self.cpid].start()
            else:
                _LOGGER.info(f"Charger {cp_id} reconnected to {self.host}:{self.port}.")
                cp = self.charge_points[self.cpid]
                await self.charge_points[self.cpid].reconnect(websocket)
        except Exception as e:
            _LOGGER.error(f"Exception occurred:\n{e}", exc_info=True)

        finally:
            _LOGGER.info(f"Charger {cp_id} disconnected from {self.host}:{self.port}.")

[drc38]

To prevent the connected charger from incorrectly assuming the websocket is still open after super().start() has completed, the following two lines are required in the finally statement:

await self._connection.close()
self._connection = None

before the HA integration reports the charger is no longer contactable. If the cause is something like a reboot the charger will then initiate a new websocket request and the reconnect code is executed.

[drc38]

@OrangeTux, another reason for the PR is for our upstream integration to easily close the super().start() awaitable that is initiated by gather. If gather has more than one indefinite awaitable, and say one throws an exception and you want to cancel the other then it can be a little tricky otherwise as gather.cancel() will not work.