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

feat: verify packet is not an older packet #112

Merged
merged 5 commits into from
Mar 2, 2024
Merged

feat: verify packet is not an older packet #112

merged 5 commits into from
Mar 2, 2024

Conversation

thecode
Copy link
Contributor

@thecode thecode commented Mar 1, 2024

No description provided.

Comment on lines 449 to 453
elif (
last_packet_id is not None
and new_packet_id <= last_packet_id
and last_packet_id - new_packet_id < 128
):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make this a named function to better show intent and document with a docstring its handling the packet id wrapping

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also moved the logic for comparing packet time to make it clear what are the conditions for dropping a packet

@bdraco bdraco changed the title feat: verfiy packet is not an older packet feat: verify packet is not an older packet Mar 1, 2024
Copy link

codecov bot commented Mar 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.28%. Comparing base (c428f92) to head (8a23fbd).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
+ Coverage   80.92%   81.28%   +0.36%     
==========================================
  Files           6        6              
  Lines         519      529      +10     
  Branches       79       82       +3     
==========================================
+ Hits          420      430      +10     
  Misses         78       78              
  Partials       21       21              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@thecode thecode marked this pull request as ready for review March 2, 2024 12:08
Copy link
Collaborator

@Ernst79 Ernst79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, some small remarks. Thanks!

@Ernst79 Ernst79 merged commit 113e49d into main Mar 2, 2024
8 checks passed
@Ernst79 Ernst79 deleted the fix-ble-parser branch March 2, 2024 18:10
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