ros2 port

[solonovamax]

closes #5

• add back change header filter
• clean up CMakeLists.txt
• automatically generate nodes from template
• implement composable nodes
• implement lifecycle nodes
  • add lifecycle node for the image filter chain on rolling/lyrical
• check if it compiles for:
  • jazzy
  • kilted
  • maybe rolling/lyrical?
• test on all platforms

this is some initial work on the ros2 port for sensor_filters

I'm currently targeting humble, however this will likely work perfectly fine on jazzy & kilted without any changes, however I have not tested either of those.

this has not been tested at all and likely still needs some work (namely the CMakeLists.txt & ChangeHeaderFilter), as well as a few other places.
however, it does compile.

I'd like to move the logging over to using ros2_fmt_logger (ros index), thoughts on that? I honestly much prefer it to what ros does with rclcpp.

I've also reformatted the code using a code style that I prefer as well as renamed .h -> .hpp & .cc -> .cpp, however both of those are their own commits so they can always be cherry picked/dropped.

[solonovamax]

also, ros2's composite nodes are more or less equivalent to ros1's nodelets: https://docs.ros.org/en/humble/Concepts/Intermediate/About-Composition.html#ros-1-nodes-vs-nodelets

would you want me to introduce composite nodes for all the different node types as well, to allow consumers the option to use the composite nodes to avoid IPC?
it would be rather trivial to add as they can now both share basically all their code between them.

[peci1]

Thanks for the proposal.

I think this package should only implement composable nodes in code. They can be quite easily turned into standalone nodes in CMake:

https://github.com/ctu-vras/ros-utils/blob/07595a9a71012b117c4fad2cd9215d892c1cb15f/cras_topic_tools/CMakeLists.txt#L43-L44

So that should simplify the implementation quite a lot (no more need for shared custom spinning code etc.).

I'm also thinking if it would be possible to simplify the generic filters by using a template that would generate the .cpp file during build phase (of course not for images/pointclouds which need the extra stuff around transports). That would shrink the number of files in this repo quite a lot (I'm just not sure if there's a possible rebuild-time penalty - whether CMake is clever enough to figure out the generated files did not change).

I also foresee a possible feature that could influence the design even at this stage: lifecycle nodes. As lifecycle support will be added to image_transport in Lyrical, it is something that so far only lives on the Rolling release: ros-perception/image_common#352 . But I think it would make sense to also provide these filters as lifecycle nodes. I'm just not sure about the transport-based ones on older distros where there's no support for lifecycle nodes. We will probably not be able to provide them on older distros, which means some #ifs in the code.

In general, I'd like to keep this repo as a single ros2 branch for all supported ROS 2 releases. Until there's a lot of changes, it is usually quite okay with a few #ifs. In the future, the code might get overcomplicated, but for now, I think it should be doable.

Our repos in general support only Jazzy and newer, but I don't mind having Humble supported here as long as you can test it and it doesn't bring too much pain to the code (in general, I consider Humble as an alpha release of ROS 2, with Jazzy becoming close to a good beta).

Regarding ros2_fmt_logger, I understand it is easier to use, but with the "amount" of logging in this package, I don't think it's worth it. It is a non-core package and as such it has unclear support/changes policy. I'd like to stay with the standard logging interface here.

[solonovamax]

I think this package should only implement composable nodes in code. They can be quite easily turned into standalone nodes in CMake:

yeah, that's what I was thinking if you wanted composable nodes. I just did the very straightforward way first, but changing to composable nodes is quite easy

I'm also thinking if it would be possible to simplify the generic filters by using a template that would generate the .cpp file during build phase (of course not for images/pointclouds which need the extra stuff around transports). That would shrink the number of files in this repo quite a lot (I'm just not sure if there's a possible rebuild-time penalty - whether CMake is clever enough to figure out the generated files did not change).

not sure how I'd do that as I'm not familiar with cmake, however it shouldn't be too difficult afaik.
and I've got no clue if cmake would realize that the files haven't changed or not.
but in development I'm compiling with clang + mold + ccache, which does help speed up compilation significantly, which is very nice.

