pavel-kirienko commented 4 years ago

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 https://github.com/UAVCAN/libuavcan/issues/185. The new implementation supports both Classic CAN and CAN FD without the need for compile-time configuration selection (https://github.com/UAVCAN/libcanard/pull/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 become legacy-v0.

Linjieqiang commented 4 years ago

Great News!

LorenzMeier commented 4 years ago

Codecov Report

Merging #142 into uavcan-v1.0 will not change coverage by %. The diff coverage is n/a.

@@             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.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b5f1b77...b5f1b77. Read the comment docs.

codecov-io commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (uavcan-v1.0@c1cf930). Click here to learn what that means. The diff coverage is 96.67%.

@@              Coverage Diff               @@
##             uavcan-v1.0     #142   +/-   ##
==============================================
  Coverage               ?   46.76%           
==============================================
  Files                  ?       13           
  Lines                  ?     4965           
  Branches               ?        0           
==============================================
  Hits                   ?     2322           
  Misses                 ?     2643           
  Partials               ?        0

Impacted Files	Coverage Δ
libcanard/canard.c	`100% <ø> (ø)`
tests/test_self.cpp	`100% <100%> (ø)`
tests/exposed.hpp	`100% <100%> (ø)`
tests/test_private_rx.cpp	`100% <100%> (ø)`
tests/test_private_crc.cpp	`100% <100%> (ø)`
libcanard/canard_dsdl.c	`100% <100%> (ø)`
tests/test_public_tx.cpp	`100% <100%> (ø)`
tests/test_dsdl.cpp	`100% <100%> (ø)`
tests/test_public_rx.cpp	`100% <100%> (ø)`
tests/test_private_tx.cpp	`100% <100%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c1cf930...b5f1b77. Read the comment docs.

pavel-kirienko commented 4 years ago

Disabled the codecov integration to prevent confusion. SonarQube is already set up to track the coverage correctly.

thirtytwobits commented 4 years ago

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.

pavel-kirienko commented 4 years ago

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.

sonarcloud[bot] commented 4 years ago

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

100.0% Coverage
0.0% Duplication

OpenCyphal / libcanard

UAVCAN v1.0 done #142

Codecov Report

Codecov Report