-
Notifications
You must be signed in to change notification settings - Fork 189
Seeker coordinator #2473
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
base: ros2
Are you sure you want to change the base?
Seeker coordinator #2473
Conversation
Squid5678
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some changes
| rclcpp::Subscription<rj_msgs::msg::SeekingCoordinator>::SharedPtr subscription_; | ||
|
|
||
| bool am_i_member_{false}; | ||
| rj_geometry::Point selected_target_{-1, -1}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this initialized with -1?
|
|
||
| std::string get_current_state() override; | ||
|
|
||
| void die() override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: better name
| SeekingCoordinator& operator=(SeekingCoordinator&&) = delete; | ||
|
|
||
| void service_callback(RequestPtr request, ResponsePtr response); | ||
| static rj_geometry::Point invalidPoint() { return rj_geometry::Point{-1, -1}; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we replace this invalidPoint() with just a null value once a robot leaves seeking?
| client_ = node_->create_client<rj_msgs::srv::SeekingCoordinator>("seeking_coordinator_srv"); | ||
| } | ||
|
|
||
| void SeekingClient::join_group(StatusCallback callback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a check for am_i_member_ prior to the callback to ensure that we aren't making repeated join_group() calls?
| const FieldDimensions& field_dimensions) const { | ||
| return Seeker::calculate_open_point(3.0, .2, current_position, world_state, field_dimensions); | ||
| rj_geometry::Point SeekingCoordinator::get_open_point( | ||
| int robot_id, const WorldState world_state, rj_geometry::Point current_position, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be const WorldState& world_state to avoid copying in the entire world state?
Description
Added Seeker coordinator to handle seekers and seeker points without requiring several robots to keep track of each other.
Single source of truth for current seeker points.
Associated / Resolved Issue
Resolves # or ClickUp card
Resolves Clickup Issue: 86b6ubv6c
Design Documents
Link
Steps to Test
Test Case 1
Expected result: Seekers go brrr and no weird Seeker requests and responses needed (except with the coordinator ofc)
Key Files to Review
Group 1
Group 2
Review Checklist
(Optional) Sub-issues (for drafts)
Note: if you find yourself breaking this PR into many smaller features, it may make sense to break up the PR into logical units based on these features.