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

Faulty CAN wiring can crash board #27

Closed
dalathegreat opened this issue Jul 26, 2023 · 5 comments
Closed

Faulty CAN wiring can crash board #27

dalathegreat opened this issue Jul 26, 2023 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@dalathegreat
Copy link
Owner

dalathegreat commented Jul 26, 2023

The CAN code is a bit strange on this board. If there is not someone alive on the CAN bus to ack the messages that the board sends, then the entire board will crash. The problem is that after the watchdog resets the board, the crash will continue to happen (since the code will try to send more CAN messages right away). After restoring the wires the board starts working again.

In proper CAN terminology, I believe this is called a BUS error state.

Steps to reproduce:

  • Setup LilyGo board with a CAN device connected (I use Kvaser reader for testing)
  • Setup terminating resistors to get 120Ohm in both ends (LilyGo has one built in)
  • Start the program, for instance with "LEAF Battery" enabled.
  • Observe how the green status LED on the board is blinking slowly, indicating that the board is alive
  • Remove terminating resistor OR one CAN wire
  • Intended behaviour: Board stays operational with pulsing green LED
  • Actual behaviour: Board freezes up, the LED stays solid green on the board, indicating a crash

This isn't really a problem, since the Inverter that is connected should act accordingly and shut everything down in a CAN failure situation. But it would be very nice if the LilyGo could stay ON in this bus error state, since the board now can control contactors. In the event of CAN failure, after 60seconds the LilyGo enters faulted state, and in this state contactors are opened. So this bug is preventing this shutdown process from happening. Again, not a massive problem since the inverter will shut down the charge process.

Help wanted! :)

@dalathegreat dalathegreat added the help wanted Extra attention is needed label Jul 26, 2023
@rjsc
Copy link
Collaborator

rjsc commented Jul 27, 2023

The miwagner arduino driver and Thomas Barthe's arduino ESP-32 CAN driver are significantly different than the current ESP-IDF CAN driver. They are NOT direct ports.

The miwagner and Thomas Barth Arduino ESP32 CAN drivers have some big issues with queues, and it doesnt have any protection/safety with large multi-threaded/tasked programs. If you are trying to send CAN messages from several different areas of your multi-tasked code, it usually gets choked sooner or later....

And the biggest issue of all is that it doesnt have the IRAM_ATTR directive, which would allow it to try to access random parts of flash/code at ANY time....this includes during an OTA update (which disables flash-cache). If you leave the miwagner/Thomas Barth Arduino ESP32 CAN driver running during an OTA update, it was guaranteed to crash the whole ESP32.

The official ESP-IDF CAN driver doesnt have any of these issues, and is completely safe to use with any program, and have running at all times, including during an OTA update.

https://esp32.com/viewtopic.php?t=12607

Seems that the Arduino can stack indeed has problems and we need to use the official one from the esp32 idf.

@dalathegreat dalathegreat changed the title CAN controller can crash board Faulty CAN wiring can crash board Jul 28, 2023
@dalathegreat
Copy link
Owner Author

Some good info here: sandeepmistry/arduino-CAN#118

@dalathegreat
Copy link
Owner Author

Another repo with same problem: timurrrr/arduino-CAN#2

@rjsc
Copy link
Collaborator

rjsc commented Aug 7, 2023

There is a pending pull request to add a timeout to the write operation but the repo owner didn't respond for over a year.
You can try to merge it privately and test.
miwagner/ESP32-Arduino-CAN#45

@dalathegreat
Copy link
Owner Author

Pull request merged to main that prevents the crash 💪 ! Thanks @rjsc and Caboom! #48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants