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

Hyundai: Car Port for Ioniq Plug-in Hybrid 2019 #24089

Merged
merged 13 commits into from
Apr 18, 2022

Conversation

sunnyhaibin
Copy link
Contributor

@sunnyhaibin sunnyhaibin commented Mar 31, 2022

Checklist

  • added entry to CarInfo in selfdrive/car/*/values.py and ran selfdrive/car/docs.py to generate new docs
  • test route added to routes.py
  • route with openpilot: 3f29334d6134fcd4|2022-03-30--22-00-50
  • route with stock system: 3f29334d6134fcd4|2022-03-29--19-28-39
  • harness type: Hyundai C

Thanks to community Ioniq Plug-In Hybrid owner camwow13.

@sunnyhaibin sunnyhaibin marked this pull request as ready for review April 4, 2022 16:19
@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

Make sure you always upload rlogs for all related routes, there's no rlogs in the stock system route.

@adeebshihadeh
Copy link
Contributor

Make sure you always upload rlogs for all related routes, there's no rlogs in the stock system route.

Stock system doesn't matter so much. It's more of a nice to have.

@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

Hmm, the only difference between the existing Ioniq PHEV here is this one isn't in the use_fca list, everything else is duplicate. I wonder if we can clean up the use_fca list and just detect by CAN fingerprint. I checked a 2020 Ioniq PHEV and it actually has both SCC12 and FCA11, so I wonder if the signals we care about (AEB/FCW) are on both and we can move IONIQ_PHEV out of use_fca and avoid adding another platform.

@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

Decided we're going to merge as a new platform for now and then work on using the CAN fingerprint to clean the list up as well as merge any otherwise duplicate candidates.

@sshane sshane force-pushed the master-ioniq-phev-2019-port branch from b56212d to de91673 Compare April 5, 2022 18:46
@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

@sunnyhaibin Can you have the user test the current commit out and see how it goes?

@sunnyhaibin
Copy link
Contributor Author

sunnyhaibin commented Apr 7, 2022

@sunnyhaibin Can you have the user test the current commit out and see how it goes?

@sshane 3f29334d6134fcd4|2022-04-06--17-36-04 - was on stock longitudinal, rlogs uploaded!

The user tested with INDI lateral control and mentioned that the ping pong effect compared with the previous PIF tune was significantly reduced. He tested it on my fork with the INDI lateral tune. I've added in this car port. Route ID with INDI lateral control below:

  • 3f29334d6134fcd4|2022-04-05--18-51-40

The 2019 Hyundai Ioniq Plug-In Hybrid is also affected by the low speed lockout at 32 MPH.

@sshane
Copy link
Contributor

sshane commented Apr 7, 2022

Thanks for confirming the minSteerSpeed, I'll run final checks on the route today. By the way, that second route is on a custom branch to remove the minimum steer speed lockout, don't see any INDI changes.

@sunnyhaibin
Copy link
Contributor Author

@sshane Route ID with INDI changes attached!

  • 3f29334d6134fcd4|2022-04-05--16-51-15