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

Honda Radarless: log nonAdaptive with openpilot longitudinal #31849

Closed
MarcoTheDingo opened this issue Mar 13, 2024 · 4 comments · Fixed by #31853
Closed

Honda Radarless: log nonAdaptive with openpilot longitudinal #31849

MarcoTheDingo opened this issue Mar 13, 2024 · 4 comments · Fixed by #31853
Labels

Comments

@MarcoTheDingo
Copy link
Contributor

MarcoTheDingo commented Mar 13, 2024

Describe the bug

After either #31763 or #31799 , and the March 12th master-ci build, my dashboard no longer works as expected. The lead car (mini car) no longer shows along with the adjacent cars, which show on my trim. It used to show up to 2 in each adjacent lane, even when using alpha longitudinal control.
Also when setting ACC under 25mph, or when the current speed falls under 25mph, a message shows on screen "Cannot set cruise: speed too low". Openpilot never disengages, but these issues never occurred before today.

EDIT: Seems to be related to being in standard cruise mode.

Provide a route where the issue occurs

5130484aa8069bad/00000010--c76b9d991a/2

openpilot version

0.9.7 / master-ci / 224ee9f / Mar 12

Additional info

No response

@sshane
Copy link
Contributor

sshane commented Mar 13, 2024

Can you revert this and try again? #31799

Can you also get a stock route where you cycle from closest to max distance on the dash and mark the beginning and end of that section?

@MarcoTheDingo
Copy link
Contributor Author

MarcoTheDingo commented Mar 13, 2024

Issue seemed to be when testing the distance button, accidentally set it to standard cruise mode, but the UI did not update. This made something think the car was in standard cruise mode, while openpilot was sending ACC commands.

If using Stock ACC, the UI icon updates back and forth when switching and car makes a long beep.
If using openpilot ACC, the UI icon doesn't update, nor does it make a beep. Only symptom is mini cars no longer show and shows "slow speed" error when under 25 mph.

op long, with car in "standard" cruise mode: 5130484aa8069bad/00000012--f2f58aca39/0
Stock long, starting in "standard" cruise mode, then switching to ACC. Also played with lead distance button from 22:35:17 to 22:35:42: 5130484aa8069bad/00000013--e85305cd64/1
op long starting in ACC mode, then switching back and forth between "standard" and ACC modes with lead (ACC @22:38:12, standard @22:38:37, ACC @22:38:47): 5130484aa8069bad/00000014--26d854a52d/1

@sshane
Copy link
Contributor

sshane commented Mar 13, 2024

Ah, so you're saying some ECU besides the camera is in non-adaptive cruise mode, and therefore is not caused by the distance button PR recently merged? The new 25 mph minimum speed sounds about right for that.

@sshane sshane changed the title Honda: Mini cars missing and speed too slow errors Honda Radarless: log nonAdaptive with openpilot longitudinal Mar 13, 2024
@sshane
Copy link
Contributor

sshane commented Mar 13, 2024

I checked the first two routes in your last message, and no other signal outside of ACC_HUD matched exactly with the non-adaptive cruise state. My guess is the PCM also manages an internal state of whether it should be doing standard cruise control or not (both camera and PCM must look at the buttons to sync their states) since we don't forward ACC_HUD.

Here you can see the PCM is reporting "ACC" is active when you engaged non-adaptive cruise control, but the camera is not sending anything. This hints that the PCM keeps its own adaptive cruise state as well as does non-adaptive cruise internally:

image

Going to block enabling in this non-adaptive state like we do when we're using stock long.

@sshane sshane linked a pull request Mar 13, 2024 that will close this issue
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 a pull request may close this issue.

2 participants