Make Player and Recorder Composable (#902) #1419

roncapat · 2023-07-13T16:51:50Z

Completion Criteria

Allow rosbag2_transport::Player node configuration via parameters.
Register rosbag2_transport::Player node as a component.
Allow rosbag2_transport::Recorder node configuration via parameters.
Register rosbag2_transport::Recorder node as a component.
Write tests
- Be able to access to the protected storage_options, player_options and record_options for verifications via mock_player and mock_recorder.
- Create node with expected parameters and check all parametwrs w.r.t. what is provided in the YAML files.
- Check if composition works.

Closes #902

Please feel free to comment, review, share opinions...

Tagging @MichaelOrlov and @emersonknapp as per #902 comment from Emerson. FYI :)

roncapat · 2023-07-13T16:54:03Z

Topic 1 - types of the attributes to be exposed as params

So, let's start with the first issue: RecordOptions, PlayOptions and StorageOptions feature many uint64_t and even some size_t attributes. However, rclcpp parameter API only supports signed integers, being int64_t the widest. We would loose the ability to set the upper half of the uint64_t value range. About size_t, it is both unsigned and platform dependent (even if I think on 90% of current ROS 2 setups with 64 bit x86 or ARM, it should correspond again to an uint64_t).

For now, I've done casts all around, since the numeric range of int64_t is extraordinarily wide to express a size in bytes or number of messages.

Topic 2 - where to place the parsing

For now, I filled the missing constructor with param-parsing code. However, doing this means that we loose a lot of constructor chaining logic that is in place for the pre-existing ones. I ended up making this constructor independent on the others, and generating a new method init() (to be renamed of course with something appropriate) to avoid duplicating lots of code.
I think it is not possible to embed instead everything in the initializer list (before constructor body), but suggestions are welcome.

Topic 3 - parameter validation

I took inspiration from:
https://github.com/ros2/rclcpp/blob/6e97990a32afa17ab98ef4c800783c5cce801786/rclcpp_components/src/component_manager.cpp#L51-L61
however, do we actually want to put descriptive text of each parameters?

rosbag2_transport/src/rosbag2_transport/player.cpp

roncapat · 2023-07-14T10:09:04Z

I think it would be nice to move parameter declaration/parsing to static methods in StorageOptions, PlayOptions and RecordOptions (passing node handle). This way, no duplication of code happens between Player and Recorder.

MichaelOrlov

@roncapat Thank you for your contribution.
It looks like it is going to be a significant effort.
I like the idea of moving options initialization from node to the separate functions and the ability to reuse them from player and recorder.

However, I think that PR is more or less in the draft stage and some parts of it need some "love".

Here are some general comments for the review.

Could you please rename declare_record_options_rw_params(std::shared_ptr<rclcpp::Node> nh, RecordOptions & ro) to the init_record_options_from_node_params(std::shared_ptr<rclcpp::Node> node, RecordOptions & record_options); and declare_play_options_rw_params(..) to the init_play_options_from_node_params?
Could you please combine declare_storage_options_r_params(..) and declare_storage_options_rw_params() in one function and name it to the init_storage_options_from_node_params(std::shared_ptr<rclcpp::Node> node, StorageOptions & storage_options)? It's Ok to call it from both the player and recorder
Please use full names for the parameters and variables. e.g.
(std::shared_ptrrclcpp::Node node, StorageOptions & storage_options) instead of the (std::shared_ptr<rclcpp::Node> nh, StorageOptions & so) to adhere a common style in the rosbag2 and ROS2.
I think it makes sense to change structured node parameters definitions to the plain one-layer parameters list as we are doing it in CLI options.
For instance, currently, we have
```
  "record.all",
  "record.is_discovery_disabled",
  "storage.max_bagfile_size",
 "storage.max_bagfile_duration",
  // And so on parameters definitions for the node
```
It would be better to have plain parameter structures without recorder and other
prefixes. It will simplify the definition of the parameters in launch files and command line for the composable recorder and player. And will match with parameters from CLI args for standalone rosbag2 recorder and player.
Will need to add a few unit tests. I envision the test approach as follows:
- Make test class for the player and recorder via inheritance to be able to have access to the protected storage_options, player_options and record_options for verifications.
- Create node with expected parameters.
- Create relevant test player or recorder classes via passing previously instantiated node with parameters.
- Test if storage_options, player_options and record_options have the expected parameters.

rosbag2_storage/src/rosbag2_storage/storage_options.cpp

rosbag2_storage/include/rosbag2_storage/storage_options.hpp

rosbag2_transport/src/rosbag2_transport/record_options.cpp

rosbag2_transport/include/rosbag2_transport/play_options.hpp

roncapat · 2023-07-22T19:30:42Z

Thank you! Will surely implement, starting next week :)

