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.

Sign up for GitHub

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

Jump to bottom

Add ~/get_type_description service (rep2011) (dupe of #1052) #1059

Closed
wants to merge 17 commits into from
Closed

Add ~/get_type_description service (rep2011) (dupe of #1052) #1059

wants to merge 17 commits into from

Conversation

james-rms
Copy link

@james-rms james-rms commented Apr 10, 2023 •
edited
Loading

Dupe of #1052 , rebased to fix a test.

@james-rms james-rms force-pushed the jrms/rep2011_get_type_description_srv branch from 388dd40 to bf20c9d Compare April 10, 2023 06:08
achim-k and others added 17 commits April 10, 2023 16:08
@achim-k @james-rms
Move node impl struct to separate header
7c7e9e4 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Add functions to convert between rosidl_runtime_c / type_description_…
5250463 
…interfaces structs

Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Add node type cache
a8e0942 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Add ~/get_type_description service
cfb6c02 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Fix missing include
411d71c 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Adapt to latest type support interface changes
9eb1491 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @emersonknapp @james-rms
Apply suggestions from code review
e3e811a 
Co-authored-by: Emerson Knapp <537409+emersonknapp@users.noreply.github.com>
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Remove unnecessary preprocessor directives
c1eba79 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Improve argument naming
58800b1 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Move type cache init function declarations to node_impl.h
8efb552 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Use direct rcl_service_t instead of pointer in node impl struct
9081bb9 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Decrease log severity
afc64a0 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Adapt to latest interface changes
6bcbfd9 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
fix test errors
6c264e2 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Apply suggestions from code review
d39b312 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
Fix error messages being overwritten
0158d61 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@achim-k @james-rms
add get_type_description test
baf3d15 
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: James Smith <james@foxglove.dev>
@james-rms james-rms force-pushed the jrms/rep2011_get_type_description_srv branch from bf20c9d to baf3d15 Compare April 10, 2023 06:09
@james-rms
Copy link
Author

Gist: https://gist.githubusercontent.com/james-rms/e1c0c45c7e56d86cb09c5bbc28aa5f72/raw/ef08db1b38130acc2c1b27715c98cf640f14e24c/ros2.repos
BUILD args: --packages-above-and-dependencies rcl rcl_action
TEST args: --packages-above rcl rcl_action
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11833

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

@clalancette
Copy link
Contributor

Instead of a completely new PR, can we just open a PR to that PR? It will keep all of the history intact.

@achim-k
Copy link
Contributor

achim-k commented Apr 10, 2023

Instead of a completely new PR, can we just open a PR to that PR? It will keep all of the history intact.

This would then be a new PR on top of #1052 with a single merge commit?

@clalancette
Copy link
Contributor

This would then be a new PR on top of #1052 with a single merge commit?

In general, we like to keep all development of a branch in one PR. If we need to rebase #1052, then please feel free to do that. If we need to add new PRs on top of #1052 , then please just push them there. And then close this PR.

The only reasons to open a new PR is if we want to split some work out, or if the original author is not responding and we can't push to their fork anymore for some reason.

@achim-k
Copy link
Contributor

achim-k commented Apr 10, 2023

OK, I have rebased #1052 so I believe we can close this PR

@clalancette
Copy link
Contributor

OK, I have rebased #1052 so I believe we can close this PR

OK, sounds good. Will close this one out. @james-rms If we need this one for some reason, please feel free to reopen.

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

@audrow audrow Awaiting requested review from audrow audrow is a code owner

@ivanpauno ivanpauno Awaiting requested review from ivanpauno ivanpauno is a code owner

@wjwwood wjwwood Awaiting requested review from wjwwood wjwwood is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@james-rms @clalancette @achim-k