pylessard / python-can-isotp

A Python package that provides support for ISO-TP (ISO-15765) protocol
MIT License
261 stars 81 forks source link

Complete event not set when "blocking_send" set to True #129

Closed jpvolkmann closed 3 months ago

jpvolkmann commented 3 months ago

Hello,

just started using the isotp and wanted to send a simple message (Single Frame). When "blocking_send" is set to True the send() command times out while if "blocking_send" is set to False everything is working as expected.

I tried to understand the FSM in protocol.py and realized that self.active_send_request.complete(True) never get called on the transmitted request.

Checked especially line 1060ff of protocol.py

I temporarily fixed it by adding a check for a active_send_reguest but not sure if this will break anything else.

        immediate_rx_msg_required = False
        if self.tx_state == self.TxState.IDLE:
            **if self.active_send_request and self.active_send_request.generator.depleted():
                self.active_send_request.complete(True)
                self.active_send_request = None**        

            read_tx_queue = True  # Read until we get non-empty frame to send
            while read_tx_queue:

Do I use something wrong?

The code is integrated in some other class:

    def connect(self):
        self._bus = can.Bus(
            interface=self._interface,
            channel=self._channel,
            bitrate=self._bitrate,
            fd=self._can_fd,
            fd_bitrate=self._can_fd_bitrate,
            can_termination=self._can_termination,
        )
        # self._bus.set_filters([{"can_id": self._filter_id, "can_mask": 0xFFFF}])

        addr = isotp.Address(
            isotp.AddressingMode.Normal_29bits,
            rxid=self._filter_id,
            txid=self._arbitration_id,
        )
        params = {
            "blocking_send": True, 
            "can_fd": True, 
            "tx_data_length": 64,
        }
        self._stack = isotp.CanStack(
            self._bus, address=addr, error_handler=self._error_handler, params=params
        )

        self._stack.start()
        self.is_connected = True

    def write(self, command):
        msg_bytes = bytes(command)
        self._stack.send(msg_bytes, send_timeout=self.timeout)
pylessard commented 3 months ago

Are you able to share a reproducing code? I will check in more details later.

You can enable logging in debug to get more info You can also run the unit tests to see if you broke something.

blocking mode is unit tested. Maybe there's a case that is not covered and if that is the case, I want to recreate the case in the unit test and fix it after.

jpvolkmann commented 3 months ago

It's easy to reproduce with the test framework if you reduce the number of bytes to fit into one frame (CAN Single Frame),

e.g. from 100 to 5 in the snippet below:

    def test_blocking_send(self):
        self.layer1.params.blocking_send = True
        self.layer1.load_params()
        # layer2 has a thread to handle reception
        self.layer1.send(bytes([1] * 5), send_timeout=5)
        self.assert_no_error_reported()
pylessard commented 3 months ago

Oh. blocking_send is a relatively new feature. It is possible that there's an issue with it. I will add a test case to test the tsingle frame case and fix soon. If it goes well, you can expect a new bugfixed release tomorrow

jpvolkmann commented 3 months ago

Thanks, I can also use "non-blocking send", so it's not urgent... Just want to avoid that somebody else is facing the same problem.

pylessard commented 3 months ago

I can reproduce. Your fix is close to what we need. There is indeed no indication that a single frame is sent for blocking send. Fixing seems to cause troubles with the rate limiter, I will double check that

Thanks for the bug report.

pylessard commented 3 months ago

Fixed by #130

pylessard commented 3 months ago

V2.0.6 is released and includes the fix to that problem Regards