From 00ee7089c77f8c56bf98bb19e44a047e449f41bc Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Sun, 10 May 2020 13:41:54 +0530 Subject: [PATCH 1/8] [Unity/cmake] Ignore unknown warning options, unused functions warning --- Unity/AirLibWrapper/AirsimWrapper/cmake/rpc-setup.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Unity/AirLibWrapper/AirsimWrapper/cmake/rpc-setup.cmake b/Unity/AirLibWrapper/AirsimWrapper/cmake/rpc-setup.cmake index e1de801535..b637eab622 100644 --- a/Unity/AirLibWrapper/AirsimWrapper/cmake/rpc-setup.cmake +++ b/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) From 43f2b0eb52a09ae52d8d33bc7a9907e5bb7809f0 Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Sun, 10 May 2020 18:36:49 +0530 Subject: [PATCH 2/8] [cmake] Disable unknown warning option warnings --- cmake/cmake-modules/CommonSetup.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/cmake-modules/CommonSetup.cmake b/cmake/cmake-modules/CommonSetup.cmake index bca7a8f9b6..ff67145760 100644 --- a/cmake/cmake-modules/CommonSetup.cmake +++ b/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") From 60cf11fb7bc453dbf217eb17e5f79489b28fdcfe Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Sun, 10 May 2020 17:56:53 +0530 Subject: [PATCH 3/8] Make base class destructors virtual --- AirLib/include/sensors/SensorFactory.hpp | 2 ++ AirLibUnitTests/TestBase.hpp | 1 + MavLinkCom/src/impl/MavLinkNodeImpl.hpp | 2 +- MavLinkCom/src/serial_com/SerialPort.hpp | 2 +- MavLinkCom/src/serial_com/TcpClientPort.hpp | 2 +- MavLinkCom/src/serial_com/UdpClientPort.hpp | 2 +- 6 files changed, 7 insertions(+), 4 deletions(-) diff --git a/AirLib/include/sensors/SensorFactory.hpp b/AirLib/include/sensors/SensorFactory.hpp index 1fffe86411..ed69b4abb7 100644 --- a/AirLib/include/sensors/SensorFactory.hpp +++ b/AirLib/include/sensors/SensorFactory.hpp @@ -57,6 +57,8 @@ class SensorFactory { } } } + + virtual ~SensorFactory() = default; }; diff --git a/AirLibUnitTests/TestBase.hpp b/AirLibUnitTests/TestBase.hpp index 13342dd2c2..d1916fb963 100644 --- a/AirLibUnitTests/TestBase.hpp +++ b/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) { diff --git a/MavLinkCom/src/impl/MavLinkNodeImpl.hpp b/MavLinkCom/src/impl/MavLinkNodeImpl.hpp index b30a87bc8b..295c3f2e6a 100644 --- a/MavLinkCom/src/impl/MavLinkNodeImpl.hpp +++ b/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 connection); void close(); diff --git a/MavLinkCom/src/serial_com/SerialPort.hpp b/MavLinkCom/src/serial_com/SerialPort.hpp index 49101d36fd..9ed32d5d16 100644 --- a/MavLinkCom/src/serial_com/SerialPort.hpp +++ b/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); diff --git a/MavLinkCom/src/serial_com/TcpClientPort.hpp b/MavLinkCom/src/serial_com/TcpClientPort.hpp index ee1ca733f9..bcca77138d 100644 --- a/MavLinkCom/src/serial_com/TcpClientPort.hpp +++ b/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 diff --git a/MavLinkCom/src/serial_com/UdpClientPort.hpp b/MavLinkCom/src/serial_com/UdpClientPort.hpp index 9908ee5e66..0c83c494ad 100644 --- a/MavLinkCom/src/serial_com/UdpClientPort.hpp +++ b/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. From ad0beb2a0a91ad011bd19e997f54d47d8f87e130 Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Sun, 10 May 2020 13:50:13 +0530 Subject: [PATCH 4/8] [Car] Remove unused variable state_ --- .../include/vehicles/car/firmwares/ardurover/ArduRoverApi.hpp | 3 +-- AirLib/include/vehicles/car/firmwares/physxcar/PhysXCarApi.hpp | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/AirLib/include/vehicles/car/firmwares/ardurover/ArduRoverApi.hpp b/AirLib/include/vehicles/car/firmwares/ardurover/ArduRoverApi.hpp index c9b0d84a41..37ce927c36 100644 --- a/AirLib/include/vehicles/car/firmwares/ardurover/ArduRoverApi.hpp +++ b/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 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(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_; }; diff --git a/AirLib/include/vehicles/car/firmwares/physxcar/PhysXCarApi.hpp b/AirLib/include/vehicles/car/firmwares/physxcar/PhysXCarApi.hpp index 1b9362da26..8e2cbfe47f 100644 --- a/AirLib/include/vehicles/car/firmwares/physxcar/PhysXCarApi.hpp +++ b/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 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_; }; From 02925349b156f5d297f382c64e60da0906c6085f Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Mon, 11 May 2020 12:36:02 +0530 Subject: [PATCH 5/8] [Mavlinkcom] Remove some unused variables --- MavLinkCom/src/impl/MavLinkConnectionImpl.cpp | 2 -- MavLinkCom/src/serial_com/TcpClientPort.cpp | 2 -- 2 files changed, 4 deletions(-) diff --git a/MavLinkCom/src/impl/MavLinkConnectionImpl.cpp b/MavLinkCom/src/impl/MavLinkConnectionImpl.cpp index 741e3b8cc9..2e4655b7bc 100644 --- a/MavLinkCom/src/impl/MavLinkConnectionImpl.cpp +++ b/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; diff --git a/MavLinkCom/src/serial_com/TcpClientPort.cpp b/MavLinkCom/src/serial_com/TcpClientPort.cpp index 502f02eb25..10568ba6ac 100644 --- a/MavLinkCom/src/serial_com/TcpClientPort.cpp +++ b/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(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(result), bytesToRead, 0); if (rc < 0) { From bfc29a7f2ae8ceba5b4a0b060b37f9a4ab4da82f Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Sat, 23 May 2020 10:17:02 +0530 Subject: [PATCH 6/8] [ros] Fix warnings --- ros/src/airsim_ros_pkgs/src/airsim_settings_parser.cpp | 2 ++ .../airsim_ros_pkgs/src/pd_position_controller_simple.cpp | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/ros/src/airsim_ros_pkgs/src/airsim_settings_parser.cpp b/ros/src/airsim_ros_pkgs/src/airsim_settings_parser.cpp index 26f0638e97..e0be99330d 100644 --- a/ros/src/airsim_ros_pkgs/src/airsim_settings_parser.cpp +++ b/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 { diff --git a/ros/src/airsim_ros_pkgs/src/pd_position_controller_simple.cpp b/ros/src/airsim_ros_pkgs/src/pd_position_controller_simple.cpp index 9ed9b5fd17..a50d0957b0 100644 --- a/ros/src/airsim_ros_pkgs/src/pd_position_controller_simple.cpp +++ b/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? From 85eceae686688db40558558576ca5304a9ef74a5 Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Tue, 30 Jun 2020 15:04:02 +0530 Subject: [PATCH 7/8] Fix misleading indentation warning in RpclibServerBase --- AirLib/src/api/RpcLibServerBase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AirLib/src/api/RpcLibServerBase.cpp b/AirLib/src/api/RpcLibServerBase.cpp index 2cb514e0de..e215f0b251 100644 --- a/AirLib/src/api/RpcLibServerBase.cpp +++ b/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 { From bb8bfa144c3300baf2bd96f68a7364cb3fdd21dd Mon Sep 17 00:00:00 2001 From: Rajat Singhal Date: Tue, 30 Jun 2020 15:06:41 +0530 Subject: [PATCH 8/8] MavlinkNodeImpl: Fix warning to catch exception by const reference --- MavLinkCom/src/impl/MavLinkNodeImpl.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MavLinkCom/src/impl/MavLinkNodeImpl.cpp b/MavLinkCom/src/impl/MavLinkNodeImpl.cpp index 7fe2740da1..4772c747fe 100644 --- a/MavLinkCom/src/impl/MavLinkNodeImpl.cpp +++ b/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); } }