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.

Sign up for GitHub

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

Jump to bottom

OP Low Gear/High RPM issue on 2020 Lexus RX 350 #2106

Closed
rpaulhanna opened this issue Aug 30, 2020 · 41 comments · Fixed by #23079
Closed

OP Low Gear/High RPM issue on 2020 Lexus RX 350 #2106

rpaulhanna opened this issue Aug 30, 2020 · 41 comments · Fixed by #23079
Labels
bug car vehicle-specific

Comments

@rpaulhanna
Copy link
Contributor

Describe the bug
Most notably at freeway speeds, engaging OP results in two downshifts shortly thereafter and an unnecessarily high RPM until disengaged or upon encountering a vehicle within following distance intervention. Manual or stock cruise control driving (OP disabled) over the same stretch of road at the same speed does not result in this low gear/high RPM behavior. As indicated by several Discord posts, this same behavior is also evident on other RX’s of earlier year models as well.

How to reproduce or log data
Engage OP at freeway speeds and observe downshifts/high RPMs (2500-4000) shortly thereafter. Alternatively, accelerate to freeway speeds with OP engaged and observe high RPMs. Disengage OP and observe the car promptly upshift and RPMs return to a normal freeway cruising range 1500-1800.

Expected behavior
Engaging OP or using it at freeway speeds should not result in downshifts and maintaining a highly inefficient DOUBLING of engine RPM just because OP is in use. OP long should result in mimicking driving the car manually at speed or as the stock cruise control does on the freeway—overdrive gear and 1500-1800 RPM.

Device/Version information

  • Dongle ID: 3a5c5101bd71ad5d
  • Route: 3a5c5101bd71ad5d|2020-08-30--13-12-43
  • Timestamp: Reference multiple engagements/disengagements starting 2 minutes into the clip.
  • Version: release2 v0.7.8
  • Car make/model: 2020 Lexus RX 350

Additional context
There appears to be a discrepancy between UI_SET_SPEED and SPEED (actual) that could be causing the problem. @nelsonjchen developed a “set speed scaling” solution that does resolve the problem. Reference his comment to @cecnic1989’s PR about the same issue: commaai/opendbc#279 (comment)

When implementing @nelsonjchen’s solution and adjusting the speed scale factor to 0.974 (which represents the 2.6% difference between UI_SET_SPEED and SPEED on my vehicle at approximately 65 mph), the low gear/high RPM issue is completely resolved. Reference route 3a5c5101bd71ad5d|2020-08-30--14-30-28 where over the same stretch of freeway the correct gear is maintained and the RPM stays within 1500-1800 except for a brief period of road surface incline and then it promptly upshifts and returns to the normal RPM cruising range.

OP code needs to be modified so that the speed discrepancy between what is set and what the actual speed is does not trigger inappropriate OP long commands that cause undesired downshifts and high RPMs that result in considerable inefficiency and make OP use on the freeway impractical. This modification could be in the form of a “Set Speed Scale” menu interface as proposed by @nelsonjchen (which I support) or fundamental changes to OP code on a per-vehicle basis to account for the speed discrepancies. In any case, a change is needed to correct this undesired behavior. Thanks.
@rpaulhanna rpaulhanna added the bug label Aug 30, 2020
@nelsonjchen
Copy link
Contributor

Just to be clear, the proof-of-concept I produced only modifies the cruise state's speed and not the speed reading of the vehicle. I actually like the GPS/close to GPS reading of the speed of the Comma 2.

@adeebshihadeh adeebshihadeh added car bug and removed bug labels Aug 30, 2020
@pd0wm
Copy link
Contributor

pd0wm commented Aug 31, 2020

