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

Subaru: update carinfo supported model years and good steering torque #24118

Merged
merged 4 commits into from
Apr 5, 2022

Conversation

martinl
Copy link
Contributor

@martinl martinl commented Apr 4, 2022

This PR updates Subaru supported model years (included in initial Subaru FPv2 import) and adds stars for good steering torque

Checklist

  • added entry to CarInfo in selfdrive/car/*/values.py and ran selfdrive/car/docs.py to generate new docs

@sshane sshane added the docs label Apr 4, 2022
@sshane sshane self-assigned this Apr 4, 2022
@sshane
Copy link
Contributor

sshane commented Apr 4, 2022

In the future we are going to use data to generate the torque star columns, but I'm fine with manually updating this for now, as long as it has enough torque for comfortable highway driving. Where did you get the years from? Are there any CAN fingerprint car port PRs?

@sshane
Copy link
Contributor

sshane commented Apr 4, 2022

Actually can you revert the torque changes except to the Forester and just update the model years? We saw from data that the Impreza has about the same torque as some Hondas. We'll use data from the fleet to fill the torque column out later.

@michaelhonan
Copy link
Contributor

Might be worth noting that the Imprezas might not be as limited as previously thought:
#23530

@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

Yeah, will look into that eventually. For now though, let's use the data for the good torque columns, so it's only Forester for now.

@martinl
Copy link
Contributor Author

martinl commented Apr 5, 2022

Which model year Impreza steering torque did you measure - 2020-21 has lower steering torque limit in openpilot because of eps fault issue but Impreza 2016-19 and Crosstrek 2017-19 should have no issues on highway driving. Would it be ok to split Impreza and Crosstrek and remove steering torque star from 20-21?

@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

We looked at Impreza Sport 2020, which had 0.66, 0.71, 0.96 m/s/s lateral acceleration from the data at three speed brake points while Forester had 2.01, 1.67, NA m/s/s, so that seems to make sense from your claim.

I see the 2020 years were added with the new IMPREZA_2020 platform in PR #21011 so I split it out correctly as you suggested, also added good_torque as you can especially see that from the max torque in the CarControllerParams

@sshane sshane merged commit 5e1095e into commaai:master Apr 5, 2022
@ClockeNessMnstr
Copy link
Contributor

ClockeNessMnstr commented Apr 7, 2022

Which model year Impreza steering torque did you measure - 2020-21 has lower steering torque limit in openpilot because of eps fault issue but Impreza 2016-19 and Crosstrek 2017-19 should have no issues on highway driving. Would it be ok to split Impreza and Crosstrek and remove steering torque star from 20-21?

20-21 look to have more torque not less. Even with the lower request value. (Steering torque / steering request) doubled on 2020+

2017-2019 can hit steering limits on hwy @2071
2020+ are fine TMK at ~1400

Forester eps and 2020+ eps response is double that of the of the 2017-2019 imp/Crosstrek scale.

Crosstrek 2020+ is using ~2850 max torque, and Forester is using 4142 max torque compared to 2017-2019

@martinl martinl deleted the subaru-cars-update branch April 7, 2022 17:28
budney pushed a commit to budney/openpilot that referenced this pull request Apr 12, 2022
* Subaru: update carinfo supported model years and good steering torque

* update torque

* model year 2020 was added to IMPREZA_2020, split out

update docs

* order

Co-authored-by: Shane Smiskol <shane@smiskol.com>
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.

4 participants