Skip to content
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.

Sign up for GitHub

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

Jump to bottom

build tools: bring GNU Arm Embedded Toolchain Version 9 to CI and setup scripts #14691

Closed
wants to merge 5 commits into from
Closed

build tools: bring GNU Arm Embedded Toolchain Version 9 to CI and setup scripts #14691

wants to merge 5 commits into from

Conversation

TSC21
Copy link
Member

@TSC21 TSC21 commented Apr 17, 2020

Describe problem solved by this pull request
Following the validation of the binaries built with GNU Arm Embedded Toolchain Version 9-2019-q4-major in #14580, I am bringing the new version to both CI and setup scripts.

Describe your solution

  1. Update container tags with the tag that brought version 9 to px4-dev-nuttx-bionic and px4-dev-nuttx-focal;
  2. Update setup scripts, with a verification for an existing version there, and if it isn't version 9, then it does a replacement inline with the new path for the version 9.

@TSC21 TSC21 added Tools Sub-tools used within PX4 ecosystem (scripts, etc) CI (Continuous Integration) 🤖 labels Apr 17, 2020
@TSC21 TSC21 requested review from dagar and MaEtUgR April 17, 2020 11:12
@TSC21 TSC21 self-assigned this Apr 17, 2020
@TSC21
Copy link
Member Author

TSC21 commented Apr 17, 2020

@MaEtUgR seems like we are still having issues in the Nuttx side of things - have you been testing master lately with the new GCC9 toolchain?

@TSC21 TSC21 mentioned this pull request Apr 17, 2020
Merged
@dagar dagar added this to the Release v1.12.0 milestone Apr 17, 2020
@weekly-digest weekly-digest bot mentioned this pull request Apr 19, 2020
Closed
@TSC21 TSC21 mentioned this pull request May 28, 2020
Closed
@TSC21 TSC21 mentioned this pull request Jun 6, 2020
Merged
@TSC21 TSC21 force-pushed the containers_bring_arm_gcc9 branch from 99c24bc to 64969bc Compare June 6, 2020 13:23
@TSC21
Copy link
Member Author

TSC21 commented Jun 6, 2020 •
edited
Loading

@dagar @davids5 we continue to run into some errors here, but this time I think these might be false positives regarding the usage of strncpy, which seems to be present since GCC 8.2. Should we silent these where possible? And if yes, instead of using #pragma GCC diagnostic ignored..., where could we add -Wno-stringop-truncation compile option?

@TSC21
Copy link
Member Author

TSC21 commented Jun 6, 2020

Note that I tried adding add_compile_options(-Wno-error=stringop-truncation) to https://github.com/PX4/Firmware/blob/master/boards/px4/fmu-v5/optimized.cmake but same luck.

@davids5
Copy link
Member

davids5 commented Jun 6, 2020

@TSC21 it may be a bug in the code. Have a look at it.

@TSC21
Copy link
Member Author

TSC21 commented Jun 6, 2020

@TSC21 it may be a bug in the code. Have a look at it.

I did and I didn't see anything that might lead to a bug. Further research lead me to think that this might be actually a problem in the compiler, given that several users reported the same.

@TSC21 TSC21 mentioned this pull request Jun 23, 2020
Open
MaEtUgR
MaEtUgR reviewed Jun 30, 2020
View reviewed changes
src/modules/sensors/vehicle_imu/VehicleIMU.cpp
@@ -219,7 +219,7 @@ void VehicleIMU::Run()

// rotate sensor clip counts into vehicle body frame
const Vector3f clipping{_accel_corrections.getBoardRotation() *
Vector3f{(float)accel.clip_counter[0], (float)accel.clip_counter[1], (float)accel.clip_counter[2]}};
Vector3f{(float)accel.clip_counter[0], (float)accel.clip_counter[1], (float)accel.clip_counter[2]}};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have this problem with make format too but if you try to change it CI will complain to change it back because it's using a different version of astyle. That's a good example for why keeping the versions in sync like propsed in PX4/PX4-containers#267 makes sense.

@weekly-digest weekly-digest bot mentioned this pull request Jul 5, 2020
Closed
@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Oct 4, 2020
@TSC21
Copy link
Member Author

TSC21 commented Oct 4, 2020

Already solved. Closing

@TSC21 TSC21 closed this Oct 4, 2020
@TSC21 TSC21 deleted the containers_bring_arm_gcc9 branch October 4, 2020 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaEtUgR MaEtUgR MaEtUgR left review comments

@dagar dagar Awaiting requested review from dagar

Assignees

@TSC21 TSC21

Labels
CI (Continuous Integration) 🤖 stale Tools Sub-tools used within PX4 ecosystem (scripts, etc)
Projects
None yet
Milestone
Release v1.12.0
Development

Successfully merging this pull request may close these issues.
4 participants
@TSC21 @davids5 @MaEtUgR @dagar