Fix ARM64 ABI notes: eliminate std::pair<double,double> from function signatures in bitbots_localization#871
Fix ARM64 ABI notes: eliminate std::pair<double,double> from function signatures in bitbots_localization#871jaagut wants to merge 6 commits into
Conversation
|
@copilot Please fix the following build warnings in the ARM CI: 4
|
Agent-Logs-Url: https://github.com/bit-bots/bitbots_main/sessions/506bd666-9895-407c-b056-607f1ffb4b08 Co-authored-by: jaagut <34797331+jaagut@users.noreply.github.com>
Addressed in commit 58895ef.
|
|
now fix this please: Finished <<< bitbots_localization [1min 40s]
|
Agent-Logs-Url: https://github.com/bit-bots/bitbots_main/sessions/5026fd65-fe24-4049-b9c6-6a5b176eee87 Co-authored-by: jaagut <34797331+jaagut@users.noreply.github.com>
Fixed in commit 02e8a7b. I removed the remaining No UI changes were made (no screenshot applicable). |
There was a problem hiding this comment.
Pull request overview
This PR expands the repository CI to run on an ARM64 GitHub Actions runner and includes a handful of C++ adjustments likely intended to address stricter ARM toolchain diagnostics (e.g., uninitialized variables and unnecessary copies).
Changes:
- Run the CI build/test job on both
ubuntu-latestand an ARM64 runner via a workflow matrix. - Initialize an otherwise uninitialized variable in the Yesense driver.
- Reduce copying in localization utilities by switching
std::make_pair/by-value parameters to brace-init andconst¶meters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Adds a job matrix to run CI on x86_64 and ARM64 runners. |
src/lib/livelybot_hardware_sdk/src/yesense_ros/yesense/src/yesense_driver.cpp |
Initializes gps_header_sum to avoid uninitialized-use warnings. |
src/bitbots_navigation/bitbots_localization/src/tools.cpp |
Uses brace-init returns for std::pair to simplify code. |
src/bitbots_navigation/bitbots_localization/src/StateDistribution.cpp |
Updates ctor parameter passing (but currently introduces a build-breaking assignment to a const ref). |
src/bitbots_navigation/bitbots_localization/include/bitbots_localization/StateDistribution.hpp |
Updates ctor signatures to take std::pair by const&. |
src/bitbots_navigation/bitbots_localization/src/map.cpp |
Passes observations by const& and uses brace-init for a pair. |
src/bitbots_navigation/bitbots_localization/include/bitbots_localization/map.hpp |
Keeps Map API consistent with the updated const& signature. |
src/bitbots_navigation/bitbots_localization/src/localization.cpp |
Updates call sites to construct std::pair via brace-init. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot ARM CI Build warning: Finished <<< bitbots_localization [1min 33s] |
The message is truncated in the PR. Could you provide the complete build warning/error message so I can properly address it? |
GCC 10.1+ emits ABI compatibility notes on AArch64 for any function with
std::pair<double, double>in its ABI (return type or by-value parameter) when compiled with C++17. These notes blocked clean ARM64 CI builds.Proposed changes
Replaced all
std::pair<double, double>occurrences in function ABI boundaries with output parameters or separate doubles:tools.hpp/cpp—cartesianToPolarandpolarToCartesiannow use output parameters instead of returning a pair:map.hpp/cpp—getObservationCoordinatesInMapFrameno longer takes a pair or returns one; uses individual input doubles and output parametersStateDistribution.hpp/cpp—RobotStateDistributionStartLeftreplacesstd::pair<double, double> field_sizemember withfield_size_x_/field_size_y_doubles; constructor updated accordinglyMotionModel.cpp,ObservationModel.cpp,localization.cpp— all call sites updated to match new signaturesRelated issues
Part of ARM64 CI support (PR #871
feature/ARM64_CI).Checklist
pixi run build