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

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

Open
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

jmachowinski
Copy link
Contributor

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

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
Contributor Author

@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
Contributor

alsora commented Jan 10, 2025

CI

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

Copy link
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

Janosch Machowinski added 3 commits January 10, 2025 11:03
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>
…ases

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
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
Contributor Author

jmachowinski commented Jan 10, 2025

rebased. @ahcorde can you rerun the CI ?

@ahcorde
Copy link
Contributor

ahcorde commented Jan 10, 2025

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

Copy link
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)

@@ -161,7 +162,8 @@ rcl_action_get_zero_initialized_server(void);
* \param[out] action_server handle to a preallocated, zero-initialized action server structure
* to be initialized.
* \param[in] node valid node handle
* \param[in] clock valid clock handle
* \param[in] expire_timer An initialized and canceled timer. The caller must guarantee the lifetime of the
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is weird, uncrustify should warn this because maximum line length is 100 characters...

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.

5 participants