-
Notifications
You must be signed in to change notification settings - Fork 25
Add support for ESP32 boards using the native TWAI (CAN) driver. #20
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
base: master
Are you sure you want to change the base?
Conversation
Changes: - Add ODriveESP32TWAI.hpp interface that calls twai_transmit/twai_receive directly - Add IS_ESP32_TWAI option to SineWaveCAN example - Add esp32 PlatformIO target
samuelsadok
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending desk test.
src/ODriveESP32TWAI.hpp
Outdated
| twai_message_t tx_msg = {}; | ||
| tx_msg.identifier = id; | ||
| tx_msg.data_length_code = length; | ||
| tx_msg.extd = (id > 0x7FF) ? 1 : 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing drivers use id & 0x80000000 as the extended-id flag, so we should do the same here.
…, make twai_transmit non-blocking
|
@samuelsadok Desk tested OK w/ ESP32-WROOM-32 devkit + SN65HVD230 breakout. Hit your comments. Should be good to merge if all looks good to you. Only possible change is moving the TWAI setup code (namely the large case/switch statement) to |
samuelsadok
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok looks good to me.
The setupCan() is a bit of visual noise, that is true, but that is also true for the larger section that does the hardware setup. The thing is that it's quite board dependent, so it can't be straight-up in a library header (you could even have multiple CAN interfaces, so you can't just make simple GPIO/baudrate defines before the library include).
What you could do, is just move the generic portion (the switch statement) over to a helper function in the header. Up to you if you want to do that in this PR.
Long term maybe we'll start putting each of these sections in its separate board header that users can copy and adapt for their board.
Changes:
This PR supersedes #11 .
Not tested yet - will desk test ASAP.