Brakebream availability in gameplay#3462
Conversation
zuperzane
requested review from
itsarune,
sauravbanna and
williamckha
as code owners
williamckha
left a comment
williamckha
left a comment
There was a problem hiding this comment.
I think your approach looks good!
| { | ||
| Team new_enemy_team = enemy_team_filter.getFilteredData(enemy_team, robot_detections); | ||
| Team new_enemy_team = enemy_team_filter.getFilteredData( | ||
| enemy_team, robot_detections, friendly_robot_id_with_ball_in_dribbler); |
There was a problem hiding this comment.
Since we don't know the breakbeam status for the enemy team, I think it makes more sense to pass false here instead of friendly_robot_id_with_ball_in_dribbler
| // find breakbeam_status | ||
| bool breakbeam_tripped = false; | ||
|
|
||
| if (breakbeam_tripped_id.has_value()) | ||
| { | ||
| breakbeam_tripped = breakbeam_tripped_id.value() == this->getRobotId(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I believe this can be simplified to
bool breakbeam_tripped = breakbeam_tripped_id == getRobotId();since if !breakbeam_tripped_id.has_value(), comparing it with a value will implicitly return false (see #21 here)
zuperzane
changed the title
There was a problem hiding this comment.
you should refactor isNearDribbler as well. You can basically short-circuit the Computation by checking the breakbeam. Otherwise, great work so far! Excited to see this come together.
Software/src/software/world/robot.cpp
Line 98 in 1ee0ead
itsarune
left a comment
itsarune
left a comment
There was a problem hiding this comment.
seems like you might need to modify robot_test.cpp: https://github.com/UBC-Thunderbots/Software/blob/master/src/software/world/robot_test.cpp
|
@GrayHoang I see, I just changed isNearDribbler to use the breakbeam and that's littered all throughout AI, if u think there's smth still lmk |
| if (vector_to_test_point.length() > POSSESSION_THRESHOLD_METERS) | ||
| { | ||
| return false; | ||
| return breakbeam || false; |
…ane_kaber_software into zane/brakebream_availability
| Point ball_position_ssl(0.75, 0.75); | ||
| Vector velocity(0, 0); | ||
| ball_state = BallState{ball_position_ssl, velocity}; | ||
|
|
||
| yellow_robot_states.clear(); | ||
| blue_robot_states.clear(); | ||
|
|
||
| RobotState robot_state(Point(0, 0), Vector(0, 0), Angle::fromRadians(2), | ||
| AngularVelocity::zero()); | ||
| RobotStateWithId robot_id = {2, robot_state}; | ||
| yellow_robot_states.push_back(robot_id); | ||
|
|
There was a problem hiding this comment.
these variables seem unused
There was a problem hiding this comment.
I believe 1019 to 1029 can be deleted. see https://github.com/UBC-Thunderbots/Software/pull/3462/files#r2136549546
| yellow_robot_states.clear(); | ||
| blue_robot_states.clear(); | ||
|
|
||
| RobotState robot_state(Point(0, 0), Vector(0, 0), Angle::fromRadians(2), | ||
| AngularVelocity::zero()); | ||
| RobotStateWithId robot_id = {2, robot_state}; | ||
| yellow_robot_states.push_back(robot_id); |
There was a problem hiding this comment.
I believe they're used in initDetectionFrame
There was a problem hiding this comment.
yellow_robot_status and blue_robot_states are already initialized in the constructor:
This is already initialized with three robots already. Since other tests might be relying on how the yellow_robot_states and blue_robot_states are initialized in the constructor, I think we should avoid updating it (line 1074-1080) can be deleted.
itsarune
left a comment
itsarune
left a comment
There was a problem hiding this comment.
looks good! tiny tiny nits
| bool vision_confirm_ball = (ball_to_robot_angle < Angle::fromDegrees(45.0)); | ||
| return breakbeam || vision_confirm_ball; |
There was a problem hiding this comment.
Why is this an OR not an AND?
There was a problem hiding this comment.
in case vision noise is preventing AI from realizing the ball is in the breakbeam
Andrewyx
left a comment
Andrewyx
left a comment
There was a problem hiding this comment.
left some minor comments
Co-authored-by: Andrew Mao <72371419+Andrewyx@users.noreply.github.com>
|
I didnt see that lol my b |
5 participants
Please fill out the following before requesting review on this PR
Description
Making it so breakbeam status is accessible from gameplay
Testing Done
Added Tests
Resolved Issues
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue