nasa / fprime

F´ - A flight software and embedded systems framework
https://fprime.jpl.nasa.gov
Apache License 2.0
10.08k stars 1.31k forks source link

FileUplink packet sequence repeat and CRC #1378

Open timcanham opened 2 years ago

timcanham commented 2 years ago
F´ Version 3.0
Affected Component Svc/FileUplink

Feature Description

Add duplicate packet detection based on sequence number and don't add to CRC computation

Rationale

If the radio link is marginal, a radio can do a retry and send the same packet. In that case, you get the following warning:

FileUplink_PacketOutOfOrder: Received packet 342 after packet 342

The repeated packet still gets added to the CRC computation, so even with a successful retry, you get this warning at the end:

FileUplink_BadChecksum: Bad checksum value during receipt of file XXXXX: computed 0x6A4F6972, read 0x92A51AAA

The update would to be to not update the CRC only in the case of a repeated packet sequence number. This wouldn't be to try to track all cases of dropped and retried packets.

DJKessler commented 2 weeks ago

@timcanham -- In the case of a repeated packet, should the repeated packet still get written to the file? In that case, I'll simply protect against updating the checksum by wrapping THIS LINE in a condition.

If we don't want to write the repeated packet to the file, then I'll wrap THIS BLOCK OF CODE in a condition that uses the same logic as checkSequenceIndex().

timcanham commented 2 weeks ago

@DJKessler I would skip the second write if the first copy was received successfully.

LeStarch commented 1 week ago

@DJKessler I think this should be reassigned given the schedule change-up. Do you agree?

DJKessler commented 1 week ago

@LeStarch -- Schedule change-up? I've actually got this fix implemented. I'm about to submit the pull request.