-
Notifications
You must be signed in to change notification settings - Fork 187
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
Basic defense play #1671
Basic defense play #1671
Conversation
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.
quick comments to speed up review
@@ -9,11 +9,11 @@ | |||
from typing import Dict, Generic, Iterator, List, Optional, Tuple, Type, TypeVar | |||
import numpy as np | |||
|
|||
|
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.
this file is identical to the file merged into ros2 recently by #1661, shows here bc I ran make pretty-lines
@@ -46,7 +45,8 @@ def __init__(self, | |||
self.root.setup_with_descendants() | |||
self.__name__ = 'move skill' | |||
|
|||
def tick(self, robot: rc.Robot, world_state: rc.WorldState): #returns dict of robot and actions | |||
def tick(self, robot: rc.Robot, |
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.
again, make pretty-lines
the only changes here
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 minor stuff, looks pretty good overall.
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.
Left a bunch of comments for files that you can skip when reviewing, at least this time around.
@@ -0,0 +1,38 @@ | |||
"""This module contains the interface and action for intercept.""" |
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.
Broken intercept, not sure if it's motion planning side or here. Feel free to skip for now since it's not used.
@@ -0,0 +1,52 @@ | |||
from abc import ABC, abstractmethod |
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.
again, intercept isn't used because it's broken, feel free to skip
@@ -18,30 +18,33 @@ | |||
from stp.utils.constants import RobotConstants, BallConstants | |||
import stp.global_parameters as global_parameters | |||
|
|||
MIN_WALL_RAD = None |
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.
the only changes to this file from what's in #1661 are from make pretty-lines
, skip this too
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.
Looks good, I think add a pass/clear if the goalie gets the ball, otherwise it'll just sit there.
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.
LGTM, I agree with Hussain about adding pass/clear thing
Other than that, worked great!
Description
There's a surprising amount of new stuff here:
world_state.game_info.goalie_id
)Steps to test
FIRST: Change the number of robots to 6. This can be done by clicking on grSim, then in the left menu finding Game > Robots Count and changing the count to 6. (If you can't see the current number, scroll to the right.)
Test Case 1
(This is set off of the
/referee/goalie_id
topic; you can check rqt to confirm that they match.)Test Case 2
Expected result:
Test Case 3
play/basic_defense.py
(to disable the wall).Expected result:
This last test case is the most likely to fail, since I essentially hacked around the non-working intercept motion command with my own move to point command.
Test Case 4
Expected result: