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

Enable some warnings #3945

Merged
merged 15 commits into from
Dec 15, 2021
Merged

Enable some warnings #3945

merged 15 commits into from
Dec 15, 2021

Conversation

rajat2004
Copy link
Contributor

@rajat2004 rajat2004 commented Aug 10, 2021 •
edited
Loading

Fixes: #

About

Enables a few warnings for AirSim and the ROS wrapper, needs testing

How Has This Been Tested?

Compilation and running Multirotor in UE4
ROS testing apart from compilation is left

Screenshots (if appropriate):

@rajat2004 rajat2004 mentioned this pull request Aug 10, 2021
Merged
@rajat2004 rajat2004 force-pushed the warnings branch 2 times, most recently from 95ac469 to 026355d Compare August 12, 2021 17:56
@rajat2004 rajat2004 mentioned this pull request Aug 26, 2021
Merged
10 tasks
@rajat2004 rajat2004 marked this pull request as ready for review August 29, 2021 07:17
@rajat2004 rajat2004 force-pushed the warnings branch 2 times, most recently from da31d73 to e5d7693 Compare September 11, 2021 05:24
@rajat2004 rajat2004 force-pushed the warnings branch 2 times, most recently from 165efce to 3dfde0d Compare December 4, 2021 12:13
zimmy87
zimmy87 suggested changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@zimmy87 zimmy87 left a comment

Choose a reason for hiding this comment

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

Tested building this locally and everything appears to be working fine, so this looks like a safe change to make, but I have a question about what the expected output of build.cmd should be as well as a few style comments.

cmake/cmake-modules/CommonSetup.cmake Show resolved Hide resolved
DroneShell/include/SimpleShell.hpp Outdated Show resolved Hide resolved
ros/src/airsim_ros_pkgs/src/pd_position_controller_simple.cpp Outdated Show resolved Hide resolved
@rajat2004
CMake: Remove -Wno-parentheses flag, cleanup
efea406
@rajat2004
Enable all warnings for ROS wrapper
44d06cf
@rajat2004
Mavlinkcom: Fix self-assignment warnings
3efb2f7
@rajat2004
Delete copy constructor, assignment methods for Signal
6db4ee3
@rajat2004
Cleanup ROS wrapper, upgrade to C++17
6aae272 
Improve log lines
@rajat2004
Mark nanRotation as noexcept
25fcc09
@rajat2004
FrequencyLimiter: Mark param as unused
e2f44ef
@rajat2004
Fix ROS wrapper initialization warnings, disable unused warnings
c9ea070
@rajat2004
Use AirLib GeodeticConverter in pd_position_controller
0364c2d
@rajat2004
Remove unused ROS Geodetic Converter
22b7675
@rajat2004
Change use_eth_lib to constexpr
bbb1806
@rajat2004
Fix incorrect spelling, add .py file extension
bdf3a52
@rajat2004
Ignore downloaded Blocks env, packaged binaries, Sublime Text files
00fe201
@rajat2004
Add non-virtual destructor warning, fix lots of them
45c7e6d 
Plus some cleanup for constructors
@rajat2004
Remove commented code
0841d85
@zimmy87
Copy link
Contributor

zimmy87 commented Dec 15, 2021 •
edited
Loading

Tested build.sh and the ros wrapper are still working on Ubuntu 20.04; going to move ahead with merging this. Thanks for the contribution @rajat2004!

@zimmy87 zimmy87 merged commit 042eabf into microsoft:master Dec 15, 2021
@rajat2004 rajat2004 deleted the warnings branch December 15, 2021 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zimmy87 zimmy87 zimmy87 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rajat2004 @zimmy87