rosbag2_storage/package.xml

rosbag2_transport/src/rosbag2_transport/player.cpp

emersonknapp

Left one pass of structural comments, I'd like to see this change look a little different top down.

However, please don't take the feedback as discouragement, it's a great start and I totally appreciate you taking on this issue!

rosbag2_transport/src/rosbag2_transport/player.cpp

rosbag2_transport/include/rosbag2_transport/play_options.hpp

rosbag2_transport/src/rosbag2_transport/play_options.cpp

rosbag2_transport/include/rosbag2_transport/player.hpp

roncapat · 2023-07-25T19:04:35Z

No worries, and thank you both for the reviews!

Will do a first pass of minor comments solving, so that I/we can focus then on more architectural issues you pointed out.

roncapat · 2023-08-01T04:25:34Z

@MichaelOrlov @emersonknapp did a first round of cleanup following your suggestions.
My proposal here is to address first the location of the parsing functions (waiting on your feedback here under) and afterwards the style of the Player constructor(s) (the one with that init() fun).

Moreover, please check my replies about the constructor signature needed for componentization, feel free to mark as resolved those points if they are clearer to you now :)

I will then implement tests

rosbag2_storage/include/rosbag2_storage/storage_options.hpp

rosbag2_transport/src/rosbag2_transport/play_options.cpp

rosbag2_storage/include/rosbag2_storage/storage_options.hpp

rosbag2_transport/src/rosbag2_transport/player.cpp

MichaelOrlov

@roncapat Thank you for moving forward with this PR and addressing my comments.
I confirm changes which you have marked as resolved among a few nitpicks, see inline.

As regards the @emersonknapp comments I agree that it would be better to move storage options init from node params to the rosbag2_transport layer I have tried to prototype and made in on my branch. See my pull request roncapat#1 feel free to merge it on your branch.

As regards to the Player constructor and those Init() method I agree that it would be better to have the cascade of the embaded constructors and get rid from the Init() method but don;t envision how to do this without decomposing base node class from inheritance to the inner member variable. BUt not sure if it is a god idea and if it will be possible.
@emersonknapp may be you will have some ideas?
Please take a look on my latest changes in the roncapat#1

roncapat · 2023-08-08T06:38:43Z

@MichaelOrlov very appreciated! Will surely merge, and also move Play and Record Options parsing in dedicated files in the meantime.

roncapat · 2023-08-08T07:38:59Z

@MichaelOrlov

If I do not include "<memory>"... cpplint says:

Error: /__w/rosbag2/rosbag2/rosbag2_transport/include/rosbag2_transport/play_options_from_node_params.hpp:24:  Add #include <memory> for shared_ptr<>  [build/include_what_you_use] [4]
Error: /__w/rosbag2/rosbag2/rosbag2_transport/include/rosbag2_transport/record_options_from_node_params.hpp:24:  Add #include <memory> for shared_ptr<>  [build/include_what_you_use] [4]
Error: /__w/rosbag2/rosbag2/rosbag2_transport/include/rosbag2_transport/storage_options_from_node_params.hpp:24:  Add #include <memory> for shared_ptr<>  [build/include_what_you_use] [4]
Error: Category 'build/include_what_you_use' errors found: 3
Error: Total errors found: 3

MichaelOrlov · 2023-08-08T07:44:47Z

@MichaelOrlov

If I do not include ""... cpplint says:
@roncapat Ok, you are right. We need it for the shared pointer.

roncapat · 2023-08-10T13:14:37Z

As regards to the Player constructor and those Init() method I agree that it would be better to have the cascade of the embaded constructors and get rid from the Init() method but don;t envision how to do this without decomposing base node class from inheritance to the inner member variable. BUt not sure if it is a god idea and if it will be possible. @emersonknapp may be you will have some ideas?

