Skip to content

Conversation

@idesign0
Copy link
Contributor

@idesign0 idesign0 commented Nov 11, 2025

This PR introduces the necessary platform-specific implementation for thread prioritization within nav2_util::setSoftRealTimePriority() for macOS systems.

Context

macOS does not support the standard POSIX real-time scheduling policies (SCHED_FIFO or SCHED_RR) for user-level threads, which are relied upon in the existing Linux (#else) implementation. When running on macOS, This resulted in a compilation error (e.g., "undeclared identifier") on macOS, completely blocking the build process for the package.

Solution

The code is updated within the #ifdef __APPLE__ block to use the native Mach kernel API, which is the correct and most effective method for this platform:

  1. Policy: Uses the Mach kernel's THREAD_PRECEDENCE_POLICY, setting the importance to the maximum value (63). This achieves the highest available user-space priority, fulfilling the requirement for soft real-time performance on macOS.
  2. Diagnostics: The error handling is significantly improved to aid in debugging. It now includes the header <mach/error.h> and uses the function mach_error_string() to provide a human-readable description of the kern_return_t error code if the policy fails to set.

Files Changed

  • nav2_util/src/node_utils.cpp

Fix: std::chrono::duration Casting Error

This resolves a strict type conversion error specific to Clang/libc++ on macOS builds.

Problem:
The Tree::sleep() function expects a std::chrono::microseconds duration (or system_clock::duration), but the calling code provides a higher-resolution std::chrono::nanoseconds duration (time_to_sleep_ns). The macOS compiler rejects this implicit conversion due to potential data loss (truncation).

Solution:
Added an explicit std::chrono::duration_cast<std::chrono::microseconds>() wrapper to explicitly convert the nanosecond duration before passing it to tree_->sleep(), resolving the compilation error.

Files Changed:

  • nav2_behavior_tree/include/nav2_behavior_tree/utils/loop_rate.hpp

Signed-off-by: Dhruv Patel <[email protected]>
@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

❌ Patch coverage is 89.13858% with 29 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...clude/nav2_behavior_tree/bt_action_server_impl.hpp 60.00% 4 Missing ⚠️
...2_bt_navigator/src/navigators/navigate_to_pose.cpp 63.63% 4 Missing ⚠️
...avigator/src/navigators/navigate_through_poses.cpp 76.92% 3 Missing ⚠️
nav2_mppi_controller/src/optimizer.cpp 62.50% 3 Missing ⚠️
..._pure_pursuit_controller/src/collision_checker.cpp 0.00% 3 Missing ⚠️
...llowing/opennav_following/src/following_server.cpp 60.00% 2 Missing ⚠️
...ree/plugins/condition/are_poses_near_condition.cpp 0.00% 1 Missing ⚠️
...nclude/nav2_behaviors/plugins/drive_on_heading.hpp 0.00% 1 Missing ⚠️
...tmap_2d/plugins/costmap_filters/keepout_filter.cpp 50.00% 1 Missing ⚠️
nav2_costmap_2d/plugins/static_layer.cpp 0.00% 1 Missing ⚠️
... and 6 more
Files with missing lines Coverage Δ
nav2_amcl/src/amcl_node.cpp 86.31% <100.00%> (ø)
nav2_amcl/src/map/map_cspace.cpp 90.76% <100.00%> (+0.14%) ⬆️
...nclude/nav2_behavior_tree/behavior_tree_engine.hpp 100.00% <ø> (ø)
...ree/plugins/action/compute_path_to_pose_action.hpp 100.00% <ø> (ø)
...avior_tree/plugins/action/compute_route_action.hpp 100.00% <100.00%> (ø)
...lugins/action/remove_in_collision_goals_action.hpp 100.00% <100.00%> (ø)
...tree/plugins/action/remove_passed_goals_action.hpp 100.00% <100.00%> (ø)
...tree/plugins/condition/is_path_valid_condition.hpp 100.00% <100.00%> (ø)
...e/plugins/condition/is_pose_occupied_condition.hpp 100.00% <100.00%> (ø)
...or_tree/plugins/condition/is_stopped_condition.hpp 100.00% <100.00%> (ø)
... and 88 more

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@idesign0 idesign0 changed the title Fix(macOS): Use Mach Precedence Policy for Soft Real-Time Thread Priority Fix(macOS): Use Mach Precedence Policy for Soft Real-Time Thread Priority and std::chrono::duration Casting Error Nov 14, 2025
Signed-off-by: Dhruv Patel <[email protected]>
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Woah - this is now a 1000 line PR and its broken a few linting rules. Checkout the CI failed job. Otherwise I approve.

@idesign0
Copy link
Contributor Author

Woah - this is now a 1000 line PR and its broken a few linting rules. Checkout the CI failed job

i ran pre-commit to all the files. 😅

@SteveMacenski
Copy link
Member

SteveMacenski commented Nov 19, 2025

Alot of these changes look correct, I'm confused why they weren't flagged before. I've been noticing for months now that some parts of the linting profile aren't being flagged correctly. It seems like a change in the ament linting rules or something that I don't understand. If you have any ideas, I'd love to hear it. I've looked into it before and didn't come up with anything

Edit: So I'd say just fix the few things it flags as wrong and leave the rest. thanks for the service 🫡

@idesign0
Copy link
Contributor Author

Alot of these changes look correct, I'm confused why they weren't flagged before. I've been noticing for months now that some parts of the linting profile aren't being flagged correctly. It seems like a change in the ament linting rules or something that I don't understand. If you have any ideas, I'd love to hear it. I've looked into it before and didn't come up with anything

Edit: So I'd say just fix the few things it flags as wrong and leave the rest. thanks for the service 🫡

here we go! 😇

@SteveMacenski
Copy link
Member

Just waiting on CI

@mergify
Copy link
Contributor

mergify bot commented Nov 19, 2025

@idesign0, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@ros-navigation ros-navigation deleted a comment from mergify bot Nov 20, 2025
@ros-navigation ros-navigation deleted a comment from mergify bot Nov 20, 2025
@SteveMacenski SteveMacenski merged commit 2f741fd into ros-navigation:main Nov 20, 2025
15 of 16 checks passed
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.

2 participants