-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
AP_RC_Protocol: protect SBUS against bad values #25169
Conversation
67d159c
to
58a2954
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This seems like an incomplete fix. We do still have places which will read ranges from RC channels/ranges which might misbehave.
Additionally, there's some odd things happening up in the higher-numbered RC input ranges:
(graph RC_CHANNELS.chan9_raw RC_CHANNELS.chan10_raw RC_CHANNELS.chan11_raw RC_CHANNELS.chan12_raw RC_CHANNELS.chan13_raw RC_CHANNELS.chan14_raw RC_CHANNELS.chan15_raw RC_CHANNELS.chan16_raw RC_CHANNELS.chan17_raw RC_CHANNELS.chan18_raw
)
Looks like 17 channel values are being supplied in total, and many of them are acting strangely at the end there.
58a2954
to
5b47429
Compare
@andyp1per @peterbarker needs re-approval as I have expanded this to add a test suite and change sbus output to match sbus input, also changed HAL_Linux to use the common SBUS decoder |
cover all values and check special handling of 875
this fixes test_sbus on clang
5b47429
to
3037d5b
Compare
I have logs here from FrSky XM+ receivers glitching that glitch down to 880 on channels. Any chance we can parameterize this and raise it? From a general perspective I'd almost be interested to tag that "no RC channel input should be outside this (min, max) range" with user facing parameters and reject everything if it was outside of it. |
I don't know how but I think this PR has somehow introduced a bug in the SBUS output. See issue #26815 I can't see a bug however. EDIT: No, it's not this PR. It's something after this PR. |
values at 875 on the first 4 channels are almost certainly errors. This has been seen in flight on one vehicle