@emersonknapp @MichaelOrlov proposal: why don't we PIMPL Player like Recorder? If you notice, adding parameter parsing to Recorder was a way smoother process than Player, thants to its structure (based on PIMPL pattern).

roncapat · 2023-08-10T13:28:04Z

To implement test, starting from this point:

Make test class for the player and recorder via inheritance to be able to have access to the protected storage_options, player_options and record_options for verifications.

may I just expand MockPlayer and MockRecorder classes with getters for the PlayOptions, StorageOptions and RecordOptions internal structures?

emersonknapp · 2023-08-20T04:38:03Z

why don't we PIMPL Player like Recorder?

I don't see any specific reason why not, just not something we've done. Unless @MichaelOrlov has a different perspective.

may I just expand MockPlayer and MockRecorder classes with getters for the PlayOptions, StorageOptions and RecordOptions internal structures?

No major concerns - just be wary of mocking so much that you're not testing any of the real code :)

Sorry if my responses are a bit slow - I'm just starting a new job and figuring out the balance between that and my open source maintenance as I get ramped up.

roncapat · 2023-08-21T08:09:26Z

Don't worry!

I'm implementing PIMPL in Player as a separate PR, so it can be reviewed individually.

EDIT: see #1447. Give me some time to let it work properly and check if some changes in tests are required or not :)

roncapat

One more quick thing: better executable names.

roncapat · 2023-12-09T07:49:00Z

rosbag2_transport/CMakeLists.txt

@@ -65,6 +69,12 @@ target_link_libraries(${PROJECT_NAME}
  yaml-cpp
 )

+rclcpp_components_register_node(
+  ${PROJECT_NAME} PLUGIN "rosbag2_transport::Player" EXECUTABLE Rosbag2Player)


Suggested change

${PROJECT_NAME} PLUGIN "rosbag2_transport::Player" EXECUTABLE Rosbag2Player)

${PROJECT_NAME} PLUGIN "rosbag2_transport::Player" EXECUTABLE player)

@MichaelOrlov one last thing... I believe it's better to rename the nodes, so that:

ros2 run rosbag2_transport player

The "Rosbag2" prefix is redundant IMHO.
And I believe that for node names lowercase convention is preferred.

(Same for Recorder here below)

@roncapat Out of curiosity, is it possible to make it ros2 run rosbag2 player without renaming rosbag2_transport to the rosbag2?
Maybe using an alias for the entry point?

Or maybe if can get rid of rosbag2_transport keyword in front. To be ros2 run Rosbag2Player ?

I believe that the semantic of the ros2 run command is 'package_name node_name' and can't be changed. We can only act on the node name.

Maybe we can then find a way to register the node under package rosbag2 nstead of 'roabag2_transport`. But it may be very tricky.

However, in the long term (I can open an Issue for later implementation) I just had a potentially nice idea: to upgrade ros2 bag play CLI support with a parameter for an existing container name. This way, the CLI would load the component in an existing container, start playing (and eventually unloading it at the end of the playback). This would be a neat feature IMHO. Thoughts?

Summing up: simplest solution here is at least for now just to choose a simple and idiomatic node name. We may act on moving the nodes to rosbag2 package later on.

