rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 202 forks source link

CAN trait intent unclear and possibly not well applicable across different hardware #447

Open ericyanush opened 1 year ago

ericyanush commented 1 year ago

Hello!

I'm opening this issue as a continuation of the discussion on the bxCAN driver stm32-rs/bxcan#57 surrounding the behaviour of the driver, specifically as it relates to priority inversion.

As part of the linked discussion, I raised the question as to if the API surface of the embedded can traits should be modified to be better suited for implementation across different CAN hardware peripherals. As a short description of the specific changes I think should be made to the CAN HAL traits:

  1. A way to identify/select if the peripheral should operate in FIFO mode or ID priority mode for transmission. Depending on the higher layer protocol being used either may be necessary. From a hardware perspective, only one of these modes may be available in hardware natively, and in some cases the hardware supports neither of these modes directly. For example, the ST bxCAN peripheral can support either FIFO mode or ID priority mode within its transmit buffers, but not both at once. In contrast the NXP LPC line C_CAN peripheral supports neither, and instead will transmit messages in the order they appear within its transmit buffers (lower number TX slot = higher transmit priority), which is different than NXP's Kinetis line MSCAN where each message is explicitly assigned an 8bit local "priority", which determines the order messages are transmitted. Implementing one or both of ID priority or FIFO mode can come with a significant performance penalty on each individual set of hardware.
  2. A way for the client code to indicate if a previously queued, unsent frame with the same ID is in the TX queue if it should be updated/replaced by the new frame being submitted to the queue. Again, depending on the higher level protocol, it may be necessary to replace the message that was previously queued (time triggered value updates), or it may be required that the new message be transmitted after the last one with matching ID (segmented transfers).

Without the ability for driver implementers to either provide a way to select different modes of operation (and or support only specific modes), I fear that the current generalized API surface will result in unexpectedly low bus throughput on some devices, in ways that application implementers may not expect.

At the very least, I think that the intent of the current trait's transmit method should be clarified; right now it seems to suggest that the driver should be operating in ID priority mode while respecting FIFO ordering for messages with the same ID, however the requirement to be operating in ID priority mode is not explicitly stated.