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

Reduce steering error with better lane width calculation #755

Closed
wants to merge 3 commits into from

Conversation

Gernby
Copy link

@Gernby Gernby commented Jul 23, 2019

This change is related to this PR from a couple weeks ago, and was prompted by the comment today from @zorrobyte about steering error when 1 lane line disappeared before a turn.

After some discussion with with the community on Discord, I tested a few changes that made a very significant improvement for lateral stability.

The root issues with the current lane width logic are:

  • Lane width estimate changes too fast, which causes the MPC to overreact to noise in the lane lines
  • Speed-based lane width has too much impact when 1 lane line has a low probability
  • Lane width change is only time-based, not distance.

The proposed logic addresses all of those issues.

Below is a screen capture of my dashboard before and after this change on the same turn within a few minutes of each other.
image

Here is a video of the turn
image

Here is a link to the drive in Cabana
https://my.comma.ai/cabana/?route=ddd3e089e7bbe0fc%7C2019-07-22--19-18-49&max=22&url=https%3A%2F%2Fchffrprivate-vzn.azureedge.net%2Fchffrprivate3%2Fv2%2Fddd3e089e7bbe0fc%2F6d8537a9b83f1a8c6695f84e3de7a108_2019-07-22--19-18-49

@zorrobyte
Copy link
Contributor

zorrobyte commented Jul 23, 2019

I have objectively tested this PR with 0.6.1 on my '19 Prius Prime Advanced with good steering angle data and have concluded that straight line performance is in fact, enhanced. I will perform more testing over the next few days and update this PR if I have additional findings.

Good work @Gernby

@rbiasini this PR is based on a fix for this other PR:
#737

@ncflagg
Copy link

ncflagg commented Jul 23, 2019

I tested a version of this fix a few hours older (diff showed here - Gernby@a324a47) and ran it for a few hours today while trying other INDI tuning numbers. It's a much more consistent lateral experience on my stock 2017 Prius Prime Premium. Until now, I ran timeconstant = 3.0 (vs 1.0) to calm over-active lane-keeping; not regular ping-pong, but frequent,noticeable corrections. I found today that timeconstant = 1.0 is tolerable now.

I'll be testing the updated version in this PR tomorrow, but am already pretty happy with what I've seen. I'm excited to see where this PR leads.

@ncflagg
Copy link

ncflagg commented Jul 25, 2019

3d6af31 isn't nearly as good as the commit I previously referenced.

@Gernby
Copy link
Author

Gernby commented Jul 25, 2019

@ncflagg I removed the change that limited the probability of the speed_lane_width. That seemed to provide better performance.

@zorrobyte
Copy link
Contributor

zorrobyte commented Jul 29, 2019

def mean(numbers):
    return float(sum(numbers)) / max(len(numbers), 1)

self.lane_width = 3
self.frame = 0
self.readings = []

# Find current lanewidth
    if l_prob > 0.49 and r_prob > 0.49:
        self.frame += 1
        if self.frame % 20 == 0:
            self.frame = 0
            current_lane_width = sorted((2.8, abs(l_poly[3] - r_poly[3]), 3.6))[1]
            max_samples = 30
            self.readings.append(current_lane_width)
            avg = mean(self.readings)
            self.lanewidth = avg
            if len(self.readings) == max_samples:
                self.readings.pop(0)

I'm just going to leave this here. Great interstate camber handling, I can loose a lane line and stay perfectly centered. I even had success on an unmarked road with OP. No more exit diving.

I don't know what the hell is going on with stock OP
image

@Gernby
Copy link
Author

Gernby commented Jul 29, 2019

@zorrobyte I'm not sure what the last screenshot is supposed to show, but it seems that you've reversed lane_width_certainty and lane_width_estimate. The certainty shouldn't ever be larger than 1.0, and the estimate would normally be about 3.7.

@zorrobyte
Copy link
Contributor

zorrobyte commented Jul 29, 2019

Well, anyway - thanks for that @Gernby
image

However, the figure it estimates is still 3.68 while the actual lane width is 2.73
current_lane_width = abs(1.4096942 - -1.3224893)

@pd0wm has said he'll look into the learner and fix it if needed.

@Gernby
Copy link
Author

Gernby commented Aug 7, 2019

After testing this PR logic side-by-side with existing logic, I'm closing this PR. It seems that standard logic has fewer vulnerabilities.

@Gernby Gernby closed this Aug 7, 2019
debugged-tech pushed a commit to debugged-tech/DebuggedPilot that referenced this pull request Dec 23, 2020
* feat(lat): import latcontrol_indi from src

* feat(lat): add live tuning to indi controller

* feat(params): update car params for new indi breakpoints

* feat(params): port op changes from src

* feat(params): port op params changes from src and add live indi params

* fix interp to pass varied length tests

* fix(params): fix infinite loop in op params

* fix(params): make sure op_edit is seen as an executable file
debugged-tech pushed a commit to debugged-tech/DebuggedPilot that referenced this pull request Dec 23, 2020
debugged-tech pushed a commit to debugged-tech/DebuggedPilot that referenced this pull request Dec 23, 2020
rav4kumar pushed a commit to rav4kumar/openpilot that referenced this pull request Dec 24, 2020
* feat(lat): import latcontrol_indi from src

* feat(lat): add live tuning to indi controller

* feat(params): update car params for new indi breakpoints

* feat(params): port op changes from src

* feat(params): port op params changes from src and add live indi params

* fix interp to pass varied length tests

* fix(params): fix infinite loop in op params

* fix(params): make sure op_edit is seen as an executable file
cgw1968-5779 added a commit to cgw1968-5779/openpilot that referenced this pull request Dec 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants