Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

break Node into several separate interfaces #277

Merged
merged 38 commits into from
Dec 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5133756
add the NodeBaseInterface and impl NodeBase
wjwwood Nov 16, 2016
e1e9709
refactor rclcpp to use NodeBaseInterface
wjwwood Nov 16, 2016
a9947d5
triggering a guard condition is not const
wjwwood Nov 16, 2016
4fd3ef2
remove unnecessary pure virtual destructor
wjwwood Nov 28, 2016
f205de7
remove unused private member, style
wjwwood Nov 28, 2016
242f459
create NodeTopics interface, refactor pub/sub
wjwwood Dec 1, 2016
68ccfa6
add convenience functions to fix API breaks
wjwwood Dec 1, 2016
b63b960
fix compilation errors from NodeTopics refactor
wjwwood Dec 1, 2016
aafb440
move "Event" based exceptions to exceptions.hpp
wjwwood Dec 2, 2016
db5e0eb
add the NodeGraphInterface and related API's
wjwwood Dec 2, 2016
778dedc
update node and graph_listener to use NodeGraph API
wjwwood Dec 2, 2016
36a206a
initialize node_topics_ and node_graph_ in Node
wjwwood Dec 2, 2016
8a61d74
remove methods from Node and reorganize the order
wjwwood Dec 3, 2016
40cfa22
add the NodeServices API and implementation
wjwwood Dec 3, 2016
345a137
add the NodeParameters API and refactor Node
wjwwood Dec 6, 2016
36e746b
mixups
wjwwood Dec 6, 2016
c4cb6a2
fixup NodeParameters constructor
wjwwood Dec 6, 2016
31e726f
added NodeTimers API and refactor Node
wjwwood Dec 6, 2016
060b69b
make new create_publisher and create_subscription free template funct…
wjwwood Dec 7, 2016
10631a9
fixup
wjwwood Dec 7, 2016
c59b1d1
fixup
wjwwood Dec 7, 2016
821a1ad
fixup
wjwwood Dec 7, 2016
18db364
fixup share pointer to node in any_executable
wjwwood Dec 7, 2016
edac3ee
free env value before throwing on Windows
wjwwood Dec 7, 2016
8507ac9
uncrustify and cpplint
wjwwood Dec 7, 2016
9fd2ad9
address constness issues
wjwwood Dec 7, 2016
9ad8342
do not store the topic name as a std::string in subscription
wjwwood Dec 8, 2016
6252e72
fixes to support const char * topic name
wjwwood Dec 8, 2016
7150837
fix incomplete type specification, which fails on Windows
wjwwood Dec 8, 2016
1122e25
refactor after rebase from type support changes
wjwwood Dec 9, 2016
365705a
fixup Windows build
wjwwood Dec 9, 2016
dfa0cf7
fix template issues on Windows
wjwwood Dec 9, 2016
053cd5b
uncrustify
wjwwood Dec 9, 2016
c18b6ab
remove the unnecessary callback group argument from the add_publisher…
wjwwood Dec 9, 2016
28a2cfc
remove unnecessary using = directive
wjwwood Dec 9, 2016
83b610e
do not store node name in C++
wjwwood Dec 9, 2016
f0afcab
fix client and service creation in Node constructor
wjwwood Dec 9, 2016
23a7728
fix include orders
wjwwood Dec 9, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion rclcpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,17 @@ set(${PROJECT_NAME}_SRCS
src/rclcpp/intra_process_manager_impl.cpp
src/rclcpp/memory_strategies.cpp
src/rclcpp/memory_strategy.cpp
src/rclcpp/node.cpp
src/rclcpp/node_interfaces/node_base.cpp
src/rclcpp/node_interfaces/node_graph.cpp
src/rclcpp/node_interfaces/node_parameters.cpp
src/rclcpp/node_interfaces/node_services.cpp
src/rclcpp/node_interfaces/node_timers.cpp
src/rclcpp/node_interfaces/node_topics.cpp
src/rclcpp/parameter.cpp
src/rclcpp/parameter_client.cpp
src/rclcpp/parameter_service.cpp
src/rclcpp/publisher.cpp
src/rclcpp/node.cpp
src/rclcpp/service.cpp
src/rclcpp/subscription.cpp
src/rclcpp/timer.cpp
Expand Down
9 changes: 7 additions & 2 deletions rclcpp/include/rclcpp/any_executable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@

#include <memory>

#include "rclcpp/callback_group.hpp"
#include "rclcpp/client.hpp"
#include "rclcpp/macros.hpp"
#include "rclcpp/node.hpp"
#include "rclcpp/node_interfaces/node_base_interface.hpp"
#include "rclcpp/service.hpp"
#include "rclcpp/subscription.hpp"
#include "rclcpp/timer.hpp"
#include "rclcpp/visibility_control.hpp"

namespace rclcpp
Expand All @@ -44,7 +49,7 @@ struct AnyExecutable
rclcpp::client::ClientBase::SharedPtr client;
// These are used to keep the scope on the containing items
rclcpp::callback_group::CallbackGroup::SharedPtr callback_group;
rclcpp::node::Node::SharedPtr node;
rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base;
};

} // namespace executor
Expand Down
14 changes: 9 additions & 5 deletions rclcpp/include/rclcpp/callback_group.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ namespace rclcpp
{

// Forward declarations for friend statement in class CallbackGroup
namespace node
namespace node_interfaces
{
class Node;
} // namespace node
class NodeServices;
class NodeTimers;
class NodeTopics;
} // namespace node_interfaces

