Free kick play tests and validation added#3666
Free kick play tests and validation added#3666Muxite wants to merge 4 commits intoUBC-Thunderbots:masterfrom
Conversation
Muxite
requested review from
Andrewyx,
sauravbanna and
williamckha
as code owners
| # FSM path: SetupPositionState -> AttemptPassState -> PassState | ||
| # -> AttemptPassState -> ... -> X (chip or second pass) | ||
| # Triggers: passFound=True, shouldAbortPass=True | ||
| # shouldAbortPass fires when ratePass() drops below abs_min_pass_score (0.05) |
There was a problem hiding this comment.
What are these comments referring to? There are already doc strings included in the tests below.
There was a problem hiding this comment.
I was trying to make it really clear what the test was for in relation to the fsms, I'll delete them and make the docstring a little more detailed
|
@nycrat Can you confirm that this PR is not containing any duplicate work? |
|
I think this PR does add additional test cases which are useful. I think it would be good to merge this first and I could handle the conflicts in #3632 |
| inv_eventually = [[FriendlyTeamEventuallyScored()]] | ||
| ag_eventually = [[FriendlyTeamEventuallyScored()]] | ||
| else: | ||
| inv_eventually = [[BallSpeedEventuallyAtOrAboveThreshold(0.05)]] | ||
| ag_eventually = [[BallSpeedEventuallyAtOrAboveThreshold(0.05)]] |
There was a problem hiding this comment.
nit: it is possible to just specify one eventually_validation_set variable instead since they are the same
| "play_name": PlayName.FreeKickPlay, | ||
| } | ||
| setup=setup, | ||
| params=[0], |
There was a problem hiding this comment.
nit: params does not need to be specified, this applies to all the run_test() calls in the file
| setup=setup, | ||
| params=[0], | ||
| inv_always_validation_sequence_set=[ | ||
| [BallAlwaysStaysInRegion(regions=[field.fieldBoundary()])] |
There was a problem hiding this comment.
fieldBoundary() is actually the entire physical field so this validation technically could never fail. Perhaps this does not need to be validated? Or instead use the playing area
| inv_always_validation_sequence_set=[[]], | ||
| inv_eventually_validation_sequence_set=[[]], | ||
| inv_eventually_validation_sequence_set=[ | ||
| [BallSpeedEventuallyAtOrAboveThreshold(0.05)] |
There was a problem hiding this comment.
Could potentially use BallIsOffGround validation to check for chip
3 participants
-->
Description
added validation to free kick play test, checks every branch of the fsm.
Testing Done
write tests for each branch, watched the test videos to confirm they are valid, all pass.
Resolved Issues
resolves #3636
Length Justification and Key Files to Review
free_kick_play_test.py
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