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

make writer_guid uint8_t[] instead of int8_t[] for consistency with rmw_gid_t #329

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

ihasdapie
Copy link
Member

This PR address another inconsistency I noticed with with gids in rmw, namely that rmw_gid_t is an uint8_t array and rmw_request_id_t.writer_guid is a int8_t array. Changing this would silence some rcl compilation warnings introduced by the service introspection feature (ros2/ros2#1285).

Reference:

rmw/rmw/include/rmw/types.h

Lines 356 to 363 in 2259c3f

typedef struct RMW_PUBLIC_TYPE rmw_request_id_s
{
/// The guid of the writer associated with this request
int8_t writer_guid[16];
/// Sequence number of this service
int64_t sequence_number;
} rmw_request_id_t;

rmw/rmw/include/rmw/types.h

Lines 620 to 627 in 2259c3f

typedef struct RMW_PUBLIC_TYPE rmw_gid_s
{
/// Name of the rmw implementation
const char * implementation_identifier;
/// Bype data Gid value
uint8_t data[RMW_GID_STORAGE_SIZE];
} rmw_gid_t;

I'm going to keep this PR as draft until I review the rmw implementations to check if any notable assumptions are made about the signedness of writer_guid, but I don't expect it to be an issue.

Signed-off-by: Brian Chen brian.chen@openrobotics.org

@ihasdapie ihasdapie mentioned this pull request Aug 23, 2022
14 tasks
@fujitatomoya
Copy link
Collaborator

@ihasdapie
Copy link
Member Author

AFAIK, Fast-DDS takes it just byte code, either int8_t or uint8_t during copy.

I've dug through a number of rmw implementations and all of them take it bytecode as well. I doubt making this change will break anything for "unofficial" rmws and we can run CI for the official impls

See: #328 (comment)

@ihasdapie
Copy link
Member Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@ihasdapie ihasdapie marked this pull request as ready for review August 29, 2022 20:51
@jacobperron jacobperron self-assigned this Sep 1, 2022
…_gid_t

Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
@clalancette
Copy link
Contributor

clalancette commented Jan 31, 2023

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status

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.

None yet

4 participants