From 0cd2a2478f594a915bad64b5886c06e54e672512 Mon Sep 17 00:00:00 2001 From: Javier Gil Aviles Date: Fri, 24 Jan 2025 10:24:44 +0100 Subject: [PATCH] New review corrections Signed-off-by: Javier Gil Aviles --- .../include/sustainml_cpp/types/types.hpp | 4 +- sustainml_cpp/src/cpp/core/NodeImpl.cpp | 12 ++--- sustainml_cpp/src/cpp/core/RequestReplier.cpp | 44 +++++++++---------- .../src/cpp/orchestrator/OrchestratorNode.cpp | 4 +- sustainml_cpp/src/cpp/types/types.cpp | 20 ++++----- 5 files changed, 41 insertions(+), 43 deletions(-) diff --git a/sustainml_cpp/include/sustainml_cpp/types/types.hpp b/sustainml_cpp/include/sustainml_cpp/types/types.hpp index 08c8ce5..635b489 100644 --- a/sustainml_cpp/include/sustainml_cpp/types/types.hpp +++ b/sustainml_cpp/include/sustainml_cpp/types/types.hpp @@ -2440,7 +2440,7 @@ class RequestType * @param x Reference to the object RequestTypeImpl that will be copied. */ eProsima_user_DllExport RequestType& operator =( - const RequestTypeImpl* x); + const RequestTypeImpl& x); /*! * @brief Comparison operator. @@ -2594,7 +2594,7 @@ class ResponseType * @param x Reference to the object ResponseTypeImpl that will be copied. */ eProsima_user_DllExport ResponseType& operator =( - const ResponseTypeImpl* x); + const ResponseTypeImpl& x); /*! * @brief Comparison operator. diff --git a/sustainml_cpp/src/cpp/core/NodeImpl.cpp b/sustainml_cpp/src/cpp/core/NodeImpl.cpp index cbe34c1..dffb34b 100644 --- a/sustainml_cpp/src/cpp/core/NodeImpl.cpp +++ b/sustainml_cpp/src/cpp/core/NodeImpl.cpp @@ -87,12 +87,12 @@ NodeImpl::NodeImpl( req_res_ = new RequestReplier([this, name](void* input) { RequestTypeImpl* in = static_cast(input); - types::RequestType req; - req = in; - if (req.node_id() == static_cast(common::get_node_id_from_name(name))) + if (in->node_id() == static_cast(common::get_node_id_from_name(name))) { types::ResponseType res; + types::RequestType req; + req = *in; req_res_listener_.on_configuration_request(req, res); req_res_->write_res(res.get_impl()); } @@ -120,12 +120,12 @@ NodeImpl::NodeImpl( req_res_ = new RequestReplier([this, name](void* input) { RequestTypeImpl* in = static_cast(input); - types::RequestType req; - req = in; - if (req.node_id() == static_cast(common::get_node_id_from_name(name))) + if (in->node_id() == static_cast(common::get_node_id_from_name(name))) { types::ResponseType res; + types::RequestType req; + req = *in; req_res_listener_.on_configuration_request(req, res); req_res_->write_res(res.get_impl()); } diff --git a/sustainml_cpp/src/cpp/core/RequestReplier.cpp b/sustainml_cpp/src/cpp/core/RequestReplier.cpp index 4fd597f..da9cad3 100644 --- a/sustainml_cpp/src/cpp/core/RequestReplier.cpp +++ b/sustainml_cpp/src/cpp/core/RequestReplier.cpp @@ -1,4 +1,4 @@ -// Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// Copyright 2025 Proyectos y Sistemas de Mantenimiento SL (eProsima). // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -27,10 +27,10 @@ namespace sustainml { namespace core { RequestReplier::RequestReplier( - std::function callback, - const char* topicw, - const char* topicr, - void* data) + std::function callback, + const char* topicw, + const char* topicr, + void* data) : callback_(callback) , topicw_(topicw) , topicr_(topicr) @@ -58,15 +58,15 @@ RequestReplier::RequestReplier( typeReq_.register_type(participant_); // Create a Topics - if(std::string(topicw_) == "sustainml/request") + if (std::string(topicw_) == "sustainml/request") { - topicR_= participant_->create_topic(topicr_, typeRes_.get_type_name(), TOPIC_QOS_DEFAULT); - topicW_= participant_->create_topic(topicw_, typeReq_.get_type_name(), TOPIC_QOS_DEFAULT); + topicR_ = participant_->create_topic(topicr_, typeRes_.get_type_name(), TOPIC_QOS_DEFAULT); + topicW_ = participant_->create_topic(topicw_, typeReq_.get_type_name(), TOPIC_QOS_DEFAULT); } else { - topicR_= participant_->create_topic(topicr_, typeReq_.get_type_name(), TOPIC_QOS_DEFAULT); - topicW_= participant_->create_topic(topicw_, typeRes_.get_type_name(), TOPIC_QOS_DEFAULT); + topicR_ = participant_->create_topic(topicr_, typeReq_.get_type_name(), TOPIC_QOS_DEFAULT); + topicW_ = participant_->create_topic(topicw_, typeRes_.get_type_name(), TOPIC_QOS_DEFAULT); } // Configure DataReader QoS @@ -96,19 +96,19 @@ RequestReplier::~RequestReplier() } void RequestReplier::write_res( - ResponseTypeImpl* res) + ResponseTypeImpl* res) { writer_->write(res); } void RequestReplier::write_req( - RequestTypeImpl* req) + RequestTypeImpl* req) { writer_->write(req); } RequestReplier::RequestReplyControlListener::RequestReplyControlListener( - RequestReplier* node) + RequestReplier* node) : node_(node) { @@ -120,7 +120,7 @@ RequestReplier::RequestReplyControlListener::~RequestReplyControlListener() } void RequestReplier::RequestReplyControlListener::on_data_available( - eprosima::fastdds::dds::DataReader* reader) + eprosima::fastdds::dds::DataReader* reader) { // Create a data and SampleInfo instance SampleInfo info; @@ -136,27 +136,27 @@ void RequestReplier::RequestReplyControlListener::on_data_available( } void RequestReplier::RequestReplyControlListener::on_subscription_matched( - eprosima::fastdds::dds::DataReader* reader, - const eprosima::fastdds::dds::SubscriptionMatchedStatus& status) + eprosima::fastdds::dds::DataReader* reader, + const eprosima::fastdds::dds::SubscriptionMatchedStatus& status) { // New remote DataWriter discovered if (status.current_count_change == 1) { - matched_ = status.current_count; - std::cout << "Subscriber matched." << std::endl; + matched_ = status.current_count; + std::cout << "Subscriber matched." << std::endl; } // New remote DataWriter undiscovered else if (status.current_count_change == -1) { - matched_ = status.current_count; - std::cout << "Subscriber unmatched." << std::endl; + matched_ = status.current_count; + std::cout << "Subscriber unmatched." << std::endl; } // Non-valid option else { - std::cout << status.current_count_change - << " is not a valid value for SubscriptionMatchedStatus current count change" << std::endl; + EPROSIMA_LOG_ERROR(RequestReplier, status.current_count_change + << " is not a valid value for SubscriptionMatchedStatus current count change"); } } diff --git a/sustainml_cpp/src/cpp/orchestrator/OrchestratorNode.cpp b/sustainml_cpp/src/cpp/orchestrator/OrchestratorNode.cpp index b523e3c..84109c5 100644 --- a/sustainml_cpp/src/cpp/orchestrator/OrchestratorNode.cpp +++ b/sustainml_cpp/src/cpp/orchestrator/OrchestratorNode.cpp @@ -118,12 +118,10 @@ OrchestratorNode::OrchestratorNode( req_res_ = new core::RequestReplier([this](void* input) { ResponseTypeImpl* in = static_cast(input); - types::ResponseType res; - res = in; { std::lock_guard lock(this->mtx_); - this->res_ = res; + this->res_ = *in; } this->cv_.notify_all(); }, "sustainml/request", "sustainml/response", res_.get_impl()); diff --git a/sustainml_cpp/src/cpp/types/types.cpp b/sustainml_cpp/src/cpp/types/types.cpp index 169c3b6..8acf05c 100644 --- a/sustainml_cpp/src/cpp/types/types.cpp +++ b/sustainml_cpp/src/cpp/types/types.cpp @@ -2171,11 +2171,11 @@ RequestType& RequestType::operator =( } RequestType& RequestType::operator =( - const RequestTypeImpl* x) + const RequestTypeImpl& x) { - this->impl_->node_id() = x->node_id(); - this->impl_->transaction_id() = x->transaction_id(); - this->impl_->configuration() = x->configuration(); + this->impl_->node_id() = x.node_id(); + this->impl_->transaction_id() = x.transaction_id(); + this->impl_->configuration() = x.configuration(); return *this; } @@ -2312,13 +2312,13 @@ ResponseType& ResponseType::operator =( } ResponseType& ResponseType::operator =( - const ResponseTypeImpl* x) + const ResponseTypeImpl& x) { - this->impl_->node_id() = x->node_id(); - this->impl_->transaction_id() = x->transaction_id(); - this->impl_->success() = x->success(); - this->impl_->err_code() = x->err_code(); - this->impl_->configuration() = x->configuration(); + this->impl_->node_id() = x.node_id(); + this->impl_->transaction_id() = x.transaction_id(); + this->impl_->success() = x.success(); + this->impl_->err_code() = x.err_code(); + this->impl_->configuration() = x.configuration(); return *this; }