-
Notifications
You must be signed in to change notification settings - Fork 429
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
Runtime Interface Reflection: rclcpp (Rebased) #2176
base: rolling
Are you sure you want to change the base?
Conversation
3a3922f
to
f8c11e9
Compare
I spoke to William, and got some preliminary reviews on this PR. These notes do not yet constitute a full review, so there might be even more issues to uncover... I'll just go about the changes in detail over here, for posterity's sake (no timeline for how/when they'll get done just yet though.) Non-ABI BreakingThese changes can and should be made in this PR (or a very quick follow-up) since they won't break ABI in Iron Minor Renames / Comments
Getters
SharedPtr Getters
Free Functions
ABI BreakingClass Refactors
SharedPtr Refactors
Self-Describing Messages Description BindingSee next post!!! Extra Work
|
Self-Describing Messages Description BindingCurrently, there is no way to use a DynamicMessage, DynamicMessageType or DynamicMessageTypeBuilder to iterate through the fields of the message. That is, these objects are not self-describing. It's weird because you can access the fields of DynamicMessage if you knew the structure, but there's no way to list that structure otherwise (there's an expectation that you have access to a separate type description alongside the dynamic message.) In order to achieve self-description and fix this, we can store a reference/pointer to the description. That is the idea is to:
So the descriptions (or at least references to the descriptions) will be baked into the Additional note:
Open question to @wjwwood : Should we associate the type description with the C versions of the structs instead? These C++ classes wrap the C versions. EDIT: Then the intention is to keep the TypeDescription message (or a reference to it) available.
|
Specifically for the helper for Iron (to get around ABI issues with the changes we want), I was imagining something like this (pseudocode): namespace rclcpp::future {
#if IRON
class DynamicSubscription : public rclcpp::DynamicSubscription
{
public:
using DynamicMessageT = rclcpp::SelfDescribingDynamic;
DynamicSubscription(
rclcpp::node_interfaces::NodeBaseInterface * node_base,
rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::SharedPtr type_support,
const std::string & topic_name,
const rclcpp::QoS & qos,
rclcpp::DynamicSubscription::DynamicCallback callback,
const rclcpp::SubscriptionOptionsWithAllocator<AllocatorT> & options)
:
rclcpp::DynamicSubscription(
node_base,
type_support,
topic_name,
qos,
std::bind(&DynamicSubscription::wrapper_callback, this, ...),
options)
{}
void
wrapper_callback(const rclcpp::DynamicMessage& msg)
{
if constexpr (std::holds_alternative_v<callback_, SelfDescribingDynamicCallback>()) {
rclcpp::SelfDescribingDynamic sddm(msg, this->get_description());
callback_(sddm);
} else {
callback_(msg);
}
}
};
#else
// deprecate
using DynamicSubscription = rclcpp::DynamicSubscription;
#endif
} #if IRON
using rclcpp::future::DynamicSubscription;
#else
using rclcpp::DynamicSubscription;
#endif
auto callback = [](const DynamicSubscription::DynamicMessageT & msg) {
// ... msg is either rclcpp::DynamicMessage or rclcpp::SelfDescribingDynamicMessage, depending on the version
};
auto sub = rclcpp::create_subscription<DynamicSubscription>(node, ..., callback, ...); Idea being to minimize the user's version of the code between Iron and Rolling. |
I just got some time again, so I'm gonna be spinning this up again and breaking up the PR into smaller PRs to progressively get this in. |
d2368ff
to
be61087
Compare
Signed-off-by: methylDragon <[email protected]>
be61087
to
dd5c608
Compare
This PR is part of the runtime interface reflection subscription feature of REP-2011: ros2/ros2#1374
It's a rebased (and slightly modified version of #2077)
Description
This PR includes support for runtime type subscriptions, including: