openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

Honda Bosch Radarless longitudinal behind toggle

Open royjr opened this issue 3 years ago • 12 comments

Possible pull for #25355. Works great except stops way too close to the lead car. See panda PR

royjr avatar Aug 04 '22 19:08 royjr

Can you provide some routes with rlogs uploaded in stop and go traffic and highway speeds (and maybe some heavy brake and accel instances)? I'd like to confirm we understand the the accel interface and it follows our request well enough

sshane avatar Aug 04 '22 19:08 sshane

Also should figure out what sends AEB and FCW so we can make sure those are never blocked (I think the camera does it?) @royjr do you know about AEB?

sshane avatar Aug 04 '22 19:08 sshane

I assume FCW is the warning before AEB takes over and starts braking. I believe the camera sends those messges from our last findings. I was never able to trigger AEB on my car confirm the AEB messages (AEB_BRAKING, AEB_STATUS)

royjr avatar Aug 05 '22 03:08 royjr

1a5d045d2c531a6d|2022-09-01--13-46-28 1a5d045d2c531a6d|2022-09-01--13-36-53

royjr avatar Sep 01 '22 21:09 royjr

tmux: undefined signal STANDSTILL - 456 There is no STANDSTILL in ACC_CONTROL for this car.

royjr avatar Sep 04 '22 11:09 royjr

Should be fixed now. Can you tell me a few things?

  • With stock ACC, do you need to press the resume button or press the accelerator to resume from a stop?
  • Before the STANDSTILL addition and on openpilot ACC, do you need to press the resume button or press the accelerator to resume from a stop?
  • After the STANDSTILL addition, does that change?

sshane avatar Sep 05 '22 07:09 sshane

Stock ACC - needs resume button OP + Stock ACC - does not need resume button, but sometimes doesnt resume automatically (once in a blue) OP + OP ACC - does not need resume button, but short terms tests may not be conclusive based on above OP + OP ACC + STANDSTILL addition - does not need resume button, but short terms tests may not be conclusive based on above

royjr avatar Sep 07 '22 03:09 royjr

@royjr @sshane what is the status of this PR is it ready to merge?

dmdreon avatar Dec 01 '22 02:12 dmdreon

@dmdreon It definitely works, just has a few ui kinks left to iron out.

  • Mismatching speed when setting speed below 25
  • Dashboard blinks speed sometimes
  • Dashboard "stopped" remains after resume

royjr avatar Dec 04 '22 22:12 royjr

A few things come to mind..

  • There was no conversion logic before, it was always CV.MS_TO_KPH which doesn't work for the civic. This might break non-radarless cars. Should we only apply the conversion to radarless?
  • Do we still need the gas_override in the hondacan ui function or can we just do that check in carcontroller since both could set to the same 255 value?

royjr avatar Dec 20 '22 00:12 royjr

Yes, how you had it before with the radar less check was fine. We can bring out the override check too

sshane avatar Dec 20 '22 01:12 sshane

done, unless you wanted to move the override check to another PR since thats new to other hondas

royjr avatar Dec 20 '22 02:12 royjr

@royjr when this is merged, can you open new PRs for the following?

  • idlestop timer, if stock uses a timer and doesn't set it immediately or statically
  • match stock hide speed behavior on override

sshane avatar Mar 28 '23 07:03 sshane