Skip to content

NotifierBasedCanStack takes a long time to process error frames #142

@TonyBeswick

Description

@TonyBeswick

Problem:

  • NotifierBasedCanStack._rx_canbus() calls _python_can_to_isotp_message() to convert the message type from a can.Message to a isotp.CanMessage.

  • _python_can_to_isotp_message() returns None if the input message is None, or if the input message is an error frame or remote frame (See https://github.com/pylessard/python-can-isotp/blob/v2.x/isotp/protocol.py#L1713).

  • When _relay_thread_fn reads the None I believe it interprets it as a "buffer empty" and performs a sleep (20ms sleep on my machine) before attempting to read the next frame.

Under error conditions (effectively a problematic device shorting the bus for several seconds), I'm seeing 10,000+ consecutive error frames.
The problematic device then recovers and begins transmitting frames as it should, but it takes several minutes for the NotifierBasedCanStack to process all the error frames (at 20ms each). Application code then times out because it expects to read a valid message off the stack within a few seconds, not several minutes.

Steps to reproduce:

import unittest
import isotp
import can


class TestNotifierBasedCanStack(unittest.TestCase):
    def test_receiving_many_error_frames(self):
        # Setup
        bus = can.Bus(interface='virtual', channel='vcan0', receive_own_messages=True)
        notifier = can.Notifier(bus, [])

        address = isotp.Address(isotp.AddressingMode.Normal_29bits, txid=0x2, rxid=0x1)
        isotp_stack = isotp.NotifierBasedCanStack(bus, notifier, address=address)

        isotp_stack.start()
        self.addCleanup(isotp_stack.stop)

        # Check the stack can correctly receive a message
        msg = can.Message(arbitration_id=0x1, data=b'\x00\x05test1')
        bus.send(msg)

        data = isotp_stack.recv(block=True, timeout=1)
        self.assertEqual(data, b'test1')

        # Send a bunch of error frames:
        error_frame = can.Message(arbitration_id=0x8, is_extended_id=False, is_error_frame=True, dlc=4,
                                  data=[0x0, 0x19, 0x81, 0x0])
        for i in range(1000):
            bus.send(error_frame)

        # Check the stack can correctly receive another message
        msg = can.Message(arbitration_id=0x1, data=b'\x00\x05test2')
        bus.send(msg)

        data = isotp_stack.recv(block=True, timeout=10)
        self.assertEqual(data, b'test2')

Note: The above test passes if these lines are commented out in _python_can_to_isotp_message

    # if msg.is_error_frame or msg.is_remote_frame:
    #     return None

But I doubt thats a proper fix for this problem.

Environment

OS: Windows 10
Python version: 3.12.10
can-isotp version: 2.0.6
python can version: 4.5.0
Can adapter: PCAN (but not needed for above test)
PCAN PEAK-Drivers version: 4.5.0

Possible Solution:

Perhaps NotifierBasedCanStack could inspect the frames provided by the Notifier and not put the frame on the buffered_reader if its an error or remote frame?

I'm happy to submit a PR if above solution is acceptable, or take guidance on a better solution. Thank you for your consideration.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions