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

Unify Rav4 tuning - lat is very unresponsive on non-FW version Rav4s #21429

Merged
merged 1 commit into from
Jul 6, 2021
Merged

Unify Rav4 tuning - lat is very unresponsive on non-FW version Rav4s #21429

merged 1 commit into from
Jul 6, 2021

Conversation

zorrobyte
Copy link
Contributor

@zorrobyte zorrobyte commented Jun 28, 2021

The stock tune for non-FW Ravs is much too lazy and OP is borderline unusable at lower speeds. OP will drift in the lane and sometimes will exit the lane as steering sensitivity is much too low. I've been running stock master on my device and better stock values need to be used.

I don't think it's so much that differing FW versions are the culprit. Toyota wouldn't revise EPS firmware so darastically and the part numbers for the EPS motor are the same. It seems that more like the majority of Rav 4 users are on that FW version. I have a 2019 without that FW version and the stock tuning values result in sloppy lat control. I was proven wrong with research, TSS2 Rav4 has two rather different steering racks.

@adeebshihadeh
Copy link
Contributor

adeebshihadeh commented Jun 28, 2021

It may be that we need to split on your fw version as well. Let's get some data from other Rav4 users on what their fw version is and whether this is an improvement for them.

@pd0wm
Copy link
Contributor

pd0wm commented Jun 29, 2021

We need to check tuning for the following versions:

0.6

  • b'8965B42170\x00\x00\x00\x00\x00\x00' Original PR
  • b'8965B42171\x00\x00\x00\x00\x00\x00' @zorrobyte + @C0mpl3t3N00b (bdad50b8a1f9fa8e|2021-06-29--10-37-41)

0.15

  • b'\x028965B0R01200\x00\x00\x00\x008965B0R02200\x00\x00\x00\x00' @adnanlanewla (d2ef5aa690e849d0)
  • b'\x028965B0R01300\x00\x00\x00\x008965B0R02300\x00\x00\x00\x00' @mattbsea (9388d771f95175f5)

Not confirmed:

  • b'8965B42180\x00\x00\x00\x00\x00\x00'
  • b'8965B42181\x00\x00\x00\x00\x00\x00'
  • b'\x028965B0R01400\x00\x00\x00\x008965B0R02400\x00\x00\x00\x00'

@C0mpl3t3N00b
Copy link

C0mpl3t3N00b commented Jun 29, 2021

Just tested it with a 2020 Rav4 Hybrid and it feels fantastic on city streets, boring highway, expressways, and even mountain overpasses. Fantastic tune. Please upstream.

Edit: adding route - bdad50b8a1f9fa8e|2021-06-29--10-37-41 @zorrobyte

@C0mpl3t3N00b
Copy link

@yufeng66 @mattbsea @dpanych @RICKYMAMA1 @Taik @adnanlanewla

New tune from Zorrobyte and he asked me to reach out to the previous tune testers:

Install:
https://smiskol.com/fork/zorrobyte/master_Rav4Tune

@pd0wm
Copy link
Contributor

pd0wm commented Jun 29, 2021

Went back through discord and comfirmed that @mattbsea has \x028965B0R01300\x00\x00\x00\x008965B0R02300\x00\x00\x00\x00, which was claimed to be better with 0.15

@zorrobyte
Copy link
Contributor Author

Dang, I was hoping we wouldn’t need to fragment so much @pd0wm but if that’s what needs to be done, I can update the PR. I can see what noob’s FW version is as well.

@mattbsea
Copy link
Contributor

Went back through discord and comfirmed that @mattbsea has \x028965B0R01300\x00\x00\x00\x008965B0R02300\x00\x00\x00\x00, which was claimed to be better with 0.15

Installing the new branch right now... Will let you know!

@adnanlanewla
Copy link

Will test it tomorrow and let you know how it performs

@yufeng66
Copy link
Contributor

I will test it this weekend. But looks like it essentially reverse the pull request #1260
I suspect the original issue I had, rapid oscillating steer angle, will probably still be there with the lane line model. But with lane less model things might be tolerable.

@pd0wm
Copy link
Contributor

pd0wm commented Jun 30, 2021

When reporting back please mention your dongle id so we can cross reference the FW versions.

@mattbsea
Copy link
Contributor

Dongle: 9388d771f95175f5
Sample Drive: 9388d771f95175f5|2021-06-29--19-07-52--0
Cell phone video: https://photos.app.goo.gl/a2kyy77wdZ2fYrdW6

The steering on this branch is jerky. On corners it over corrects then comes back to a more correct angle. The current release 0.8.x is much smoother.

@adnanlanewla
Copy link

adnanlanewla commented Jul 1, 2021

I have the exact same issue as @mattbsea
Sample Drive: d2ef5aa690e849d0|2021-06-30--22-02-47--0
Cell phone video: https://photos.app.goo.gl/AaKbZ3xqmS8XkXs79

I have 2020 Rav4 LE Canadian version. In the city driving this tune is not useable. It gives me motion sickness. The wheel is continuously trying to adjust itself even on the straight roads. Sorry :( @zorrobyte

@zorrobyte
Copy link
Contributor Author

zorrobyte commented Jul 4, 2021

Alright! So I did some research with @pd0wm - looks like there are two distinct steering racks for TSS2 RAV:

image0
image1
image2

https://ahparts.com/buy-used/2020-toyota-rav-4-and-gear-box-power-steering-rack-pinion-chk-44250-0r012-442500r012/351778-1
https://ahparts.com/buy-used/2019-toyota-rav-4-and-gear-box-power-steering-rack-pinion-44250-42190-4425042190/332087-1

https://discord.com/channels/469524606043160576/574796986822295569/860481431707713606

Willem Melching — 07/02/2021
@zorrobyte My best guess is we should use 0.15 for all EPS firmwares starting with \x02, and 0.6 for the others
Can you update your PR accordingly? (post the info about the 2 racks in the PR as well so know why this difference exists if we want to go back to it later)

And here's the PR!

Please check carParams and make sure \x02 actually matches the carFw eps string of the vehicles we are trying to target. Mine is fwVersion = "8965B42171\000\000\000\000\000\000", and I want to be sure /x02 is actually fwVersion = "\x028965B0R01200\x00\x00\x00\x008965B0R02200\x00\x00\x00\x00" with the backslash -- dongle d2ef5aa690e849d0

@pd0wm
Copy link
Contributor

pd0wm commented Jul 5, 2021

Can you also put a comment in the code with a short explanation?

Saw some confusion on discord around the b"\x02". It's an escape sequence for a byte value with value 2, so the length is actually just one. So alternatively you can check with fw.fwVersion[0] == b"\x02", but using startswith is fine too.

@zorrobyte
Copy link
Contributor Author

Done and done! I left startswith in there so the code is a bit more self explanatory.

@zorrobyte zorrobyte requested a review from pd0wm July 5, 2021 14:02
@adnanlanewla
Copy link

Will these PR means that the tune will remain intact for the other steering rack FW ?

@adeebshihadeh
Copy link
Contributor

Let's also combine CAR.RAV4_TSS2 and CAR.RAVH4_TSS2 as all the parameters are the same except mass.

@zorrobyte
Copy link
Contributor Author

Done, as requested

Split tuning on eps fwVersion \x02 only. See #21429 (comment) for findings.

Unify Rav4 & Rav4 Hybrid

Average mass between ICE & Hybrid

Co-Authored-By: Willem Melching <willem.melching@gmail.com>
@zorrobyte
Copy link
Contributor Author

Fixed the failing tests and should be gold now

@Taik
Copy link
Contributor

Taik commented Jul 6, 2021

Was able to take this out earlier tonight. Definitely feels more planted for me; solid handling. The car feels like it over-compensates less when exiting curves vs. original 0.8.5.

Will be driving on this for the rest of the week and provide feedback. Thanks for the tune @zorrobyte !

Car: 2020 Toyota RAV4 Hybrid LE
Dongle ID: 5e3a19ff9d0eb905
Route: 5e3a19ff9d0eb905|2021-07-05--20-15-16

@zorrobyte zorrobyte marked this pull request as ready for review July 6, 2021 05:18
@pd0wm pd0wm merged commit f40e75e into commaai:master Jul 6, 2021
@zorrobyte
Copy link
Contributor Author

Whohoo! Happy days!

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

Successfully merging this pull request may close these issues.

8 participants