Fix SeeedStudio recv crash on empty serial read

[jafrivacation]

Summary of Changes

Fix Seeed Studio CAN interface handling of empty reads.

Related Issues / Pull Requests

• Closes SeeedBus: Crash occsionally. #2024
• Related to #

Type of Change

• Bug fix
• New feature
• Documentation update
• Refactoring
• Other (please describe):

Checklist

• I have followed the contribution guide.
• I have added or updated tests as appropriate.
• I have added or updated documentation as appropriate.
• I have added a news fragment for towncrier.
• All checks and tests pass (tox).

Additional Notes

This fixes the crash described in #2024 by guarding against empty reads before trying to parse the returned data. I reproduced the failure path, confirmed that an empty read could be returned, and updated the receive logic so empty data is skipped instead of being treated as a valid frame.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[LucienDufour]

This is very nice as it completely omits crashes (exceptions). I personally would prefer to have this merged, as it clearly is an improvement. Tough, I haven't tested it yet.

However with this solution, we still have occasionally "packet loss" (~1% for my use-case).
Reason:
We have the following data flow, coming from the Seeedstudio dongle to the user on the computer:
CAN packet --> UART stream --> USB packet --> parser
The current implementation of python-can is a parser that expects the USB packet to already be correctly framed, however the UART stream is often (~1% on my computer) "cut into two pieces", as the USB packets are not appearing 1:1 synchronized with the CAN packets.
We additionally need something to recover the correct frame:
CAN packet --> UART stream --> USB packet --> framing --> parser
It would be nice, if we could fully split the "framing" from the "parsing", which is not that easy.
In our python-can impementation we currently have this automatically entangled, since the serial.read() is already consuming the buffer, even though we do not have it framed yet. We rely on luck!

A possible solution for correctly detecting the frame and parsing the data would be:

1. Create a "non-consuming" parse-function (use a buffer to ensure the serial data is not consumed yet)
2. Use this parse-function to recover the frame
3. Take the frame, parse and consume it from the buffer

Demo implementation of exactly this solution:
https://github.com/Luz/seeedcan/blob/master/src/lib.rs

1. Recovery of the UART stream: self.sbuf.extend_from_slice(&buf[..new]);
2. Use this parse-function (See line 357: fn parse(&mut self) -> Result<(u16, Vec<u8>), SeeedError> { )
3. Consume the frame (See line 411: self.sbuf.drain(0..5 + datalen); )

Additionally noteworthy are the following cases:
A) Opening the serial interface and then attaching the dongle --> Does not work as the dongle needs to be plugged in to open the serial interface. --> Generally no problem
B) Attaching the dongle to a "silent" CAN network, then opening the serial interface, then start the data stream --> This works without initial snchronisation, but it is limiting the use-cases. --> Generally no problem
C) Attaching the dongle to a CAN network that has a running data stream and afterwards opening the serial interface --> There is a high likelyhood that the first data is only a "cut-in-half" CAN packet. --> Need to get rid of some bytes at the beginning to ensure synchronisation.

So to cover the case "C)", it is very important to initially "synchronize" to the CAN data. (Or did I call this framing before?)
The implementation needs some logic like:
"If the data does not parse, but is long enough, the first byte shall be removed"
(Not sure if this is correctly implemented in the demo, don't copy it from there.)

The case "B)" is useful for development of the parser, it is easier to implement the parser, when the synchronisation does not need to be final yet.