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

Lateral tuning of E9x #2

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open

Lateral tuning of E9x #2

wants to merge 37 commits into from

Conversation

OxygenLiu
Copy link

Car
E90 325i, MY2008

Tuning

  • Experimental mode ON
  • Acceptable performance matching to my driving style: more active acceleration, closer following distance, and less unnecessary braking.

Copy link

github-actions bot commented Oct 22, 2024

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

@github-actions github-actions bot added the car label Oct 22, 2024
@dzid26
Copy link
Owner

dzid26 commented Nov 15, 2024

need to figure out what values in toml causes:

openpilot/selfdrive/car/tests/test_lateral_limits.py::TestLateralLimits_7_BMW_E90::test_jerk_limits failed: self = <selfdrive.car.tests.test_lateral_limits.TestLateralLimits_7_BMW_E90 object at 0x7f95b2dd5340>

    def test_jerk_limits(self):
      up_jerk, down_jerk = self.calculate_0_5s_jerk(self.control_params, self.torque_params)
      assert up_jerk <= MAX_LAT_JERK_UP + MAX_LAT_JERK_UP_TOLERANCE
>     assert down_jerk <= MAX_LAT_JERK_DOWN
E     assert 6.0 <= 5.0

@dzid26
Copy link
Owner

dzid26 commented Nov 15, 2024

@OxygenLiu any idea why it was constantly requesting max torque to the right and being undrivable? Is it related to lateralTuning.torque.latAccelOffset = -0.255 ? - I left it at default (0).

Quite large Live parameters offset:
image

...but the similar on the master:
image

@OxygenLiu
Copy link
Author

this is the torque params for cycloidal reducer 11:1, probably not suitable for planetary gearbox.

I had similar issue due to wrong stepper direction. :(

@dzid26
Copy link
Owner

dzid26 commented Nov 16, 2024

For 5:1 shouldn't be that much different. Strange that it always go full to the right, not just opposite.

I will test longitudinal for now.

@dzid26 dzid26 changed the title Longtitudinal tuning of E9x Lateral tuning of E9x Nov 21, 2024
@@ -33,7 +33,7 @@
MIN_ENGAGE_BUFFER = 2 # secs

VERSION = 1 # bump this to invalidate old parameter caches
ALLOWED_CARS = ['toyota', 'hyundai']
ALLOWED_CARS = ['bmw', 'toyota', 'hyundai']
Copy link
Owner

Choose a reason for hiding this comment

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

bmw probably should not be allowed to use live values, because overriding the steering by hand cannot be detected and could sometimes skew the results.

Copy link
Author

Choose a reason for hiding this comment

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

hm, I agree. The learned live parameters can be saved in override.toml, and then delete bmw in ALLOWED_CARS.

In practise I don't override steering, but instead press the brake pedal to disengage OP.

Besides, without a proper torque sensor, it is probably quite challenge to achieve a smooth overriding.

Copy link
Owner

Choose a reason for hiding this comment

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

Overriding works well when going nore or less straight and torque is low. OP still doesn't work great when there is no lane lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants