CAN API is insufficient and underspecified

[ducky64]

Description

• Type: Question on API-changing (or clarifying) Enhancement
• Priority: Minor, but probably only because CAN isn't used that much

CAN API issues

Since this is an issue with an API that has been implemented on several devices already, it's unclear how this could be best resolved. Some would require new features (like querying available messageboxes or attach errors), while others may update definitions in a way that would make some implementations incorrect (though perhaps it might be better than having ambiguous definitions?).

Anyways, the issues are:

• Error handling is lacking.
  • The only way to determine error conditions (like bus-off or error warning) is through interrupts, and these aren't always implemented. There is no polling API, other than maybe inferring from error counters.
  • reset() is poorly defined. On some implementations (like LPC15xx), this won't clear a bus-off condition. Also, what should get reset? filters? CAN data? error counters? The purpose and effects of this should be clarified in API documentation.
  • Side effects are also poorly defined. For example, the LPC15xx implementation will reset a bus-off condition on any read or write operation.
• More generally, when should the CAN module turn on, especially since some configuration must occur as runtime statements (like frequency and filters)?
  • It seems like both the LPC15xx and STM32F3 implementation turn on the CAN module during init at the default 100kbits/s rate, which may not be desirable for buses running at different rates, especially since the controller can detect and respond to errors in the background. There may be a compromise between ease of use (automatic initialization, declare the object and immediately transmit) and correctness (never initialized until at least frequency is set) though.
  • An alternative may be to add optional parameters for initialization frequency and mode (active, silent, loopback, etc) in the CAN constructor.
• Not all IRQ types are supported by all drivers, and more importantly there is no failure in attempting to attach an interrupt that is not supported.
  • Neither CAN::attach nor the internal can_irq_set functions can return statuses. It may make sense for can_irq_set to return a failure status, which CAN::attach can either give to the user or call mbed_die.
  • Specifically for the LPC15xx CAN peripheral, there is no dedicated interrupt status for error types (bus-off, error passive, error warning), meaning that once an error flag is set, all subsequent interrupts may (repeatedly) set off error interrupts while those are active. This may be a consideration for a more formal attach() description.
• Notion of CAN message boxes ("handles" in mbed terminology) aren't well defined or fully usable
  • There should be some method to query for the number of available messageboxes.
  • filter() has a handle parameter that isn't well-documented. It seems that a default handle of zero means to select an arbitrary handle (which may be one a filter was previously applied to - in which case it is overwritten) and apply the filter to it, then return the handle. The overwriting doesn't seem like safe behavior, and at a first glance I would imagine a parameter-less filter would apply a filter to all messageboxes.
  • Lower priority would be methods to disable messageboxes and to read from a range of messageboxes (both to be used with filter).

[bridadan]

@ducky64 Thanks for raising this issue and submitting your PRs! I'm not all that familiar with the CAN API, but hopefully I can bring some people in who are!

cc @0xc0170 @sg-

GitHib issue review: Closed due to inactivity. Please re-file if critical issues found.