I also foresee a possible feature that could influence the design even at this stage: lifecycle nodes. As lifecycle support will be added to image_transport in Lyrical, it is something that so far only lives on the Rolling release: ros-perception/image_common#352 . But I think it would make sense to also provide these filters as lifecycle nodes. I'm just not sure about the transport-based ones on older distros where there's no support for lifecycle nodes. We will probably not be able to provide them on older distros, which means some #ifs in the code.

lifecycle nodes might be able to be introduced for everything except point cloud 2 & image, and then only have those two as normal nodes. I don't think that would be too difficult.

In general, I'd like to keep this repo as a single ros2 branch for all supported ROS 2 releases. Until there's a lot of changes, it is usually quite okay with a few #ifs. In the future, the code might get overcomplicated, but for now, I think it should be doable.

afaik the way you'd do this with ros2 is by adding smth in the cmake to expose ROS_DISTRO_* with the current distro name, yeah?

because from what I can tell, ros2 has no equivalent to ros1's ROS_VERSION_MINIMUM, so you need to put together an equivalent yourself

Our repos in general support only Jazzy and newer, but I don't mind having Humble supported here as long as you can test it and it doesn't bring too much pain to the code (in general, I consider Humble as an alpha release of ROS 2, with Jazzy becoming close to a good beta).

I'm targetting humble because my primary usecase uses humble (nvidia jetson orin nano, and nvidia currently only supports hunble/ubuntu 22.04 for it, sigh. I want to move to jazzy so badly)
but in terms of this, the differences between humble & jazzy should be relatively minor, so supporting humble shouldn't be too difficult here.

Regarding ros2_fmt_logger, I understand it is easier to use, but with the "amount" of logging in this package, I don't think it's worth it. It is a non-core package and as such it has unclear support/changes policy. I'd like to stay with the standard logging interface here.

the author has been rather responsive with me for support/changes.
and since it's header-only in a single file, if there's ever some change which needs to be made that can't wait for upstream, it can always be vendored in temporarily (or permanently) (which is what I did when I had an issue with it at one point)

but if you don't want it then I won't add it, as it's not by any means critical.

[solonovamax]

actually, it seems like there is RCLCPP_VERSION_GTE:

• [rclcpp] Generate version header ros2/rclcpp#1805
• Add ament_cmake_gen_version_h package ament/ament_cmake#198
• https://github.com/ament/ament_cmake/blob/rolling/ament_cmake_gen_version_h/cmake/version.h.in#L37-L44

unsure if this exists in humble, however some people are saying it does, so I'll give that a try.

though the way the version increments for rclcpp is a little odd.
in humble it's currently 16.0.18, then in iron (which we wouldn't support because it's EOL due to not being an LTS) it's 21.0.8, then in jazzy it's 28.1.17, then in kilted it's 29.5.7

so the major version is not necessarily tied 1-to-1 with the ros distro. but it can be used to approximate it, I guess?

[peci1]

though the way the version increments for rclcpp is a little odd.
in humble it's currently 16.0.18, then in iron (which we wouldn't support because it's EOL due to not being an LTS) it's 21.0.8, then in jazzy it's 28.1.17, then in kilted it's 29.5.7

Yes, this is a bit PITA. I've ended up with CMake feature tests as the most reliable way: ctu-vras/compass@86e8b40 .

[solonovamax]

updated the description with a checklist for tracking the remaining things that need to be done

[peci1]

