Odrive Canbus Arduino

Hi. I’ve been working on implementing the ODrive protocol on Arduino. I looked around, but didn’t find anything that fit. Mine is far from complete, but I’d like to share it now to get any feedback from future users, or even ODrive devs themselves.

What do you think of the interface? Would you use this?

Or else, maybe somebody has done this already and can spare me the effort?

Thanks in advance.

Awesome thanks! I know that @Diablo on the Discord channel also made one, so you guys might want to collab.

A couple things:

  1. The enum definitions are available in the ODriveArduino code, and they get automatically generated for each build. So I would ask users to get that library instead.
  2. Consider using the CAN-Helpers library I wrote for encode/decode (which is also used in ODrive firmware): GitHub - Wetmelon/CAN-Helpers: Header-only library for handling CAN signals on embedded devices</title

My version: GitHub - Malaphor/ODriveTeensyCAN

Its a bit outdated but should still work. Probably needs to be updated to pull the list of enums automatically. It’s easier for me to chat in discord if you want to join

Nice use of union in CAN-Helpers. And support for both endians, and extended messages. I might give this a try.

Is it normal that I only see the axis state enum in ODriveArduino? Indeed I’d rather pull from you than maintain my own definitions. (I’d like the others - input modes, control modes, even named error codes would be cool.)

Thanks, this helps. You did what I tried to avoid, which was to wrap the Canbus read/writes. Yours would be easier to use, and mine is more ‘bring your own Canbus.’ I’ll probably borrow your SetPosition and SetVelocity encodings though! Cheers

Heh the union aliasing is technically UB, so idk if it’s “nice” but it is fast xD

So I tried both can_helpers.hpp and can_helpers.hpp but neither initially compiled under Arduino.

Here it the build output with Wetmelon’s repo’s version:

include/CAN-Helpers\can_helpers.hpp: In instantiation of 'constexpr T can_getSignal(const can_Message_t&, size_t, size_t, bool) [with T = long unsigned int; size_t = unsigned int]':
src\ODriveFlexCAN.h:493:127:   required from here
include/CAN-Helpers\can_helpers.hpp:51:5: error: uninitialized variable '<anonymous>' in 'constexpr' function
     };
     ^
include/CAN-Helpers\can_helpers.hpp:47:11: note: 'union can_getSignal(const can_Message_t&, size_t, size_t, bool) [with T = long unsigned int; size_t = unsigned int]::<anonymous>' has no user-provided default 
constructor
     union {
           ^
include/CAN-Helpers\can_helpers.hpp:48:18: note: and the implicitly-defined constructor does not initialize 'uint64_t can_getSignal(const can_Message_t&, size_t, size_t, bool) [with T = long unsigned int; size_t = unsigned int]::<anonymous union>::tempVal'
         uint64_t tempVal;
                  ^
*** [.pio\build\teensy41\src\main.cpp.o] Error 1

According to cppreference,

It’s undefined behavior to read from the member of the union that wasn’t most recently written. Many compilers implement, as a non-standard language extension, the ability to read inactive members of a union.

So maybe the Arduino compiler hasn’t implemented this behaviour? Dunno, defeated for now.

Here’s the build output from the ODrive firmware version, which doesn’t use union:

include/ODrive\Firmware\communication\can\can_helpers.hpp: In instantiation of 'constexpr T can_getSignal(can_Message_t, uint8_t, uint8_t, bool) [with T = long unsigned int; uint8_t = unsigned char]':
src\ODriveFlexCAN.h:493:127:   required from here
include/ODrive\Firmware\communication\can\can_helpers.hpp:44:7: error: uninitialized variable 'retVal' in 'constexpr' function
     T retVal;
       ^
*** [.pio\build\teensy41\src\main.cpp.o] Error 1

Ok, seems easy enough to fix. This

    T retVal;
    std::memcpy(&retVal, &tempVal, sizeof(T));
    return retVal;

…to this…

    T* retVal = (T*)std::malloc(sizeof(T));
    std::memcpy(retVal, &tempVal, sizeof(T));
    return *retVal;

And now can_getSignal works in my class. Next up to test is can_setSignal.

Tbf, there might be ways to configure the compiler that would allow these patterns. I have not looked in that direction.

I’ve never compiled the ODrive fw myself, but I’d bet money there’s at least a compiler warning associated with that bit of code. :wink:

Yes, type punning through unions is undefined behavior, but it results in better optimization. I haven’t checked it on AVR-gcc yet but it’s fine on arm.

Oh I saw that warning but it’s actually I think a bug in the older compilers. What you need to do is create an instance of the union instead.

Oh man you can’t malloc and return like that! You’re leaking memory and you’re going to run out and start crashing the board almost immediately.

2 Likes

After experimenting, I see that they’re still on gcc 5.4.1… I will make sure the code compiles nicely under this old compiler.

1 Like

Ok, the updated can_helpers.hpp should compile without warning on 5.4.1 with Teensy.

Thanks! Works great.

Peter

2 Likes