diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c4e37a..487497a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,6 +31,16 @@ if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) endif() +add_compile_options( + -Wall; + -Wswitch-enum; + -Wcast-qual; + -Wuninitialized; + -Wconversion; + -Wpedantic; + -Werror; +) + file(GLOB_RECURSE SRC_FILES "${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp") add_library(${PROJECT_NAME} ${SRC_FILES}) diff --git a/src/ZenohUTransport.cpp b/src/ZenohUTransport.cpp index 9481679..932730e 100644 --- a/src/ZenohUTransport.cpp +++ b/src/ZenohUTransport.cpp @@ -136,6 +136,18 @@ zenoh::Priority ZenohUTransport::mapZenohPriority(v1::UPriority upriority) { return Z_PRIORITY_INTERACTIVE_HIGH; case v1::UPriority::UPRIORITY_CS6: return Z_PRIORITY_REAL_TIME; + // These sentinel values come from the protobuf compiler. + // They are illegal for the enum, but cause linting problems. + // In order to suppress the linting error, they need to + // be included in the switch-case statement. + // It is deemed acceptable to use an exception here because + // it is in the sending code. An exception would not be + // acceptable in receiving code. The correct strategy wopuld be + // to drop the message. + case v1::UPriority::UPriority_INT_MIN_SENTINEL_DO_NOT_USE_: + case v1::UPriority::UPriority_INT_MAX_SENTINEL_DO_NOT_USE_: + throw std::runtime_error( + "Sentinel values detected in priority switch-case"); case v1::UPriority::UPRIORITY_UNSPECIFIED: default: return Z_PRIORITY_DATA_LOW; @@ -273,12 +285,17 @@ v1::UStatus ZenohUTransport::sendRequest_(const std::string& zenoh_key, auto on_done = []() {}; try { + // -Wpedantic disallows named member initialization until C++20, + // so GetOptions needs to be explicitly created and passed with + // std::move() + zenoh::Session::GetOptions options; + options.target = Z_QUERY_TARGET_BEST_MATCHING; + options.consolidation = + zenoh::QueryConsolidation(Z_CONSOLIDATION_MODE_NONE); + options.payload = zenoh::Bytes::serialize(payload); + options.attachment = zenoh::Bytes::serialize(attachment); session_.get(zenoh_key, "", std::move(on_reply), std::move(on_done), - {.target = Z_QUERY_TARGET_BEST_MATCHING, - .consolidation = zenoh::QueryConsolidation( - {.mode = Z_CONSOLIDATION_MODE_NONE}), - .payload = zenoh::Bytes::serialize(payload), - .attachment = zenoh::Bytes::serialize(attachment)}); + std::move(options)); } catch (const zenoh::ZException& e) { return uError(v1::UCode::INTERNAL, e.what()); } @@ -302,8 +319,13 @@ v1::UStatus ZenohUTransport::sendResponse_(const std::string& payload, spdlog::debug("sendResponse_ to query: {}", query->get_keyexpr().as_string_view()); auto attachment = uattributesToAttachment(attributes); - query->reply(query->get_keyexpr(), payload, - {.attachment = zenoh::Bytes::serialize(attachment)}); + // -Wpedantic disallows named member initialization until C++20, + // so PutOptions needs to be explicitly created and passed with + // std::move() + zenoh::Query::ReplyOptions options = + zenoh::Query::ReplyOptions::create_default(); + options.attachment = zenoh::Bytes::serialize(attachment); + query->reply(query->get_keyexpr(), payload, std::move(options)); return v1::UStatus(); } @@ -317,11 +339,15 @@ v1::UStatus ZenohUTransport::sendPublishNotification_( auto priority = mapZenohPriority(attributes.priority()); try { + // -Wpedantic disallows named member initialization until C++20, + // so PutOptions needs to be explicitly created and passed with + // std::move() + zenoh::Session::PutOptions options; + options.priority = priority; + options.encoding = zenoh::Encoding("app/custom"); + options.attachment = attachment; session_.put(zenoh::KeyExpr(zenoh_key), - zenoh::Bytes::serialize(payload), - {.priority = priority, - .encoding = zenoh::Encoding("app/custom"), - .attachment = attachment}); + zenoh::Bytes::serialize(payload), std::move(options)); } catch (const zenoh::ZException& e) { return uError(v1::UCode::INTERNAL, e.what()); } @@ -358,6 +384,19 @@ v1::UStatus ZenohUTransport::sendImpl(const v1::UMessage& message) { case v1::UMessageType::UMESSAGE_TYPE_RESPONSE: { return sendResponse_(payload, attributes); } + // These sentinel values come from the protobuf compiler. + // They are illegal for the enum, but cause linting problems. + // In order to suppress the linting error, they need to + // be included in the switch-case statement. + // It is deemed acceptable to use an exception here because + // it is in the sending code. An exception would not be + // acceptable in receiving code. The correct strategy wopuld be + // to drop the message. + case v1::UMessageType::UMessageType_INT_MIN_SENTINEL_DO_NOT_USE_: + case v1::UMessageType::UMessageType_INT_MAX_SENTINEL_DO_NOT_USE_: + throw std::runtime_error( + "Sentinel values detected in attribute type switch-case"); + case v1::UMessageType::UMESSAGE_TYPE_UNSPECIFIED: default: { return uError(v1::UCode::INVALID_ARGUMENT, "Wrong Message type in v1::UAttributes"); diff --git a/test/extra/RpcClientServerTest.cpp b/test/extra/RpcClientServerTest.cpp index 42b9947..b7188a6 100644 --- a/test/extra/RpcClientServerTest.cpp +++ b/test/extra/RpcClientServerTest.cpp @@ -50,18 +50,18 @@ struct MyUUri { } }; +const MyUUri rpc_service_uuri{"me_authority", 65538, 1, 32600}; +const MyUUri ident{"me_authority", 65538, 1, 0}; + class RpcClientServerTest : public testing::Test { protected: - MyUUri rpc_service_uuri_{"me_authority", 65538, 1, 32600}; - MyUUri ident_{"me_authority", 65538, 1, 0}; - using Transport = uprotocol::transport::ZenohUTransport; - std::shared_ptr transport_; + std::shared_ptr transport_ = nullptr; // NOLINT // Run once per TEST_F. // Used to set up clean environments per test. void SetUp() override { - transport_ = std::make_shared(ident_, ZENOH_CONFIG_FILE); + transport_ = std::make_shared(ident, ZENOH_CONFIG_FILE); EXPECT_NE(nullptr, transport_); } @@ -78,23 +78,21 @@ class RpcClientServerTest : public testing::Test { static void TearDownTestSuite() {} }; -TEST_F(RpcClientServerTest, SimpleRoundTrip) { - using namespace std; - - string client_request{"RPC Request"}; +TEST_F(RpcClientServerTest, SimpleRoundTrip) { // NOLINT + std::string client_request{"RPC Request"}; // NOLINT uprotocol::datamodel::builder::Payload client_request_payload( client_request, UPayloadFormat::UPAYLOAD_FORMAT_TEXT); bool client_called = false; - UMessage client_capture; + UMessage client_capture; // NOLINT bool server_called = false; - UMessage server_capture; - string server_response{"RPC Response"}; + UMessage server_capture; // NOLINT + std::string server_response{"RPC Response"}; // NOLINT uprotocol::datamodel::builder::Payload server_response_payload( server_response, UPayloadFormat::UPAYLOAD_FORMAT_TEXT); - auto serverOrStatus = RpcServer::create( - transport_, rpc_service_uuri_, + auto server_or_status = RpcServer::create( + transport_, rpc_service_uuri, [this, &server_called, &server_capture, &server_response_payload](const UMessage& message) { server_called = true; @@ -102,13 +100,13 @@ TEST_F(RpcClientServerTest, SimpleRoundTrip) { return server_response_payload; }, UPayloadFormat::UPAYLOAD_FORMAT_TEXT); - ASSERT_TRUE(serverOrStatus.has_value()); - ASSERT_NE(serverOrStatus.value(), nullptr); + ASSERT_TRUE(server_or_status.has_value()); + ASSERT_NE(server_or_status.value(), nullptr); - auto client = RpcClient(transport_, rpc_service_uuri_, + auto client = RpcClient(transport_, rpc_service_uuri, UPriority::UPRIORITY_CS4, 1000ms); - uprotocol::communication::RpcClient::InvokeHandle client_handle; + uprotocol::communication::RpcClient::InvokeHandle client_handle; // NOLINT EXPECT_NO_THROW( client_handle = client.invokeMethod( std::move(client_request_payload),