Transmission pipeline and cosmetic adjustments to the API

pavel-kirienko commented 1 year ago

Please focus on udpard.c and udpard.h. Upon a closer inspection it appeared to be easier to produce a completely new implementation that exploits the design features of Cyphal/UDP instead of adapting the original CAN implementation.

The code is fully covered by tests; Sonar reports a slightly lower value because of imperfections of llvm-cov.

Golden tests were produced with the help of PyCyphal; relevant Python snippets are provided in the comments.

The commit history is uninformative and should not be looked at.

pavel-kirienko commented 1 year ago

I'm assuming we don't have the work in here to handle a low-priority, long-timeout item in a tx queue?

No, I considered this to not be part of what we defined as "significant completion". This will be taken care of after the core functionality is in place: https://github.com/OpenCyphal-Garage/libudpard/issues/33

I do think we should switch. I don't mind having a bunch of individual test executables. It's how I structured CETL even without the need to test cpp internals.

Okay, I just did that and discovered (too late) that it won't work. The problem is that it is difficult to compile a C file using a C++ compiler without suppressing a very large set of sensible compiler warnings. Clang-Tidy and Sonar can also be made to not emit diagnostics for the included C file but it it slightly less trivial than I would like it to be. As C and C++ are, after all, very different languages, I'm worried that by compiling C with a C++ compiler we may create maintenance issues for us in the future, particularly if we decide to switch to a newer version of either standard at one point. You can see what it looks like in the second-to-last commit (the next one reverts it).

I suggest keeping the existing design as-is. The presence of exposed.hpp pains me but it is less undesirable than the alternative.

thirtytwobits commented 1 year ago

Okay, I just did that and discovered (too late) that it won't work. The problem is that it is difficult to compile a C file using a C++ compiler without suppressing a very large set of sensible compiler warnings...

import the .c into an exposed.c that defines test_hooks. for example:

// realcode.c
static int foo(const char* p)
{
 ...
}

// exposed.c
#include "realcode.c"

EXPORT int exposed_foo(const char* p)
{
    return foo(p);
}

pavel-kirienko commented 1 year ago

Could you please elaborate on the value of this approach compared to exposed.hpp? I see one in that there will be no need to customize UDPARD_INTERNAL but it doesn't seem significant; plus it doesn't obviate the need for exposed.hpp.

thirtytwobits commented 1 year ago

Could you please elaborate on the value of this approach compared to exposed.hpp? I see one in that there will be no need to customize UDPARD_INTERNAL but it doesn't seem significant; plus it doesn't obviate the need for exposed.hpp.

It avoids polluting the production code with a test macro or any knowledge of the tests. I can accept any crazy plan needed in unit tests but not in the product.