arduino-CAN icon indicating copy to clipboard operation
arduino-CAN copied to clipboard

CAN bus filter does not work as expected

Open heliophagus opened this issue 4 years ago • 11 comments

Greetings, all. This is a nice library but I am tearing my hair out because I can't get the CAN filter to work. If I don't set the filter, all incoming packets trigger the onReceive() callback, as they should. However, if I set the filter as Sandeep documents, I do not receive ANY packages at all. Here is a code snippet that shows a packet I'd like to receive, the filter mask to select the wanted bits, and the filter ID that should trigger the onReceive event after masking. According to the documentation (see snippet in code below), everything appears to be correct and the filter should allow the packet through ((packetID & filterMask) == filterID)). This evaluates as true. BUT I receive nothing at all!

Any help or suggestions would be GREATLY appreciated!

`void setup() { Serial.begin(115200); // The filter documentation says, regarding CAN.filterExtended(id, mask); // Only packets that meet the following criteria are acknowledged and received, other packets are ignored: // if ((packetId & mask) == id) { // // acknowledged and received // } else { // // ignored // }

// here is an incoming package ID that needs to trigger // the onReceive event. uint32_t packetID = 0b10001001110000000010010000001; //// I only want to trigger on the bits allowed by this mask: uint32_t filterMask = 0b00011111111111111111000000000; // after masking, compare to this ID: uint32_t filterID = 0b00001001110000000010000000000; // According to Sandeep, this is the operation implemented // by the CAN.filterExtended(filterID, filterMask) call: Serial.println((packetID & filterMask) == filterID); // ... which results in 1, as expected. The packet should be accepted.

// HOWEVER, if I use this call in setup after initializing CAN: // CAN.filterExtended(filterID, filterMask); // ABSOLUTELY NO packages are received! }

void loop() { }`

heliophagus avatar Jul 03 '20 20:07 heliophagus

@heliophagus I found this document on the Web on filtering:

http://www.cse.dmu.ac.uk/~eg/tele/CanbusIDandMask.html

Also the ID & Mask are in HEX like 0x1FFFFFFF

(Extended) ID = Your ID (also Hex) Mask = 0x1FFFFFFF

(Normal) ID = 0x100 Mask = 0x7ff


I Do software filtering because I need a lot of Differend ID ranges. The ESP should be more than enough to do the job! You could also run this on the Other Core of the ESP f if you dont use WIFI or BT.

I hope this helps.

Petros144 avatar Jul 05 '20 00:07 Petros144

Thanks, @Petros144! I thought that binary format numbers were automagically translated by the compiler, but I may be incorrect. I will try again, this time expressing the numbers in Hex. I expressed them in binary format to clarify exactly what I was trying to do on the bit level. The article was also helpful. I can't help wondering whether there is a mistake when assigning the filter registers in the library, yet I doubt that. It would be great if @sandeepmistry also responded to my question - if I am making a stupid mistake, please let me know what it is! :)

heliophagus avatar Jul 06 '20 19:07 heliophagus

Do you use MCP2515 or ESP32?

I'm aware of at least three bugs in the implementation of filterExtended() for MCP2515, and I've documented them as TODOs in my fork of this library: https://github.com/timurrrr/arduino-CAN/blob/26d094ac4240875a80693731aba15a8dc786a119/src/MCP2515.cpp#L452 https://github.com/timurrrr/arduino-CAN/blob/26d094ac4240875a80693731aba15a8dc786a119/src/MCP2515.cpp#L459 https://github.com/timurrrr/arduino-CAN/blob/26d094ac4240875a80693731aba15a8dc786a119/src/MCP2515.cpp#L481 as well as incorrect macro definitions for macros REG_RXFn****(n) for n >= 3: https://github.com/sandeepmistry/arduino-CAN/pull/47/commits/b366242ddba22e4fa9a87b764b12488b156ab211

I don't think either of these four issues would cause behavior like that though.

I suggest looking at the source code of the library and the data sheet of the MCP to see if there are any more bugs. If you find the issue, I would be curious what's the resolution, and will be happy to fix the bug in my fork.

timurrrr avatar Aug 16 '20 08:08 timurrrr

Actually, can you try replacing the writeRegister(REG_RXBnCTRL(n), FLAG_RXM1); lines with a single writeRegister(REG_RXBnCTRL(n), 0); line and see if it helps?

timurrrr avatar Aug 16 '20 08:08 timurrrr

Skimming through the datasheet, it looks like we also shouldn't be setting FLAG_EXIDE here: https://github.com/sandeepmistry/arduino-CAN/blob/bcdd7ac0de63cab438b6bf2c7a3918d67de873ef/src/MCP2515.cpp#L333

I wonder how many more bugs are there in a single filterExtended() method.

timurrrr avatar Aug 16 '20 18:08 timurrrr

Also in these two places the 0x03s next to >> 18s should probably be replaced with 0x07: https://github.com/sandeepmistry/arduino-CAN/blob/bcdd7ac0de63cab438b6bf2c7a3918d67de873ef/src/MCP2515.cpp#L333 https://github.com/sandeepmistry/arduino-CAN/blob/bcdd7ac0de63cab438b6bf2c7a3918d67de873ef/src/MCP2515.cpp#L340

timurrrr avatar Aug 17 '20 03:08 timurrrr

@timurrrr & @heliophagus: Did you have any luck fixing these issues? I just moved my code form using standard frame filtering to extended. The standard filter worked fine, however the extended filter is doing exactly what @heliophagus first stated: only exact ID matches are being received by the onReceive interrupt no matter what the filter is set to. I've implemented a soft solution to do my own filter within the onReceive, however would prefer to stop the interrupt from triggering when it shouldn't. TIA

bcorwen avatar Oct 27 '20 16:10 bcorwen

Check out the pull requests on this project, or go straight to my fork on GitHub for even more fixes. I don't think I've updated/fixed "filter extended", but if you look at what I did for the standard filtering and what's written in the data sheet, you should be able to do something similar for extended PIDs.

timurrrr avatar Oct 27 '20 16:10 timurrrr

I have switched to your project for those bug fixes! The problem is still there but I'll see what you did for the standard! Thanks for the reply :)

bcorwen avatar Oct 27 '20 18:10 bcorwen

To timurrr: I am using your version of the CAN library. While compiling your library, I get a warning that I think is problematic:

d:\Arduino\libraries\CAN-0.3.2\src\MCP2515.cpp:426:60: warning: narrowing conversion of 'filter0' from 'uint16_t {aka unsigned int}' to 'uint8_t {aka unsigned char}' inside { } [-Wnarrowing] {filter0, filter1, filter2, filter3, filter4, filter5};

425  uint8_t filter_array[6] =
426    {filter0, filter1, filter2, filter3, filter4, filter5};
427  for (int n = 0; n < 6; n++) { ... } // Working with 8-bit-values?!`

8-bit values are passed to the array; these are used in the following for loop. But filters are 16-bit values.

RudolfAtHome avatar Aug 04 '21 19:08 RudolfAtHome

Thanks Rudolf for pointing out, this is super useful!

I just found out that my fork had the issue tracking disabled, so I've enabled it. Let's continue on https://github.com/timurrrr/arduino-CAN/issues/1

timurrrr avatar Aug 05 '21 05:08 timurrrr