(To register the nodes under "rosbag2" instead of "rosbag2_transport" we may need to hack the inner workings of this: https://github.com/ros2/rclcpp/blob/rolling/rclcpp_components%2Fcmake%2Frclcpp_components_register_node.cmake)

@roncapat Ahh never mind. Feel free to go ahead and commit your suggestions with signoff.

As regards:

This way, the CLI would load the component in an existing container, start playing (and eventually unloading it at the end of the playback). This would be a neat feature IMHO. Thoughts?

Curious why it would be useful at all if we already have entry point for ros2 bag play and ros2 bag record?
I only envision a case when we can run player or recorder and specify settings yaml file instead of the CLI parameters directly. Something like ros2 bag play --settings playback_settings.yaml

The same questions apply to how to use ros2 run rosbag2_transport player in a real use case? How we will provide settings or node parameters in this case?

As regards:

This way, the CLI would load the component in an existing container, start playing (and eventually unloading it at the end of the playback). This would be a neat feature IMHO. Thoughts?

Curious why it would be useful at all if we already have entry point for ros2 bag play and ros2 bag record?

Imagine people have already scripted their ros2 bag play/record commands (for example, in some launch files). With a small modification (like the addition of one CLI parameter) they will be able to "upgrade their setup" and inject player/recorder in their component containers. Increased performance with almost zero effort (if you have specified all parameters in the CLI command, you won't have to migrate to a .yaml configuration file).

The same questions apply to how to use ros2 run rosbag2_transport player in a real use case? How we will provide settings or node parameters in this case?

Like normal node parameters via YAML file. Notice that this also enables people to use Node() objects in LaunchConfiguration() of launchfiles instead of the current only way to run player/recorder via launchfiles (to specify the full CLI command).

In the next days I'll write some docs to push as separate PR. It may be the right context to sum up all the past, present and potentially future ways to run players/recorders if it's ok for you :)

@roncapat As regards:

magine people have already scripted their ros2 bag play/record commands (for example, in some launch files). With a small modification (like the addition of one CLI parameter) they will be able to "upgrade their setup" and inject player/recorder in their component containers. Increased performance with almost zero effort (if you have specified all parameters in the CLI command, you won't have to migrate to a .yaml configuration file).

I don't really understand about what increased performance and benefits you are talking about there. Maybe with documentation or some real-world examples will be clear to me.

As regards:

The same questions apply to how to use ros2 run rosbag2_transport player in a real use case? How we will provide settings or node parameters in this case?

Like normal node parameters via YAML file. Notice that this also enables people to use Node() objects in LaunchConfiguration() of launchfiles instead of the current only way to run player/recorder via launchfiles (to specify the full CLI command).

Still unclear to me how node parameters will be settled with ros2 run rosbag2_transport player command. What you described seems more relevant for launch files launching composed nodes via the launcher. But it is irrelevant to the ros2 run rosbag2_transport player command as far as understand.

In the next days I'll write some docs to push as separate PR. It may be the right context to sum up all the past, present and potentially future ways to run players/recorders if it's ok for you :)

Anyway, documentation would be very helpful and grateful to have.

rosbag2_transport/CMakeLists.txt

roncapat · 2023-12-09T07:52:17Z

Latest failures (linux):

/tmp/ws/src/rosbag2/rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp:238
Expected equality of these values:
  storage_options.max_bagfile_size
    Which is: 2147483646
  8601600678

/tmp/ws/src/rosbag2/rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp:239
Expected equality of these values:
  storage_options.max_bagfile_duration
    Which is: 2147483646
  54321689657

rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp

…der.cpp Signed-off-by: Patrick Roncagliolo <[email protected]>

Signed-off-by: Patrick Roncagliolo <[email protected]>

MichaelOrlov · 2023-12-09T19:13:07Z

Reruning CI after addressing Windows CI failures

BUILD args: --packages-above-and-dependencies rosbag2_storage rosbag2_transport rosbag2_tests
TEST args: --packages-above rosbag2_storage rosbag2_transport rosbag2_tests
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/13037

Linux
Linux-aarch64
Windows

Signed-off-by: Michael Orlov <[email protected]>

MichaelOrlov · 2023-12-10T23:27:58Z

re-run Windows CI after fixing warnings

Windows

roncapat · 2023-12-11T07:10:47Z

About the warnings, what if we append the f suffix to the literals in those lines? Should it work or do you think it is caused by something different?

MichaelOrlov · 2023-12-11T07:36:13Z

About the warnings, what if we append the f suffix to the literals in those lines? Should it work or do you think it is caused by something different?

Ahh.. Windows!!! Need to debug it locally.

roncapat · 2023-12-11T08:24:58Z

Very sorry of not having a W10 machine to help you in this!

MichaelOrlov

@roncapat See my suggestions for Windows warnings fixes.
Now the warnings should be 100% fixed. I tested the build locally and it is clean without warnings after the propoused fixes.

rosbag2_transport/src/rosbag2_transport/config_options_from_node_params.cpp

rosbag2_transport/test/rosbag2_transport/test_composable_player.cpp

…de_params.cpp Co-authored-by: Michael Orlov <[email protected]> Signed-off-by: Patrick Roncagliolo <[email protected]>

…r.cpp Co-authored-by: Michael Orlov <[email protected]> Signed-off-by: Patrick Roncagliolo <[email protected]>

MichaelOrlov · 2023-12-11T16:07:36Z

@ros-pull-request-builder retest this please

MichaelOrlov · 2023-12-11T16:40:14Z

re-run Windows CI after fixing warnings

Windows

MichaelOrlov · 2023-12-11T19:49:08Z

@emersonknapp Friendly ping here again. Could you please approve this PR additionally?
We will not be able to merge this PR since you made a request for changes on one of your reviews.
All your concerns and suggestions have been taken into account and addressed.
The CI is green.

emersonknapp

Ok, I haven't reviewed thoroughly but it looks like you've all put in the hard work on this so it's fine by me

roncapat · 2023-12-11T20:08:42Z

Thank you very much!

ros-discourse · 2023-12-18T22:01:34Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-for-2023-12-14/35153/1

berndpfrommer · 2024-03-08T08:42:53Z

I finally got around to testing the composable recorder and it works GREAT! I can start recording by loading the recorder into a running camera driver node, and stop by unloading it. I can compress and record 10 camera streams at 40fps 1920x1200. CPU load is about 35% less than with separate recorder/driver setup and with the reduced memcpys the host feels much more responsive when running other tasks.
THANK YOU VERY MUCH for all the hard work you have put in!

berndpfrommer · 2024-03-08T08:46:13Z

Question though: right now I'm running on Rolling on Ubuntu 22.04, but upgrading my host to Ubuntu 24.04 is very difficult since we must be recording daily. Is there a chance that this feature will be backported to Iron? I tried backporting to humble but the code base has diverged so much in other places that I gave up. Haven't tried Iron yet.

roncapat · 2024-03-08T12:36:48Z

@berndpfrommer thank you very much for your feedback on this, it always feels great to see adoption and appreciation for this kind of contributions :)

