Can not receive CAN packets with DLC=0

[balazsracz]

The API for parsePacket is defined as returning 0 when there is no packet in the input buffer, and returning DLC when there is a packet to read in the input buffer. Therefore packets with DLC=0 will appear the same as when no packets have arrived.

As a consequence of this confusion, the callback onReceive will not be executed when a packet with DLC=0 arrives on the CAN-bus.

Packets with DLC=0 (no data payload bytes) are perfectly valid packets on a CAN-bus. They should be able to be sent to and received from the bus using this library.

[RufusVS]

I verified this issue and the example in Adafruit_MCP2515 has the same flaw. It can be worked around by checking the packetId in the other library because it is set to -1 when there is no message. The driver here doesn't do that, so it will have to be modified.

[fastbike]

I am experiencing this same problem. I have a node from another system that is sending a message with RTR set and DLC = 0.
The message is not being intercepted by this library, which makes it a bit useless for my purposes (my system starts by scanning all nodes - sending an RTR message - and the receiving node is needs to answer to identify its type.)

My work around is to comment out the code at the start of _parsePacket and to change the last return statement to this

  //https://github.com/adafruit/Adafruit_CAN/issues/4
  if (!hw->RXF0S.bit.F0FL) {
    return -1;
  }
  return _rxDlc;

[caternuson]

The general issue with the existing logic is pretty simple and obvious. However, the question about what the behavior should be seems to still be a point of discussion. See discussion here:
sandeepmistry/arduino-CAN#7
and here:
sandeepmistry/arduino-CAN#22

[TheKitty]

@caternuson so this issue is in limbo? We have two comments that the receive code on the guide doesn't work.

[balazsracz]

So your question is what would be a reasonable API? Let's look at SocketCAN from the Linux kernel: ``` struct can_frame f; ssize_t len = read(can_fd, &f, sizeof(struct can_frame)); if (len == sizeof(struct can_frame)) { // I got something :) } else if (len == 0) { // I didn't get anything :( } else if (len < 0) { // I got an error :/ } ``` I think this is a reasonable API. `struct can_frame` then has all the necessary values like flags, id (uint32_t to cover standard or extended), DLC (can be zero!), and an `uint8_t data[8]` that is filled with 0 to 8 bytes.

[caternuson]

@TheKitty They should post in the forums with details. This issue is for a specific scenario. More than just "receive code doesn't work", which could be anything (soldering, etc.).

@balazsracz Yes, question is what's a good suggested API update to deal with this scenario, ideally one that is non-breaking change (i.e., backwards compatible). It's tricky, since the return is currently an int, which leads to 0 being ambiguous.

[balazsracz]

TBH I described pretty well what the issue is in the opening post here. The minimum change that is necessary is for the onReceive callback to be invoked for a zero-DLC incoming CAN frame.

There are perfectly valid protocols that use zero-DLC frames for perfectly valid reasons. I don't think that can be argued.

So until there is a change in the implementation of this driver, fixing onReceive or adding a new API that actually is usable, I will keep recommending to people not to purchase the adafruit CAN board, because the software supporting it is incompatible with the protocol I care about.

(Btw This is not the only flaw in the API. There are some more in the email linked above. For example this entire parsePacket API is fundamentally flawed, because an interrupt will asynchronously overwrite all local variables even if the application code has not read them out yet. So this API has a conceptual race condition.)

[caternuson]

There is a proposed fix in the following pull request:
#6

Please test that with your setups to see if it solves the issue.