[Ovmsdev] MCP2515 driver bug?

[Collin Kidder]

I totally do not disagree with you. The handling is not very good at
all - and I might have even written that part... Surely it could be
handled better. However, nobody has fixed it yet. Perhaps I ought to
create an issue on GitHub to remind myself that someone has to revisit
that code at some point.

One thing I would like to inject into this discussion, though, is that
counting on proper in-order reception and no dropped frames is not
generally advisable. CAN is best treated like UDP. Some packets might
arrive out of order, some not at all. Any multiframe transmission over
CAN really needs to include some sort of means to identify lost or out
of order frames. You simply cannot count on CAN traffic to reliably
get from point A to point B 100% of the time. This is especially true
once you get onto a real bus where other frames are coming in and the
bus is loaded up. It's one thing to try this on an empty bus and get
it to work but that work is not likely to transition over reliably to
"the real thing." It's best to not even entertain the notion that it
could be possible. Some form of higher level protocol that keeps track
of the order and sequence is necessary. Yes, I know technically no
dropped frames and in order reception are possible if the drivers on
both sides are properly coded. But, in practice it tends to be quite
difficult to ensure this 100% of the time. Once again, this is JUST
like UDP. It can be reliable... most of the time. But, it isn't
guaranteed to be so and it's best to take proper precautions against
the problem. This is why things like ISO-TP have sequence bytes. Also,
see here: https://datatracker.ietf.org/doc/html/draft-cafi-can-ip-00

On Sun, Jun 6, 2021 at 4:16 PM Michael Balzer <dexter at expeedo.de> wrote:
> That issue isn't addressed by that driver as well, in fact their
> handling is much worse than ours, even without rollover enabled. As you
> can see in their interrupt handler…
>
> https://github.com/collin80/esp32_can/blob/master/src/mcp2515.cpp#L892
>
> …they read the interrupt flags, then check & read both buffers based on
> the flags read, then clear all (!) interrupt flags.
>
> So if an interrupt occurs for buffer 0, and the next frame comes in for
> buffer 1 right after the driver read the interrupts, their driver will
> only read buffer 0 and then clear all interrupts, thus losing that for
> buffer 1.
>