That said, about backporting, I think it's undoable. However, I can tell you that in my lab I run entire systems with source builds of Iron, and I deployed this updated package simply by checking out rolling versions of rcpputils and rosbag2 and rebuilding everything.

roncapat requested a review from a team as a code owner July 13, 2023 16:51

roncapat requested review from gbiggs and emersonknapp and removed request for a team July 13, 2023 16:51

roncapat marked this pull request as draft July 13, 2023 16:52

MichaelOrlov requested review from MichaelOrlov and removed request for gbiggs July 13, 2023 22:24

MichaelOrlov reviewed Jul 14, 2023

View reviewed changes

rosbag2_transport/src/rosbag2_transport/player.cpp Outdated Show resolved Hide resolved

roncapat force-pushed the patch-2 branch from 2c0d965 to 8332f70 Compare July 17, 2023 08:58

roncapat marked this pull request as ready for review July 20, 2023 11:56

MichaelOrlov requested changes Jul 22, 2023

View reviewed changes

MichaelOrlov mentioned this pull request Jul 22, 2023

Recording a bag file from a launch file. #1429

Closed

emersonknapp reviewed Jul 25, 2023

View reviewed changes

rosbag2_storage/package.xml Outdated Show resolved Hide resolved

emersonknapp reviewed Jul 25, 2023

View reviewed changes

rosbag2_transport/src/rosbag2_transport/player.cpp Outdated Show resolved Hide resolved

emersonknapp requested changes Jul 25, 2023

View reviewed changes

MichaelOrlov reviewed Aug 8, 2023

View reviewed changes

roncapat force-pushed the patch-2 branch from 7888b96 to ec59d0e Compare August 10, 2023 13:19

roncapat commented Dec 9, 2023

View reviewed changes

rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp Outdated Show resolved Hide resolved

roncapat added 3 commits December 9, 2023 08:59

Update rosbag2_transport/test/rosbag2_transport/test_composable_recor…

db3aa34

…der.cpp Signed-off-by: Patrick Roncagliolo <[email protected]>

Update rosbag2_transport/CMakeLists.txt

af07a04

Signed-off-by: Patrick Roncagliolo <[email protected]>

Update rosbag2_transport/CMakeLists.txt

6f9bbe8

Signed-off-by: Patrick Roncagliolo <[email protected]>

MichaelOrlov mentioned this pull request Dec 10, 2023

Address warnings from Windows CI job in composable player and recorder roncapat/rosbag2#10

