Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix two socketcand issues: start up switch to raw mode, crash due to partial frame #495

Merged
merged 3 commits into from
Sep 19, 2022

Conversation

AndyHuska
Copy link
Contributor

The startup failure to switch to raw mode was due to data frames coming in the same message as the "< ok >" ack message for the switch to raw, so added parsing to check for these cases

The partial frame failure was an index out of bounds crash that came from assuming only complete frames would be received and the parser would always return a certian number of substrings, so added some sanity checking

Andy Huska added 3 commits September 16, 2022 15:38
…mplete frame.

Not sure if my method drops the partial or just sends the buffer back to continue filling.
…onse being received in the same message as the first data frame

Sometimes when opening socketcand 1 or more busses won't start right and there's a ton of debug data saying "received datagramm: ...."
It seems to happen because the switch to RAW mode is in progress and we receive the OK with a frame in the same datagramm and we don't parse the OK out of the string
but rather expect the whole string to just be OK.  Offending code is in socketcand.cpp SocketCANd::procRXData line 386

Fixed by looking to see if the < ok > message was at index 0 of the incoming message...could also look for it elsewhere (as in index != -1), but not sure if necessary
…er with other messages...

For now I made it its own else statement so it can be detected
@collin80 collin80 merged commit cc3c043 into collin80:master Sep 19, 2022
@AndyHuska AndyHuska deleted the fix_socketcand branch September 29, 2022 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants