Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Improved CAN infrastructure #294

Closed
rleh opened this issue Sep 13, 2019 · 7 comments
Closed

RFC: Improved CAN infrastructure #294

rleh opened this issue Sep 13, 2019 · 7 comments

Comments

@rleh
Copy link
Member

rleh commented Sep 13, 2019

For a project I need a driver for the FDCAN IP present the new STM32H7- and STM32G4-series devices.
To integrate UAVCAN into modm (and for other use cases) prioritized transmit queues are required, see #216 (comment).

Some thoughts:

  • Remove the buffer option from the (only) stm32 can driver. This will simplify the existing can driver and upcoming can drivers.
  • Add a (semi-)hardware-independent wrapper which buffers can messages (tx and rx). Buffer queues will be prioritized (optional?).

This will likely introduce some incompatible changes.

Comments? @salkinium @chris-durand @strongly-typed @asmfreak @dergraaf

@salkinium
Copy link
Member

I'm on-board with this, as long as the XPCC protocol behavior is not changed.

I think externalizing the buffering for packet-oriented peripherals (ie. UART, ADC, DAC, CAN, Ethernet, etc) would be a great benefit. This would require perhaps a generic interface with RX/TX function hooks executed in an IRQ context instead of the polling based approach that we have now. The Buffer interface could provide a generic IRQ-safe implementation of a polling interface instead.

@salkinium
Copy link
Member

And to add further: The CAN peripheral isn't used much inside modm, except for XPCC, yet is implemented as a relatively complicated peripheral, so it's a very good candidate to experiment with new interfaces.

@rleh
Copy link
Member Author

rleh commented Oct 6, 2019

The new STM32 Can IP supports FD-CAN and long frame mode (LFM) with up to 64 Bytes of payload.
Increasing the modm::can::Message size ans thus the buffer sizes seems like a stupid idea to me.
How should we handle large payloads?

Option A

Continue to use the current modm::can::Message and expand the data array from 8 bytes to 64 bytes (or a size configurable via lbuild).

Option B

Change the modm::can::Message structure: Store the message payload (up to 64 Bytes) externally, e.g. on the heap (or build a dedicated heap-like data structure for Can message payloads).
Hybrid solution: Payloads up to 8 Bytes get stored inside the modm::can::Message object, larger payloads on heap.

Any opinions?

@salkinium
Copy link
Member

Does UAVCAN even make use of LFMs?

The overhead of the heap is at least 8B, due to alignment and heap info requirements (newlib might even be 16B) so the overall memory saving isn't going to be so large.

I'd choose option A for now, also because the UART buffers also copy the data and that makes it consistent (altough for const data, you could pass a std::string_view or something like that in theory).

@rleh
Copy link
Member Author

rleh commented Oct 8, 2019

As far as I know, UAVCAN doesn't use LFM.
Nevertheless I would like to support LFM for other applications.

@salkinium
Copy link
Member

Maybe having two types like ::can::Message and ::can::LongMessage, where the first can be converted into LongMessage (efficiently, without copying somehow?), because then the implementation of the interface can decide to provide one or the other.

@asmfreak
Copy link
Contributor

asmfreak commented Oct 9, 2019

Maybe we can use something like a flex array?
See: https://youtu.be/IAdLwUXRUvg around 29:44
But we need to somehow initialize it to correct size without allocation, which is not mentioned in the video.

@chris-durand chris-durand mentioned this issue Apr 4, 2021
5 tasks
@modm-io modm-io locked and limited conversation to collaborators Sep 29, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Development

No branches or pull requests

3 participants