Runtime Interface Reflection: rclcpp by methylDragon · Pull Request #2077 · ros2/rclcpp · GitHub

This PR is part of the runtime interface reflection subscription feature of REP-2011: ros2/ros2#1374

Description

This PR includes support for runtime type subscriptions, including:

Edits to SubscriptionBase and executor code to allow for use of runtime type
- This includes new pure virtual methods for SubscriptionBase that have also been implemented in all child classes
A new RuntimeTypeSubscription subscription class that handles a subscription callback using ser_dynamic_data

Signed-off-by: methylDragon <methylDragon@gmail.com>

⚠️ The sha of the head commit of this PR conflicts with #2160. Mergify cannot evaluate rules on this PR. ⚠️

Can all these bad cases be true if ret == RCL_RET_OK?

🤔 probably not.

I might still leave in the typesupport identifier check though

⚠️ The sha of the head commit of this PR conflicts with #2160. Mergify cannot evaluate rules on this PR. ⚠️

manage is an overloaded term - this is really a make_shared function to put ownership of the object into a shared pointer, right?

Yeah that's true.. I wanted it to be like a: "We are taking ownership/managing the lifetime of the pointer now)

It's a private method though, so I don't have any issues with changing the name

The distinction here isn't clear to me between dynamic message direct and serialized.

What does this wording pattern "take as X and handle as Y" mean? Does "handle" refer to the argument type for the callback?

Yep, "handle" is for the callback.

The way it works with ROS is that a message has to be taken, then passed to a handle function that passes it to a callback. So take -> handle.

What about a dynamic subscription that passes serialized data to the handler function? GenericDynamicSubscription, perhaps. It's what we would want in rosbag2 recording. Or is that the case for DYNAMIC_MESSAGE_FROM_SERIALIZED, I couldn't quite tell.

In that case you would want to use a serialized subscription (I.e. one that takes and handles serialized data), then use the DynamicMessage class to deserialize the serialized message into the DynamicMessage inside the callback

You probably would need to subclass that subscription so you can instantiate a DynamicMessage and have it accessible inside the callback

(Basically you just need a way to implement what the executor is doing here, in the callback)

    // Take serialized and then convert to dynamic message
    case rclcpp::SubscriptionType::DYNAMIC_MESSAGE_FROM_SERIALIZED:
      {
        std::shared_ptr<SerializedMessage> serialized_msg =
          subscription->create_serialized_message();

        // NOTE(methylDragon): Is this clone necessary? If I'm following the pattern, it seems so.
        DynamicMessage::SharedPtr dynamic_message = subscription->create_dynamic_message();
        take_and_do_error_handling(
          "taking a serialized message from topic",
          subscription->get_topic_name(),
          [&]() {return subscription->take_serialized(*serialized_msg.get(), message_info);},
          [&]()
          {

            // === <<< THIS IS THE MOST IMPORTANT LINE!!! >>> ===
            bool ret = dynamic_message->deserialize(&serialized_msg->get_rcl_serialized_message());
            // ^^^ <<< THIS IS THE MOST IMPORTANT LINE!!! >>> ^^^

            if (!ret) {
              throw_from_rcl_error(ret, "Couldn't convert serialized message to dynamic data!");
            }
            subscription->handle_dynamic_message(dynamic_message, message_info);
          }
        );
        subscription->return_serialized_message(serialized_msg);
        subscription->return_dynamic_message(dynamic_message);
        break;
      }
  }

Closing this in favor of #2176

⚠️ The sha of the head commit of this PR conflicts with #2160. Mergify cannot evaluate rules on this PR. ⚠️

methylDragon requested review from hidmic, ivanpauno and wjwwood as code owners January 3, 2023 23:52

methylDragon marked this pull request as draft January 3, 2023 23:52

methylDragon force-pushed the runtime_interface_reflection branch 6 times, most recently from 42802ae to af692ec Compare January 25, 2023 08:40

methylDragon mentioned this pull request Jan 30, 2023

Dynamic Subscription (REP-2011 subset) ros2/ros2#1374

Open

31 tasks

methylDragon force-pushed the runtime_interface_reflection branch 3 times, most recently from 41a2df1 to 0ee1cee Compare January 30, 2023 09:24

methylDragon force-pushed the runtime_interface_reflection branch from 0ee1cee to d2b1743 Compare February 28, 2023 01:19

methylDragon force-pushed the runtime_interface_reflection branch 2 times, most recently from a298ef5 to dd1ab8c Compare March 16, 2023 18:10

methylDragon mentioned this pull request Mar 20, 2023

Runtime Interface Reflection: Dynamic Typesupport Wrappers #2134

Closed