namespace callback_group
{
Expand All @@ -46,7 +48,9 @@ enum class CallbackGroupType

class CallbackGroup
{
friend class rclcpp::node::Node;
friend class rclcpp::node_interfaces::NodeServices;
friend class rclcpp::node_interfaces::NodeTimers;
friend class rclcpp::node_interfaces::NodeTopics;

public:
RCLCPP_SMART_PTR_DEFINITIONS(CallbackGroup)
Expand Down Expand Up @@ -78,7 +82,7 @@ class CallbackGroup
const CallbackGroupType &
type() const;

private:
protected:
RCLCPP_DISABLE_COPY(CallbackGroup)

RCLCPP_PUBLIC
Expand Down
17 changes: 10 additions & 7 deletions rclcpp/include/rclcpp/client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include "rclcpp/function_traits.hpp"
#include "rclcpp/macros.hpp"
#include "rclcpp/node_interfaces/node_graph_interface.hpp"
#include "rclcpp/type_support_decl.hpp"
#include "rclcpp/utilities.hpp"
#include "rclcpp/visibility_control.hpp"
Expand All @@ -39,10 +40,10 @@
namespace rclcpp
{

namespace node
namespace node_interfaces
{
class Node;
} // namespace node
class NodeBaseInterface;
} // namespace node_interfaces

namespace client
{
Expand All @@ -54,7 +55,8 @@ class ClientBase

RCLCPP_PUBLIC
ClientBase(
std::shared_ptr<rclcpp::node::Node> parent_node,
rclcpp::node_interfaces::NodeBaseInterface * node_base,
rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph,
const std::string & service_name);

RCLCPP_PUBLIC
Expand Down Expand Up @@ -98,7 +100,7 @@ class ClientBase
rcl_node_t *
get_rcl_node_handle() const;

std::weak_ptr<rclcpp::node::Node> node_;
rclcpp::node_interfaces::NodeGraphInterface::WeakPtr node_graph_;
std::shared_ptr<rcl_node_t> node_handle_;

rcl_client_t client_handle_ = rcl_get_zero_initialized_client();
Expand Down Expand Up @@ -127,10 +129,11 @@ class Client : public ClientBase
RCLCPP_SMART_PTR_DEFINITIONS(Client)

Client(
std::shared_ptr<rclcpp::node::Node> parent_node,
rclcpp::node_interfaces::NodeBaseInterface * node_base,
rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph,
const std::string & service_name,
rcl_client_options_t & client_options)
: ClientBase(parent_node, service_name)
: ClientBase(node_base, node_graph, service_name)
{
using rosidl_typesupport_cpp::get_service_type_support_handle;
auto service_type_support_handle =
Expand Down
51 changes: 51 additions & 0 deletions rclcpp/include/rclcpp/create_publisher.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2016 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef RCLCPP__CREATE_PUBLISHER_HPP_
#define RCLCPP__CREATE_PUBLISHER_HPP_

#include <memory>
#include <string>

#include "rclcpp/node_interfaces/node_topics_interface.hpp"
#include "rclcpp/publisher_factory.hpp"
#include "rmw/qos_profiles.h"

namespace rclcpp
{

template<typename MessageT, typename AllocatorT, typename PublisherT>
std::shared_ptr<PublisherT>
create_publisher(
rclcpp::node_interfaces::NodeTopicsInterface * node_topics,
const std::string & topic_name,
const rmw_qos_profile_t & qos_profile,
bool use_intra_process_comms,
std::shared_ptr<AllocatorT> allocator)
{
auto publisher_options = rcl_publisher_get_default_options();
publisher_options.qos = qos_profile;

auto pub = node_topics->create_publisher(
topic_name,
rclcpp::create_publisher_factory<MessageT, AllocatorT, PublisherT>(allocator),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this factory when we pass the allocator as a template argument?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The factory captures all template code into non template functions so they can be interleaved with non template code in the cpp file. As I mentioned before, I think we could remove the factories now in favor of more linear code that lives in the template functions. The trade off being that there is more code in template functions which increase compile time.

I'd rather do that refactor later though so that we can merge this and let other work build on top in the mean time.

publisher_options,
use_intra_process_comms);
node_topics->add_publisher(pub);
return std::dynamic_pointer_cast<PublisherT>(pub);
}

} // namespace rclcpp

#endif // RCLCPP__CREATE_PUBLISHER_HPP_
62 changes: 62 additions & 0 deletions rclcpp/include/rclcpp/create_subscription.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2016 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef RCLCPP__CREATE_SUBSCRIPTION_HPP_
#define RCLCPP__CREATE_SUBSCRIPTION_HPP_

#include <memory>
#include <string>
#include <utility>

#include "rclcpp/node_interfaces/node_topics_interface.hpp"
#include "rclcpp/subscription_factory.hpp"
#include "rmw/qos_profiles.h"

namespace rclcpp
{

template<typename MessageT, typename CallbackT, typename AllocatorT, typename SubscriptionT>
typename rclcpp::subscription::Subscription<MessageT, AllocatorT>::SharedPtr
create_subscription(
rclcpp::node_interfaces::NodeTopicsInterface * node_topics,
const std::string & topic_name,
CallbackT && callback,
const rmw_qos_profile_t & qos_profile,
rclcpp::callback_group::CallbackGroup::SharedPtr group,
bool ignore_local_publications,
bool use_intra_process_comms,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can the two booleans go into a dedicated struct. Similar to the qos_profile_t maybe something like intra_process_profile_t? Also to lower the number of arguments here. Or expose the subscription_options ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need an options structure in c++ for these classes, but I didn't want to tackle that in this pr.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm creating an issue for this right now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typename rclcpp::message_memory_strategy::MessageMemoryStrategy<MessageT, AllocatorT>::SharedPtr
msg_mem_strat,
typename std::shared_ptr<AllocatorT> allocator)
{
auto subscription_options = rcl_subscription_get_default_options();
subscription_options.qos = qos_profile;
subscription_options.ignore_local_publications = ignore_local_publications;

auto factory =
rclcpp::create_subscription_factory<MessageT, CallbackT, AllocatorT, SubscriptionT>(
std::forward<CallbackT>(callback), msg_mem_strat, allocator);

auto sub = node_topics->create_subscription(
topic_name,
factory,
subscription_options,
use_intra_process_comms);
node_topics->add_subscription(sub, group);
return std::dynamic_pointer_cast<SubscriptionT>(sub);
}

} // namespace rclcpp

#endif // RCLCPP__CREATE_SUBSCRIPTION_HPP_
16 changes: 16 additions & 0 deletions rclcpp/include/rclcpp/exceptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,22 @@ class RCLInvalidArgument : public RCLErrorBase, public std::invalid_argument
RCLInvalidArgument(const RCLErrorBase & base_exc, const std::string & prefix);
};

/// Thrown when an invalid rclcpp::Event object or SharedPtr is encountered.
class InvalidEventError : public std::runtime_error
{
public:
InvalidEventError()
: std::runtime_error("event is invalid") {}
};

/// Thrown when an unregistered rclcpp::Event is encountered where a registered one was expected.
class EventNotRegisteredError : public std::runtime_error
{
public:
EventNotRegisteredError()
: std::runtime_error("event already registered") {}
};

} // namespace exceptions
} // namespace rclcpp

Expand Down
54 changes: 44 additions & 10 deletions rclcpp/include/rclcpp/executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,21 @@
#include "rcl/guard_condition.h"
#include "rcl/wait.h"

#include "rclcpp/any_executable.hpp"
#include "rclcpp/macros.hpp"
#include "rclcpp/node_interfaces/node_base_interface.hpp"
#include "rclcpp/memory_strategies.hpp"
#include "rclcpp/memory_strategy.hpp"
#include "rclcpp/node.hpp"
#include "rclcpp/utilities.hpp"
#include "rclcpp/visibility_control.hpp"

namespace rclcpp
{

// Forward declaration is used in convenience method signature.
namespace node
{
class Node;
} // namespace node

namespace executor
{

Expand Down Expand Up @@ -114,7 +119,12 @@ class Executor
*/
RCLCPP_PUBLIC
virtual void
add_node(rclcpp::node::Node::SharedPtr node_ptr, bool notify = true);
add_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify = true);

/// Convenience function which takes Node and forwards NodeBaseInterface.
RCLCPP_PUBLIC
virtual void
add_node(std::shared_ptr<rclcpp::node::Node> node_ptr, bool notify = true);

/// Remove a node from the executor.
/**
Expand All @@ -125,7 +135,12 @@ class Executor
*/
RCLCPP_PUBLIC
virtual void
remove_node(rclcpp::node::Node::SharedPtr node_ptr, bool notify = true);
remove_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify = true);

/// Convenience function which takes Node and forwards NodeBaseInterface.
RCLCPP_PUBLIC
virtual void
remove_node(std::shared_ptr<rclcpp::node::Node> node_ptr, bool notify = true);

/// Add a node to executor, execute the next available unit of work, and remove the node.
/**
Expand All @@ -136,7 +151,7 @@ class Executor
*/
template<typename T = std::milli>
void
spin_node_once(rclcpp::node::Node::SharedPtr node,
spin_node_once(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node,
std::chrono::duration<int64_t, T> timeout = std::chrono::duration<int64_t, T>(-1))
{
return spin_node_once_nanoseconds(
Expand All @@ -145,13 +160,30 @@ class Executor
);
}

/// Convenience function which takes Node and forwards NodeBaseInterface.
template<typename NodeT = rclcpp::node::Node, typename T = std::milli>
void
spin_node_once(std::shared_ptr<NodeT> node,
std::chrono::duration<int64_t, T> timeout = std::chrono::duration<int64_t, T>(-1))
{
return spin_node_once_nanoseconds(
node->get_node_base_interface(),
std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
);
}

/// Add a node, complete all immediately available work, and remove the node.
/**
* \param[in] node Shared pointer to the node to add.
*/
RCLCPP_PUBLIC
void
spin_node_some(rclcpp::node::Node::SharedPtr node);
spin_node_some(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node);

/// Convenience function which takes Node and forwards NodeBaseInterface.
RCLCPP_PUBLIC
void
spin_node_some(std::shared_ptr<rclcpp::node::Node> node);

/// Complete all available queued work without blocking.
/**
Expand Down Expand Up @@ -247,7 +279,9 @@ class Executor
protected:
RCLCPP_PUBLIC
void
spin_node_once_nanoseconds(rclcpp::node::Node::SharedPtr node, std::chrono::nanoseconds timeout);
spin_node_once_nanoseconds(
rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node,
std::chrono::nanoseconds timeout);

/// Find the next available executable and do the work associated with it.
/** \param[in] any_exec Union structure that can hold any executable type (timer, subscription,
Expand Down Expand Up @@ -284,7 +318,7 @@ class Executor
wait_for_work(std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));

RCLCPP_PUBLIC
rclcpp::node::Node::SharedPtr
rclcpp::node_interfaces::NodeBaseInterface::SharedPtr
get_node_by_group(rclcpp::callback_group::CallbackGroup::SharedPtr group);

RCLCPP_PUBLIC
Expand Down Expand Up @@ -318,7 +352,7 @@ class Executor
private:
RCLCPP_DISABLE_COPY(Executor)

std::vector<std::weak_ptr<rclcpp::node::Node>> weak_nodes_;
std::vector<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> weak_nodes_;
};

} // namespace executor
Expand Down
Loading