Skip to content

Support multiple hardware interfaces in the coordinator #72

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

Merged
merged 4 commits into from
Jul 31, 2025

Conversation

evan-palmer
Copy link
Member

Changes Made

This PR adds support for activating/deactivating multiple hardware interfaces using the controller coordinator.

Associated Issues

Testing

TBD

@evan-palmer evan-palmer self-assigned this Jul 29, 2025
@evan-palmer evan-palmer requested a review from Copilot July 29, 2025 23:46
Copilot

This comment was marked as outdated.

@evan-palmer evan-palmer requested a review from Copilot July 30, 2025 02:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables the controller coordinator to support multiple hardware interfaces instead of being limited to a single interface. The main enhancement allows users to specify an ordered list of hardware interfaces that can be activated/deactivated sequentially through the coordinator service.

Key changes:

  • Modified parameter configuration to accept multiple hardware interfaces via hardware_sequence array
  • Updated the coordinator implementation to handle sequential activation/deactivation of multiple hardware interfaces
  • Bumped package versions across all modules to 0.3.3

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
controller_coordinator/src/coordinator_parameters.yaml Changed parameter from single hardware_interface to hardware_sequence array
controller_coordinator/src/coordinator.hpp Updated member variables to handle multiple hardware interface requests
controller_coordinator/src/coordinator.cpp Implemented sequential processing of multiple hardware interfaces with proper error handling
end_effector_trajectory_controller/CHANGELOG.md Fixed incorrect package name in changelog header
Multiple package.xml files Version bump to 0.3.3 across all packages
Multiple CHANGELOG.md files Added 0.3.3 release entries

Comment on lines 118 to 120
rclcpp::spin_until_future_complete(this->get_node_base_interface(), hardware_future);
}

Copy link
Preview

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

Sequential blocking calls to spin_until_future_complete may cause performance issues if there are many hardware interfaces. Consider implementing parallel activation with proper synchronization or adding timeout handling to prevent indefinite blocking.

Suggested change
rclcpp::spin_until_future_complete(this->get_node_base_interface(), hardware_future);
}
futures.push_back(hardware_future.share());
}
for (auto & future : futures) {
if (future.wait_for(std::chrono::seconds(10)) != std::future_status::ready) {
RCLCPP_ERROR(this->get_logger(), "Timeout while waiting for hardware activation");
response->success = false;
response->message = "Timeout during hardware activation";
}
}

Copilot uses AI. Check for mistakes.

Comment on lines 157 to 159
rclcpp::spin_until_future_complete(this->get_node_base_interface(), hardware_future);
}

Copy link
Preview

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

Sequential blocking calls to spin_until_future_complete may cause performance issues if there are many hardware interfaces. Consider implementing parallel deactivation with proper synchronization or adding timeout handling to prevent indefinite blocking.

Suggested change
rclcpp::spin_until_future_complete(this->get_node_base_interface(), hardware_future);
}
futures.push_back(hardware_future.share());
}
for (auto & future : futures) {
if (future.wait_for(std::chrono::seconds(10)) != std::future_status::ready) {
RCLCPP_ERROR(this->get_logger(), "Timeout while deactivating hardware interface");
response->success = false;
response->message = "Timeout while deactivating hardware interface";
}
}

Copilot uses AI. Check for mistakes.

@evan-palmer evan-palmer merged commit 734f1a0 into main Jul 31, 2025
2 of 4 checks passed
@evan-palmer evan-palmer deleted the dev-multiple-interfaces branch July 31, 2025 03:30
mergify bot pushed a commit that referenced this pull request Jul 31, 2025
* add support for multiple hardware interfaces

* changelog

* make hardware activation synchronous

* Fixed coordinator hardware bug

(cherry picked from commit 734f1a0)
evan-palmer added a commit that referenced this pull request Aug 1, 2025
* add support for multiple hardware interfaces

* changelog

* make hardware activation synchronous

* Fixed coordinator hardware bug

(cherry picked from commit 734f1a0)

Co-authored-by: Evan Palmer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support activating/deactivating multiple hardware interfaces from the coordinator
1 participant