methylDragon force-pushed the runtime_interface_reflection branch 9 times, most recently from 86522b4 to 28ce1bf Compare March 24, 2023 09:39

methylDragon force-pushed the runtime_interface_reflection branch from f7f548f to f71bea7 Compare March 28, 2023 19:57

methylDragon marked this pull request as ready for review March 28, 2023 22:36

methylDragon force-pushed the runtime_interface_reflection branch from 8933e83 to 67a958e Compare March 30, 2023 20:35

methylDragon added 6 commits April 3, 2023 15:07

Remove dynamic data/type aliases

aac28e7

Signed-off-by: methylDragon <methylDragon@gmail.com>

Migrate methods to use return types

113dc01

Signed-off-by: methylDragon <methylDragon@gmail.com>

Lint

7e0d063

Signed-off-by: methylDragon <methylDragon@gmail.com>

Support type hashes

8571b28

Signed-off-by: methylDragon <methylDragon@gmail.com>

Add initializers for description and source getters

e7e9a29

Signed-off-by: methylDragon <methylDragon@gmail.com>

Support type hashes

ca17caf

Signed-off-by: methylDragon <methylDragon@gmail.com>

methylDragon force-pushed the runtime_interface_reflection branch 3 times, most recently from 2217287 to b22731b Compare April 4, 2023 00:10

Use dynamic typesupport identifier getter

000f788

Signed-off-by: methylDragon <methylDragon@gmail.com>

methylDragon force-pushed the runtime_interface_reflection branch 4 times, most recently from ac43cb6 to 0a21288 Compare April 5, 2023 23:28

Move dynamic type support struct to rosidl and fix mem issues

29577da

Signed-off-by: methylDragon <methylDragon@gmail.com>

methylDragon force-pushed the runtime_interface_reflection branch from 0a21288 to 29577da Compare April 6, 2023 06:12

methylDragon added 3 commits April 6, 2023 21:13

Use create instead of init

d871af0

Signed-off-by: methylDragon <methylDragon@gmail.com>

Remove print methods

30210a2

Signed-off-by: methylDragon <methylDragon@gmail.com>

Remove RMW interfaces

32859cd

Signed-off-by: methylDragon <methylDragon@gmail.com>

methylDragon force-pushed the runtime_interface_reflection branch from b961350 to 32859cd Compare April 8, 2023 01:16

wjwwood mentioned this pull request Apr 10, 2023

Dynamic Subscription (BONUS: Allocators): rclcpp #2160

Closed

emersonknapp reviewed Apr 13, 2023

View reviewed changes

methylDragon mentioned this pull request Apr 28, 2023

Runtime Interface Reflection: rclcpp (Rebased) #2176

Open

methylDragon closed this Apr 28, 2023

+                ROS_MESSAGE = 1,  // take message as ROS message and handle as ROS message
+                SERIALIZED_MESSAGE = 2,  // take message as serialized and handle as serialized
+                DYNAMIC_MESSAGE_DIRECT = 3,  // take message as DynamicMessage and handle as DynamicMessage
+                DYNAMIC_MESSAGE_FROM_SERIALIZED = 4  // take message as serialized and handle as DynamicMessage

+                if (ret != RCL_RET_OK) {
+                  throw std::runtime_error("error initializing rosidl message type support");
+                }
+                if (!ts) {

+                RCLCPP_PUBLIC
+                void
+                manage_description_(rosidl_runtime_c__type_description__TypeDescription * description);

Runtime Interface Reflection: rclcpp by methylDragon · Pull Request #2077 · ros2/rclcpp · GitHub

Uh oh!

Runtime Interface Reflection: rclcpp - #2077

Runtime Interface Reflection: rclcpp#2077methylDragon wants to merge 27 commits intorollingros2/rclcpp:rollingfrom runtime_interface_reflectionros2/rclcpp:runtime_interface_reflectionCopy head branch name to clipboard

Conversation

methylDragon commented Jan 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

mergify Bot commented Apr 9, 2023

Uh oh!

emersonknapp Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

methylDragon Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Apr 13, 2023

Uh oh!

emersonknapp Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

methylDragon Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

emersonknapp Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

methylDragon Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

emersonknapp Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

methylDragon Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

methylDragon Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

methylDragon Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

methylDragon commented Apr 28, 2023

Uh oh!

mergify Bot commented Apr 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Runtime Interface Reflection: rclcpp#2077
methylDragon wants to merge 27 commits into
rollingfrom
runtime_interface_reflection

methylDragon commented Jan 3, 2023 •
edited
Loading

emersonknapp Apr 13, 2023 •
edited
Loading

methylDragon Apr 13, 2023 •
edited
Loading

methylDragon Apr 13, 2023 •
edited
Loading