Skip to content

Conversation

@n8hui
Copy link

@n8hui n8hui commented May 3, 2025

Add function rcl_timer_get_next_call_time to support humble backport of rclpy EventsExecutor (ros2/rclpy#1454).

Cherry-picked from commit 2e9549c (#1146)

(cherry-picked from commit 2e9549c)

Signed-off-by: Nathan Hui <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented May 5, 2025

Pulls: #1237
Gist: https://gist.githubusercontent.com/ahcorde/5bd65e2e0ca27ab1098c3a2218d70338/raw/046435f802f9811c8cc50aa7de21ced70d32169c/ros2.repos
BUILD args: --packages-above-and-dependencies rcl
TEST args: --packages-above rcl
ROS Distro: humble
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15908

  • 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.

@n8hui why we are seeing the difference from #1146 original fix? which includes the improvements and tests. if the PR can be backported to humble, maybe we should cherry-pick entire fix from #1146? my major concern is the test and stability for this additional function, currently this PR just picks up the new function and never tested in rcl.

@jmachowinski
Copy link
Contributor

@fujitatomoya I don't know, #1146 slightly alters the behavior of the waits. It does not fix a critical bug, just too much wakeups if simulation time is slower than real time. From my point of view Humble is outdated (event though official EOL is 2027) and its not worth the risk of introducing a regression of some kind.

@fujitatomoya
Copy link
Collaborator

@jmachowinski thanks! yeah we obviously want to avoid possible regression for humble, totally agree. if that is the case, probably we could add some unit tests for this free function just to make sure that works fine with CI? @n8hui can you add unit tests for this function with this PR?

@jmachowinski
Copy link
Contributor

Pulls: #1237
Gist: https://gist.githubusercontent.com/jmachowinski/64f9d2df390edbcea11454e79b9b30df/raw/8feb2361b5352445c5829a5335832e3284ecd8e1/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16339

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows 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.

5 participants