Skip to content
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

Add support for single Core ESP32 / multiple TWAI controllers #65

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

outlandnish
Copy link

@outlandnish outlandnish commented Mar 16, 2024

Changes

  • Runs xTaskCreate instead of xTaskCreatePinnedToCore if used on a device with CONFIG_FREERTOS_UNICORE enabled (e.g. on single core ESP32s)
  • Uses a secondary internal TWAI controller if SOC_TWAI_CONTROLLER_NUM = 2 (like on the ESP32 C6 with two CAN controllers) and IDF version >= 5.2 (which introduces the v2 TWAI API)
  • Adds a preprocessor define HAS_EXTERNAL_CAN_CONTROLLER that a user can set before including esp32_can.h to enable support for an MCP2515 / MCP2517FD controller. If SOC_TWAI_CONTROLLER_NUM = 2 and IDF version >= 5.2, the MCP device shows up as CAN2 (and CAN1 otherwise)

Example for external controller

#define HAS_EXTERNAL_CAN_CONTROLLER
#include esp3_can.h

@TechOverflow
Copy link

Any timeline on merging this @collin80?
Seems like a great update now that the C6 modules are available.

@collin80
Copy link
Owner

My only real trepidation about merging this is that it defaults to not loading the driver for external controllers. Since that was the old default this would break all existing programs that relied on the fact that an external controller was automatically loaded. I do realize that perhaps I should never have made that the default but that's what happened. So, I have to decide what I want to do about that. I should have done that back when this pull request was created but I've been busy. I will try to merge this in soon.

@outlandnish
Copy link
Author

@collin80 one option for not introducing a breaking change is that I can have it default to defining HAS_EXTERNAL_CAN_CONTROLLER when the IDF version < 5.2 and SOC_TWAI_CONTROLLER_NUM is not 2. That way it should work out of the box with the existing solution.

Any thoughts?

@Fyod
Copy link

Fyod commented Aug 28, 2024

Hi @outlandnish, espressif/arduino-esp32 updated the core IDF to 5.3 in 3.1.0 RC today.
When trying to compile your fork, I get these errors:

/Users/***/Documents/Arduino/libraries/esp32_can/src/esp32_can_builtin.cpp: In constructor 'ESP32CAN::ESP32CAN(gpio_num_t, gpio_num_t, uint8_t)':
/Users/***/Documents/Arduino/libraries/esp32_can/src/esp32_can_builtin.cpp:57:16: error: request for member 'controller_id' in '((ESP32CAN*)this)->ESP32CAN::bus_handle', which is of pointer type 'twai_handle_t' {aka 'twai_obj_t*'} (maybe you meant to use '->' ?)
   57 |     bus_handle.controller_id = busNumber;
      |                ^~~~~~~~~~~~~
/Users/***/Documents/Arduino/libraries/esp32_can/src/esp32_can_builtin.cpp: In member function 'virtual void ESP32CAN::enable()':
/Users/***/Documents/Arduino/libraries/esp32_can/src/esp32_can_builtin.cpp:373:33: error: 'g_config' was not declared in this scope; did you mean 'gpio_config'?
  373 |     if (twai_driver_install_v2(&g_config, &t_config, &f_config, &bus_handle) == ESP_OK) {
      |                                 ^~~~~~~~
      |                                 gpio_config
/Users/***/Documents/Arduino/libraries/esp32_can/src/esp32_can_builtin.cpp:373:44: error: 't_config' was not declared in this scope
  373 |     if (twai_driver_install_v2(&g_config, &t_config, &f_config, &bus_handle) == ESP_OK) {
      |                                            ^~~~~~~~
/Users/***/Documents/Arduino/libraries/esp32_can/src/esp32_can_builtin.cpp:373:55: error: 'f_config' was not declared in this scope
  373 |     if (twai_driver_install_v2(&g_config, &t_config, &f_config, &bus_handle) == ESP_OK) {
      |                                                       ^~~~~~~~

exit status 1

Compilation error: exit status 1

Could you recommend fixes to them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants