Skip to content
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

Extend TypeDescription codegen with referenced type inclusion, disable option, raw sources #10

Conversation

emersonknapp
Copy link
Owner

@emersonknapp emersonknapp commented Apr 4, 2023

Part of ros2/ros2#1159

Bundled with (must be merged together):

Replaces (or extends) ros2#727. Key differences

  • type_support_t->get_type_hash_func rather than bare value
  • get_type_description and get_type_sources take a typesupport struct argument (for use by dynamic types)
  • Type descriptions and sources include referenced types, rather than baking in full recursive dependencies per-interface
  • Expected hashes of referenced types are included in codegen to optionally error-check with at runtime

Key features:

  • Changes TYPE_HASH_TUPLES to more accurate TYPE_DESCRIPTION_TUPLES in cmake
  • Adds new C interface functions get_type_description, get_individual_type_description_source, and get_type_description_sources
  • Implements above, generating code for type descriptions and raw interface definition sources, embedded in the binaries
  • Optionally disables embedding that information in the generated code by the CMake flag ROSIDL_GENERATOR_C_DISABLE_TYPE_DESCRIPTION_CODEGEN
  • Refactors rosidl_generator_type_description to output a new structure of type_description_msg plus a flat list of all referenced types and their hashes at the time of generation. These hashes are used at runtime (in Debug builds) to detect ABI breaks when constructing type descriptions the first time

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
…of date

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp
Copy link
Owner Author

Windows also needs to open(encoding='utf-8') explicitly:

  • Windows Build Status

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewers - these files are the copied generated sources, so they give a good sense for what the templates do, since those are not super easy to read

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp force-pushed the emersonknapp/type-description-include-nested branch from e57ee67 to f581176 Compare April 4, 2023 23:29
Copy link

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short, I don't have much to say here; it looks generally good. I'll suggest we go ahead and merge this one into ros2#727 now, and run CI. I'll do one final pass on the combination before approving over there.

@emersonknapp emersonknapp merged commit e482986 into emersonknapp/type-description-struct Apr 5, 2023
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.

3 participants