Skip to content

Commit

Permalink
Add pedantic compiler flags, and resolve problems.
Browse files Browse the repository at this point in the history
This PR copies the compiler warning flags from up-cpp to this repo, and
resolves warnings that result.
  • Loading branch information
debruce committed Aug 13, 2024
1 parent d0658a3 commit cfed0f3
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 30 deletions.
10 changes: 10 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
61 changes: 50 additions & 11 deletions src/ZenohUTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand All @@ -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();
}
Expand All @@ -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());
}
Expand Down Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion test/extra/PublisherSubscriberTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void ValidateMessages(std::queue<v1::UMessage>& rx_queue, size_t num_messages,
auto message = rx_queue.front();
rx_queue.pop();

int pos = message.payload().find(prefix);
auto pos = message.payload().find(prefix);
int num = std::stoi(message.payload().substr(prefix.size()));
sum += num;
EXPECT_NE(pos, std::string::npos);
Expand Down
34 changes: 16 additions & 18 deletions test/extra/RpcClientServerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> transport_;
std::shared_ptr<Transport> transport_ = nullptr; // NOLINT

// Run once per TEST_F.
// Used to set up clean environments per test.
void SetUp() override {
transport_ = std::make_shared<Transport>(ident_, ZENOH_CONFIG_FILE);
transport_ = std::make_shared<Transport>(ident, ZENOH_CONFIG_FILE);
EXPECT_NE(nullptr, transport_);
}

Expand All @@ -78,37 +78,35 @@ 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;
server_capture = message;
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),
Expand Down

0 comments on commit cfed0f3

Please sign in to comment.