KevinOConnor / can2040

Software CAN bus implementation for rp2040 micro-controllers
GNU General Public License v3.0
636 stars 63 forks source link

Added support for a ctx parameter to be passed into the RX callback. #12

Closed linted closed 1 year ago

linted commented 1 year ago

Hello, This pull request is an attempt to allow for better memory management schemes when using this library.

The goal of the context data pointer is to allow the programmer to keep state or configuration data without needing to use globals. This solves some implementation problems which arise from using both PIOs since the current configuration makes it difficult to use one rx_cb on both concurrently.

Since this is a breaking API change, an alternate option would be to document how to allocate the struct can2040 at the start of a meta struct containing user data. This technique is usually considered to be advanced and error prone, so it is usually frowned upon.

KevinOConnor commented 1 year ago

Thanks. I'm not sure it is necessary to add this capability as there are two ready alternatives (as you have noted).

  1. As there are at most two PIO instances, one could use two separate rx_cb callback functions.
  2. One could embed the struct can2040 in a user-defined struct and then use the container_of() macro (as popularized by the Linux kernel code) to obtain the user struct from the existing rx_cb struct can2040 *cd parameter.

For what it is worth, I documented that "The cd parameter of the callback contains the same pointer passed to can2040_callback_config()" to make it clear that one could use container_of().

In general, I'd prefer to keep the interface simple than to add a 3rd mechanism.

Cheers, -Kevin

github-actions[bot] commented 1 year ago

Hello,

It looks like there hasn't been any recent updates on this github ticket. We prefer to only list tickets as "open" if they are actively being worked on. Feel free to provide an update on this ticket. Otherwise the ticket will be automatically closed in a few days.

Best regards,

~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.