Extend CanMsg to allow to distinguish between standard and extended ids … #194
Conversation
…Ds following a <linux/can.h> inspired approach.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #194 +/- ##
==========================================
+ Coverage 95.77% 95.93% +0.16%
==========================================
Files 13 14 +1
Lines 970 1009 +39
==========================================
+ Hits 929 968 +39
Misses 41 41
☔ View full report in Codecov by Sentry. |
…n order to eliminate the 3 MSBits encoded status flags.
…ardId", "isExtendedId".
… files, inline functions to prevent linker errors, and define static constants so that the compiler can place them in memory. All of those changes broaden adoption.
…ds max data lenght.
|
Hi @facchinm ☕ 👋 This PR is required by
which support both standard (11-Bit) and extended (29-Bit) CAN IDs by the respective platform libraries. I'm not a 💯 on the proposed design, especially |
| @@ -82,11 +92,45 @@ class CanMsg : public Printable | |||
| return n; | |||
| } | |||
|
|
|||
|
|
|||
| uint32_t getStandardId() const { | |||
There was a problem hiding this comment.
Would it make sense to make the ID an ADT? That way you could have something like:
class CanId : public Printable
{
virtual uint32_t toInt() const = 0;
};
class ExtendedCanId : public CanId
...
class StandardCanId: public CanId
...
This would allow libraries to be written that extend this to things like:
class DroneCANId : public ExtendedCanId
...
class CyphalId : public ExtendedCandId
...
class ARINC825 : public ExtendedCanId
...
// etc.
| @@ -31,14 +33,19 @@ namespace arduino | |||
| class CanMsg : public Printable | |||
| { | |||
| public: | |||
| static size_t constexpr MAX_DATA_LENGTH = 8; | |||
| static uint8_t constexpr MAX_DATA_LENGTH = 8; | |||
There was a problem hiding this comment.
Of course, the thing that is most irksome when you finally add CAN-FD support is the quantization of the DLC. If you are trying to get ahead of adoption and avoid future churn you might want to start moving away from an integer data_length type to an enumerated one now.
There was a problem hiding this comment.
I had been thinking of making a CanFdMsg whose MAX_DATA_LENGTH would be 64 then. Overall I'm a fan of const/constexpr over enum because they provide a strong type guarantee (vs. enum that easily and without warning converts to int, etc.).
Note: enum classes in public Arduino APIs are discouraged (they'd provide strong type support).
I'm ok for leaving it public to keep the APIs space simpler and allow some optimizations for libraries adopting this. |
… following a <linux/can.h> inspired approach.