-
Notifications
You must be signed in to change notification settings - Fork 13.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
Mixer parsing fixes #6226
Mixer parsing fixes #6226
Conversation
This introduces a correctly designed pre-check for the input parsers. This fixes the mixer unit test and should fix all issues occuring on real hardware. ;
…central implementation
…ral implementation
Yeah: ______ __ __ ___
| ___ \ \ \ / / / |
| |_/ / \ V / / /| |
| __/ / \ / /_| |
| | / /^\ \ \___ |
\_| \/ \/ |_/
px4 starting.
pxh> tests mixer
INFO [tests] RUNNING TEST: loadIOPass
INFO [tests] TEST PASSED: loadIOPass
INFO [tests] RUNNING TEST: loadQuadTest
INFO [tests] TEST PASSED: loadQuadTest
INFO [tests] RUNNING TEST: loadVTOL1Test
INFO [tests] TEST PASSED: loadVTOL1Test
INFO [tests] RUNNING TEST: loadVTOL2Test
INFO [tests] TEST PASSED: loadVTOL2Test
INFO [tests] RUNNING TEST: mixerTest
INFO [tests] TEST PASSED: mixerTest
INFO [modules__unit_test] ALL TESTS PASSED
INFO [modules__unit_test] Tests passed : 5
INFO [modules__unit_test] Tests failed : 0
INFO [modules__unit_test] Tested assertions : 810
pxh> @AndreasAntener Can you try with the hardware with this branch? |
Great! I verified this on my bench Pixhawk. I also changed the vtol2 mixer slightly to have the combination that failed for me before (I checked again which one it was exactly). Ran the mixer test on hardware as well. |
@@ -0,0 +1,28 @@ | |||
M: 1 |
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.
O-oh, loading fail when this is set to 5
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.
INFO [tests] RUNNING TEST: loadComplexTest
ERROR [tests] Mixer text length overflow
ERROR [tests] Mixer load failed with chunk size 16
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.
Stupid me. I'll fix. Can you test your airframe meanwhile?
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.
Home office today, later this week.
@AndreasAntener I got a little crazy and I'm now actually linking the test to the IO buffer sizes and I'm loading ALL mixers. That means you now can't contribute a mixer which will not load OK. And it found corner cases which I fixed with the next commits:
|
This is necessary to allow more and better unit testing.
We use the real defines now and test them against every mixer on the system. This means we should catch transfer errors now before even hitting master.
Here is the output of the fixed test:
|
@AndreasAntener Please review / test on HW and merge if it passes. |
Merged via #6245 |
Fixed a parsing corner case and validated in unit test that the fix holds up independent of the packetization interval.
To run in one command:
To run from the shell:
make posit_sitl_shell none pxh> tests mixer
Fixes #5118