Skip to content
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

New KeeperMotion #1560

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

New KeeperMotion #1560

wants to merge 9 commits into from

Conversation

larskna
Copy link
Contributor

@larskna larskna commented Jan 9, 2025

Why? What?

renamed the WideStance to KeeperMotion and added JumpDirections for the GoalKeeper

Fixes #

I have to do a little bit tuning

How to Test

I want to test it in a test Game please, so we can it see all

@larskna larskna enabled auto-merge January 9, 2025 20:34
Copy link
Contributor

@schluis schluis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it 🚀 . Minor things

crates/control/src/behavior/defend.rs Outdated Show resolved Hide resolved
Comment on lines 41 to 42
keeper_jump_left_joints_command:
Input<MotorCommands<Joints<f32>>, "keeper_jump_left_joints_command">,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming suggestion: keeper_jump_left_joints_command -> keeper_jump_left_motor_commands to be consistent with walk_motor_commands

crates/control/src/motion/motion_selector.rs Show resolved Hide resolved
crates/control/src/motion/motion_selector.rs Show resolved Hide resolved
Comment on lines 45 to 48
keeper_jump_left_joints_command:
Input<MotorCommands<Joints<f32>>, "keeper_jump_left_joints_command">,
keeper_jump_right_joints_command:
Input<MotorCommands<Joints<f32>>, "keeper_jump_right_joints_command">,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> keeper_jump_*_motor_commands

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't rename all instances correctly! motor_command != motor_commands

crates/types/src/parameters.rs Outdated Show resolved Hide resolved
Comment on lines 51 to 53
if context.motion_selection.current_motion == MotionType::KeeperJumpLeft {
self.interpolator
.advance_by(last_cycle_duration, context.condition_input);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extract context.condition_input into a variable like the last_cycle_duration, similar to the other motion files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now please also add a newline between the if and the variables. I know I'm very picky, but all files are exactly written like this so I would like to keep this as consistent as possible 🙃

#[context]
#[derive(Default)]
pub struct MainOutputs {
pub keeper_jump_left_joints_command: MainOutput<MotorCommands<Joints<f32>>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename keeper_jump_left_joints_command -> keeper_jump_left_motor_commands

@schluis schluis self-assigned this Jan 12, 2025
@larskna larskna force-pushed the spring_zur_Seite_keepy branch from 5d0b0d7 to 187bdb5 Compare January 29, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Request for Review
Development

Successfully merging this pull request may close these issues.

2 participants