commaai
Honda: Add support for 2018 Honda N-Box
Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:
- Convert your PR to a draft unless it's ready to review
- Read the contributing docs
- Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Thank you for the PR! At a glance it looks good, except we do need a CI test route. Can you share a route that you don't mind making public, for automated testing purposes? We just need 5-10 minutes of driving with some openpilot engagement. For your privacy, this route shouldn't start or end at your home or workplace. Be sure to upload logs from comma connect.
This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.
@jyoung8607 I uploaded n-box test driving route and added it as test route to code.
@jyoung8607 I uploaded n-box test driving route and added it as test route to code.
It looks like rlogs aren't uploaded for that route. Can you upload those logs, or replace the route with one that does have logs uploaded? Your device will need to be on Wi-Fi, and then in comma connect, click on the route you want to upload, and select Files->Upload (all logs). Then, enable More info->Public access and More info->Preserved.
@jyoung8607 Sorry, I only uploaded camera data. Now I upload all data to comma connect. Could you check this?
@jyoung8607 Sorry, I only uploaded camera data. Now I upload all data to comma connect. Could you check this?
Yes, I see your data uploaded, thank you. I will review it soon.
CI tests aren't passing due to a syntax error on your most recent change. I'm setting your PR to Draft, once that's fixed you can remove it from Draft.
@jyoung8607 I tested current branch on 414af83891dbf72c/00000008--a634d20a68. This branch work.
Hello @miettal,
Thank you for your patience. We've learned some important things recently from merging support for other Hondas, and I'd like your help with a couple more tests.
First, we need a dashcam-mode route showing your stock LKA system working. Just set the "openpilot enabled" toggle to off, turn on your stock LKA system, and record it controlling your steering during some lane departures. I think the current values of lateralParams.torqueBP and lateralParams.torqueV are probably wrong, and this will tell us the correct values. When you're done, upload logs and let me know the route ID as you've done before.
Once we have a dashcam-mode route, I'll update this branch with some improved lateral control parameters, and have you test to make sure that works well. That's the last thing we should need to merge.
@jyoung8607 OK, no problem, I will do it. Thank you for huge refactoring and migration!!
@jyoung8607 here
414af83891dbf72c/0000000d--582a030f1f/0
Thank you, this was helpful. Can you try driving on the most current changes in this PR?
I've made a testing branch that you can use: reinstall using custom shortcut commaai/honda-testing.
It's helpful for testing/calibration if your hands are not on the steering wheel while openpilot is engaged. You can always override or take back control if you need to, but we don't really know how your car responds to torque control if we're trying to move your hands as well.
I tested, but controll reaction is very slow.
414af83891dbf72c/00000001--44dfd9fbac
BTW, my connect page was in "Loading..." forever... Console show javascript error, but I'm no sure this is related to forever loading.
I tested, but controll reaction is very slow.
414af83891dbf72c/00000001--44dfd9fbac
Yes, looking at your logs, I agree that control wasn't very good. I've switched N-Box back to its original tuning.
Thank you for the PR!