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.

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

Fix warnings #1660

Merged
merged 2 commits into from
Nov 29, 2021
Merged

Fix warnings #1660

merged 2 commits into from
Nov 29, 2021

Conversation

mortenfyhn
Copy link
Contributor

I have some build warnings in the mavros and mavros_extras packages when I build locally with Clang, which I've attempted to fix.

I think some of these changes are uncontroversial, such as

  • reordering constructor initializer lists to avoid "-Wreorder-ctor",
  • initializing yaw and yaw_rate explicitly to zero in the setpoint_trajectory plugin, and
  • fixing the probably incorrect gps_rate initialization.

There was also a warning where MissionBase::initialize(ros::NodeHandle *_wp_nh) upsets the compiler because it "hides the overloaded virtual function PluginBase::initialize(UAS &uas) because MissionBase inherits from PluginBase. I've avoided this by renaming MissionBase::initialize but you may want to solve it differently, or give it a different name.

After this, all that remains when I build are unused variable warnings, mostly about COV_SIZE which is only used in debug builds. I suppose this could be fixed with the casting to void trick (void)COV_SIZE or with [maybe_unused](https://en.cppreference.com/w/cpp/language/attributes/maybe_unused) (but that would require C++17). Locally I've silenced this with a few #pragma GCC diagnostic ignored "-Wunused-variable".

Copy link
Member

@vooon vooon left a comment

Choose a reason for hiding this comment

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

LGTM. Personally i prefer [[maybe_unused]], but for v2...

@vooon vooon added this to the Version 1.13 milestone Nov 29, 2021
@vooon vooon merged commit b11258f into mavlink:master Nov 29, 2021
@mortenfyhn
Copy link
Contributor Author

mortenfyhn commented Nov 29, 2021

LGTM. Personally i prefer [[maybe_unused]], but for v2...

I agree, [[maybe_unused]] is much nicer than the (void) "hack". (A well-established hack but a hack nontheless.)

Anyway thanks for the merge!

@mortenfyhn mortenfyhn deleted the fix-warnings branch November 29, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants