Fix some warnings (microsoft#2682)

* [Unity/cmake] Ignore unknown warning options, unused functions warning

* [cmake] Disable unknown warning option warnings

* Make base class destructors virtual

* [Car] Remove unused variable state_

* [Mavlinkcom] Remove some unused variables

* [ros] Fix warnings

* Fix misleading indentation warning in RpclibServerBase

* MavlinkNodeImpl: Fix warning to catch exception by const reference

AirLib/include/sensors/SensorFactory.hpp

@@ -57,6 +57,8 @@ class SensorFactory {
             }
         }
     }
+
+    virtual ~SensorFactory() = default;
 };
 
 

AirLib/include/vehicles/car/firmwares/ardurover/ArduRoverApi.hpp

@@ -33,7 +33,7 @@ class ArduRoverApi : public CarApiBase {
     ArduRoverApi(const AirSimSettings::VehicleSetting* vehicle_setting, std::shared_ptr<SensorFactory> sensor_factory,
                  const Kinematics::State& state, const Environment& environment, const msr::airlib::GeoPoint& home_geopoint)
     : CarApiBase(vehicle_setting, sensor_factory, state, environment),
-      state_(state), home_geopoint_(home_geopoint)
+      home_geopoint_(home_geopoint)
     {
         connection_info_ = static_cast<const AirSimSettings::MavLinkVehicleSetting*>(vehicle_setting)->connection_info;
         sensors_ = &getSensors();
@@ -277,7 +277,6 @@ class ArduRoverApi : public CarApiBase {
     const SensorCollection* sensors_;
 
     CarControls last_controls_;
-    const Kinematics::State& state_;
     GeoPoint home_geopoint_;
     CarState last_car_state_;
 };

AirLib/include/vehicles/car/firmwares/physxcar/PhysXCarApi.hpp

@@ -13,7 +13,7 @@ class PhysXCarApi : public CarApiBase {
     PhysXCarApi(const AirSimSettings::VehicleSetting* vehicle_setting, std::shared_ptr<SensorFactory> sensor_factory, 
                 const Kinematics::State& state, const Environment& environment, const msr::airlib::GeoPoint& home_geopoint)
     : CarApiBase(vehicle_setting, sensor_factory, state, environment),
-      home_geopoint_(home_geopoint), state_(state)
+      home_geopoint_(home_geopoint)
     {}
 
     ~PhysXCarApi()
@@ -88,7 +88,6 @@ class PhysXCarApi : public CarApiBase {
     bool api_control_enabled_ = false;
     GeoPoint home_geopoint_;
     CarControls last_controls_;
-    const Kinematics::State& state_;
     CarState last_car_state_;
 
 };

AirLib/src/api/RpcLibServerBase.cpp

@@ -181,7 +181,8 @@ RpcLibServerBase::RpcLibServerBase(ApiProvider* api_provider, const std::string&
             sim_world_api->reset();
         else
             getVehicleApi("")->reset();
-            resetInProgress = false;
+
+        resetInProgress = false;
     });
 
     pimpl_->server.bind("simPrintLogMessage", [&](const std::string& message, const std::string& message_param, unsigned char severity) -> void {

AirLibUnitTests/TestBase.hpp

@@ -9,6 +9,7 @@ namespace msr { namespace airlib {
 
 class TestBase {
 public:
+    virtual ~TestBase() = default;
     virtual void run() = 0;
 
     void testAssert(double lhs, double rhs, const std::string& message) {

MavLinkCom/src/impl/MavLinkConnectionImpl.cpp

@@ -430,8 +430,6 @@ void MavLinkConnectionImpl::readPackets()
             }
             else if (frame_state == MAVLINK_FRAMING_OK)
             {
-                int msgId = msg.msgid;
-
                 // pick up the sysid/compid of the remote node we are connected to.
                 if (other_system_id == -1) {
                     other_system_id = msg.sysid;

MavLinkCom/src/impl/MavLinkNodeImpl.cpp

@@ -592,8 +592,9 @@ void MavLinkNodeImpl::sendCommand(MavLinkCommand& command)
     try {
         sendMessage(cmd);
     }
-    catch (std::exception e) {
+    catch (const std::exception& e) {
         // silently fail since we are on a background thread here...
+        unused(e);
     }
 }
 

MavLinkCom/src/impl/MavLinkNodeImpl.hpp

@@ -15,7 +15,7 @@ namespace mavlinkcom_impl {
     {
     public:
         MavLinkNodeImpl(int localSystemId, int localComponentId);
-        ~MavLinkNodeImpl();
+        virtual ~MavLinkNodeImpl();
 
         void connect(std::shared_ptr<MavLinkConnection> connection);
         void close();

MavLinkCom/src/serial_com/SerialPort.hpp

@@ -37,7 +37,7 @@ class SerialPort : public Port
 {
 public:
 	SerialPort();
-	~SerialPort();
+	virtual ~SerialPort();
 
 	// open the serial port
 	virtual int connect(const char* portName, int baudRate);

MavLinkCom/src/serial_com/TcpClientPort.cpp

@@ -227,7 +227,6 @@ class TcpClientPort::TcpSocketImpl
 	// write to the serial port
 	int write(const uint8_t* ptr, int count)
 	{
-		socklen_t addrlen = sizeof(sockaddr_in);
 		int hr = send(sock, reinterpret_cast<const char*>(ptr), count, 0);
 		if (hr == SOCKET_ERROR)
 		{
@@ -262,7 +261,6 @@ class TcpClientPort::TcpSocketImpl
 
 		while (!closed_)
 		{
-			socklen_t addrlen = sizeof(sockaddr_in);
 			int rc = recv(sock, reinterpret_cast<char*>(result), bytesToRead, 0);
 			if (rc < 0)
 			{

MavLinkCom/src/serial_com/TcpClientPort.hpp

@@ -10,7 +10,7 @@ class TcpClientPort : public Port
 {
 public:
 	TcpClientPort();
-	~TcpClientPort();
+	virtual ~TcpClientPort();
 
 	// Connect can set you up two different ways.  Pass 0 for local port to get any free local
 	// port. localHost allows you to be specific about which local adapter to use in case you 

MavLinkCom/src/serial_com/UdpClientPort.hpp

@@ -10,7 +10,7 @@ class UdpClientPort : public Port
 {
 public:
 	UdpClientPort();
-	~UdpClientPort();
+	virtual ~UdpClientPort();
 
 	// Connect can set you up two different ways.  Pass 0 for local port to get any free local
 	// port and pass a fixed remotePort if you want to send to a specific remote port. 

Unity/AirLibWrapper/AirsimWrapper/cmake/rpc-setup.cmake

@@ -125,7 +125,7 @@ if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
   # clang is the compiler used for developing mainly, so
   # this is where I set the highest warning level
   list(APPEND RPCLIB_BUILD_FLAGS
-    -Wall -Wextra -Wno-c++98-compat
+    -Wall -Wextra -Wno-unused-function -Wno-c++98-compat -Wno-unknown-warning-option
     -Wno-c++98-compat-pedantic -Wno-padded -Wno-missing-prototypes
     -Wno-undef -pthread)
 
@@ -268,7 +268,7 @@ if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
   # clang is the compiler used for developing mainly, so
   # this is where I set the highest warning level
   list(APPEND RPCLIB_BUILD_FLAGS
-    -Wall -Wextra -Wno-c++98-compat
+    -Wall -Wextra -Wno-unused-function -Wno-c++98-compat -Wno-unknown-warning-option
     -Wno-c++98-compat-pedantic -Wno-padded -Wno-missing-prototypes
     -Wno-undef -pthread)
 

cmake/cmake-modules/CommonSetup.cmake

@@ -59,7 +59,7 @@ macro(CommonSetup)
                 ${RPC_LIB_DEFINES} ${CMAKE_CXX_FLAGS}")
 
             if (${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
-                set(CMAKE_CXX_FLAGS "-stdlib=libc++ -Wno-documentation ${CMAKE_CXX_FLAGS}")
+                set(CMAKE_CXX_FLAGS "-stdlib=libc++ -Wno-documentation -Wno-unknown-warning-option ${CMAKE_CXX_FLAGS}")
                 set(CXX_EXP_LIB "-lc++fs -ferror-limit=10")
             else()
                 set(CXX_EXP_LIB "-lstdc++fs -fmax-errors=10 -Wnoexcept -Wstrict-null-sentinel")

ros/src/airsim_ros_pkgs/src/airsim_settings_parser.cpp

@@ -51,6 +51,8 @@ bool AirSimSettingsParser::initializeSettings()
         std::cout << "simmode_name: " << simmode_name << std::endl; 
 
         AirSimSettings::singleton().load(std::bind(&AirSimSettingsParser::getSimMode, this));
+
+        return true;
     }
     else
     {

ros/src/airsim_ros_pkgs/src/pd_position_controller_simple.cpp

@@ -127,6 +127,10 @@ bool PIDPositionController::local_position_goal_srv_cb(airsim_ros_pkgs::SetLocal
         reset_errors(); // todo
         return true;
     }
+
+    // Already have goal, and have reached it
+    ROS_INFO_STREAM("[PIDPositionController] Already have goal and have reached it");
+    return false;
 }
 
 bool PIDPositionController::local_position_goal_srv_override_cb(airsim_ros_pkgs::SetLocalPosition::Request& request, airsim_ros_pkgs::SetLocalPosition::Response& response)
@@ -206,6 +210,10 @@ bool PIDPositionController::gps_goal_srv_cb(airsim_ros_pkgs::SetGPSPosition::Req
         reset_errors(); // todo
         return true;
     }
+
+    // Already have goal, this shouldn't happen
+    ROS_INFO_STREAM("[PIDPositionController] Goal already received, ignoring!");
+    return false;
 }
 
 // todo do relative altitude, or add an option for the same?