Refactor set_params helper to reduce tests runtime#1127
Conversation
|
@1Blademaster the broken tests seem to be because it is returning the old values that you changed last week in #1083 ? Like this is the diff of why the test is failing
And this is the diff of the changes you made last week
|
|
@Turnlings that's interesting. I'm honestly not sure because I had issues with it as well, but I did eventually get it working and pushed to docker hub so the action runner should pick up the change. Does it pass locally out of curiosity? |
|
@1Blademaster Nope. The exact same errors locally too, where I'm using the :latest image. |
|
I'm kinda lost tbh, it's very annoying error. |
|
@1Blademaster I get the same errors just running the tests on the main branch locally too. |
The x, y and z in the test being 0 is not correct; something must be going wrong somewhere I think. |
There was a problem hiding this comment.
Pull request overview
Refactors the radio/tests/helpers.py:set_params() test helper to reduce overall test runtime by batching MAVLink parameter writes and consolidating retry/ack handling during session-scoped test setup.
Changes:
- Replaces per-parameter retry loops with a batched “send all pending params” approach.
- Introduces pending/retry tracking maps and a buffer-drain helper to reduce stale ACK interference.
- Updates timing behavior (new timeout loop + end-of-function settle sleep).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Yeah turned out that it was a me issue... SITL Simulator clearly doesn't handle new docker images the way I thought it would, so I was on the wrong version and hunting for the wrong bug. Fix for this bug was it just needed some time to settle for the state to line up (Settling time is much less than the time it took before so still saving a lot of time) SITL thing raises the issue of what behaviour we expect for that when you update the docker image. Probably a warning label and button to update? (Don't want it downloading 4GB out of the blue, also should it delete the old one?) Anyway lots of questions for a new ticket... I've requested a review, but in the meantine I'm going to run the tests a lot to make sure that it is definitely stable. |
1Blademaster
left a comment
There was a problem hiding this comment.
Seems fine to me from reading through 👍



Significantly reduces test runtime by refactoring function used in setup that used to take 120s.
Reduces a python tests run on Github from about 5m 20s to about 2m 40s. So pretty much exactly in half.