Expose type hash on typesupports (rep2011)#729
Conversation
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
emersonknapp
force-pushed
the
emersonknapp/hash-typesupport
branch
from
5b9f0b6 to
3ee9e4c
Compare
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
emersonknapp
force-pushed
the
emersonknapp/hash-typesupport
branch
from
3ee9e4c to
1a8fce4
Compare
emersonknapp
left a comment
emersonknapp
left a comment
There was a problem hiding this comment.
Calling out some key spots for reviewers
| // Note: this define is for MSVC, where the static const var can't be used in downstream aggregate initializers | ||
| #define @(hash_var)__INIT @(type_hash_to_c_definition(type_hash['service'], line_final_backslash=True)) | ||
| static const rosidl_type_hash_t @(hash_var) = @(hash_var)__INIT; | ||
| static const rosidl_type_hash_t @(idl_structure_type_to_c_typename(service.namespaced_type))__@(TYPE_HASH_VAR) = @(type_hash_to_c_definition(type_hash['service'])); |
There was a problem hiding this comment.
I am getting rid of the #define __INIT by using pointers in the other places this is used as an initializer. I think everything is nicer this way.
|
|
||
| set(_target_suffix "__rosidl_generator_cpp") | ||
| add_library(${rosidl_generate_interfaces_TARGET}${_target_suffix} INTERFACE) | ||
| target_compile_features(${rosidl_generate_interfaces_TARGET}${_target_suffix} INTERFACE cxx_std_17) |
There was a problem hiding this comment.
Is this a concern? We've upgraded everything else to C++17 but the generated interface libraries themselves hadn't actually specified a standard version. This is needed for the static constexpr rosidl_type_hash_t in non-templated Service and Action classes, for the inline variable thing that's new in C++17
There was a problem hiding this comment.
I don't think this is a concern; we've specified in REP-2000 that we target C++17, and we've upgraded most of the packages at this point to C++17. So I'm fine with adding this here.
| /// Pointer to the service type support handler function | ||
| rosidl_service_typesupport_handle_function func; | ||
| /// Service request message typesupport | ||
| const rosidl_message_type_support_t * request_typesupport; |
There was a problem hiding this comment.
These rosidl_message_type_support instances were not available in all contexts. rosidl_typesupport_fastrtps exposed them in its ServiceMembers, but rosidl_typesupport_introspection only exposes nested MessageMembers with no access to the message typesupport.
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
emersonknapp
requested review from
adityapande-1995,
methylDragon,
quarkytale and
sloretz
as code owners
|
Smoke check (one run with 3 PRs combined)
|
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
I've left one question inline. Otherwise, this looks fine to me with green CI.
rosidl_generator_type_description/rosidl_generator_type_description/__init__.py
Show resolved
Hide resolved
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
|
Gist: https://gist.githubusercontent.com/emersonknapp/6f64e848cf178a828c180607a21fa967/raw/ba310891c24e6d45715e2fdd58d3a978b39d9442/ros2.repos
|
|
I'm just trying Windows Windows: |
It didn't, that's a flaky test. Everything else looks good there, so I'll go ahead and merge the three of them in this series. |
3 participants
No dependencies.
Followup to #722
Part of ros2/ros2#1159
Bundled with (must be merged together)
Changes:
rosidl_type_hash_ttomessage_type_support_t(andservice_type_support_t,action_type_support_t)TYPE_VERSION_HASHconsistently toTYPE_HASHbefore it's too late__INITworkaround by using the type hashes as pointers elsewhere instead of static initializers