Conversation
📊 Size Comparison ReportBuild completed for commit 9e378e5 📦 Size ChangesUpdated automatically by the size test workflow |
peterbarker
force-pushed
the
pr/rewrite-test-size
branch
from
9a6c0f2 to
529a35d
Compare
…1-ASAUAV board Co-authored-by: Leonardo Garcia <lgarciaos@protonmail.com>
…SAUAV board Co-authored-by: Leonardo Garcia <lgarciaos@protonmail.com>
each of the affected gimbals can already send angle data at the rate at which we call these functions; we don't need to treat the retracted and neutral positions any more specially than we treat normal angle targets
Necessary to work around a bug in newlib. This applies to all hardfloat targets, including Cortex-M4. The jmp_buf is now 12 bytes larger, totaling 104 bytes. Info from newlib commit 15ad816dddf836def06cd0330ec0efa9ce50e5bf.
Necessary to work around a bug in newlib. This applies to all hardfloat targets, including Cortex-M4. The jmp_buf is now 12 bytes larger, totaling 104 bytes. Info from newlib commit 15ad816dddf836def06cd0330ec0efa9ce50e5bf.
It properly saves and restores the floating point registers.
No longer necessary; verified with a debugger. These were also never applied to Cortex-M4.
Should instead use ap_setjmp.
Probably could stand a refactor to use the CLI like other tools.
C++ driver now handles ViewPro gimbals, lua version no longer needed
Avoids unnecessary loads and stores and zeros of the KH array at the cost of more computation. But it turns out on hardware the computation is cheaper.
Removes the extra computation introduced in the previous commit.
For a particular axis, one H value is always 1, and the other two which would be 1 for a different axis are zero. Therefore we can process only the relevant P row and avoid meaningless computation of the other two.
This fixes make_secure_bl writing to the incorrect file (sys.argv[1]). The resultant file was whatever argv[1] was passed in as, which could have been --omit-ardupilot-keys. This fixes make_secure_fw not running at all because of a possible return outside of a function. Also improves argument passing so we can get ourselves a better help file.
Adds ability to pass --omit-ardupilot-keys to build_bootloaders.py Adds ability to pass multiple public keys to the signing of the bootloader. This extends the functionality of the single key that was previously possible. All keys are prefixed with --signing-key and are appended to the args.signing_key array. All keys are checked for presense, and type before being used to sign the bootloader. General tidy up of the argument parser, prints a proper description of the role of the file.
…d with the compact method of signing firmware If the firmware file is built and signed using the compact method (ie. build and sign firmware at the same time) then the firmware file is not updated with the field signed_firmware. If the firmware file is built using the long form method (ie. build the firmware first, then sign after) then the apj is updated with the field signed_firmware. This ensures both signed firmware is consistent and contains the 'signed_firmware' field.
The binutils size utility has a peculiar way of tallying the section sizes that also includes the .heap section. As the .heap section is the remainder of (the first section of) free RAM, analogous to .crash_dump being the remainder of flash, we also need to remove it from the BSS size to make it useful. The tally method also explains why we needed to remove .crash_dump in the first place. The BSS metric now correctly shows the amount of zero-initialized non-heap space used so that e.g. removing a global variable reduces the BSS used by the size of that variable (subject to alignment).
mixing the progress with stdout is causing problems for tools trying to consume stdout: [update.py]: feature EF: Running (arm-none-eabi-nm --demangle --print-size /home/autotest/build/tmp/binaries.build/ZeroOneX6_Air/bin/blimp) in (.) (ZeroOneX6_Air,Blimp) not in build_options.py [update.py]: feature EF: Running (strings /home/autotest/build/tmp/binaries.build/ZeroOneX6_Air/bin/blimp) in (.) (ZeroOneX6_Air,Blimp) not in build_options.py
no command-line option at the moment and we don't want this stuff emitted by default anyway.
Lua calls our registered panic function if a Lua error is raised outside of any protected mode set up by `lua_pcall` and friends. As scripts themselves are run in protected mode, such an error will only originate from our engine code. The current code handles panics, but does so incorrectly by neglecting to free the error message buffer during a panic exit. As the underlying heap is eventually destroyed and recreated, that static buffer pointer is to a heap that no longer exists. If the new state raises an error it will try to free that pointer, causing heap corruption and sadness. Fix this issue and avoid similar ones by refactoring to run the main part of the scripting engine in Lua protected mode. This creates exactly one exit path from the scripting engine and avoids the need for fragile panic handling infrastructure that duplicates what is built into Lua. Quoth the Lua manual, "The panic function, as its name implies, is a mechanism of last resort. Programs should avoid it." This is unlikely to be triggerable in flight as the main engine loop does not appear to use the Lua API in a way which can trigger errors. But the possibility can't be fully excluded. It is, however, possible to trigger beforehand using a perfectly wrong heap size that causes a memory error during initialization. Note that due to the error message buffer being freed properly now, an error message originating from the engine will not cause a pre-arm failure. This could be improved in the future.
Nominally cannot be called as we do not call error-raising Lua functions outside of protected mode. In the event a panic happens somehow anyway, Lua calls `lua_abort` which we already handle safely.
This should never be called so there's not much to be gained from a panic message. If it is, the call should be identifiable by the internal error line number.
If Lua raises an error we won't ever link the new_script into the list and won't find it to remove it and free its components later. The components do ultimately get freed by the destruction of the heap, and there's no way to access them after, but we'd rather it be empty and know nothing is dangling. This should be replaced by storing the components as Lua objects and letting Lua's GC take care of them. That way the code could be made much less fragile. Also removes a chance of a leak in run_next_script.
Significantly simplifies the test_size workflow by replacing ~300 lines of manual build steps with a single call to size_compare_branches.py. Changes: - Removed matrix build strategy - Use single job with size_compare_branches.py parallelization (--parallel-copies 4) - Removed manual build logic for base and PR branches - Removed manual size comparison steps (pretty_diff_size.py) - Removed manual feature extraction steps (extract_features.py) - Use --all-vehicles to let size_compare_branches.py determine what to build - Removed artifact upload (output stays in job logs) Benefits: - Reduced workflow from ~330 lines to ~110 lines (-67%) - Single source of truth for size comparison logic - Easier to maintain - Same experience for CI and local development - Parallelization handled by size_compare_branches.py 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add a step to fetch the base branch as 'upstream' remote before running size_compare_branches.py. This fixes the "Not a valid object name master" error by ensuring the base branch exists as a git reference. Changes: - Add git remote for base repository - Fetch base branch to upstream remote - Pass --master-branch upstream/<ref> instead of just <ref> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The --parallel-copies option requires rsync which is not available in the CI environment. Use --jobs instead to control build parallelization.
Add step to install rsync package which is required by size_compare_branches.py when using --parallel-copies. Restore --parallel-copies 4 with -j 2 for better parallelization across multiple source tree copies.
Keep rsync installed but don't use --parallel-copies. Use -j 8 for build parallelization instead.
Disable mount support by default to reduce flash usage.
Add a new step that captures the output from size_compare_branches.py and posts it as a formatted PR comment. The comment includes: - Size changes section with code block formatting - Feature changes section with readable ✅/❌ symbols for added/removed features - Auto-updates on each CI run - Clear headings and organization The comment is created on first run and updated on subsequent runs to avoid cluttering the PR with multiple bot comments.
peterbarker
force-pushed
the
pr/rewrite-test-size
branch
from
529a35d to
b2112d6
Compare
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
20 participants
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.
No description provided.