-
-
Notifications
You must be signed in to change notification settings - Fork 195
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
UAVCAN v1.0 done #142
UAVCAN v1.0 done #142
Conversation
Great News! |
Codecov Report
@@ Coverage Diff @@
## uavcan-v1.0 #142 +/- ##
============================================
Coverage 46.76% 46.76%
============================================
Files 13 13
Lines 4965 4965
============================================
Hits 2322 2322
Misses 2643 2643 Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## uavcan-v1.0 #142 +/- ##
==============================================
Coverage ? 46.76%
==============================================
Files ? 13
Lines ? 4965
Branches ? 0
==============================================
Hits ? 2322
Misses ? 2643
Partials ? 0
Continue to review full report at Codecov.
|
…racked by SonarQube.
Disabled the codecov integration to prevent confusion. SonarQube is already set up to track the coverage correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial feedback.
So, I can approve this based on a cursory reading of the code but I'm not saying I did a "good" code review because it's big. If you want a real code review it'll take longer than this weekend to go though and compile this myself, use it, and dive into the implementation. |
State that the frame payload pointer may be NULL if the size is zero.
That's understandable. I think we can afford some laxity until Libcanard v1.0 is released (the current version is 0.100); I suppose we will have a few months at least to get major API-breaking changes in, shall the need arise. For now, seeing as there don't seem to be any major issues, I would like to have it merged and the branches swapped. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs |
The library has been somewhat reworked. Don't look at the diff, it is too large -- this change is impossible to do incrementally in a sensible way.
The new implementation offers hard time complexity guarantees as discussed in OpenCyphal-Garage/libcyphal#185. The new implementation supports both Classic CAN and CAN FD without the need for compile-time configuration selection (#103). The new implementation offers a much simplified API that operates on contiguous memory buffers instead of small fixed-size blocks (https://forum.uavcan.org/t/canardrxtransfer-payload-reading/600).
After this PR is merged, I am going to do the branch swap:
master
will be v1.0, and the current master will becomelegacy-v0
.Fixes #131
Fixes #129 (this case is covered by tests)
Fixes #126
Fixes #125
Fixes #117 (completely new memory management)
Fixes #105
Fixes #104 (there is no DESIGN.md, the description has been moved into the header file)
Fixes #102 (100% test coverage)
Fixes #69 (https://forum.uavcan.org/t/platform-specific-components/768)
Closes #132