Skip to content

Runtime Interface Reflection: type_description_interfaces_utils#155

Closed
methylDragon wants to merge 5 commits intorollingfrom
runtime_interface_reflection
Closed

Runtime Interface Reflection: type_description_interfaces_utils#155
methylDragon wants to merge 5 commits intorollingfrom
runtime_interface_reflection

Conversation

@methylDragon
Copy link

@methylDragon methylDragon commented Mar 10, 2023

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

Description

This PR adds a utilities library for manipulating and constructing the C structs generated by type_description_interfaces, including:

  • Handy print functions
  • Hash map construction
  • Sorting and pruning of referenced types (to prep them for hashing)
  • Checking of type validity
  • Appending fields and referenced type descriptions
  • Creating TypeDescriptions from referenced type descriptions (description subsetting)

I also pre-generate the type_description_interfaces sources and headers to create a new package (type_description_interfaces_static) to remove a dependency cycle downstream. (That's why there are so many lines.)

I haven't implemented tests yet, so some memory management issues/segfaults are expected. I'm working on it, but I am wary of spending too much time getting 100% coverage because we're trying to meet the deadline for feature freeze...

I've implemented a bunch of tests and I'm fairly confident that most of the core functionalities work as expected now, so I'm going to mark the PR as ready to review.

Asan runs green too!

clear && \
colcon build --packages-select type_description_interfaces_utils \
  --build-base=build-asan --install-base=install-asan \
  --cmake-args -DOSRF_TESTING_TOOLS_CPP_DISABLE_MEMORY_TOOLS=ON \
               -DINSTALL_EXAMPLES=OFF -DSECURITY=ON --no-warn-unused-cli \
               -DCMAKE_BUILD_TYPE=Debug \
  --mixin asan-gcc && \
colcon test --build-base=build-asan --install-base=install-asan \
  --event-handlers sanitizer_report+ --packages-select type_description_interfaces_utils

There are some less important tests waiting to be written (printing), and some tests that are already implicitly covered by other tests (but are not explicit yet.)

TODO

  • More minor tests
  • 100% docstring coverage

Open Questions

  • This only operates on the C version of the type_description_interfaces struct... Do I need to write a C++ version? (Is there a way to convert between the two versions of the message, or are they otherwise interchangeable?)
    • Perhaps that C++ utils library would just have a conversion function, since we know the layout of the struct already???
  • Should this be type_description_interfaces_utils_c instead? 😬

@methylDragon methylDragon force-pushed the runtime_interface_reflection branch 2 times, most recently from 9a608df to 3760e4d Compare March 10, 2023 01:15
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the runtime_interface_reflection branch 7 times, most recently from 457c9ba to 7ab657d Compare March 14, 2023 00:55
@methylDragon methylDragon marked this pull request as ready for review March 14, 2023 01:26
@methylDragon methylDragon requested a review from gbiggs as a code owner March 14, 2023 01:26
@methylDragon methylDragon force-pushed the runtime_interface_reflection branch from 7ab657d to 5810619 Compare March 14, 2023 02:22
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the runtime_interface_reflection branch 2 times, most recently from 338cc7c to ffcdfa8 Compare March 14, 2023 23:04
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the runtime_interface_reflection branch from ffcdfa8 to 78d4f90 Compare March 15, 2023 08:33
Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Copy link
Author

Closing this in favor of ros2/rosidl#728

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant