OpenCyphal-Garage / cyphal.rs

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

Convert to embedded-hal types. #96

Open davidlenfesty opened 2 years ago

davidlenfesty commented 2 years ago

With embedded-hal 1.0.0 on it's way, and CAN types included, we should now migrate to using those types to improve our support.

Tasks to complete:

teamplayer3 commented 2 years ago

The second step is not that easy. We maintain a timestamp in our CanFrame and the embedded_hal trait wants to implement a new function without this timestamp. We, can't create the timestamp with "now" because a timestamp needs the reference clock to do so.

pub struct CanFrame<C: embedded_time::Clock> {
    pub timestamp: Timestamp<C>,
    pub id: ExtendedId,
    pub payload: ArrayVec<[u8; 8]>,
}

impl<C: embedded_time::Clock> embedded_hal::can::Frame for CanFrame<C> {
    fn new(id: impl Into<Id>, data: &[u8]) -> Option<Self> {}
}

Another thing is, that the trait uses the Id enum. In our case, we only use ExtendedId Frames. So if we allow the user to pass a frame with the trait, we must check if it's a ExtendedId by calling the special function on the trait. Furthermore, the given frame wouldn't have the arrival timestamp.

davidlenfesty commented 2 years ago

Hrm, I had assumed there was good CAN FD support and didn't fully look into it. That is very unfortunate. I think we could have gotten away with wrapping the embedded-hal frame in another struct, but looks like there are other structural issues with how it's implemented that make it hard to integrate generically.

I'll still keep this open as I would like to eventually sync with embedded-hal and maybe have better support for the entire ecosystem, but for now we can just leave this on the backburner.

teamplayer3 commented 2 years ago

For further investigation, we can track how stm32/fdcan processes, especially the frames.

teamplayer3 commented 2 years ago

I think this relates to #91.