OpenCyphal-Garage / cyphal.rs

Please use Canadensis by Sam Crow instead
https://github.com/samcrow/canadensis
Apache License 2.0
44 stars 5 forks source link

API rework #86

Open teamplayer3 opened 3 years ago

teamplayer3 commented 3 years ago

When I saw the Arduino API, I thought of having the same here in Rust. I think that could be possible.

The main thoughts were on how we subscribe to data and how we set up a Node.

For the first point, I think of subscribing on the Node rather on the session manager. To do it like this, the startup process of the Node should be explicit. I think of something like the rocket crate does in a smaller fashion. This should indicate when the Node is in config state and when it can be used to receive and transmit messages.

I don't know if this would restrict, when to subscribe. But I think for a realtime and deterministic application it would be great to do the subscription and this stuff at the beginning and then start up the node. Furthermore, it would add another generic parameter (maybe bad).

This pattern is often used in Rust:

trait State {}

struct Config;
struct Running;

impl State for Config {}
impl State for Running {}

struct Node<S: State> {
    _state: PhantomData<S>
}

impl Node<Config> {
    fn start() -> Node_<Running> {}
}

Pros:

Cons:

Thoughts?

davidlenfesty commented 3 years ago

I'm generally fine with re-working the higher level Node API (and I still want to implement a higher-level structure on top of that for implementing all of the application-level functions), but I don't quite see the reasoning why we need to maintain a configuration vs. running state in this. If you could explain the benefits of the structured configuration?

teamplayer3 commented 3 years ago

I read the description of the Node struct here. So, your idea is to divide the Node into the separate structs like Publisher, Subscriber, ... . When I thought of the configuration steps, I meant to have the Node as a central manager on which you do all calls.

Your idea could look like this:

let node = Node::init();
let subscription = node.subscribe::<DataType>(id); // a subscription to data
let publisher = node.publisher::<DataType>(id);    // to publish data
let listener = node.listen::<DataType>(id);        // for function requests
let caller = node.caller::<DataType>(id);          // for external function calls
node.start();  // <- maybe consume the node to not allow other calls

All the sub-structs will have a ref to the node, or only parts of it which they need.

If we think of a higher application, this could look like this:

impl HeartBeat {
    impl new(node: &Node) -> Self {
        let publisher = node.publisher::<DataType>(id);
    }
}

let node = Node::init();
let heart_beat = HeartBeat::new(&node);
...
loop {
    delay(1000);
    heart_beat.beat();
}

The design with the start() function or the config state will represent the static configuration of the network at first.

To go further into details on the separate structs. The next idea is to do something like this to get back dynamic subs:

let subscription = node::subscribe::<DataType>(id);
// stop/start for now
let dead_sub = subscription.stop(); // will call a unsubscribe with the ID underneath
let subscription = dead_sub.start(); 

Maybe this could be an idea and will help to think further about stuff we need in the Node API. Or how we process further.

pavel-kirienko commented 3 years ago

This is slightly unrelated but the port-ID cannot be a compile-time parameter. It should be a runtime parameter: node.subscribe(id).

teamplayer3 commented 3 years ago

I took the android API as a reference and there is the subscribe message type with the port ID a compile-time thing. Why should it be a runtime parameter @pavel-kirienko?

pavel-kirienko commented 3 years ago

It is a known deficiency of the Arduino-UAVCAN library that was discussed in the past and is mentioned in the README: https://github.com/107-systems/107-Arduino-UAVCAN#example

Making it a compile-time parameter would make it impossible to use data types with non-fixed port-IDs with this library.