It might have to do with the PERMIT_BRAKING bit in the ACC message (https://github.com/commaai/openpilot/blob/master/selfdrive/car/toyota/toyotacan.py#L39). Stock Toyota only sets this when there is a lead car, since otherwise it won't need to brake. So far we haven't seen any problems by always settings this to 1, but your car might react differently.

@rpaulhanna
Copy link
Contributor Author

rpaulhanna commented Aug 31, 2020 •
edited
Loading

@pd0wm I noticed this change in the code from the solution provided by @nelsonjchen. However, with his code left intact except for the speed scaling (i.e. PERMIT_BRAKING set to lead and the speed scale factor set to 1.0) the low gear/high RPM issue returns. The only thing I have found so far that corrects the issue is adjusting the speed scale factor (in my case to 0.974) as he proposed.

I don't know whether or not this is helpful, but another curious item is that without the speed scaling, the ACCEL_CMD with OP engaged is a strongly negative value the entire time. With the speed scaling set to 0.974, the ACCEL_CMD is mostly a positive value with dips into negative territory (as would be expected if adjusting the throttle going downhill). So, the behavior of ACCEL_CMD is wildly different depending on whether or not speed scaling is active.

@pd0wm
Copy link
Contributor

pd0wm commented Aug 31, 2020 •
edited
Loading

The change in the code was a noop. We have always been sending that bit. But reading your complaint better I don't think it's related.

My current guess is you're triggering a safety in the Toyota PCM. I have seen this happen when you accelerate too much beyond the set speed. The car then aggressively slows you down. This could be caused by a scaling issue somewhere.

We should probably try to figure where this discrepancy is coming from. When you're using stock ACC, does the car's speed stay below the set speed (according to EON set speed and car speed)? Do you have a link to a route with stock ACC without a lead car (e.g. where it can reach the set speed).

@rpaulhanna
Copy link
Contributor Author

rpaulhanna commented Aug 31, 2020 •
edited
Loading

Stock ACC is a bit sloppy compared to OP. Whether in town at low speed or on the freeway, it allows the car to drift a few mph both below and over the set speed before it takes action to either accelerate or slow the car to the set speed. However, it never exhibits the aggressive downshifting/high rev behavior. Perhaps this is by design for fuel economy? In equilibrium (level road surface, no curves) it does a good job of maintaining the set speed. Overall, OP seems more aggressive in maintaining a set speed over a variety of changing road conditions--not abrupt but more precise. The car's CAN speed and the speed displayed on the Comma Two is ALWAYS a few mph less than the set cruise speed as displayed on the vehicle's HUD. In reviewing the cabana info from my drives, that's how I determined that the car's speed was 2.6% (on average at 65 mph) less than the set speed and thus the 0.974 speed factor. So, the car's set speed is 67 let's say and the speedometer is indicating 67, but the actual speed is a few mph less as shown on the Comma Two and in the cabana data. I will try to get a stock ACC route today for comparison.

@rpaulhanna
Copy link
Contributor Author

@pd0wm here is a Stock ACC route I just performed over the same stretch of freeway as the other two routes in this issue report (albeit at a slightly slower speed to eliminate a lead vehicle over the majority of the route.)

3a5c5101bd71ad5d|2020-08-31--10-03-02

Interestingly, although I set the car's cruise to maintain 62 mph, the SET_SPEED parameter of PCM_CRUISE shows 97 kph which equates to 60.27 mph. The SPEED parameter seemed to stay within 96 to 99 kph (59.65 to 61.52 mph). On my vehicle's HUD, with the cruise set at 62 mph, the speedometer indicating steady at 62 mph, the Comma Two displayed 59 mph. The car's speedometer stayed within 2 mph of the set speed. When it reached 64 mph, it didn't downshift/rev but simply eased back to 62 mph. There were a couple brief single gear downshifts with RPMs rising to approximately 2,100 RPM to handle a decline in the road surface but nowhere near the constant maintaining of 2 gears too low and 3,000 RPM with OP engaged and without speed scaling.

@cecnic1989 has an earlier model RX with the same high rev issue and approached the solution differently by modifying the relevant opendbc files so that the set speed and speed matched. I think @bobmeeks01 has a 2020 RX with the same high rev issue and he also modified the relevant opendbc files to solve the problem as well. His fork is at https://github.com/bobmeeks01/openpilot-071. His custom .dbc file is lexus_rx350_2020_cabana.dbc.

Being a newbie, it was easier for me to implement @nelsonjchen's solution since his code changes didn't require the files to be compiled on release2.

I hope this additional context and the additional route demonstrating stock ACC behavior helps you identify the problem and the best way to correct the issue. Thanks.

@rpaulhanna
Copy link
Contributor Author

Additionally, with the cruise set to 62 mph as above, the PCM_CRUISE_SM UI_SET_SPEED matches the 62 mph as set whereas, simultaneously, the PCM_CRUISE_2 SET_SPEED shows 97 kph which equates to 60.27 mph.

@brown1428
Copy link
Contributor

FWIW My 2021 RX350 also suffers from this issue and I too decided to use one of @nelsonjchen's solutions.

@rpaulhanna
Copy link
Contributor Author

rpaulhanna commented Dec 6, 2020 via email

@brown1428
Copy link
Contributor

I have also (just today) experimented with @bobmeeks01 opendbc revisions, which also work well. The opendbc approach is attractive because now all speeds (speedo, cruise, OP) appear to very closely match.

@bobmeeks01
Copy link

What OP versions are you guys working with? I've been stuck on 0.7.1 with my custom .DBC file but just tried to upgrade to 0.8.1. Stock version still has this issue. I tried re-applying my custom .DBC values but it's like the changes aren't being recognized. No matter what I do it still drives the same and the mph and cruise set read-outs vary substantially between OP and car. If you are able to update .DBC values in recent OP versions then friend me on discord. I'd like to find out more.

@rpaulhanna
Copy link
Contributor Author

rpaulhanna commented Jan 4, 2021 •
edited
Loading

I am on 0.8.1 and, yes, the issue has not yet been addressed. With each new version, I simply add two lines of code that @nelsonjchen came up with to scale the speed to 98% and completely eliminate the issue. This, of course, will not make the speedometer and Comma 2 speeds match, but I would much rather have the proper shifting behavior.

rpaulhanna@0302170

@Dillinja2
Copy link

Adding my voice to the chorus to get this addressed. '17 RX 450h with high rpm issue has taken OP off the table for freeway driving.

@sumedhekaru
Copy link

Confirming that issue is still present with 0.8.3. 2016 Lexus Rx with SDSU.

@nelsonjchen
Copy link
Contributor

I built this continuous Lexus RX fork(s) generator as a stopgap:

https://github.com/LexusRXopenpilotUG/openpilot

It'll continually apply my hardcode hack atop branches daily and push them to the LexusRXopenpilotUG/openpilot repository. Also, some light instructions for no-SSH/no-Git installation via Shane's Fork installer are included at the bottom.

@brown1428
Copy link
Contributor

Well, for some reason the WHEEL_SPEED_xx values I was using to fix this issue no longer seem to have the desired effect beginning with 0.8.6. Not clear why.

@subwaymatch
Copy link

@nelsonjchen Thank you for creating the continuous Lexus RX fork! I returned my comma two because of the weird RPM issue (on top of a few other issues) but am excited to use your fork once my comma three arrives.

nelsonjchen referenced this issue in LexusRXopenpilotUG/openpilot Nov 11, 2021
@nelsonjchen
Update README.md
ec1fd7c
@carl98nes
Copy link

I ordered the C3 about a month ago and installed on my Lexus Rx350L. Very excited about the whole autonomous driving experience but something happened when upgraded to 0.8.10, there was an issue of shifting to lower gears when driving on the highways. revving the engine to as high as 4500 when I was doing 75mph. This was a very serious issue.
Went to discord to see if other users have had this problem and thankfully @nelsonjchen suggested the fork he created. I uninstalled comma software and reinstalled using the url. Now I dont have the issue anymore. Had a good driving day today using C3, city and highways, and the rpms stayed to under 2000. I drove for 2 hours in city driving didn't see/feel any bad behavior. I cruised a good half an hour in highways and to about 75mph and saw no issues. Thanks for the fork @nelsonjchen

@haraschax
Copy link
Contributor

@carl98nes Do you have a segment for where this happens?

@carl98nes
Copy link

carl98nes commented Nov 11, 2021 via email

@bobmeeks01
Copy link

+1 where my custom dbc file updates to address this rpm issue no longer seem to work after going to 0.8.10. I just upgraded from 0.8.2. I’ll give @nelsonjchen fork a try.

@bobmeeks01
Copy link

@HaraldSchafer If you need any additional data please reach out. I'm on Discord @bobmeeks01. I'd be happy to grab driving segments to help troubleshoot. We've been hacking our way past this issue for 2 years now and it's not safe for anyone who isn't aware and thinks the RX350 is "supported" and tries to drive this way.

@pd0wm
Copy link
Contributor

pd0wm commented Nov 22, 2021

Some driving segments with rlogs would be great (turn on the "upload raw logs" or upload trough useradmin). We would also need a stock drive (turn off the "enable openpilot" toggle in the settings) and drive the same stretch of road.

@haraschax
Copy link
Contributor

haraschax commented Nov 23, 2021 •
edited
Loading

@bobmeeks01 Do you mind testing this branch just to confirm getting too close to the set speed is the issue? I think it should work.

https://github.com/commaai/openpilot/tree/fudge_cruise_speed

@bobmeeks01
Copy link

@HaraldSchafer Certainly! I'll throw it on when I get home from work today and give it a try.

@bobmeeks01
Copy link

@HaraldSchafer I'm getting a black screen on startup. Here's what I did to install via SSH:

cd /data; cp -rf ./openpilot ./openpilot.bob;
rm -rf ./openpilot;
git clone https://github.com/commaai/openpilot.git openpilot; cd openpilot; git checkout fudge_cruise_speed;
reboot;

Everything seemed to download and install, but no luck getting past the black screen after two reboot attempts. Is there a better way to install??

Bob

@adeebshihadeh
Copy link
Contributor

You need to init the submodules, but there's an easier way. Uninstall openpilot and use the following URL: installer.comma.ai/commaai/fudge_cruise_speed.

@bobmeeks01
Copy link

Ok, seems pretty good! No RPM issues up to 80 mph (speedometer). RPM's sat around 2k, which is normal, and did not spike up to 3k then 4k after about 5 seconds ... which is the behavior I've seen since 0.7.1 without some code intervention.

With cruise set at 80 mph on the speedometer, the Comma had cruise set at 75 MPH. At 35 mph (speedometer), comma had 33 mph.

I would encourage other RX owners to give it a try to verify. I have rlog upload on, but not sure how to point you to it. I can give you a URL from Cabana and the time when I had OP engaged if that helps.

Thanks for looking into this. It will be great to have this resolved in released version!

@bobmeeks01
Copy link

You need to init the submodules, but there's an easier way. Uninstall openpilot and use the following URL: installer.comma.ai/commaai/fudge_cruise_speed.

Thanks, @adeebshihadeh. That did the trick.

@pd0wm
Copy link
Contributor

pd0wm commented Nov 24, 2021

Good to know!

Do you have a route with openpilot disabled? I'd like to see if the stock system is also driving slower than the cruise set speed.

@bobmeeks01
Copy link

@pd0wm Here are a couple of routes.

This one is with Openpilot disabled. There is good highway driving starting around 370 seconds with cruise enabled. I did notice that even with just the dashcam software (no OP) that when speedometer was at 80 mph, dashcam on comma showed 75 mph.
41e96f2034770443%7C2021-11-24--15-19-20

This one is from yesterday (referenced above) using the "Fudge_cruise_speed" branch and OP enabled. There is a good stretch of highway driving starting at 930 with OP engaged.
41e96f2034770443%7C2021-11-23--17-23-31

Let me know what else you need. Thanks again to everyone helping with this.
Bob

@pd0wm
Copy link
Contributor

pd0wm commented Nov 25, 2021 •
edited
Loading

image

vs a random rav4 segment:
image

Seems like there is a massive difference between the speeds reported by the wheels and the actual speeds (compared to GPS). Instead of fixing this in the wheel speed sensors they added some stuff to the PCM and the dashboard to fix this.

@cydia2020 UI_SET_SPEED is even higher in this case (after converting from mph to km/h) to match the units. So that's also not the solution.

Seems like the intention of the initial DBC change was correct, but changed both the wheel speeds and the set speed. It would work if you only scale the wheel speeds but leave the set speed as is.

@HaraldSchafer What do you think about adding a wheel speed multiplier in carParams to fix cars like this with egregious errors? We can expose the multiplier as a locationd output since it's already in the filter anyway, which would make it trivial to extract it from the qlogs of a bunch of routes.

@carl98nes
Copy link

I have installed the fudge_cruise_speed and at first it seemed to be working fine on slow speeds and at high speeds on the freeway. the RX speed is actually closer to c3 speed. Then it happened on just one occasion and only for a brief moment (I was not sure why it happened). I was on a freeway on a little uphill so I expected the gears to change to a lower gear and thinking the rpm would increase to about 3000rpm but the rpm shot up to 5000 rpm. I knew then something is wrong I had to disengaged immediately by pressing the brake. it happened this afternoon Thu, 25 Nov @ 15:07:20. It didn't happen any other time but only on that instance.
Maybe it's an anomaly but I can't trust C3 on highway speeds.
dongle: c5c8fc431bbe68e5_2021-11-25--14-34-47--32--qlog.bz2

@haraschax
Copy link
Contributor

Given Willem's plots, the 3% fudge is likely just not enough.

@pd0wm
Copy link
Contributor

pd0wm commented Nov 30, 2021

Made it 3.5%.

@carl98nes @bobmeeks01 Can you try this branch? #23079 (wheel-speed-factor). Make sure to update your submodules.

While at a steady speed with this branch please write down the following speeds and the date/time:

  • Speed on car HUD
  • Speed on openpilot UI
  • Set speed on car HUD
  • Set speed on openpilot UI

@bobmeeks01
Copy link

Here is a route from this morning: 41e96f2034770443%7C2021-12-01--08-22-41

Starting at around 8:30 am (480 seconds) I engaged for about 2 minutes. The car set speed was 73 mph. OP set speed was 71 mph. When not following (full speed) the OP speed was 71 mph, but the car speed was 76 mph!! 3 mph higher than the set speed? I've never seen that before. I'll upload pictures I took with my phone.

Unfortunately, though, after about 2 minutes, the car starting downshifting like crazy and ramped up over 4k RPM's again. As with @carl98nes, this might have started on a slight uphill grade but once the downshifting occurred the car got stuck and wouldn't resolve itself. I'll upload pictures but the set speeds and actual speeds were the same as before, but RPM's are double. On this route, this was happening at around 570 seconds for about 10 seconds before I had to disengage.

These are at normal RPM's (480 seconds)
IMG_1447
IMG_1448

These are during rev'd RPM's (570 seconds)
IMG_1455
IMG_1456

@carl98nes
Copy link

carl98nes commented Dec 2, 2021 via email

@pd0wm
Copy link
Contributor

pd0wm commented Dec 2, 2021

@bobmeeks01 Thanks for the detailed report. I accidentally applied the changes to the TSS1 version of the RX instead of the TSS2 (LSS2) version. Let me know if it's better now.

@bobmeeks01
Copy link

Ha! No problem. Here's a new route: 41e96f2034770443%7C2021-12-02--11-42-10

Much better. Drove engaged at 70+ mph for about 8 or 9 minutes straight with no revving issue, including some uphill driving.

Starting at 11:51:

Speed on car HUD = 73 mph
Speed on openpilot UI = 70 mph
Set speed on car HUD = 73 mph
Set speed on openpilot UI = 70 mph

Then starting at around 11:54 I bumped the speed:

Speed on car HUD = 80 mph
Speed on openpilot UI = 77 mph
Set speed on car HUD = 80 mph
Set speed on openpilot UI = 77 mph

I might be paranoid by now, but I did seem to think that there were 4-5 odd little bumps or hits in the RPM's ... like on uphills it would bump up but immediately come down, or on downhills the RPM's would suddenly drop and then come back up. I can't really point to an exact time because it just seemed a little random. But again, this might have happened even without OP ... I can't tell.

@pd0wm
Copy link
Contributor

pd0wm commented Dec 2, 2021

I'll merge this to master then! Feel free to update this issue if you have more feedback after merging it.

@pd0wm pd0wm linked a pull request Dec 3, 2021 that will close this issue
add CarParams.wheelSpeedFactor #23079
Merged
@cecnic1989 cecnic1989 mentioned this issue Jan 3, 2022
Closed
@cecnic1989
Copy link

Thank you all for resolving this issue for RX owners! This is perfect.

I've been using my fork with the custom opendbc modifications for the past year. I tried the latest release from OP, and I'm still facing this issue on my 2017 Lexus RX. It looks like the above PR didn't add the wheelSpeedFactor to the 2016-2017 Lexus RX section. Therefore, I raised a quick PR to add wheelSpeedFactor fix for 2016-2017 Lexus RXs. See #23367.

@pd0wm pd0wm mentioned this issue May 25, 2022
Closed
@pd0wm pd0wm added bug car vehicle-specific and removed car bug labels Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug car vehicle-specific
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add CarParams.wheelSpeedFactor commaai/openpilot