From 282b995c1e94d4080cdba2eaeab270900daae596 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 15 May 2015 18:41:38 +0300 Subject: [PATCH] Partially refactored ServiceClient, tests are failing, the code is totally broken --- .../include/uavcan/node/service_client.hpp | 354 ++++++++++++------ .../distributed/raft_core.hpp | 25 +- .../node_discoverer.hpp | 14 +- .../protocol/network_compat_checker.hpp | 16 +- .../uavcan/protocol/node_info_retriever.hpp | 13 +- .../uavcan/transport/transfer_listener.hpp | 97 ++--- libuavcan/include/uavcan/util/multiset.hpp | 1 + libuavcan/src/node/uc_service_client.cpp | 29 +- libuavcan/test/node/service_client.cpp | 30 +- .../test/protocol/data_type_info_provider.cpp | 66 ++-- .../test/protocol/node_status_provider.cpp | 11 +- libuavcan/test/protocol/param_server.cpp | 32 +- .../test/protocol/restart_request_server.cpp | 8 +- .../protocol/transport_stats_provider.cpp | 42 +-- .../linux/include/uavcan_linux/helpers.hpp | 4 +- 15 files changed, 435 insertions(+), 307 deletions(-) diff --git a/libuavcan/include/uavcan/node/service_client.hpp b/libuavcan/include/uavcan/node/service_client.hpp index 067d3b890c..2aefd9c1f3 100644 --- a/libuavcan/include/uavcan/node/service_client.hpp +++ b/libuavcan/include/uavcan/node/service_client.hpp @@ -6,6 +6,7 @@ #define UAVCAN_NODE_SERVICE_CLIENT_HPP_INCLUDED #include +#include #include #include @@ -20,12 +21,39 @@ namespace uavcan { -template +template class UAVCAN_EXPORT ServiceResponseTransferListenerInstantiationHelper { public: // so much templating it hurts typedef typename TransferListenerInstantiationHelper::Type Type; + NumStaticReceiversAndBuffers, + NumStaticReceiversAndBuffers, + TransferListenerWithFilter>::Type Type; +}; + +/** + * This struct describes a pending service call. + * Refer to @ref ServiceClient to learn more about service calls. + */ +struct ServiceCallID +{ + NodeID server_node_id; + TransferID transfer_id; + + ServiceCallID() { } + + ServiceCallID(NodeID arg_server_node_id, TransferID arg_transfer_id) + : server_node_id(arg_server_node_id) + , transfer_id(arg_transfer_id) + { } + + bool operator==(const ServiceCallID rhs) const + { + return (rhs.server_node_id == server_node_id) && + (rhs.transfer_id == transfer_id); + } + + bool isValid() const { return server_node_id.isUnicast(); } }; /** @@ -33,29 +61,39 @@ public: // so much templating it hurts * Note that application ALWAYS gets this result, even when it times out or fails because of some other reason. */ template -struct UAVCAN_EXPORT ServiceCallResult +class UAVCAN_EXPORT ServiceCallResult { +public: typedef ReceivedDataStructure ResponseFieldType; enum Status { Success, ErrorTimeout }; - const Status status; ///< Whether successful or not. Failure to decode the response causes timeout. - NodeID server_node_id; ///< Node ID of the server this call was addressed to. - ResponseFieldType& response; ///< Returned data structure. Undefined if the service call has failed. +private: + const Status status_; ///< Whether successful or not. Failure to decode the response causes timeout. + ServiceCallID call_id_; ///< Identifies the call + ResponseFieldType& response_; ///< Returned data structure. Value undefined if the service call has failed. - ServiceCallResult(Status arg_status, NodeID arg_server_node_id, ResponseFieldType& arg_response) - : status(arg_status) - , server_node_id(arg_server_node_id) - , response(arg_response) +public: + ServiceCallResult(Status arg_status, ServiceCallID arg_call_id, ResponseFieldType& arg_response) + : status_(arg_status) + , call_id_(arg_call_id) + , response_(arg_response) { - UAVCAN_ASSERT(server_node_id.isUnicast()); - UAVCAN_ASSERT((status == Success) || (status == ErrorTimeout)); + UAVCAN_ASSERT(call_id_.isValid()); + UAVCAN_ASSERT((status_ == Success) || (status_ == ErrorTimeout)); } /** * Shortcut to quickly check whether the call was successful. */ - bool isSuccessful() const { return status == Success; } + bool isSuccessful() const { return status_ == Success; } + + Status getStatus() const { return status_; } + + ServiceCallID getCallID() const { return call_id_; } + + const ResponseFieldType& getResponse() const { return response_; } + ResponseFieldType& getResponse() { return response_; } }; /** @@ -67,10 +105,11 @@ static Stream& operator<<(Stream& s, const ServiceCallResult& scr) { s << "# Service call result [" << DataType::getDataTypeFullName() << "] " << (scr.isSuccessful() ? "OK" : "FAILURE") - << " server_node_id=" << int(scr.server_node_id.get()) << "\n"; + << " server_node_id=" << int(scr.getCallID().server_node_id.get()) + << " tid=" << int(scr.getCallID().transfer_id.get()) << "\n"; if (scr.isSuccessful()) { - s << scr.response; + s << scr.getResponse(); } else { @@ -82,42 +121,64 @@ static Stream& operator<<(Stream& s, const ServiceCallResult& scr) /** * Do not use directly. */ -class ServiceClientBase : protected DeadlineHandler +class ServiceClientBase : public ITransferAcceptanceFilter, Noncopyable { const DataTypeDescriptor* data_type_descriptor_; ///< This will be initialized at the time of first call protected: - MonotonicDuration request_timeout_; - bool pending_; + class CallState : DeadlineHandler + { + ServiceClientBase& owner_; + const ServiceCallID id_; - explicit ServiceClientBase(INode& node) - : DeadlineHandler(node.getScheduler()) - , data_type_descriptor_(NULL) + virtual void handleDeadline(MonotonicTime); + + public: + CallState(INode& node, ServiceClientBase& owner, ServiceCallID call_id) + : DeadlineHandler(node.getScheduler()) + , owner_(owner) + , id_(call_id) + { + UAVCAN_ASSERT(id_.isValid()); + DeadlineHandler::startWithDelay(owner_.request_timeout_); + } + + bool doesMatch(ServiceCallID call_id) const { return call_id == id_; } + + bool operator==(const CallState& rhs) const + { + return (&owner_ == &rhs.owner_) && (id_ == rhs.id_); + } + }; + + struct CallStateMatchingPredicate + { + const ServiceCallID id; + CallStateMatchingPredicate(ServiceCallID reference) : id(reference) { } + bool operator()(const CallState& state) const { return state.doesMatch(id); } + }; + + MonotonicDuration request_timeout_; + + ServiceClientBase() + : data_type_descriptor_(NULL) , request_timeout_(getDefaultRequestTimeout()) - , pending_(false) { } virtual ~ServiceClientBase() { } - int prepareToCall(INode& node, const char* dtname, NodeID server_node_id, TransferID& out_transfer_id); + int prepareToCall(INode& node, const char* dtname, NodeID server_node_id, ServiceCallID& out_call_id); + + virtual void handleTimeout(ServiceCallID call_id) = 0; public: - /** - * Returns true if the service call is currently in progress. - */ - bool isPending() const { return pending_; } - /** * It's not recommended to override default timeouts. + * Change of this value will not affect pending calls. */ static MonotonicDuration getDefaultRequestTimeout() { return MonotonicDuration::fromMSec(500); } static MonotonicDuration getMinRequestTimeout() { return MonotonicDuration::fromMSec(10); } static MonotonicDuration getMaxRequestTimeout() { return MonotonicDuration::fromMSec(60000); } - - /** - * Returns the service response waiting deadline, if pending. - */ - using DeadlineHandler::getDeadline; }; /** @@ -132,14 +193,17 @@ public: */ template = UAVCAN_CPP11 - typename Callback_ = std::function&)> + typename Callback_ = std::function&)>, #else - typename Callback_ = void (*)(const ServiceCallResult&) + typename Callback_ = void (*)(const ServiceCallResult&), #endif + unsigned NumStaticCalls_ = 1 > class UAVCAN_EXPORT ServiceClient - : public GenericSubscriber::Type > + : public GenericSubscriber::Type> , public ServiceClientBase { public: @@ -149,22 +213,32 @@ public: typedef ServiceCallResult ServiceCallResultType; typedef Callback_ Callback; + enum { NumStaticCalls = NumStaticCalls_ }; + private: typedef ServiceClient SelfType; typedef GenericPublisher PublisherType; - typedef typename ServiceResponseTransferListenerInstantiationHelper::Type TransferListenerType; + typedef typename ServiceResponseTransferListenerInstantiationHelper::Type + TransferListenerType; typedef GenericSubscriber SubscriberType; +#if 0 + typedef Multiset CallRegistry; + CallRegistry call_registry_; +#endif + PublisherType publisher_; Callback callback_; - bool isCallbackValid() const { return try_implicit_cast(callback_, true); } + virtual bool shouldAcceptFrame(const RxFrame& frame) const; // Called from the transfer listener void invokeCallback(ServiceCallResultType& result); virtual void handleReceivedDataStruct(ReceivedDataStructure& response); - virtual void handleDeadline(MonotonicTime); + virtual void handleTimeout(ServiceCallID call_id); + + int addCallState(ServiceCallID call_id); public: /** @@ -173,7 +247,6 @@ public: */ explicit ServiceClient(INode& node, const Callback& callback = Callback()) : SubscriberType(node) - , ServiceClientBase(node) , publisher_(node, getDefaultRequestTimeout()) , callback_(callback) { @@ -183,7 +256,7 @@ public: #endif } - virtual ~ServiceClient() { cancel(); } + virtual ~ServiceClient() { cancelAll(); } /** * Shall be called before first use. @@ -202,18 +275,24 @@ public: * Note that the callback will ALWAYS be called even if the service call has timed out; the * actual result of the call (success/failure) will be passed to the callback as well. * - * If this client instance is already pending service response, it will be cancelled and the new - * call will be performed. - * * Returns negative error code. */ int call(NodeID server_node_id, const RequestType& request); /** - * Cancel the pending service call. - * Does nothing if it is not pending. + * Same as plain @ref call() above, but this overload also returns the call ID of the new call. */ - void cancel(); + int call(NodeID server_node_id, const RequestType& request, ServiceCallID& out_call_id); + + /** + * Cancels certain call referred via call ID structure. + */ + void cancel(ServiceCallID call_id); + + /** + * Cancels all pending calls. + */ + void cancelAll(); /** * Service response callback must be set prior service call. @@ -221,6 +300,16 @@ public: const Callback& getCallback() const { return callback_; } void setCallback(const Callback& cb) { callback_ = cb; } +#if 0 + unsigned getNumPendingCalls() const { return call_registry_.getSize(); } +#endif + +#if 0 + bool hasPendingCalls() const { return !call_registry_.isEmpty(); } +#else + bool hasPendingCalls() const { return false; } +#endif + /** * Returns the number of failed attempts to decode received response. Generally, a failed attempt means either: * - Transient failure in the transport layer. @@ -246,10 +335,10 @@ public: // ---------------------------------------------------------------------------- -template -void ServiceClient::invokeCallback(ServiceCallResultType& result) +template +void ServiceClient::invokeCallback(ServiceCallResultType& result) { - if (isCallbackValid()) + if (try_implicit_cast(callback_, true)) { callback_(result); } @@ -259,55 +348,86 @@ void ServiceClient::invokeCallback(ServiceCallResultType& } } -template -void ServiceClient::handleReceivedDataStruct(ReceivedDataStructure& response) +template +bool ServiceClient::shouldAcceptFrame(const RxFrame& frame) const +{ + UAVCAN_ASSERT(frame.getTransferType() == TransferTypeServiceResponse); // Other types filtered out by dispatcher + +#if 0 + return call_registry_.findFirst(CallStateMatchingPredicate(ServiceCallID(frame.getSrcNodeID(), + frame.getTransferID()))) != NULL; +#else + (void)frame; + return false; +#endif +} + +template +void ServiceClient:: +handleReceivedDataStruct(ReceivedDataStructure& response) { UAVCAN_ASSERT(response.getTransferType() == TransferTypeServiceResponse); - const TransferListenerType* const listener = SubscriberType::getTransferListener(); - if (listener) - { - const typename TransferListenerType::ExpectedResponseParams erp = listener->getExpectedResponseParams(); - ServiceCallResultType result(ServiceCallResultType::Success, erp.src_node_id, response); - cancel(); - invokeCallback(result); - } - else - { - UAVCAN_ASSERT(0); - cancel(); - } + + ServiceCallID call_id(response.getSrcNodeID(), response.getTransferID()); + cancel(call_id); + ServiceCallResultType result(ServiceCallResultType::Success, call_id, response); // Mutable! + invokeCallback(result); } -template -void ServiceClient::handleDeadline(MonotonicTime) -{ - const TransferListenerType* const listener = SubscriberType::getTransferListener(); - if (listener) - { - const typename TransferListenerType::ExpectedResponseParams erp = listener->getExpectedResponseParams(); - ReceivedDataStructure& ref = SubscriberType::getReceivedStructStorage(); - ServiceCallResultType result(ServiceCallResultType::ErrorTimeout, erp.src_node_id, ref); - UAVCAN_TRACE("ServiceClient", "Timeout from nid=%i, dtname=%s", - erp.src_node_id.get(), DataType::getDataTypeFullName()); - cancel(); - invokeCallback(result); - } - else - { - UAVCAN_ASSERT(0); - cancel(); - } +template +void ServiceClient::handleTimeout(ServiceCallID call_id) +{ + cancel(call_id); + ServiceCallResultType result(ServiceCallResultType::ErrorTimeout, call_id, + SubscriberType::getReceivedStructStorage()); // Mutable! + invokeCallback(result); } -template -int ServiceClient::call(NodeID server_node_id, const RequestType& request) +template +int ServiceClient::addCallState(ServiceCallID call_id) { - cancel(); - if (!isCallbackValid()) +#if 0 + if (call_registry_.isEmpty()) + { + const int subscriber_res = SubscriberType::startAsServiceResponseListener(); + if (subscriber_res < 0) + { + UAVCAN_TRACE("ServiceClient", "Failed to start the subscriber, error: %i", subscriber_res); + return subscriber_res; + } + } + + if (call_registry_.add(CallState(SubscriberType::getNode(), *this, call_id)) == NULL) + { + SubscriberType::stop(); + return -ErrMemory; + } + + return 0; +#else + (void)call_id; + return 0; +#endif +} + +template +int ServiceClient::call(NodeID server_node_id, + const RequestType& request) +{ + ServiceCallID dummy; + return call(server_node_id, request, dummy); +} + +template +int ServiceClient::call(NodeID server_node_id, + const RequestType& request, + ServiceCallID& out_call_id) +{ + if (!try_implicit_cast(callback_, true)) { UAVCAN_TRACE("ServiceClient", "Invalid callback"); - return -ErrInvalidParam; + return -ErrInvalidConfiguration; } /* @@ -315,37 +435,35 @@ int ServiceClient::call(NodeID server_node_id, const Reque */ TransferID transfer_id; const int prep_res = - prepareToCall(SubscriberType::getNode(), DataType::getDataTypeFullName(), server_node_id, transfer_id); + prepareToCall(SubscriberType::getNode(), DataType::getDataTypeFullName(), server_node_id, out_call_id); if (prep_res < 0) { UAVCAN_TRACE("ServiceClient", "Failed to prepare the call, error: %i", prep_res); - cancel(); return prep_res; } /* - * Starting the subscriber + * Initializing the call state - this will start the subscriber ad-hoc */ - const int subscriber_res = SubscriberType::startAsServiceResponseListener(); - if (subscriber_res < 0) + const int call_state_res = addCallState(out_call_id); + if (call_state_res < 0) { - UAVCAN_TRACE("ServiceClient", "Failed to start the subscriber, error: %i", subscriber_res); - cancel(); - return subscriber_res; + UAVCAN_TRACE("ServiceClient", "Failed to add call state, error: %i", call_state_res); + return call_state_res; } /* - * Configuring the listener so it will accept only the matching response + * Configuring the listener so it will accept only the matching responses + * TODO move to init(), but this requires to somewhat refactor GenericSubscriber<> (remove TransferForwarder) */ TransferListenerType* const tl = SubscriberType::getTransferListener(); if (tl == NULL) { UAVCAN_ASSERT(0); // Must have been created - cancel(); + cancel(out_call_id); return -ErrLogic; } - const typename TransferListenerType::ExpectedResponseParams erp(server_node_id, transfer_id); - tl->setExpectedResponseParams(erp); + tl->installAcceptanceFilter(this); /* * Publishing the request @@ -353,22 +471,34 @@ int ServiceClient::call(NodeID server_node_id, const Reque const int publisher_res = publisher_.publish(request, TransferTypeServiceRequest, server_node_id, transfer_id); if (publisher_res < 0) { - cancel(); + cancel(out_call_id); + return publisher_res; } - return publisher_res; + + return 0; } -template -void ServiceClient::cancel() +template +void ServiceClient::cancel(ServiceCallID call_id) { - pending_ = false; - SubscriberType::stop(); - DeadlineHandler::stop(); - TransferListenerType* const tl = SubscriberType::getTransferListener(); - if (tl) +#if 0 + call_registry_.remove(call_id); + if (call_registry_.isEmpty()) { - tl->stopAcceptingAnything(); + SubscriberType::stop(); } +#else + (void)call_id; +#endif +} + +template +void ServiceClient::cancelAll() +{ +#if 0 + call_registry_.removeAll(); +#endif + SubscriberType::stop(); } } diff --git a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp index 17887557a3..634bd002bd 100644 --- a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp +++ b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/distributed/raft_core.hpp @@ -327,9 +327,9 @@ class RaftCore : private TimerBase for (uint8_t i = 0; i < NumRequestVoteClients; i++) { - request_vote_clients_[i]->cancel(); + request_vote_clients_[i]->cancelAll(); // TODO FIXME Concurrent calls!! } - append_entries_client_.cancel(); + append_entries_client_.cancelAll(); /* * Calling the switch handler @@ -550,22 +550,23 @@ class RaftCore : private TimerBase return; } - if (result.response.term > persistent_state_.getCurrentTerm()) + if (result.getResponse().term > persistent_state_.getCurrentTerm()) { - tryIncrementCurrentTermFromResponse(result.response.term); + tryIncrementCurrentTermFromResponse(result.getResponse().term); } else { - if (result.response.success) + if (result.getResponse().success) { - cluster_.incrementServerNextIndexBy(result.server_node_id, pending_append_entries_fields_.num_entries); - cluster_.setServerMatchIndex(result.server_node_id, + cluster_.incrementServerNextIndexBy(result.getCallID().server_node_id, + pending_append_entries_fields_.num_entries); + cluster_.setServerMatchIndex(result.getCallID().server_node_id, Log::Index(pending_append_entries_fields_.prev_log_index + pending_append_entries_fields_.num_entries)); } else { - cluster_.decrementServerNextIndex(result.server_node_id); + cluster_.decrementServerNextIndex(result.getCallID().server_node_id); } } @@ -654,15 +655,15 @@ class RaftCore : private TimerBase return; } - trace(TraceRaftVoteRequestSucceeded, result.server_node_id.get()); + trace(TraceRaftVoteRequestSucceeded, result.getCallID().server_node_id.get()); - if (result.response.term > persistent_state_.getCurrentTerm()) + if (result.getResponse().term > persistent_state_.getCurrentTerm()) { - tryIncrementCurrentTermFromResponse(result.response.term); + tryIncrementCurrentTermFromResponse(result.getResponse().term); } else { - if (result.response.vote_granted) + if (result.getResponse().vote_granted) { num_votes_received_in_this_campaign_++; } diff --git a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp index 6d351d6176..74280ac9f4 100644 --- a/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp +++ b/libuavcan/include/uavcan/protocol/dynamic_node_id_server/node_discoverer.hpp @@ -235,14 +235,14 @@ class NodeDiscoverer : TimerBase if (result.isSuccessful()) { UAVCAN_TRACE("dynamic_node_id_server::NodeDiscoverer", "GetNodeInfo response from %d", - int(result.server_node_id.get())); - finalizeNodeDiscovery(&result.response.hardware_version.unique_id, result.server_node_id); + int(result.getCallID().server_node_id.get())); + finalizeNodeDiscovery(&result.getResponse().hardware_version.unique_id, result.getCallID().server_node_id); } else { - trace(TraceDiscoveryGetNodeInfoFailure, result.server_node_id.get()); + trace(TraceDiscoveryGetNodeInfoFailure, result.getCallID().server_node_id.get()); - NodeData* const data = node_map_.access(result.server_node_id); + NodeData* const data = node_map_.access(result.getCallID().server_node_id); if (data == NULL) { return; // Probably it is a known node now @@ -250,11 +250,11 @@ class NodeDiscoverer : TimerBase UAVCAN_TRACE("dynamic_node_id_server::NodeDiscoverer", "GetNodeInfo request to %d has timed out, %d attempts", - int(result.server_node_id.get()), int(data->num_get_node_info_attempts)); + int(result.getCallID().server_node_id.get()), int(data->num_get_node_info_attempts)); data->num_get_node_info_attempts++; if (data->num_get_node_info_attempts >= MaxAttemptsToGetNodeInfo) { - finalizeNodeDiscovery(NULL, result.server_node_id); + finalizeNodeDiscovery(NULL, result.getCallID().server_node_id); // Warning: data pointer is invalidated now } } @@ -262,7 +262,7 @@ class NodeDiscoverer : TimerBase void handleTimerEvent(const TimerEvent&) { - if (get_node_info_client_.isPending()) + if (get_node_info_client_.hasPendingCalls()) { return; } diff --git a/libuavcan/include/uavcan/protocol/network_compat_checker.hpp b/libuavcan/include/uavcan/protocol/network_compat_checker.hpp index ca6635451a..3ac0ae0264 100644 --- a/libuavcan/include/uavcan/protocol/network_compat_checker.hpp +++ b/libuavcan/include/uavcan/protocol/network_compat_checker.hpp @@ -87,7 +87,7 @@ class UAVCAN_EXPORT NetworkCompatibilityChecker : Noncopyable int waitForCATSResponse() { - while (cats_cln_.isPending()) + while (cats_cln_.hasPendingCalls()) { const int res = getNode().spin(MonotonicDuration::fromMSec(10)); if (res < 0 || !result_.isOk()) @@ -119,15 +119,15 @@ class UAVCAN_EXPORT NetworkCompatibilityChecker : Noncopyable if (last_cats_request_ok_) { const DataTypeSignature sign = GlobalDataTypeRegistry::instance(). - computeAggregateSignature(checking_dtkind_, resp.response.mutually_known_ids); + computeAggregateSignature(checking_dtkind_, resp.getResponse().mutually_known_ids); UAVCAN_TRACE("NodeInitializer", "CATS response from nid=%i; local=%llu remote=%llu", - int(resp.server_node_id.get()), static_cast(sign.get()), - static_cast(resp.response.aggregate_signature)); + int(resp.getCallID().server_node_id.get()), static_cast(sign.get()), + static_cast(resp.getResponse().aggregate_signature)); - if (sign.get() != resp.response.aggregate_signature) + if (sign.get() != resp.getResponse().aggregate_signature) { - result_.conflicting_node = resp.server_node_id; + result_.conflicting_node = resp.getCallID().server_node_id; } } } @@ -138,7 +138,7 @@ class UAVCAN_EXPORT NetworkCompatibilityChecker : Noncopyable StaticAssert::check(); UAVCAN_ASSERT(nid.isUnicast()); - UAVCAN_ASSERT(!cats_cln_.isPending()); + UAVCAN_ASSERT(!cats_cln_.hasPendingCalls()); checking_dtkind_ = kind; protocol::ComputeAggregateTypeSignature::Request request; @@ -253,7 +253,7 @@ public: exit: ns_sub_.stop(); - cats_cln_.cancel(); + cats_cln_.cancelAll(); return res; } diff --git a/libuavcan/include/uavcan/protocol/node_info_retriever.hpp b/libuavcan/include/uavcan/protocol/node_info_retriever.hpp index 4c4245b6c8..c2656b645c 100644 --- a/libuavcan/include/uavcan/protocol/node_info_retriever.hpp +++ b/libuavcan/include/uavcan/protocol/node_info_retriever.hpp @@ -196,9 +196,9 @@ private: virtual void handleTimerEvent(const TimerEvent&) { - if (get_node_info_client_.isPending()) // If request is pending, this condition will fail every second time + if (get_node_info_client_.hasPendingCalls()) // If request is pending, this condition will fail every second time { - return; + return; // TODO FIXME Concurrent calls!! } const NodeID next = pickNextNodeToQuery(); @@ -277,7 +277,7 @@ private: void handleGetNodeInfoResponse(const ServiceCallResult& result) { - Entry& entry = getEntry(result.server_node_id); + Entry& entry = getEntry(result.getCallID().server_node_id); if (result.isSuccessful()) { @@ -285,9 +285,10 @@ private: * Updating the uptime here allows to properly handle a corner case where the service response arrives * after the device has restarted and published its new NodeStatus (although it's unlikely to happen). */ - entry.uptime_sec = result.response.status.uptime_sec; + entry.uptime_sec = result.getResponse().status.uptime_sec; entry.request_needed = false; - listeners_.removeWhere(NodeInfoRetrievedHandlerCaller(result.server_node_id, result.response)); + listeners_.removeWhere(NodeInfoRetrievedHandlerCaller(result.getCallID().server_node_id, + result.getResponse())); } else { @@ -298,7 +299,7 @@ private: { entry.request_needed = false; listeners_.removeWhere(GenericHandlerCaller(&INodeInfoListener::handleNodeInfoUnavailable, - result.server_node_id)); + result.getCallID().server_node_id)); } } } diff --git a/libuavcan/include/uavcan/transport/transfer_listener.hpp b/libuavcan/include/uavcan/transport/transfer_listener.hpp index dc0bc66ad2..62be67d64b 100644 --- a/libuavcan/include/uavcan/transport/transfer_listener.hpp +++ b/libuavcan/include/uavcan/transport/transfer_listener.hpp @@ -178,88 +178,71 @@ public: virtual ~TransferListener() { - // Map must be cleared before bufmgr is destructed + // Map must be cleared before bufmgr is destroyed receivers_.removeAll(); } }; +/** + * This class is used by transfer listener to decide if the frame should be accepted or ignored. + */ +class ITransferAcceptanceFilter +{ +public: + /** + * If it returns false, the frame will be ignored, otherwise accepted. + */ + virtual bool shouldAcceptFrame(const RxFrame& frame) const = 0; + + virtual ~ITransferAcceptanceFilter() { } +}; + /** * This class should be derived by callers. */ template -class UAVCAN_EXPORT ServiceResponseTransferListener - : public TransferListener +class UAVCAN_EXPORT TransferListenerWithFilter : public TransferListener { + const ITransferAcceptanceFilter* filter_; + + virtual void handleFrame(const RxFrame& frame); + public: typedef TransferListener BaseType; - struct ExpectedResponseParams - { - NodeID src_node_id; - TransferID transfer_id; - - ExpectedResponseParams() - { - UAVCAN_ASSERT(!src_node_id.isValid()); - } - - ExpectedResponseParams(NodeID arg_src_node_id, TransferID arg_transfer_id) - : src_node_id(arg_src_node_id) - , transfer_id(arg_transfer_id) - { - UAVCAN_ASSERT(src_node_id.isUnicast()); - } - - bool match(const RxFrame& frame) const - { - UAVCAN_ASSERT(frame.getTransferType() == TransferTypeServiceResponse); - return (frame.getSrcNodeID() == src_node_id) && (frame.getTransferID() == transfer_id); - } - }; - -private: - ExpectedResponseParams response_params_; - - void handleFrame(const RxFrame& frame); - -public: - ServiceResponseTransferListener(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, - IPoolAllocator& allocator) + TransferListenerWithFilter(TransferPerfCounter& perf, const DataTypeDescriptor& data_type, + IPoolAllocator& allocator) : BaseType(perf, data_type, allocator) + , filter_(NULL) { } - void setExpectedResponseParams(const ExpectedResponseParams& erp); - - const ExpectedResponseParams& getExpectedResponseParams() const { return response_params_; } - - void stopAcceptingAnything(); + void installAcceptanceFilter(const ITransferAcceptanceFilter* acceptance_filter) + { + UAVCAN_ASSERT(filter_ == NULL); + filter_ = acceptance_filter; + UAVCAN_ASSERT(filter_ != NULL); + } }; // ---------------------------------------------------------------------------- /* - * ServiceResponseTransferListener<> + * TransferListenerWithFilter<> */ template -void ServiceResponseTransferListener::handleFrame(const RxFrame& frame) +void TransferListenerWithFilter::handleFrame(const RxFrame& frame) { - if (response_params_.match(frame)) + if (filter_ != NULL) { - BaseType::handleFrame(frame); + if (filter_->shouldAcceptFrame(frame)) + { + BaseType::handleFrame(frame); + } + } + else + { + UAVCAN_ASSERT(0); } -} - -template -void ServiceResponseTransferListener::setExpectedResponseParams( - const ExpectedResponseParams& erp) -{ - response_params_ = erp; -} - -template -void ServiceResponseTransferListener::stopAcceptingAnything() -{ - response_params_ = ExpectedResponseParams(); } } diff --git a/libuavcan/include/uavcan/util/multiset.hpp b/libuavcan/include/uavcan/util/multiset.hpp index 311f4775f0..7cce1e2a17 100644 --- a/libuavcan/include/uavcan/util/multiset.hpp +++ b/libuavcan/include/uavcan/util/multiset.hpp @@ -146,6 +146,7 @@ public: /** * Does nothing if there's no such item. + * Only the first matching item will be removed. */ void remove(const T& item); diff --git a/libuavcan/src/node/uc_service_client.cpp b/libuavcan/src/node/uc_service_client.cpp index a0c6cc00e8..f686db20c5 100644 --- a/libuavcan/src/node/uc_service_client.cpp +++ b/libuavcan/src/node/uc_service_client.cpp @@ -6,12 +6,26 @@ namespace uavcan { - -int ServiceClientBase::prepareToCall(INode& node, const char* dtname, NodeID server_node_id, - TransferID& out_transfer_id) +/* + * ServiceClientBase::CallState + */ +void ServiceClientBase::CallState::handleDeadline(MonotonicTime) { - pending_ = true; + UAVCAN_ASSERT(id_.isValid()); + UAVCAN_TRACE("ServiceClient", "Timeout from nid=%d, tid=%d, dtname=%s", + int(id_.server_node_id.get()), int(id_.transfer_id.get()), + (owner_.data_type_descriptor_ == NULL) ? "???" : owner_.data_type_descriptor_->getFullName()); + owner_.handleTimeout(id_); +} +/* + * ServiceClientBase + */ +int ServiceClientBase::prepareToCall(INode& node, + const char* dtname, + NodeID server_node_id, + ServiceCallID& out_call_id) +{ /* * Making sure we're not going to get transport error because of invalid input data */ @@ -20,6 +34,7 @@ int ServiceClientBase::prepareToCall(INode& node, const char* dtname, NodeID ser UAVCAN_TRACE("ServiceClient", "Invalid Server Node ID"); return -ErrInvalidParam; } + out_call_id.server_node_id = server_node_id; /* * Determining the Data Type ID @@ -50,13 +65,9 @@ int ServiceClientBase::prepareToCall(INode& node, const char* dtname, NodeID ser UAVCAN_TRACE("ServiceClient", "OTR access failure, dtd=%s", data_type_descriptor_->toString().c_str()); return -ErrMemory; } - out_transfer_id = *otr_tid; + out_call_id.transfer_id = *otr_tid; otr_tid->increment(); - /* - * Registering the deadline handler - */ - DeadlineHandler::startWithDelay(request_timeout_); return 0; } diff --git a/libuavcan/test/node/service_client.cpp b/libuavcan/test/node/service_client.cpp index f668f791d5..fa08b7214a 100644 --- a/libuavcan/test/node/service_client.cpp +++ b/libuavcan/test/node/service_client.cpp @@ -26,9 +26,9 @@ struct ServiceCallResultHandler void handleResponse(const uavcan::ServiceCallResult& result) { std::cout << result << std::endl; - last_status = result.status; - last_response = result.response; - last_server_node_id = result.server_node_id; + last_status = result.getStatus(); + last_response = result.getResponse(); + last_server_node_id = result.getCallID().server_node_id; } bool match(StatusType status, uavcan::NodeID server_node_id, const typename DataType::Response& response) const @@ -111,17 +111,17 @@ TEST(ServiceClient, Basic) ASSERT_EQ(3, nodes.b.getDispatcher().getNumServiceResponseListeners()); // Listening now! - ASSERT_TRUE(client1.isPending()); - ASSERT_TRUE(client2.isPending()); - ASSERT_TRUE(client3.isPending()); + ASSERT_TRUE(client1.hasPendingCalls()); + ASSERT_TRUE(client2.hasPendingCalls()); + ASSERT_TRUE(client3.hasPendingCalls()); nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(20)); ASSERT_EQ(1, nodes.b.getDispatcher().getNumServiceResponseListeners()); // Third is still listening! - ASSERT_FALSE(client1.isPending()); - ASSERT_FALSE(client2.isPending()); - ASSERT_TRUE(client3.isPending()); + ASSERT_FALSE(client1.hasPendingCalls()); + ASSERT_FALSE(client2.hasPendingCalls()); + ASSERT_TRUE(client3.hasPendingCalls()); // Validating root_ns_a::StringService::Response expected_response; @@ -130,9 +130,9 @@ TEST(ServiceClient, Basic) nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(200)); - ASSERT_FALSE(client1.isPending()); - ASSERT_FALSE(client2.isPending()); - ASSERT_FALSE(client3.isPending()); + ASSERT_FALSE(client1.hasPendingCalls()); + ASSERT_FALSE(client2.hasPendingCalls()); + ASSERT_FALSE(client3.hasPendingCalls()); ASSERT_EQ(0, nodes.b.getDispatcher().getNumServiceResponseListeners()); // Third has timed out :( @@ -141,7 +141,7 @@ TEST(ServiceClient, Basic) // Stray request ASSERT_LT(0, client3.call(99, request)); // Will timeout! - ASSERT_TRUE(client3.isPending()); + ASSERT_TRUE(client3.hasPendingCalls()); ASSERT_EQ(1, nodes.b.getDispatcher().getNumServiceResponseListeners()); } @@ -178,10 +178,10 @@ TEST(ServiceClient, Rejection) ASSERT_LT(0, client1.call(1, request)); ASSERT_EQ(1, nodes.b.getDispatcher().getNumServiceResponseListeners()); - ASSERT_TRUE(client1.isPending()); + ASSERT_TRUE(client1.hasPendingCalls()); nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(200)); - ASSERT_FALSE(client1.isPending()); + ASSERT_FALSE(client1.hasPendingCalls()); ASSERT_EQ(0, nodes.b.getDispatcher().getNumServiceResponseListeners()); // Timed out diff --git a/libuavcan/test/protocol/data_type_info_provider.cpp b/libuavcan/test/protocol/data_type_info_provider.cpp index f987b3080b..1780d5f801 100644 --- a/libuavcan/test/protocol/data_type_info_provider.cpp +++ b/libuavcan/test/protocol/data_type_info_provider.cpp @@ -31,29 +31,29 @@ static bool validateDataTypeInfoResponse(const std::auto_ptrresponse.name != DataType::getDataTypeFullName()) + if (resp->getResponse().name != DataType::getDataTypeFullName()) { std::cout << "Type name mismatch: '" - << resp->response.name.c_str() << "' '" + << resp->getResponse().name.c_str() << "' '" << DataType::getDataTypeFullName() << "'" << std::endl; return false; } - if (DataType::getDataTypeSignature().get() != resp->response.signature) + if (DataType::getDataTypeSignature().get() != resp->getResponse().signature) { std::cout << "Signature mismatch" << std::endl; return false; } - if (resp->response.mask != mask) + if (resp->getResponse().mask != mask) { std::cout << "Mask mismatch" << std::endl; return false; } - if (resp->response.kind.value != DataType::DataTypeKind) + if (resp->getResponse().kind.value != DataType::DataTypeKind) { std::cout << "Kind mismatch" << std::endl; return false; } - if (resp->response.id != DataType::DefaultDataTypeID) + if (resp->getResponse().id != DataType::DefaultDataTypeID) { std::cout << "DTID mismatch" << std::endl; return false; @@ -90,7 +90,7 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(validateDataTypeInfoResponse(gdti_cln.collector.result, GetDataTypeInfo::Response::MASK_KNOWN | GetDataTypeInfo::Response::MASK_SERVING)); - ASSERT_EQ(1, gdti_cln.collector.result->server_node_id.get()); + ASSERT_EQ(1, gdti_cln.collector.result->getCallID().server_node_id.get()); /* * GetDataTypeInfo request for GetDataTypeInfo by name @@ -105,7 +105,7 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(validateDataTypeInfoResponse(gdti_cln.collector.result, GetDataTypeInfo::Response::MASK_KNOWN | GetDataTypeInfo::Response::MASK_SERVING)); - ASSERT_EQ(1, gdti_cln.collector.result->server_node_id.get()); + ASSERT_EQ(1, gdti_cln.collector.result->getCallID().server_node_id.get()); /* * GetDataTypeInfo request for NodeStatus - not used yet @@ -148,11 +148,11 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(gdti_cln.collector.result.get()); ASSERT_TRUE(gdti_cln.collector.result->isSuccessful()); - ASSERT_EQ(1, gdti_cln.collector.result->server_node_id.get()); - ASSERT_EQ(0, gdti_cln.collector.result->response.mask); - ASSERT_TRUE(gdti_cln.collector.result->response.name.empty()); // Empty name - ASSERT_EQ(gdti_request.id, gdti_cln.collector.result->response.id); - ASSERT_EQ(gdti_request.kind.value, gdti_cln.collector.result->response.kind.value); + ASSERT_EQ(1, gdti_cln.collector.result->getCallID().server_node_id.get()); + ASSERT_EQ(0, gdti_cln.collector.result->getResponse().mask); + ASSERT_TRUE(gdti_cln.collector.result->getResponse().name.empty()); // Empty name + ASSERT_EQ(gdti_request.id, gdti_cln.collector.result->getResponse().id); + ASSERT_EQ(gdti_request.kind.value, gdti_cln.collector.result->getResponse().kind.value); /* * Requesting a non-existent type by name @@ -166,11 +166,11 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(gdti_cln.collector.result.get()); ASSERT_TRUE(gdti_cln.collector.result->isSuccessful()); - ASSERT_EQ(1, gdti_cln.collector.result->server_node_id.get()); - ASSERT_EQ(0, gdti_cln.collector.result->response.mask); - ASSERT_EQ("uavcan.equipment.gnss.Fix", gdti_cln.collector.result->response.name); - ASSERT_EQ(0, gdti_cln.collector.result->response.id); - ASSERT_EQ(0, gdti_cln.collector.result->response.kind.value); + ASSERT_EQ(1, gdti_cln.collector.result->getCallID().server_node_id.get()); + ASSERT_EQ(0, gdti_cln.collector.result->getResponse().mask); + ASSERT_EQ("uavcan.equipment.gnss.Fix", gdti_cln.collector.result->getResponse().name); + ASSERT_EQ(0, gdti_cln.collector.result->getResponse().id); + ASSERT_EQ(0, gdti_cln.collector.result->getResponse().kind.value); /* * ComputeAggregateTypeSignature test for messages @@ -184,12 +184,12 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(cats_cln.collector.result.get()); ASSERT_TRUE(cats_cln.collector.result->isSuccessful()); - ASSERT_EQ(1, cats_cln.collector.result->server_node_id.get()); - ASSERT_EQ(NodeStatus::getDataTypeSignature().get(), cats_cln.collector.result->response.aggregate_signature); - ASSERT_EQ(2048, cats_cln.collector.result->response.mutually_known_ids.size()); - ASSERT_TRUE(cats_cln.collector.result->response.mutually_known_ids[NodeStatus::DefaultDataTypeID]); - cats_cln.collector.result->response.mutually_known_ids[NodeStatus::DefaultDataTypeID] = false; - ASSERT_FALSE(cats_cln.collector.result->response.mutually_known_ids.any()); + ASSERT_EQ(1, cats_cln.collector.result->getCallID().server_node_id.get()); + ASSERT_EQ(NodeStatus::getDataTypeSignature().get(), cats_cln.collector.result->getResponse().aggregate_signature); + ASSERT_EQ(2048, cats_cln.collector.result->getResponse().mutually_known_ids.size()); + ASSERT_TRUE(cats_cln.collector.result->getResponse().mutually_known_ids[NodeStatus::DefaultDataTypeID]); + cats_cln.collector.result->getResponse().mutually_known_ids[NodeStatus::DefaultDataTypeID] = false; + ASSERT_FALSE(cats_cln.collector.result->getResponse().mutually_known_ids.any()); /* * ComputeAggregateTypeSignature test for services @@ -203,13 +203,13 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(cats_cln.collector.result.get()); ASSERT_TRUE(cats_cln.collector.result->isSuccessful()); - ASSERT_EQ(1, cats_cln.collector.result->server_node_id.get()); - ASSERT_EQ(512, cats_cln.collector.result->response.mutually_known_ids.size()); - ASSERT_TRUE(cats_cln.collector.result->response.mutually_known_ids[GetDataTypeInfo::DefaultDataTypeID]); - ASSERT_TRUE(cats_cln.collector.result->response.mutually_known_ids[ComputeAggregateTypeSignature::DefaultDataTypeID]); - cats_cln.collector.result->response.mutually_known_ids[GetDataTypeInfo::DefaultDataTypeID] = false; - cats_cln.collector.result->response.mutually_known_ids[ComputeAggregateTypeSignature::DefaultDataTypeID] = false; - ASSERT_FALSE(cats_cln.collector.result->response.mutually_known_ids.any()); + ASSERT_EQ(1, cats_cln.collector.result->getCallID().server_node_id.get()); + ASSERT_EQ(512, cats_cln.collector.result->getResponse().mutually_known_ids.size()); + ASSERT_TRUE(cats_cln.collector.result->getResponse().mutually_known_ids[GetDataTypeInfo::DefaultDataTypeID]); + ASSERT_TRUE(cats_cln.collector.result->getResponse().mutually_known_ids[ComputeAggregateTypeSignature::DefaultDataTypeID]); + cats_cln.collector.result->getResponse().mutually_known_ids[GetDataTypeInfo::DefaultDataTypeID] = false; + cats_cln.collector.result->getResponse().mutually_known_ids[ComputeAggregateTypeSignature::DefaultDataTypeID] = false; + ASSERT_FALSE(cats_cln.collector.result->getResponse().mutually_known_ids.any()); /* * ComputeAggregateTypeSignature test for a non-existent type @@ -221,6 +221,6 @@ TEST(DataTypeInfoProvider, Basic) ASSERT_TRUE(cats_cln.collector.result.get()); ASSERT_TRUE(cats_cln.collector.result->isSuccessful()); - ASSERT_EQ(0, cats_cln.collector.result->response.aggregate_signature); - ASSERT_FALSE(cats_cln.collector.result->response.mutually_known_ids.any()); + ASSERT_EQ(0, cats_cln.collector.result->getResponse().aggregate_signature); + ASSERT_FALSE(cats_cln.collector.result->getResponse().mutually_known_ids.any()); } diff --git a/libuavcan/test/protocol/node_status_provider.cpp b/libuavcan/test/protocol/node_status_provider.cpp index 31c4e5cc20..5c9feacdf6 100644 --- a/libuavcan/test/protocol/node_status_provider.cpp +++ b/libuavcan/test/protocol/node_status_provider.cpp @@ -102,14 +102,15 @@ TEST(NodeStatusProvider, Basic) ASSERT_TRUE(gni_cln.collector.result.get()); // Response must have been delivered ASSERT_TRUE(gni_cln.collector.result->isSuccessful()); - ASSERT_EQ(1, gni_cln.collector.result->server_node_id.get()); + ASSERT_EQ(1, gni_cln.collector.result->getCallID().server_node_id.get()); - ASSERT_EQ(uavcan::protocol::NodeStatus::STATUS_CRITICAL, gni_cln.collector.result->response.status.status_code); + ASSERT_EQ(uavcan::protocol::NodeStatus::STATUS_CRITICAL, + gni_cln.collector.result->getResponse().status.status_code); - ASSERT_TRUE(hwver == gni_cln.collector.result->response.hardware_version); - ASSERT_TRUE(swver == gni_cln.collector.result->response.software_version); + ASSERT_TRUE(hwver == gni_cln.collector.result->getResponse().hardware_version); + ASSERT_TRUE(swver == gni_cln.collector.result->getResponse().software_version); - ASSERT_EQ("superluminal_communication_unit", gni_cln.collector.result->response.name); + ASSERT_EQ("superluminal_communication_unit", gni_cln.collector.result->getResponse().name); /* * GlobalDiscoveryRequest diff --git a/libuavcan/test/protocol/param_server.cpp b/libuavcan/test/protocol/param_server.cpp index 245864a83f..bcc38e1f1d 100644 --- a/libuavcan/test/protocol/param_server.cpp +++ b/libuavcan/test/protocol/param_server.cpp @@ -113,16 +113,16 @@ TEST(ParamServer, Basic) save_erase_rq.opcode = uavcan::protocol::param::ExecuteOpcode::Request::OPCODE_SAVE; doCall(save_erase_cln, save_erase_rq, nodes); ASSERT_TRUE(save_erase_cln.collector.result.get()); - ASSERT_TRUE(save_erase_cln.collector.result->response.ok); + ASSERT_TRUE(save_erase_cln.collector.result->getResponse().ok); save_erase_rq.opcode = uavcan::protocol::param::ExecuteOpcode::Request::OPCODE_ERASE; doCall(save_erase_cln, save_erase_rq, nodes); - ASSERT_TRUE(save_erase_cln.collector.result->response.ok); + ASSERT_TRUE(save_erase_cln.collector.result->getResponse().ok); // Invalid opcode save_erase_rq.opcode = 0xFF; doCall(save_erase_cln, save_erase_rq, nodes); - ASSERT_FALSE(save_erase_cln.collector.result->response.ok); + ASSERT_FALSE(save_erase_cln.collector.result->getResponse().ok); /* * Get/set @@ -131,17 +131,17 @@ TEST(ParamServer, Basic) get_set_rq.name = "nonexistent_parameter"; doCall(get_set_cln, get_set_rq, nodes); ASSERT_TRUE(get_set_cln.collector.result.get()); - ASSERT_TRUE(get_set_cln.collector.result->response.name.empty()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().name.empty()); // No such variable, shall return empty name/value get_set_rq.index = 0; get_set_rq.name.clear(); get_set_rq.value.value_int.push_back(0xDEADBEEF); doCall(get_set_cln, get_set_rq, nodes); - ASSERT_TRUE(get_set_cln.collector.result->response.name.empty()); - ASSERT_TRUE(get_set_cln.collector.result->response.value.value_bool.empty()); - ASSERT_TRUE(get_set_cln.collector.result->response.value.value_int.empty()); - ASSERT_TRUE(get_set_cln.collector.result->response.value.value_float.empty()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().name.empty()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().value.value_bool.empty()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().value.value_int.empty()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().value.value_float.empty()); mgr.kv["foobar"] = 123.456; // New param @@ -149,10 +149,10 @@ TEST(ParamServer, Basic) get_set_rq = uavcan::protocol::param::GetSet::Request(); get_set_rq.name = "foobar"; doCall(get_set_cln, get_set_rq, nodes); - ASSERT_STREQ("foobar", get_set_cln.collector.result->response.name.c_str()); - ASSERT_TRUE(get_set_cln.collector.result->response.value.value_bool.empty()); - ASSERT_TRUE(get_set_cln.collector.result->response.value.value_int.empty()); - ASSERT_FLOAT_EQ(123.456F, get_set_cln.collector.result->response.value.value_float[0]); + ASSERT_STREQ("foobar", get_set_cln.collector.result->getResponse().name.c_str()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().value.value_bool.empty()); + ASSERT_TRUE(get_set_cln.collector.result->getResponse().value.value_int.empty()); + ASSERT_FLOAT_EQ(123.456F, get_set_cln.collector.result->getResponse().value.value_float[0]); // Set by index get_set_rq = uavcan::protocol::param::GetSet::Request(); @@ -163,13 +163,13 @@ TEST(ParamServer, Basic) get_set_rq.value.value_string.push_back(str); } doCall(get_set_cln, get_set_rq, nodes); - ASSERT_STREQ("foobar", get_set_cln.collector.result->response.name.c_str()); - ASSERT_FLOAT_EQ(424242, get_set_cln.collector.result->response.value.value_float[0]); + ASSERT_STREQ("foobar", get_set_cln.collector.result->getResponse().name.c_str()); + ASSERT_FLOAT_EQ(424242, get_set_cln.collector.result->getResponse().value.value_float[0]); // Get by index get_set_rq = uavcan::protocol::param::GetSet::Request(); get_set_rq.index = 0; doCall(get_set_cln, get_set_rq, nodes); - ASSERT_STREQ("foobar", get_set_cln.collector.result->response.name.c_str()); - ASSERT_FLOAT_EQ(424242, get_set_cln.collector.result->response.value.value_float[0]); + ASSERT_STREQ("foobar", get_set_cln.collector.result->getResponse().name.c_str()); + ASSERT_FLOAT_EQ(424242, get_set_cln.collector.result->getResponse().value.value_float[0]); } diff --git a/libuavcan/test/protocol/restart_request_server.cpp b/libuavcan/test/protocol/restart_request_server.cpp index 7b27621bf2..8229ed8ce6 100644 --- a/libuavcan/test/protocol/restart_request_server.cpp +++ b/libuavcan/test/protocol/restart_request_server.cpp @@ -44,7 +44,7 @@ TEST(RestartRequestServer, Basic) ASSERT_TRUE(rrs_cln.collector.result.get()); ASSERT_TRUE(rrs_cln.collector.result->isSuccessful()); - ASSERT_FALSE(rrs_cln.collector.result->response.ok); + ASSERT_FALSE(rrs_cln.collector.result->getResponse().ok); /* * Accepted @@ -57,7 +57,7 @@ TEST(RestartRequestServer, Basic) nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10)); ASSERT_TRUE(rrs_cln.collector.result->isSuccessful()); - ASSERT_TRUE(rrs_cln.collector.result->response.ok); + ASSERT_TRUE(rrs_cln.collector.result->getResponse().ok); /* * Rejected by handler @@ -68,7 +68,7 @@ TEST(RestartRequestServer, Basic) nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10)); ASSERT_TRUE(rrs_cln.collector.result->isSuccessful()); - ASSERT_FALSE(rrs_cln.collector.result->response.ok); + ASSERT_FALSE(rrs_cln.collector.result->getResponse().ok); /* * Rejected because of invalid magic number @@ -79,5 +79,5 @@ TEST(RestartRequestServer, Basic) nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10)); ASSERT_TRUE(rrs_cln.collector.result->isSuccessful()); - ASSERT_FALSE(rrs_cln.collector.result->response.ok); + ASSERT_FALSE(rrs_cln.collector.result->getResponse().ok); } diff --git a/libuavcan/test/protocol/transport_stats_provider.cpp b/libuavcan/test/protocol/transport_stats_provider.cpp index e309977d2f..6c8ed9da69 100644 --- a/libuavcan/test/protocol/transport_stats_provider.cpp +++ b/libuavcan/test/protocol/transport_stats_provider.cpp @@ -28,13 +28,13 @@ TEST(TransportStatsProvider, Basic) ASSERT_TRUE(tsp_cln.collector.result.get()); ASSERT_TRUE(tsp_cln.collector.result->isSuccessful()); - ASSERT_EQ(0, tsp_cln.collector.result->response.transfer_errors); - ASSERT_EQ(1, tsp_cln.collector.result->response.transfers_rx); - ASSERT_EQ(0, tsp_cln.collector.result->response.transfers_tx); - ASSERT_EQ(1, tsp_cln.collector.result->response.can_iface_stats.size()); - ASSERT_EQ(0, tsp_cln.collector.result->response.can_iface_stats[0].errors); - ASSERT_EQ(1, tsp_cln.collector.result->response.can_iface_stats[0].frames_rx); - ASSERT_EQ(0, tsp_cln.collector.result->response.can_iface_stats[0].frames_tx); + ASSERT_EQ(0, tsp_cln.collector.result->getResponse().transfer_errors); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().transfers_rx); + ASSERT_EQ(0, tsp_cln.collector.result->getResponse().transfers_tx); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().can_iface_stats.size()); + ASSERT_EQ(0, tsp_cln.collector.result->getResponse().can_iface_stats[0].errors); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_rx); + ASSERT_EQ(0, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_tx); /* * Second request @@ -43,13 +43,13 @@ TEST(TransportStatsProvider, Basic) ASSERT_LE(0, nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10))); ASSERT_TRUE(tsp_cln.collector.result.get()); - ASSERT_EQ(0, tsp_cln.collector.result->response.transfer_errors); - ASSERT_EQ(2, tsp_cln.collector.result->response.transfers_rx); - ASSERT_EQ(1, tsp_cln.collector.result->response.transfers_tx); - ASSERT_EQ(1, tsp_cln.collector.result->response.can_iface_stats.size()); - ASSERT_EQ(0, tsp_cln.collector.result->response.can_iface_stats[0].errors); - ASSERT_EQ(2, tsp_cln.collector.result->response.can_iface_stats[0].frames_rx); - ASSERT_EQ(6, tsp_cln.collector.result->response.can_iface_stats[0].frames_tx); + ASSERT_EQ(0, tsp_cln.collector.result->getResponse().transfer_errors); + ASSERT_EQ(2, tsp_cln.collector.result->getResponse().transfers_rx); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().transfers_tx); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().can_iface_stats.size()); + ASSERT_EQ(0, tsp_cln.collector.result->getResponse().can_iface_stats[0].errors); + ASSERT_EQ(2, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_rx); + ASSERT_EQ(6, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_tx); /* * Sending a malformed frame, it must be registered as tranfer error @@ -74,11 +74,11 @@ TEST(TransportStatsProvider, Basic) ASSERT_LE(0, nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10))); ASSERT_TRUE(tsp_cln.collector.result.get()); - ASSERT_EQ(1, tsp_cln.collector.result->response.transfer_errors); // That broken frame - ASSERT_EQ(3, tsp_cln.collector.result->response.transfers_rx); - ASSERT_EQ(2, tsp_cln.collector.result->response.transfers_tx); - ASSERT_EQ(1, tsp_cln.collector.result->response.can_iface_stats.size()); - ASSERT_EQ(72, tsp_cln.collector.result->response.can_iface_stats[0].errors); - ASSERT_EQ(4, tsp_cln.collector.result->response.can_iface_stats[0].frames_rx); // Same here - ASSERT_EQ(12, tsp_cln.collector.result->response.can_iface_stats[0].frames_tx); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().transfer_errors); // That broken frame + ASSERT_EQ(3, tsp_cln.collector.result->getResponse().transfers_rx); + ASSERT_EQ(2, tsp_cln.collector.result->getResponse().transfers_tx); + ASSERT_EQ(1, tsp_cln.collector.result->getResponse().can_iface_stats.size()); + ASSERT_EQ(72, tsp_cln.collector.result->getResponse().can_iface_stats[0].errors); + ASSERT_EQ(4, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_rx); // Same here + ASSERT_EQ(12, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_tx); } diff --git a/libuavcan_drivers/linux/include/uavcan_linux/helpers.hpp b/libuavcan_drivers/linux/include/uavcan_linux/helpers.hpp index 608330fb48..d27cad4833 100644 --- a/libuavcan_drivers/linux/include/uavcan_linux/helpers.hpp +++ b/libuavcan_drivers/linux/include/uavcan_linux/helpers.hpp @@ -42,7 +42,7 @@ class BlockingServiceClient : public uavcan::ServiceClient void callback(const uavcan::ServiceCallResult& res) { - response_ = res.response; + response_ = res.getResponse(); call_was_successful_ = res.isSuccessful(); } @@ -85,7 +85,7 @@ public: const int call_res = Super::call(server_node_id, request); if (call_res >= 0) { - while (Super::isPending()) + while (Super::hasPendingCalls()) { const int spin_res = Super::getNode().spin(SpinDuration); if (spin_res < 0)