Thanks for moving this forward. I've added CI to test building on all platforms and distros. I've also updated package.xml for ROS 2 (I haven't known that ament is happy to build a package with invalid package.xml). Please update your working copy.

I can definitely help with the CMakeLists.txt cleanup (once the implementation is stabilized) and possibly also writing some simple tests to check whether the filter chains work properly.

[peci1]

because from what I can tell, ros2 has no equivalent to ros1's ROS_VERSION_MINIMUM, so you need to put together an equivalent yourself

A little bit more detail on this. ROS core packages follow the semantic versioning paradigm, so they increase major version every time there is a backward incompatible change. This cannot, of course, happen on released distros, but it does happen on rolling. And apparently, the changes in rolling can be released so often that there are multiple major version bumps between a new stable ROS is branched off.

To make your life more difficult, features from rolling get backports to older distros if possible, so you can't say simply "this feature is available in humble+". You'd have to explicitly list the particular version from which the feature was available on all released distros (up to the next stable distro to be released). This gets quite complicated because sometimes the stuff in changelogs of the core packages differs a bit from what was released (e.g. some versions in changelog were never released on buildfarm, but somebody can theoretically have them built from source).

I've tried to open a discussion about making the detection of these features easier, but nobody seemed to be interested in discussing it: https://discourse.openrobotics.org/t/should-feature-adding-deprecating-changes-to-core-repos-define-feature-flags/52119 . If you find the proposal reasonable, feel free to add a comment, I'd be glad for it.

I'm targetting humble because my primary usecase uses humble (nvidia jetson orin nano, and nvidia currently only supports hunble/ubuntu 22.04 for it, sigh. I want to move to jazzy so badly)

If you want Jazzy so badly, it's just a sudo do-release-upgrade away. This is how we run our Jetsons with 24.04, happily for over a year. I haven't noticed any negative side-effects.

but in terms of this, the differences between humble & jazzy should be relatively minor,

Look at the CI :) Especially in the filters realm, the changes were quite wild.

ros2_fmt_logger: but if you don't want it then I won't add it, as it's not by any means critical.

Please, let's do this without the fmt logger.

[solonovamax]

I'm targetting humble because my primary usecase uses humble (nvidia jetson orin nano, and nvidia currently only supports hunble/ubuntu 22.04 for it, sigh. I want to move to jazzy so badly)

If you want Jazzy so badly, it's just a sudo do-release-upgrade away. This is how we run our Jetsons with 24.04, happily for over a year. I haven't noticed any negative side-effects.

hmmm

I was planning to do that but had read somewhere that it's not a good idea to do so never did it. but if it's worked fine for you, then I'll try to push that through sometime soon. though it might be a month or so before my robotics team has sufficient downtime to be able to do that, so for now I'll continue to target humble.

[solonovamax]

@peci1 I've completed all the things I had on the checklist except for:

• lifecycle support for the image filter chain
• tests

for the tests, what have you done to test it manually in the past?

[solonovamax]

also, CI builds successfully on all distros without the need for any #ifs or detection of features using try_compile.
I would need that in order to support the lifecycle node for image transport, and I do have some changes for that shelved atm, but I'll get around to it later.
atm I'm blocked on doing any development locally for rolling, as there's some compile errors with the nix packages that I'll fix later.

for lifecycle nodes, I've introduced a second node type, so there's both the normal node (e.g. imu_filter_chain), and then there's also an _lifecycle node (e.g. imu_filter_chain_lifecycle)
the reason I've chosen to have two nodes, 1 for lifecycle and 1 for a normal node is so that if someone doesn't find lifecycle nodes necessary, they don't need to introduce any additional code to start the lifecycle node, and they can just use the normal node instead.
afaik there isn't a way to define a lifecycle node that "auto-starts" without a launch file.
this does double the number of things that need to be compiled, but eh, it's fine

[solonovamax]

I've converted REGISTER_ALL_MSG_FILTERS to using BOOST_PP_SEQ_FOR_EACH, in order to aid with future adoption of something like #8. when I eventually add #8, I will likely backport the same thing to the ros 1 version as well.

[peci1]

Great, these are all good changes! Do you want to finish the lifecycle stuff as a part of this PR, or do you want to do it later?

[peci1]

for the tests, what have you done to test it manually in the past?

I don't think I did anything rigorous. I just set up a publisher, a filter and looked at its output in console, I guess.

[solonovamax]

I don't think I did anything rigorous. I just set up a publisher, a filter and looked at its output in console, I guess.

alright, in that case I'll probably just do something similar.

the port is basically complete at this point, and there's only a few minor things left, really. so, I'll try to get to those, and then it should be ready for merging