-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Car Port Acura Integra #1722
base: master
Are you sure you want to change the base?
Conversation
This reverts commit c49a2b2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The under-2kmh speed issue is annoying. I see how you're trying to solve it, and while I'm not in love with your solution, I looked in Cabana myself for a bit and I'm not sure I see a better one.
I'm curious how stock deals with it. What's the control envelope for the factory camera-based ACC? Will it slow to a stop, and if so, will it resume? Does openpilot longitudinal control work?
// 0x158 used for all suported Hondas except Integra (use 0x309 car_speed message) | ||
if ((addr == 0x158) || (addr == 0x309)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 0x309
case and its corresponding RxCheck aren't exercised in tests. I'm not sure we can depend on it anyway, so it may be irrelevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will leave for the moment. Maybe we can find another message for speed
if self.CP.carFingerprint == CAR.ACURA_INTEGRA: | ||
ret.standstill = cp.vl["CAR_SPEED"]["CAR_SPEED"] < 1e-5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely not in love with this message, it's almost certainly intended for instrument cluster display. Low frequency, high arb ID, and has a considerable response lag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can take another look to see what we have, or can use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a pinch, there's a message that has ABS wheel speed sensor impulse integral, and that doesn't have the 2 kmh cutoff. We'd have to calibrate it against the main reference speed, but there's precedent and support for briefly suppressing engagement for steering angle sensor calibration and the like.
The reason I asked about stock ACC is that I'd be surprised if it does FtS/SnG if its own reference source has the 2kmh cutoff.
Checklist
added entry to CarInfo in selfdrive/car/*/values.py and ran selfdrive/car/docs.py to generate new docsDone by bot now?Requires
Honda: Acura Integra safety panda#2152Moved into this PR