Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiframe transfer assert expression correction #48

Merged

Conversation

bradAPD
Copy link
Contributor

@bradAPD bradAPD commented Jul 20, 2023

This pull request modifies an assert check targeting an underflow condition in multi-frame transfer receive processing. The assert aims to detect accidental underflow in a memory offset computation. The assert expression mistakenly invalidates a case where the final CAN frame of a transfer is received with all previously allocated memory blocks fully utilised. This condition requires the base address of the frame data buffer to be assigned to the rx_transfer.payload_tail field.

The assert case is triggerable by performing a multi-transfer file read operation using the DroneCAN GUI tool, such as during a firmware update process. The CAN bus is configured in non-CAN-FD mode, with the default memory allocator block size of 32 bytes.

@bradAPD bradAPD force-pushed the pr-multiframe-assert-criteria-revision branch from 1dc4796 to c14f03d Compare July 20, 2023 05:09
…r a valid operation.

Modifying underflow assert check. This check mistakenly invalidates the case where the final CAN frame of a transfer is received with all previously allocated memory blocks fully utilised.
@tridge tridge force-pushed the pr-multiframe-assert-criteria-revision branch from c14f03d to f770569 Compare August 31, 2023 01:11
@tridge
Copy link
Member

tridge commented Aug 31, 2023

this change looks correct, but I'd like @bugobliterator to comment

@bugobliterator bugobliterator merged commit e5b6842 into dronecan:master Aug 31, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants