[Feature Request] Separate Node and Cyphal implementation

[JustinOng]

⚡ Feature Request

I'm looking to use this library with a FreeRTOS-based environment targeting the ESP-IDF. Right now, much of the logic in Node assumes single-threaded operation and also bundles tools like o1heap and tx/rx buffers inside. I'm hoping to split the actual Cyphal/Pub-sub/Service logic from the rest in order to make the library thread-safe and take advantage of the CAN buffers already built into ESP-IDF.

The obvious solution here is to split the Cyphal/Pub-sub/Service logic into an abstract NodeBase, then the current logic around spinSome and various bundled tools goes into another class that implements NodeBase. Of course, there's the associated overheads with runtime polymorphism, unless more complex patterns like CTRP are used.

I'm interested to hear your thoughts on alternative potential implementations and acceptable performance tradeoffs - I'm willing to work on this and contribute a draft/full PR for this.

[pavel-kirienko]

take advantage of the CAN buffers already built into ESP-IDF

Can you provide more info on their implementation? Many implementations of CAN queues that I've seen were done by people who only have experience with traditional packet-based networks, which resulted in the inner priority inversion problem in the queue. This is not applicable to the rx queue but the tx queue is not a linear FIFO but rather a priority queue (tree). Using CAN with a FIFO TX is deadly.

Related: https://forum.opencyphal.org/t/uavcan-can-tx-buffer-management-in-can-fd-controllers/1215

[JustinOng]

I see the issue, thanks for catching that. The queue in ESP-IDF is a generic FIFO queue, so the TX queue should still be handled by libcanard.

[aentinger]

Imho it would be acceptable to further split up the current implementation for such an use-case. However, please be advised that it can already be made threadsafe by putting every access to any top-level library API within a critical section (i.e. an mutex). Furthermore, even when you can not handle those buffers and queues yourself, you can run-time configure their size to your liking, so I'm not seeing the immediate benefit there. Please remember, this is first and foremost a "batteries included" Arduino library, no (well, very little) external components needed to get started.

I'm closing this but I'd be happy to have a discussion in a separate PR presenting an idea how the solution could look like and inhowfar it would be better to the current one.