openpilot
openpilot copied to clipboard
hyundai: ev6 - use good steering angle sensor
Our EV6 has a rack mounted power steering motor, and we have signals for two different steering angles. I have a theory that one sensor is on the steering column and the other is on the steering rack, and we are currently using the one on the steering column which looks like it may be a much worse signal (see data from d824e27e8c60172c|2022-09-15--09-57-55--6 below).

Not sure if we need to worry about some CAN-FD cars having a rack mounted power steering motor and other cars having a column mounted power steering motor.
Before merging:
- [x] Decide if it is OK the logged angle rate is for a different sensor (@HaraldSchafer says that is fine)
- [x] Determine if
LAT_ACCEL_FACTORshould change (@HaraldSchafer can you take a look?) - [ ] Determine what platforms we should apply this change to (@adeebshihadeh can you look at data for other cars this change would currently affect and exclude them if necessary?)
Do we care if the steering angle rate is for a different sensor than the steering angle?
How do we re-analyze what should be used for LAT_ACCEL_FACTOR?
Do you see a real difference between the acceleration derived from the steering angle and what the localizer says at high angles/accelerations?
Do you see a real difference between the acceleration derived from the steering angle and what the localizer says?
I haven't looked at the data for that, but In the image of the above segment I see at one point the angle in one signal changing about 0.1 deg and the other changing almost 2 deg when they diverge and come back together.
I have an hda1 ev6 and can check if I have two steering angles like your ev6 if that is helpful at all. Let me know what I should look for or post.
The localizer seems to confirm this change is an improvement for our EV6

Saw @hoomoose comment in the Discord that these changes completely eliminated ping pong in his HDA1 EV6 Wind, so I cherry-picked these two commits (https://github.com/commaai/openpilot/pull/25801/commits/4cde6afd36e22cb15d3544ec3d2410e34e0892a8, https://github.com/commaai/openpilot/pull/25801/commits/508c00fd0642a30251ab678a8daf6b249cca1604) on to my local copy of his HDA1 EV6 branch. Interestingly, I had the exact opposite experience - in my HDA1 EV6 Light, these two commits on top of that branch result in extreme oversteering. Are there more recent changes in master that this PR depends on?
I also cherry picked those changes into my IONIQ 5 and drive IS now super smooth. Definitely an improvement!
From a Tucson Hybrid route (36e10531feea61a4|2022-08-16--18-14-13), it appears MDPS->STEERING_ANGLE is always zero, however MDPS->STEERING_ANGLE_2 is set. It also looks nearly identical to the current STEERING_SENSORS->STEERING_ANGLE signal.

On the ev6 STEERING_SENSORS->STEERING_ANGLE also looked extremely similar a lot of the time. It does seems like the Tuscon has a rack mounted EPS (R-MDPS).
It looks like it can vary per-platform as well. We're going to need live detection like we do for Toyota with its more accurate angle sensor (unless STEERING_ANGLE_2 is a copy of STEERING_SENSOR's signal and we can just use that):
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=1526
platform='KIA EV6 2022', segment='22de8111a8c5463c|2022-07-29--13-34-49--6', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=1377
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--0', len(new_steering_angle)=5558, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=650
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--7', len(new_steering_angle)=6000, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=1814
platform='KIA EV6 2022', segment='22de8111a8c5463c|2022-07-29--13-34-49--1', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=1257
platform='KIA EV6 2022', segment='22de8111a8c5463c|2022-07-29--13-34-49--0', len(new_steering_angle)=5930, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=1369
platform='KIA EV6 2022', segment='22de8111a8c5463c|2022-07-29--13-52-45--2', len(new_steering_angle)=6000, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=140
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--4', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=72
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--5', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=846
platform='KIA EV6 2022', segment='22de8111a8c5463c|2022-07-29--13-34-49--5', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=97
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--3', len(new_steering_angle)=6000, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=52
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--6', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=362
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--2', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
set(new_steering_angle)={-0.0}, len(set(new_steering_angle))=1, len(set(new_steering_angle_2))=1280
platform='HYUNDAI TUCSON HYBRID 4TH GEN', segment='36e10531feea61a4|2022-08-16--18-14-13--1', len(new_steering_angle)=5999, new_steering_angle == new_steering_angle_2=False
There are four different situations here:
- Tucson Hybrid:
MDPS->STEERING_ANGLEis not set, andMDPS->STEERING_ANGLE_2is nearly identical toSTEERING_SENSORS->STEERING_ANGLE(~0.05-0.1 degree angle error across 8 segments). We can useMDPS->STEERING_ANGLE_2here. - EV6:
MDPS->STEERING_ANGLEequalsMDPS->STEERING_ANGLE_2and has an average of 0.9 degrees of error across 154 segments (max segment error was 2.32 deg, min 0.18 deg). - IONIQ 5 (40 segments):
MDPS->STEERING_ANGLEequalsMDPS->STEERING_ANGLE_2and has an average of 0.80 degrees of error across 40 segments (max segment error was 1.75 deg, min 0.25 deg). - IONIQ 5 (5 segments): All from one user, but this IONIQ 5 is similar to the TUCSON in that
MDPS->STEERING_ANGLEis not set. Average angle error withSTEERING_SENSORS->STEERING_ANGLEis 0.75 deg (max segment error was 1.1 deg, min 0.62 deg) The difference between the Tucson though is that the two signals have a higher error, and precision looks to be about the same for both. Toss up on whether either signal is more accurate to the localizer's accel so it's fine to use the new signal here: https://imgur.com/a/yS4K57y
It looks like this new signal can be more noisy and diverge from the localizer accel compared to the previous signal. Since this isn't easy to merge anymore, going to leave this open for now.
EV6: https://imgur.com/a/V7lmbKU
When I zoom in on a random place where the MDPS signal is noisy, the noise seems to have the same frequency as the motor torque output. I feel like the localizer may filter this level of noise out, and things like suspension dynamics may also hide changes in steering angle from the localizer at the level we are looking here, but maybe I am wrong.

Is there any chance you (Comma) would consider merging the torque values separate from the switch to MDPS? Using the MDPS steering angle sensor makes OP unusable for me on the EV6 Light (maybe I don't have it?), but without the torque settings in this PR I have pretty strong understeer.
Update - opened a PR: https://github.com/commaai/openpilot/pull/26076
recent example where ping pong was reported that looks similar 5e31c7585bf1a946|2022-11-04--00-00-39--9

This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity.
@sshane what's the verdict on this?
@nuwandavek is it worth switching the signal for a single platform?
Looked at the data from the 2022 Hyundai Tucson Hybrid and this signal has much less noise than the current one used in upstream.
This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity.
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.