Skip to content

fix(rcl_action): Allow to pass the timer to action during initialization#1201

Merged
ahcorde merged 3 commits intoros2:rollingfrom
cellumation:expose_timer
Jan 30, 2025
Merged

fix(rcl_action): Allow to pass the timer to action during initialization#1201
ahcorde merged 3 commits intoros2:rollingfrom
cellumation:expose_timer

Conversation

@jmachowinski
Copy link
Copy Markdown
Contributor

@jmachowinski jmachowinski commented Dec 6, 2024

This change is needed in order to expose the timer to event based executors.

Precursor to ros2/rclcpp#2699

@jmachowinski jmachowinski marked this pull request as draft December 6, 2024 12:11
@jmachowinski jmachowinski marked this pull request as ready for review December 6, 2024 15:26
@ros-discourse
Copy link
Copy Markdown

ros-discourse commented Dec 6, 2024

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/next-client-library-wg-meeting-friday-6th-december-2024-8am-pt/40954/2

@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Dec 17, 2024

@alsora @mjcarroll @wjwwood can one of you do a review, and start the CI ?

This commit is the precursor to ros2/rclcpp#2699

@alsora
Copy link
Copy Markdown
Contributor

alsora commented Jan 10, 2025

CI

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

ahcorde
ahcorde requested changes Jan 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

This branch has conflicts

@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Jan 10, 2025 •
edited
Loading

rebased. @ahcorde can you rerun the CI ?

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Jan 10, 2025

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

fujitatomoya
fujitatomoya approved these changes Jan 10, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@jmachowinski do you want to merge this to rolling now? or we can wait for ros2/rclcpp#2699, and merge them all together. (i think we usually the latter procedure)

fujitatomoya
fujitatomoya reviewed Jan 10, 2025
View reviewed changes
@jmachowinski jmachowinski mentioned this pull request Jan 28, 2025
Merged
@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Jan 28, 2025

Pulls: ros2/rclcpp#2699, #1201
Gist: https://gist.githubusercontent.com/ahcorde/a4917dcff0940203284bcd592f2ecb96/raw/dfc2a9450c7b234ee278414773300dc944688bdf/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp rcl --packages-above-and-dependencies rclcpp rcl
TEST args: --packages-above rclcpp rcl --packages-above rclcpp rcl
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15100

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

Janosch Machowinski added 2 commits January 28, 2025 11:03
fix(timer): Use impl pointer in jump callback
73f7cc0 
The interface description does not explicitly state that a
rcl_timer_t may not be copied around. Therefore users may do this.
By using a known never changing pointer in the callbacks, we avoid
segfaults, even if the 'user' decides to copy the rcl_timer_t around.

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
fix(rcl_action_server_init): Do not return RCL_RET_OK in some error c…
7f5ea7b 
…ases

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
@jmachowinski jmachowinski force-pushed the expose_timer branch 3 times, most recently from 36e1bcd to 7d95d86 Compare January 28, 2025 12:05
@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Jan 28, 2025

Pulls: ros2/rclcpp#2699, ros2/system_tests#556, #1201
Gist: https://gist.githubusercontent.com/jmachowinski/711b3e342bed5cd8c16b1b4cffd6c361/raw/22af95b0a7ea9789dfda5280fe2b51d84965a9de/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15101

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

@jmachowinski jmachowinski mentioned this pull request Jan 28, 2025
Merged
fix(rcl_action): Allow to pass the timer to action during initialization
991601f 
This change is needed in order to expose the timer to event based
executors.

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Jan 28, 2025

Pulls: ros2/rclcpp#2699, ros2/system_tests#556, #1201
Gist: https://gist.githubusercontent.com/jmachowinski/7608e9da50fcb66b359b3a37b7a301b4/raw/22af95b0a7ea9789dfda5280fe2b51d84965a9de/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15102

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

@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Jan 29, 2025 •
edited
Loading

Pulls: ros2/rclcpp#2699, ros2/system_tests#556, #1201
Gist: https://gist.githubusercontent.com/jmachowinski/d0c0fd4939d4c2a8515d3e7c6039c009/raw/22af95b0a7ea9789dfda5280fe2b51d84965a9de/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15105

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

@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Jan 29, 2025

CI is green, can someone merge this ?

@jmachowinski
Copy link
Copy Markdown
Contributor Author

jmachowinski commented Jan 30, 2025

@ahcorde @mjcarroll @alsora @fujitatomoya ping for merge

@ahcorde ahcorde merged commit fc5388e into ros2:rolling Jan 30, 2025
@fujitatomoya fujitatomoya mentioned this pull request Jan 31, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

@mjcarroll mjcarroll mjcarroll approved these changes

@ahcorde ahcorde ahcorde approved these changes

@alsora alsora alsora approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jmachowinski @ros-discourse @alsora @ahcorde @mjcarroll @fujitatomoya