107-systems / 107-Arduino-MCP2515

Arduino library for controlling the MCP2515 in order to receive/transmit CAN frames.
https://107-systems.org
MIT License
90 stars 14 forks source link

Fix: Determine if CAN TX is ongoing by reading from the TXBnCTRL register. #55

Closed aentinger closed 2 years ago

aentinger commented 2 years ago

Possible alternate solution for #54, without busy waiting.

@pepeRossRobotics can you please test? :pray:

github-actions[bot] commented 2 years ago

Memory usage change @ f66a9ecd56b0748d708e1c4265736c4882caf492

Board flash % RAM for global variables %
arduino:mbed_edge:edge_control 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_nano:nano33ble 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_nano:nanorp2040connect 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_portenta:envie_m4 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_portenta:envie_m7 :small_red_triangle: 0 - +64 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkr1000 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrfox1200 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrgsm1400 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrnb1500 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrvidor4000 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrwan1300 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrwan1310 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrwifi1010 :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:samd:mkrzero :small_red_triangle: 0 - +36 0.0 - +0.01 0 - 0 0.0 - 0.0
esp32:esp32:esp32 :small_red_triangle: 0 - +40 0.0 - 0.0 0 - 0 0.0 - 0.0
rp2040:rp2040:rpipico :small_red_triangle: 0 - +32 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table Board|examples/MCP2515-CAN-Sniffer
flash|%|examples/MCP2515-CAN-Sniffer
RAM for global variables|%|examples/MCP2515-Loopback
flash|%|examples/MCP2515-Loopback
RAM for global variables|% -|-|-|-|-|-|-|-|- arduino:mbed_edge:edge_control|0|0.0|0|0.0|0|0.0|0|0.0 arduino:mbed_nano:nano33ble|0|0.0|0|0.0|0|0.0|0|0.0 arduino:mbed_nano:nanorp2040connect|0|0.0|0|0.0|0|0.0|0|0.0 arduino:mbed_portenta:envie_m4|0|0.0|0|0.0|0|0.0|0|0.0 arduino:mbed_portenta:envie_m7|0|0.0|0|0.0|64|0.01|0|0.0 arduino:samd:mkr1000|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrfox1200|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrgsm1400|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrnb1500|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrvidor4000|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrwan1300|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrwan1310|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrwifi1010|0|0.0|0|0.0|36|0.01|0|0.0 arduino:samd:mkrzero|0|0.0|0|0.0|36|0.01|0|0.0 esp32:esp32:esp32|0|0.0|0|0.0|40|0.0|0|0.0 rp2040:rp2040:rpipico|0|0.0|0|0.0|32|0.0|0|0.0
Click for full report CSV ``` Board,examples/MCP2515-CAN-Sniffer
flash,%,examples/MCP2515-CAN-Sniffer
RAM for global variables,%,examples/MCP2515-Loopback
flash,%,examples/MCP2515-Loopback
RAM for global variables,% arduino:mbed_edge:edge_control,0,0.0,0,0.0,0,0.0,0,0.0 arduino:mbed_nano:nano33ble,0,0.0,0,0.0,0,0.0,0,0.0 arduino:mbed_nano:nanorp2040connect,0,0.0,0,0.0,0,0.0,0,0.0 arduino:mbed_portenta:envie_m4,0,0.0,0,0.0,0,0.0,0,0.0 arduino:mbed_portenta:envie_m7,0,0.0,0,0.0,64,0.01,0,0.0 arduino:samd:mkr1000,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrfox1200,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrgsm1400,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrnb1500,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrvidor4000,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrwan1300,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrwan1310,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrwifi1010,0,0.0,0,0.0,36,0.01,0,0.0 arduino:samd:mkrzero,0,0.0,0,0.0,36,0.01,0,0.0 esp32:esp32:esp32,0,0.0,0,0.0,40,0.0,0,0.0 rp2040:rp2040:rpipico,0,0.0,0,0.0,32,0.0,0,0.0 ```
pepeRossRobotics commented 2 years ago

This is with my home brew wait: image

This is with this commit's code: image

As you can see the amount of errors is around double (the up-time of both nodes was around 7 minutes), but the the modification of the performance of the code is orders of magnitude better than the master branch in that regard.

I think this fix performs very similar, but it is more elegant and efficient than my version, so happy to use it instead of mine.

What do you think?

aentinger commented 2 years ago

Thank you very much for the test. I'd prefer this variant to your PR, since it does not contain any busy-wait. If this is fine with you I'll merge this and close the other. As for the remaining errors ... possibly the CAN timings are too tight leading to frame errors due to out-of-margin bit-timings? Possibly also the SPI clock is too high for the MCP2515 resulting in bit errors during transmission? Those timing issues are hard to pin down ...

pepeRossRobotics commented 2 years ago

Agree with you. Thanks for taking the time to check this and come up with a better solution.