Merged

Address warnings from Windows CI job in composable player and recorder

723a7a8

Signed-off-by: Michael Orlov <[email protected]>

MichaelOrlov reviewed Dec 11, 2023

View reviewed changes

rosbag2_transport/src/rosbag2_transport/config_options_from_node_params.cpp Outdated Show resolved Hide resolved

rosbag2_transport/test/rosbag2_transport/test_composable_player.cpp Outdated Show resolved Hide resolved

roncapat and others added 2 commits December 11, 2023 10:09

Update rosbag2_transport/src/rosbag2_transport/config_options_from_no…

9110f65

…de_params.cpp Co-authored-by: Michael Orlov <[email protected]> Signed-off-by: Patrick Roncagliolo <[email protected]>

Update rosbag2_transport/test/rosbag2_transport/test_composable_playe…

8737b72

…r.cpp Co-authored-by: Michael Orlov <[email protected]> Signed-off-by: Patrick Roncagliolo <[email protected]>

emersonknapp approved these changes Dec 11, 2023

View reviewed changes

MichaelOrlov merged commit 0fcc839 into ros2:rolling Dec 11, 2023
13 of 14 checks passed

Barry-Xu-2018 mentioned this pull request Dec 13, 2023

Implement service recording and display info about recorded services #1480

Merged

roncapat deleted the patch-2 branch March 8, 2024 12:37

MCFurry mentioned this pull request Jun 4, 2024

Build this from source to have composable nodes nobleo/ros_debian_packages#1

Closed

r7vme mentioned this pull request Jul 26, 2024

Decoding RecordOptions from YAML conflicts with chrono::milliseconds #1764

Closed

	${PROJECT_NAME} PLUGIN "rosbag2_transport::Player" EXECUTABLE Rosbag2Player)
	${PROJECT_NAME} PLUGIN "rosbag2_transport::Player" EXECUTABLE player)

Make Player and Recorder Composable (#902) #1419

Make Player and Recorder Composable (#902) #1419

Conversation

roncapat commented Jul 13, 2023 • edited Loading

Completion Criteria

roncapat commented Jul 13, 2023 • edited Loading

Topic 1 - types of the attributes to be exposed as params

Topic 2 - where to place the parsing

Topic 3 - parameter validation

roncapat commented Jul 14, 2023

MichaelOrlov left a comment

Choose a reason for hiding this comment

roncapat commented Jul 22, 2023

emersonknapp left a comment

Choose a reason for hiding this comment

roncapat commented Jul 25, 2023

roncapat commented Aug 1, 2023 • edited Loading

MichaelOrlov left a comment

Choose a reason for hiding this comment

roncapat commented Aug 8, 2023

roncapat commented Aug 8, 2023 • edited Loading

MichaelOrlov commented Aug 8, 2023

roncapat commented Aug 10, 2023 • edited Loading

roncapat commented Aug 10, 2023

emersonknapp commented Aug 20, 2023

roncapat commented Aug 21, 2023 • edited Loading

roncapat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roncapat Dec 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roncapat commented Dec 9, 2023

MichaelOrlov commented Dec 9, 2023

MichaelOrlov commented Dec 10, 2023

roncapat commented Dec 11, 2023

MichaelOrlov commented Dec 11, 2023

roncapat commented Dec 11, 2023

MichaelOrlov left a comment

Choose a reason for hiding this comment

MichaelOrlov commented Dec 11, 2023

MichaelOrlov commented Dec 11, 2023

MichaelOrlov commented Dec 11, 2023

emersonknapp left a comment

Choose a reason for hiding this comment

roncapat commented Dec 11, 2023

ros-discourse commented Dec 18, 2023

berndpfrommer commented Mar 8, 2024

berndpfrommer commented Mar 8, 2024

roncapat commented Mar 8, 2024

roncapat commented Jul 13, 2023 •

edited

Loading

roncapat commented Jul 13, 2023 •

edited

Loading

roncapat commented Aug 1, 2023 •

edited

Loading

roncapat commented Aug 8, 2023 •

edited

Loading

roncapat commented Aug 10, 2023 •

edited

Loading

roncapat commented Aug 21, 2023 •

edited

Loading

roncapat Dec 9, 2023 •

edited

Loading