OpenCyphal / libudpard

A compact implementation of the Cyphal/UDP protocol in C for high-integrity real-time embedded systems
MIT License
11 stars 8 forks source link

Start of Transfer Detection is incorrect. #19

Closed emrainey closed 1 year ago

emrainey commented 1 year ago

start of transfer is indicated by a zero, not a 1.

out->start_of_transfer = (((frame->udp_cyphal_header.frame_index_eot) & (UDPARD_MAX_FRAME_INDEX)) == 1);

Should be

out->start_of_transfer = (((frame->udp_cyphal_header.frame_index_eot) & (UDPARD_MAX_FRAME_INDEX)) == 0);

emrainey commented 1 year ago

This was found during Service transfers but I would imagine it's happening all the time?

lydia-at-amazon commented 1 year ago

Hey Erik, thanks for pointing this out.

Looks like there a couple of things we need to change.

We are incorrectly setting the frame index to 1 instead of 0 when we call txMakeFrameHeader for both single frame transfers and multiframe transfers:

  1. Single frame transfers: https://github.com/OpenCyphal-Garage/libudpard/blob/main/libudpard/udpard.c#L445
  2. For multiframe transfers, we need to:

I think there might be a few other places we need to update the logic as well.

lydia-at-amazon commented 1 year ago

Fixed by https://github.com/OpenCyphal-Garage/libudpard/pull/27