pylessard / python-udsoncan

Python implementation of UDS (ISO-14229) standard.
MIT License
580 stars 197 forks source link

isotp error handler not propagated to uds connection #233

Open elupus opened 4 months ago

elupus commented 4 months ago

When a isotp error occurs, it triggers a call to a callback, that one can supply to the constructor of the isotp stack called error_handler. The default of this callback is to just log an message to log.

The uds stack should be informed of this error and abort on-going requests in PythonIsoTpConnection.specific_wait_frame. With log request timeouts, it can take a long time for the request to abort. It also aborts with a TimeoutException instead of the actual low level fault.

elupus commented 4 months ago

In general i might be that the protocol._stop_receiving() or _stop_sending() call is not propagated to the listener. not all error_handler calls abort the reception.

elupus commented 4 months ago

Did the following hack to work around it:

class Connection(PythonIsoTpConnection):
    def transport_error(self, exception: Exception):
        self.fromIsoTPQueue.put(exception)

    def specific_wait_frame(self, timeout=2):
        if not self.opened:
            raise RuntimeError("Connection is not open")

        frame = None
        try:
            frame = self.fromIsoTPQueue.get(block=True, timeout=timeout)
        except queue.Empty as exception:
            raise TimeoutException("Did not receive frame IsoTP Transport layer in time (timeout=%s sec)" % timeout) from exception

        if isinstance(frame, Exception):
            raise frame

        if self.mtu is not None:
            if frame is not None and len(frame) > self.mtu:
                self.logger.warning("Truncating received payload to a length of %d" % (self.mtu))
                frame = frame[0:self.mtu]

        return bytes(frame)  # isotp.protocol.TransportLayer uses bytearray. udsoncan is strict on bytes format

Then hook that transport error into the isotp.CanStack.


        connection: Connection
        def transport_error(exception: Exception):
            connection.transport_error(exception)

        transport = isotp.CanStack(
            self.bus,
            address=isotp_addr,
            params=isotp_params,
            error_handler=transport_error,
        )
        connection = Connection(transport)

But it gets ugly due to when and how the transport_error is registered. Maybe the recv() function of isotp should be changed to propagate these exceptions? In my case it's usually been "FlowControlTimeoutError: Reception of FlowControl timed out. Stopping transmission" that causes issues.

pylessard commented 4 months ago

Thanks, I see what you mean. The error should be reported to the upper layer. I will check this, but maybe not short term.