Fix SITL system load metric permanently pinned at 100%, blocking arming#11720
Open
sensei-hacker wants to merge 2 commits into
Open
Fix SITL system load metric permanently pinned at 100%, blocking arming#11720sensei-hacker wants to merge 2 commits into
sensei-hacker wants to merge 2 commits into
Conversation
taskSystem() used the sample-based load estimator's UNUSED(currentTimeUs) placement even though SITL_BUILD now uses the parameter, and lacked any note on why SITL needs a different calculation than hardware targets.
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Test firmware build ready — commit Download firmware for PR #11720 243 targets built. Find your board's
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A freshly-defaulted SITL build on 9.1 cannot arm: the Configurator's Status
tab shows CPU load pinned at/above 100%, tripping
ARMING_DISABLED_SYSTEM_OVERLOADEDon an idle, correctly functioninginstance.
averageSystemLoadPercent(src/main/scheduler/scheduler.c) is a duty-cycleestimator that counts how many
scheduler()samples found a task alreadyoverdue. That only produces a sane result if
scheduler()runs far moreoften than tasks become due — true on real hardware, which busy-polls. PR
#11436 made SITL's
scheduler()sleep until just before the next task isdue instead of busy-polling (a real, worthwhile CPU-usage improvement on the
host). That eliminated the zero-load samples the old estimator relied on, so
every sample now finds >=1 task due and the average is permanently >=100.
Changes
5e47ffd356("Fix SITL system load accounting") by@xznhj8129, originally authored against
maintenance-10.xas part ofSITL & X-Plane simulation fixes #11712. Full credit for the fix
design and implementation goes to that commit's author — this PR ports it
to
release/9.1, where the same regression (8a475617c4, "SITL Tweaks")is also present.
SITL_BUILD-gated busy-time/elapsed-time duty-cycle calculation:scheduler()measures wall-clock time spent per iteration (excludingthe intentional sleep), and
taskSystem()converts that into a percentageover a rolling window, capped at 100.
SITL_BUILD) load accounting is untouched.docs/SITL/SITL.md: one-line note on how SITL load is now calculated.UNUSED(currentTimeUs)call intothe branch where it's actually still unused, and added a short comment
explaining why SITL needs a different calculation than hardware — raised
by code review, no behavior change.
Testing
taskGyro()temporarily (reverted before commit) to drive genuine load:
cpuLoad(MSP2_INAV_STATUS) steady at 3-4%, noARMING_DISABLED_SYSTEM_OVERLOADED.cpuLoad10-12%.cpuLoadstable 58-59%, metric tracksadded load, still armable, no overload flag.
SITL.elfprocess measured two ways (ps pcpuand/proc/[pid]/stattick deltas): steady 7.4-8% of one core,confirming PR [SITL] Maintenance and Improvements #11436's sleep-based scheduling is untouched (no
busy-polling reintroduced).
confirms the non-
SITL_BUILDpath is unaffected (all changes are inside#if defined(SITL_BUILD)blocks).Code Review
Reviewed with the
inav-code-reviewagent: approved, no critical orimportant issues blocking merge. Two minor suggestions (explanatory comment,
misplaced
UNUSED()) were addressed in the follow-up commit.Fixes #11710