New gameplay passing #1644
New gameplay passing #1644
Conversation
Codecov Report
@@ Coverage Diff @@
## gameplay_node_update #1644 +/- ##
========================================================
- Coverage 33.36% 30.49% -2.88%
========================================================
Files 46 51 +5
Lines 1912 2099 +187
========================================================
+ Hits 638 640 +2
- Misses 1274 1459 +185
Continue to review full report at Codecov.
|
| self.pivot_point = pivot_point | ||
| self.target_point = target_point | ||
| self.dribble_speed =dribble_speed |
There was a problem hiding this comment.
Nit: space before dribble_speed here
| dot_product = np.dot(robot_pos_unit, target_point_unit) | ||
| angle = np.arccos(dot_product) | ||
| if abs(angle - angle_threshold) < angle_threshold: |
There was a problem hiding this comment.
Could more simply return the conditional directly, but it doesn't matter if we'll end up rewriting this anyways.
| if robot is not None: | ||
| self.pivot.pivot_point = robot.pose[0:2] | ||
| actions = self.root.tick_once(robot) | ||
| return actions | ||
| # TODO: change so this properly returns the actions intent messages |
| import numpy as np | ||
|
|
||
| class PivotPlay(play.IPlay): | ||
| """A play which lines up two robots, one on the right the one on the left |
There was a problem hiding this comment.
Comment is wrong, but this is a temp play anyhow so it doesn't matter.
| if not self.pivot.is_done(world_state): | ||
| role_requests[self.pivot] = self.pivot.get_requests(world_state, None) | ||
| else: | ||
| pass |
There was a problem hiding this comment.
Delete the else: pass, unless this is a stub, in which case add a TODO above the pass here.
There was a problem hiding this comment.
(Temp play caveat applies here too though)
| else: | ||
| skills = [] | ||
|
|
||
| return (skill_dict ,skills) |
There was a problem hiding this comment.
| return (skill_dict ,skills) | |
| return (skill_dict, skills) |
|
|
||
| return (skill_dict, skills) | ||
|
|
||
| def is_done(self ,world_state): |
There was a problem hiding this comment.
| def is_done(self ,world_state): | |
| def is_done(self, world_state): |
lol just saw this
There was a problem hiding this comment.
you can directly click "Commit suggestion" on GH if you want, not sure if you were aware
There was a problem hiding this comment.
The code looks good to me, and robots moved as designed.
But you would want to tweak the ball speed a bit.
I did 4 trials:
- 1 success
- Robot 7 missed the ball 2 times
- Robot 7 fumbled the ball.
This happened probably because the ball_speed was too fast, I think.
Overall, good work!
|
|
||
| def fail(self): | ||
| return False | ||
| def __init__(self, robot_id:Optional[int]=None, chip:Optional[bool]=False, kick_speed:Optional[float]=255.0) -> None: |
There was a problem hiding this comment.
You would want to lower the kick_speed here?
There was a problem hiding this comment.
kick_speed should be in m/s, so this is definitely way too fast. Do these all really need default values? I'd prefer not to allow a default value for kick at least, and I thought we designed things in a way that you didn't need to construct actions until you know which robot they're assigned to?
|
|
||
| return intent | ||
|
|
||
| def is_done(self, world_state) -> bool: | ||
| if world_state.our_robots[self.robot_id].has_ball_sense: | ||
| if not self.robot_id is None and world_state.our_robots[self.robot_id].has_ball_sense: |
There was a problem hiding this comment.
When will robot_id be None?
There was a problem hiding this comment.
when capture action is initialized we have to default all to None
|
|
||
| def fail(self): | ||
| return False | ||
| def __init__(self, robot_id:Optional[int]=None, chip:Optional[bool]=False, kick_speed:Optional[float]=255.0) -> None: |
There was a problem hiding this comment.
kick_speed should be in m/s, so this is definitely way too fast. Do these all really need default values? I'd prefer not to allow a default value for kick at least, and I thought we designed things in a way that you didn't need to construct actions until you know which robot they're assigned to?
| return new_intent | ||
|
|
||
| def is_done(self, world_state:rc.WorldState) -> bool: | ||
| return np.linalg.norm(world_state.ball.vel) > 1 |
There was a problem hiding this comment.
I really do not trust this condition at all...
There was a problem hiding this comment.
Yeah, I could make it check that the ball is going away from the kicking robot.
| Receive action | ||
| """ | ||
|
|
||
| def __init__(self, robot_id: int = None, one_touch_target:np.ndarray = None): |
There was a problem hiding this comment.
For a vanilla receive we don't need one touch target.
There should be a separate action for one touches
Co-authored-by: Yudai Hatano <[email protected]>
…nable, tuned the receive skill so it settles at more reasonable speeds
|
|
||
| return (skill_dict, skills) | ||
|
|
||
| def is_done(self ,world_state): |
| self.robot = role.robot | ||
| self.root = py_trees.composites.Sequence("Sequence") | ||
| def __init__(self, target_point:np.array, chip:bool, kick_speed:float, robot:rc.Robot=None) -> None: | ||
| #TODO: Have something which automatically determines kick speed based on targert point distance |
There was a problem hiding this comment.
| #TODO: Have something which automatically determines kick speed based on targert point distance | |
| #TODO: Have something which automatically determines kick speed based on target point distance |
| #TODO: Have something which automatically determines kick speed based on targert point distance | ||
| self.__name__ = 'pivot kick' | ||
| self.robot = robot | ||
| self.chip= chip |
There was a problem hiding this comment.
| self.chip= chip | |
| self.chip = chip |
| def __init__(self, role: role.Role, target_point: np.array) -> None: | ||
| self.robot = role.robot | ||
| self.root = py_trees.composites.Sequence("Sequence") | ||
| def __init__(self, target_point:np.array, chip:bool, kick_speed:float, robot:rc.Robot=None) -> None: |
There was a problem hiding this comment.
| def __init__(self, target_point:np.array, chip:bool, kick_speed:float, robot:rc.Robot=None) -> None: | |
| def __init__(self, target_point: np.array, chip: bool, kick_speed: float, robot: rc.Robot=None) -> None: |
There was a problem hiding this comment.
also maybe consider putting robot first? for consistency w/ other skills
| @@ -81,7 +81,7 @@ def get_requests( | |||
|
|
|||
| role_requests: tactic.RoleRequests = {} | |||
|
|
|||
| passer_request = role.RoleRequest(role.Priority.HIGH, True, self.passer_cost) | |||
| passer_request = role.RoleRequest(role.Priority.HIGH, True, self.Passer_cost) | |||
| role_requests[self.pivot_kick] = [passer_request] | |||
| receive_request = role.RoleRequest(role.Priority.HIGH, True, self.receiver_cost) | |||
There was a problem hiding this comment.
Based on understanding, it requests receive skill and capture skill simultaneously. so when robot 1 passes the ball, robot 7 moves toward the kicker, and it makes it difficult for the receiver to settle the ball, I believe. Capture skill should be applied only when the ball gets closer to the receiver??
There was a problem hiding this comment.
this is what does happen, kind of, if you check the receive action its based off of the speed of the ball, so when the ball is slow the capture starts. I am also changing it so that if the robot has ball sense it starts capture.
There was a problem hiding this comment.
Great! Now I understand what was happening!
| role_requests: play.RoleRequests = {} | ||
| if not self.pass_tactic.is_done(world_state): | ||
| role_requests[self.pass_tactic] = self.pass_tactic.get_requests(world_state, None) | ||
| else: |
| heading_vect = np.array([np.cos(heading_angle), np.sin(heading_angle)]) | ||
| dot_product = np.dot(heading_vect, ball_vel_unit) | ||
| #TODO: Make this threshold a local param | ||
| if dot_product > 0.1: |
There was a problem hiding this comment.
This threshold should be higher, vision noise can probably hit this pretty easily.
|
|
||
| def is_done(self, world_state:rc.WorldState) -> bool: | ||
| #TODO: Change this when we get action state feedback | ||
| angle_threshold = 0.1 |
There was a problem hiding this comment.
This is definitely too big, it's about 6°. It should be closer to 1° (0.02 or so)
| if self.robot_id is None: | ||
| return False | ||
| #TODO: Use local params for this threshold | ||
| if world_state.our_robots[self.robot_id].has_ball_sense or np.linalg.norm(world_state.ball.vel) < 10**(-6): |
There was a problem hiding this comment.
Velocity threshold is again way too small...also, nit: please make it a constant (even if it's not a local param)
| import numpy as np | ||
|
|
||
|
|
||
| class Receiver_cost(role.CostFn): |
There was a problem hiding this comment.
@HussainGynai the cost functions in other tactics are styled as if they're methods, since we use/call these classes as methods. What are we going with?
There was a problem hiding this comment.
According to Kyle seems like class style is better here, when they're instantiated they should probably be in method form though I think.
| return 0.0 | ||
| return 1.0 | ||
|
|
||
| class Passer_cost(role.CostFn): |
| self.capture = capture.Capture(self.robot.id) | ||
| else: | ||
| self.receive = receive.Receive(self.robot) | ||
| self.capture = capture.Capture() |
There was a problem hiding this comment.
I'm confused by these constructors here. Why do we pass in self.robot for Receive? Isn't the first argument supposed to be an ID?
There was a problem hiding this comment.
self.robot is None is the else block so it works, I agree that it's confusing tho
|
ignore the above msg, I was wondering what the "revert" button did on GH |
Description
Everything needed for passing will go here (pivoting, pivot kicking, receiving, passing).
Current things in:
With the pivot kick skill, I had to create our own separate sequence block for behavior trees, since we need to pass actions/intent back up the tree. When an action server is implemented we will no longer need this, since the action can just make a direct call to the server to start behavior, and won't need to pass any data back up.
Steps to test
Test Case 1