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

FileUplink packet sequence repeat and CRC #1378

Closed
timcanham opened this issue Apr 11, 2022 · 5 comments
Closed

FileUplink packet sequence repeat and CRC #1378

timcanham opened this issue Apr 11, 2022 · 5 comments
Assignees
Labels
enhancement High Priority High Priority issue that needs to be resolved.

Comments

@timcanham
Copy link
Collaborator

timcanham commented Apr 11, 2022

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.

@LeStarch LeStarch added the High Priority High Priority issue that needs to be resolved. label Mar 28, 2024
@DJKessler
Copy link
Collaborator

@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
Copy link
Collaborator Author

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

@LeStarch
Copy link
Collaborator

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

@DJKessler
Copy link
Collaborator

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

LeStarch pushed a commit that referenced this issue Nov 14, 2024
* Added duplicated packet event/warning

* Added method to check for duplicated packet

* Added last packet write status member variable

* Duplicate file packets are now skipped

* Added duplicate file packet UT

* Fixed `m_lastPacketWriteStatus` initialization.

* Updated UT to test that the first packet isn't erroneously marked as a duplicate
@LeStarch
Copy link
Collaborator

Closing as complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement High Priority High Priority issue that needs to be resolved.
Projects
None yet
Development

No branches or pull requests

3 participants