Make C library overhead configurable

[asmfreak]

The standard C library and can produce a lot of unneeded noise in the firmware if it's not used. This PR provides a fix for this problem.

There are several problems with standard C functions:

• [x] assert may be implemented in a different way than libc.
• [x] std::rand can lead to unnecessary overheads from libc.
• [x] virtual destructors require an operator delete to be present (requires further investigation)

assert

A commit is added to replace assert macro with a namespaced one - kocherga_assert, which can be defined to a user-specified code.

std::rand

A commit will be added to make this configurable in UAVCAN/serial module.

virtual destructors require operator delete

See this question on why it may relevant. A change is proposed using this answer from the same post.

Since the solution above requires С++20, we should stick to a simpler solution - removing virtual from base classes. The virtual destructors aren't needed for Kocherga - all virtual classes are managed by the user and in the most cases user does not need to destroy derived objects at all. In the rare event destructon is actually needed, destruction through a base class pointer (which virtual destructors are needed for) must not be used if virtual destructors are disabled. In the commited version virtual destructors continue to be enabled by default.

[pavel-kirienko]

• kocherga_assert() is fine but it should be KOCHERGA_ASSERT().

• The CAN transport also requires randomness, so this logic should be implemented at the top level rather than in kocherga::serial. The current implementation seems wordy and relies on the preprocessor a bit too much. We can do better; how about we define a freestanding function like extern auto getPseudoRandomByte() -> uint8_t but provide no definition for it in the library, such that the user is allowed to redefine it arbitrarily? This should be much simpler than what you are proposing and without additional violations of MISRA.

• Would it not be easier to define operator delete than abusing the preprocessor with KOCHERGA_VIRTUAL_DESTRUCTOR? I don't like this at all.

[asmfreak]

• kocherga_assert -> KOCHERGA_ASSERT is on the way.
• On randomness:
  • For randomness we would need both extern auto getPseudoRandomValue() -> uint32_t and extern constexpr auto getMaxRandomValue() -> uint32_t, doesn't we?
  • My implementation is interface-compatible with #include <random> sequences like std::mt19937. Yes, it is quite wordy. Maybe we can define an template<typename Int>struct kocherga::random::Driver;, instantiate it in code as a reference (like serial/can drivers) and then force user to implement specializations like this (maybe using my impl):

    template<>
    struct kocherga::random::Driver<uint32_t>: public kocherga::random::StdCRand{
    using kocherga::random::StdCRand::StdCRand;
    };
    // or
    template<>
    struct kocherga::random::Driver<uint32_t>: public std::mt19937{
    using std::mt19937;
    };

• We can implement operator delete, it works. I've implemented a general base class and made all inheritable classed inherit from it:

  struct IKochergaBase
  {
  static void operator delete(IKochergaBase*, std::destroying_delete_t)
  {
      kocherga_assert(false);
  }
  virtual ~IKochergaBase() = default;
  };

  The only issue with this would be - we would force our users to use C++20. It may be bad, as kocherga is declared to be C++17 library.

[pavel-kirienko]

For randomness we would need both extern auto getPseudoRandomValue() -> uint32_t and extern constexpr auto getMaxRandomValue() -> uint32_t, doesn't we?

Not if we match the range of the random values returned by this function to match the range of its return type. In my example above I named it getPseudoRandomByte with the return type being uint8_t, which makes it evident that the random values range from 0 to std::numeric_limits<std::uint8_t>::max() inclusive.

We can implement operator delete, it works. I've implemented a general base class and made all inheritable classed inherit from it:

I meant a freestanding definition anywhere else in your program (not in this library):

void operator delete(void*) noexcept { std::abort(); }

[asmfreak]

the return type being uint8_t, which makes it evident that the random values range from 0 to std::numeric_limits<std::uint8_t>::max() inclusive.

I can see that. My objection to that is as follows: right now in 3 spaces (1 in serial and 2 in CAN) where random numbers are needed, int32_t is used. I'm not an expert on UAVCAN protocol in any means, so it's up to you to say - is 8 bits of random sufficient enough for the purposes of that locations? The places are: in serial, in CAN and another in CAN

[pavel-kirienko]

Yes, 8 bits of entropy is expected to be enough for these.

[asmfreak]

I'll do it shortly. Should we provide sample implementations with std::rand and/or C++'s <random> header for user convenience?

[pavel-kirienko]

Should we provide sample implementations with std::rand and/or C++'s header for user convenience?

Maybe but I am not sure where. Do you want to put it in the README?

We can't put it in the headers because that would require more preprocessing.

[asmfreak]

Maybe but I am not sure where. Do you want to put it in the README?

I see two possible ways:

1. Add files like kocherga_random_libc.hpp and kocherga_random_cpp.hpp and make user choose one of them in their main.cpp
2. Provide sample implementations in README.md for the user to copy-paste in their solution.

These files would be just a few lines of code, I assume:

// kocherga_random_libc.hpp
#pragma once
#include "kocherga.hpp"
auto getRandomByte() -> uint8_t{
   return std::rand() *std::numeric_limits<uint8_t>::max() / RAND_MAX;
}

But at the same time for <random> implementation we would need to initialize the sequence somehow, which should be done in the user code. Given all that, which one do you prefer?

[pavel-kirienko]

That's a lot of work for such a trivial feature. Let us add this one line to the README and call it a day:

auto getRandomByte() { return static_cast<std::uint8_t>(std::rand() * std::numeric_limits<std::uint8_t>::max() / RAND_MAX); }

[asmfreak]

I think I've catched all things in this single commit.

[pavel-kirienko]

Thanks. I am going to get back to this after #13 is in.

[pavel-kirienko]

@ASMfreaK please merge master into your branch (or rebase, it doesn't matter because we're going to squash your commits) and make sure the CI is green.

[asmfreak]

@pavel-kirienko Can you please take a look on which test broke? I suspect that a random number generator I used is generating a big delay, which breaks deadline. Maybe I should lower the maximum delay in library code, so even big numbers (which are more likely since we are using a coarser distribution) wouldn't break the deadlines?

[pavel-kirienko]

Thanks Pavel. I had to introduce a couple of final adjustments but there is nothing major. Our robotic overlords will have this merged once the tests have passed.