commaai / openpilot

openpilot is an operating system for robotics. Currently, it upgrades the driver assistance system in 275+ supported cars.
https://comma.ai/openpilot
MIT License
49.07k stars 8.94k forks source link

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

Closed rpaulhanna closed 2 years ago

rpaulhanna commented 3 years ago

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

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: https://github.com/commaai/opendbc/pull/279#issuecomment-679826539

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.

nelsonjchen commented 3 years ago

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.

pd0wm commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

rpaulhanna commented 3 years ago

Glad that is working for you. I settled on a speed scale factor of 0.98 and that seems to work fine for me. Also, changing the toyotacan.py file doesn't seem to make any difference. So, I have been just leaving that one as is as I apply the changes to each new release. Hopefully, Comma develops a permanent and more elegant solution to fix this problem for good.

On Sun, Dec 6, 2020 at 12:56 PM brown1428 notifications@github.com wrote:

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

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/commaai/openpilot/issues/2106#issuecomment-739546565, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF2ETWP2U7G7OQXOMFKG7D3STPHVRANCNFSM4QP2DUCQ .

brown1428 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

https://github.com/rpaulhanna/openpilot/commit/0302170af7b8b7f727dc749fc2b6f10ed8c9d000

Dillinja2 commented 3 years ago

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 commented 3 years ago

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

nelsonjchen commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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.

carl98nes commented 2 years ago

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 commented 2 years ago

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

carl98nes commented 2 years ago

Hi Harald:

Thanks for looking into this. Here is the camera log I have for Nov 7 which ran for 30 min. Notice that on a fairly flat highway road, I had to disengage many times due to the very high rev and rpms. I have a 2021 Lexus RX 350L, if that makes any difference on the code. I have other days of logs but this captured my highway driving. Also I need to point out that this happened only after the upgrade to 0.8.10. on Nov 1. Before the upgrade, when I first received the C3 version 0.8.9, it did not have this problem. Let me know if you need anything else.

Thanks,

Carl98Nes

On Thu, Nov 11, 2021 at 12:10 PM HaraldSchafer @.***> wrote:

@carl98nes https://github.com/carl98nes Do you have a segment for where this happens?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/commaai/openpilot/issues/2106#issuecomment-966472387, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWJLILDYMZFEN3AXCKLNHJDULP2HVANCNFSM4QP2DUCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

bobmeeks01 commented 2 years ago

+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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

bobmeeks01 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

pd0wm commented 2 years ago

Made it 3.5%.

@carl98nes @bobmeeks01 Can you try this branch? https://github.com/commaai/openpilot/pull/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:

bobmeeks01 commented 2 years ago

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 commented 2 years ago

Hi All, I have the similar experience, not exactly the same but something is wrong with the rpm that it just keeps on downshifting on high speeds. On city driving it's not apparent but when speeds go up to around 60mph then the car would just suddenly downshift. Cruising along at around 8:08 when I turned C3 but I should have known something was wrong because the rpm did not go lower than 2200 and at around 8:10, it started downshifting to 3.5K then go down to 2500 and back up. I had to disengage, I dont want to ruin my engine.

On Wed, Dec 1, 2021 at 5:02 PM bobmeeks01 @.***> wrote:

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 https://github.com/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) [image: IMG_1447] https://user-images.githubusercontent.com/58351215/144321039-5c7fc654-4a40-4363-ade8-1380c6a8e803.jpg [image: IMG_1448] https://user-images.githubusercontent.com/58351215/144321048-68e6302a-1f7d-4970-ae75-498d280c0a23.jpg

These are during rev'd RPM's (570 seconds) [image: IMG_1455] https://user-images.githubusercontent.com/58351215/144321070-9a633aca-2e06-4a46-8655-dbf8038fad11.jpg [image: IMG_1456] https://user-images.githubusercontent.com/58351215/144321083-7915bbb4-d371-4118-bb4c-b0ac1a2ecb59.jpg

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/commaai/openpilot/issues/2106#issuecomment-984092933, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWJLILCVMDPZS55H5RTSGL3UO2LOFANCNFSM4QP2DUCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

pd0wm commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

cecnic1989 commented 2 years ago

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.