control ball before chipping in GoalieFSM - #3869
Conversation
9799b1e to
540ea44
Compare
nycrat
left a comment
nycrat
left a comment
There was a problem hiding this comment.
These changes look good so far. I'm just a bit unsure about how the ballControlled guard works with the new FSM logic. Also as you said it would be nice to have this tested physically, but writing a simulated pytest to demonstrate that the goalie actually gains possession and moves the ball out of the no chip zone before chipping would be nice.
| bool GoalieFSM::ballControlled(const Update& event) | ||
| { | ||
| return event.common.robot.isNearDribbler(event.common.world_ptr->ball().position()); | ||
| } |
There was a problem hiding this comment.
I think that this guard should probably also check that the robot is in a good position to chip since the controlBallForChip action prevents chipping in the no chip rectangle, but this guard would immediately return true after the robot gets possession of the ball.
| DribbleFSM_S + Update_E[shouldMoveToGoalLine_G] / moveToGoalLine_A = | ||
| MoveToGoalLine_S, | ||
| DribbleFSM_S + Update_E[ballInInflatedDefenseArea_G] / retrieveFromDeadZone_A, | ||
| DribbleFSM_S + Update_E[shouldEvacuateCrease_G] / retrieveFromDeadZone_A, |
There was a problem hiding this comment.
Reason for this addition? It seems a bit unrelated to the rest of the PR but I might be missing some context
There was a problem hiding this comment.
oh wait i think this got caught in from one of my old PR changes haha.
| DribbleFSM_S + Update_E[ballInInflatedDefenseArea_G] / retrieveFromDeadZone_A, | ||
| DribbleFSM_S + Update_E[shouldEvacuateCrease_G] / retrieveFromDeadZone_A, | ||
| DribbleFSM_S + Update_E[shouldPivotChip_G] / controlBallForChip_A, | ||
| DribbleFSM_S + Update_E[ballInInflatedDefenseArea_G] / controlBallForChip_A, |
There was a problem hiding this comment.
Same here, why is this logic changed?
3 participants
Description
Previously when goalie chipped, it transitioned straight into
PivotKickFSMeven if the ball wasn't under control. Goalie was able to attempt chipping before securing ball. This PR addsDribbleFSMstep that dribbles to control the ball and orient towards chip target before we hand off to PivotKickFSM.Added a new guard
controlBallForChipaction which drivesDribbleFSMto position ball at a safe chip origin and orients towards target.Implemented new methods ballControlled and controlBallForChip. Updated
goalie_fsm_test.cppso FSM expects to land in DribbleFSM first and reaches PivotKickFSM after ball repositioned near dribbler (has possession).Testing Done
So far only unit tests in
goalie_fsm_test.cppupdated to assert new dribble state before chipping. It would be great to test on actual robot too.Resolved Issues
#2897
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