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

Cameron carson/penalty position dribbling #2336

Merged
merged 9 commits into from
Feb 19, 2025

Conversation

CameronLyon
Copy link
Contributor

Description

Members Cameron and Rocio modified the penalty-shooting.cpp and hpp files to implement a "dribbling" function that creates a small kick, a follow function, and then the scheduled shoot once the ball is close enough to the enemy goal (3.5 meters). This behavior has not been tested on the blue side.

Associated / Resolved Issue

ClickUp Post

Steps to Test

  1. Check to see if the penalty shot functions properly using SSL ref controller
  2. Check to see if the penalty shot functions on both blue side and on the other side of the field
  3. Check to see if the position of the goalie modifies the directed shot of the ball (if placed on left side, ball should be kicked to the right side of the goal)

Expected result: A fully autonomous penalty shot that follows the following steps

  1. Lines up behind the ball at a very small distance
  2. Pushes the ball at a given speed and continues to follow the ball until the robot is 3.5 meters away from the goal
  3. Lines itself back up again, calculates the best shot possible, and shoots it from now a closer distance from where the ball originated.

Key Files to Review

penalty_player.cpp
penalty_player.hpp

CameronLyon and others added 4 commits January 27, 2025 09:37
…s kicked once shortly, follows it, then kicks to goal. Only works when ball is at center field.
…llows ball by kicking with speed 0 and then kicks when closer to the goal. WARNING: Has only been tested on yellow side
@CameronLyon
Copy link
Contributor Author

Everything you've commented on has been resolved and additional comments/improvements were made.

@CameronLyon CameronLyon requested a review from rishiso February 12, 2025 00:37
@@ -43,14 +56,41 @@ PenaltyPlayer::State PenaltyPlayer::update_state() {

std::optional<RobotIntent> PenaltyPlayer::state_to_task(RobotIntent intent) {
switch (latest_state_) {
case LINE_UP: {
case START: { // First, gets the robot to the ball to begin penalty dribbling-shooting
SPDLOG_INFO("START");
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like some logging was left here!

case START: { // First, gets the robot to the ball to begin penalty dribbling-shooting
SPDLOG_INFO("START");
double y_pos = last_world_state_->ball.position.y();
y_pos -= kRobotRadius + 0.3; // added the 0.01 as a buffer space
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this doing--Why 0.3? Also, comment says 0.01.

break;
}
case SMALL_KICK: { // less of a kick, more of a "follow" ball closely
// SPDLOG_INFO("SMALL_KICK");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's clean these comments up too

}
case LINE_UP: { // gets the robot behind the ball with a certain distance (usually
// immediatly skipped if the robot is already close)
// SPDLOG_INFO("LINE_UP");
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above on cleaning up comments

// } else {
y_pos -= kRobotRadius + 0.3;
// }
y_pos -= kRobotRadius + 0.1;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the 0.1 doing? Can we relate this to the radius as a ratio or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

do you mean kOwnBallRadius possibly?

@@ -124,11 +168,16 @@ double PenaltyPlayer::distance_from_their_robots(rj_geometry::Point tail,
rj_geometry::Point PenaltyPlayer::calculate_best_shot() const {
// Goal location
rj_geometry::Point their_goal_pos = field_dimensions_.their_goal_loc();
double goal_width = field_dimensions_.goal_width(); // 1.0 meters
double goal_width =
field_dimensions_.goal_width(); // 1.0 meters // BALL SEEMS TO OCCASIONALLY MISS SHOT ON
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra double slash here. Let's change this comment to TODO format.

Comment on lines 32 to 37
if (check_is_done()) {
return SHOOTING;
return SHOOTING_START;
}
if (distance_to_ball() < kOwnBallRadius) {
return SHOOTING_START;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Combine this into one if statement

planning::LinearMotionInstant goal{target_pt, target_vel};
intent.motion_command =
planning::MotionCommand{"path_target", goal, planning::FaceBall{}};
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking but you use break here but a return statement on the other cases. Either method works but please use one or the other, not both

case SHOOTING_START: {
case SHOOTING_START: { // Positions the robot behind the ball at a certain angle so that it
// has a straight shot towards the goal
// SPDLOG_INFO("SHOOTING_START");
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this log comment

case SHOOTING: {
case SHOOTING: { // Kicks the ball with a now much higher speed (basically SMALL_KICK but
// power set to 4)
// SPDLOG_INFO("SHOOTING");
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this log comment

Comment on lines 178 to 180
// Iterates across 19 possible shot locations along the goal width in 0.05-meter increments.
// For each location, it calculates the clearance distance from opponent robots and updates the
// best shot position if a better (less obstructed) option is found.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think an extended description like this would be better as a docstring for the function

Copy link
Contributor

@rishiso rishiso left a comment

Choose a reason for hiding this comment

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

Please fix comments

Mainly cleaned up comments and fixed inconsistencies in variable naming- improved code readability
@CameronLyon CameronLyon requested a review from rishiso February 19, 2025 00:46
Copy link
Contributor

@rishiso rishiso left a comment

Choose a reason for hiding this comment

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

LGTM

@CameronLyon CameronLyon merged commit 0729903 into ros2 Feb 19, 2025
2 checks passed
@CameronLyon CameronLyon deleted the cameron-carson/penalty-position-dribbling branch February 19, 2025 01:43
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.

4 participants