search

OpenCyphal / libcanard

A compact implementation of the Cyphal/CAN protocol in C for high-integrity real-time embedded systems
http://opencyphal.org
MIT License
326 stars 192 forks source link

Should we consider switching to a more static architecture for the benefit of high-reliability applications? #75

Closed pavel-kirienko closed 5 years ago

pavel-kirienko commented 5 years ago

The current implementation of the library uses a lot of indirection, which should be minimized in high-reliability settings (see MISRA, AUTOSAR, power of 10). Should we trade-off general utility for robustness?

The two prospects for improvement here are:

  1. Replacement of the instance pointers with a static private instance structure located inside canard.c. The downside is that it will no longer be possible to use more than one instance per application. The upside is the simplification of the API -- one argument will be removed from all API calls.
  2. Replacement of the callback pointers (which are generally dangerous) with statically bound external handlers (extern void canardOnTransferReception(CanardRxTransfer* transfer); instead of the dynamic callback pointer). Same downside.

The removal of the indirection will also reduce resource usage, which is important in the applications targeted by Libcanard. Also, removal of callbacks will improve its API compatibility with fully synchronous applications (e.g., sampling & queuing ports instead of asynchronous callbacks, see ARINC 653).

pavel-kirienko commented 5 years ago

@aasmune @kjetilkjeka @tridge @thirtytwobits we could use your input here.

kjetilkjeka commented 5 years ago

I'm not convinced doing what you propose in 1 will increase robustness. It will at least lower testability somewhat.

I guess doing 2. without doing 1. Is counter productive.

I think the current implementation is on the correct point in this question. My vote goes towards not changing it.

(Or just rewrite it in a modern language that got generics and type safety. I've even heard about such language without null pointers)

aasmune commented 5 years ago

I agree with Kjetil.

Restricting the library to only one instance per application seems like a step in the wrong direction and may break applications which are connected to multiple buses and are using multiple instances.

One of the design goals for Libcanard is to not make any assumptions to the underlying hardware, and I also feel this should imply that no assumptions are made for the physical interfaces as well. Even though Libcanard doesn't support redundant interfaces by design, this should not be a limitation for the ability of interfacing concurrently with separate can buses.

pavel-kirienko commented 5 years ago

Okay, thank you, let's keep things unchanged.