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

Fix Xbox controller; introduce Takeover policy w/o brake #615

Merged
merged 10 commits into from
Jan 25, 2024

Conversation

pengzhenghao
Copy link
Member

@pengzhenghao pengzhenghao commented Jan 24, 2024

What changes do you make in this PR?

  • Please describe why you create this PR

Checklist

  • I have merged the latest main branch into current branch.
  • I have run bash scripts/format.sh before merging.
  • Please use "squash and merge" mode.

@pengzhenghao
Copy link
Member Author

@QuanyiLi could you play with drive_in_single_agent_env.py to see if this feels better?

Copy link

[BOT] Great job! You have changed the docstring coverage from 30.54% to 30.54%, improving 0.005%.

@QuanyiLi
Copy link
Member

It feels bad when the car drives fast. At least, my performance degrades. Maybe I overfit the old keyboard controller...

@QuanyiLi
Copy link
Member

It takes so long for the steering wheel to return to the center. It makes the control hard when the speed is high.

Copy link

[BOT] Great job! You have changed the docstring coverage from 30.54% to 30.54%, improving 0.005%.

@pengzhenghao
Copy link
Member Author

pengzhenghao commented Jan 24, 2024

Now I've reverted all values to previous version. The key differences:

  1. we introduce space key to indicate takeover when using keyboard
  2. A little different behavior on up and down keys:

If throttle > 0, and you press down, we will not set the brake to -brake_increment. The original code is:

        elif throttle_brake < 0.:   # WHEN YOU PRESS DOWN KEY
            self.throttle_brake = min(self.throttle_brake, 0.)  # IF throttle>0, IT WILL SET TO 0.
            self.throttle_brake -= self.BRAKE_INCREMENT  # throttle NOW EQUALS brake_increment

Instead, in this impl, we use:

        elif down_key_pressed:
            if self.throttle_brake <= 0.0:
                self.throttle_brake -= self.BRAKE_INCREMENT
            else:
                self.throttle_brake -= self.BRAKE_INCREMENT_WHEN_THROTTLE

note that

BRAKE_INCREMENT = 0.5
BRAKE_INCREMENT_WHEN_THROTTLE = 0.5

In this impl, we avoid too large change in the throttle_brake.


note: when I say press down, I say press s. Same for up.

@pengzhenghao pengzhenghao added the help wanted Extra attention is needed label Jan 24, 2024
Copy link

[BOT] Great job! You have changed the docstring coverage from 30.54% to 30.54%, improving 0.005%.

@QuanyiLi
Copy link
Member

It is good now.

Copy link

[BOT] Great job! You have changed the docstring coverage from 30.54% to 30.55%, improving 0.012%.

Copy link

[BOT] Great job! You have changed the docstring coverage from 30.54% to 30.55%, improving 0.012%.

Copy link

[BOT] Great job! You have changed the docstring coverage from 30.54% to 30.55%, improving 0.012%.

@pengzhenghao pengzhenghao changed the title Optimize keyboard experience Fix Xbox controller; introduce Takeover policy w/o brake Jan 25, 2024
@pengzhenghao pengzhenghao added Merge after all tests pass Merge this PR when all tests pass! and removed help wanted Extra attention is needed labels Jan 25, 2024
@pengzhenghao pengzhenghao merged commit 2949cd1 into main Jan 25, 2024
13 checks passed
@pengzhenghao pengzhenghao deleted the optimize-keyboard branch January 25, 2024 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge after all tests pass Merge this PR when all tests pass!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants