Transfer.is_response_to() should check transfer_id?

[matt-foundry]

Hi,

I think I've encountered a bug in how pyuavcan (legacy-v0) dispatches to the list of _outstanding_request_callbacks. When several requests with the same message_id are simultaneously in flight the wrong request callback is sometimes invoked by _recv_frame. Empirically this seems to occur when one of the requests fails to receive a complete reply. I believe the issue is in the implementation of Transfer.is_response_to(), which currently doesn't match on transfer_id but probably should.

A candidate fix is to change the implementation of Transfer.is_response_to() as follows:

def is_response_to(self, transfer):
    if (transfer.service_not_message and self.service_not_message and
            transfer.request_not_response and
            not self.request_not_response and
            transfer.dest_node_id == self.source_node_id and
            transfer.source_node_id == self.dest_node_id and
            transfer.data_type_id == self.data_type_id and
            transfer.transfer_id == self.transfer_id): # <-- Additional check added here
        return True
    else:
        return False

After applying this patch to pyuavcan I haven't seen the issue reoccur in my application. Please confirm if this is indeed a bug & feel free to apply the above fix to the legacy library.

Cheers, Matt

[pavel-kirienko]

Thank you, your assumption is correct. Fixed in legacy-v0.