这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@jyoung8607
Copy link
Collaborator

@jyoung8607 jyoung8607 commented Jul 11, 2025

The Honda gearbox message situation needed some help. Four different messages defined with an array of overlapping and conflicting names, incorrect message frequencies, different fields used from the same message in different DBCs, different signal bit widths and endianness settings, several ways to pick a message, lots of duplication. Other than that it was great.

This PR migrates the Honda port to gearbox message auto-detection with a common gearbox DBC. Virtually all cars are covered by two messages: one for traditional CVTs, and another for traditional autos, eCVTs and direct-drive EVs. The oldest ACURA_RDX is a snowflake that has to stay in its custom DBC.

Validation

Verified against several data sources using commaai/openpilot#35682:

  • commaCarSegments v2
  • upstream comma CI test routes
  • pending CI test routes from the Q3 CY2025 Honda car port backlog

TODO

  • Add back a manual trans reverse signal, don't think the current signal is good
  • Final validation
  • Monitor that auto-detect works in the real world, that we don't have startup timing issues

@jyoung8607 jyoung8607 mentioned this pull request Jul 11, 2025
@adeebshihadeh
Copy link
Contributor

Something to keep in mind here and in the future:

openpilot doesn't actually care about the specific gear we're in. It just needs to know whether we should engage or not.

@mvl-boston
Copy link
Contributor

mvl-boston commented Jul 13, 2025

All the Bosch C appear to be gearbox 50. Even the Accord which is CVT: 4df2d9d0197e20e3/0000000e--54aa88db6f

I just realized you moved dbc signals names/locations too - will check that out and report back on Bosch C.

@jyoung8607
Copy link
Collaborator Author

All the Bosch C appear to be gearbox 50. Even the Accord which is CVT: 4df2d9d0197e20e3/0000000e--54aa88db6f

Are you sure that route isn't from a hybrid Accord with an e-CVT? It's possible that I didn't get this distinction quite right, it's based on messages present aligned with Wikipedia research:

two messages: one for traditional CVTs, and another for traditional autos, eCVTs and direct-drive EVs

If it makes sense to store the detected difference in a flag rather than trans type, I can do that. But what really matters is that we're coalescing around two messages, each of which have consistent frequencies and signals.

@mvl-boston
Copy link
Contributor

mvl-boston commented Jul 14, 2025

Sorry, read discord history from the driver and it looks like it was a hybrid, and I have no DMs active with CVT Accord drivers. But did confirm that a CVT CRV is using the same CVT signal you build: 2dc4489d7e1410ca/00000000--8cc91f7e24

I presume Accord would be the same.

But there is an issue with Bosch C:

  • canfd cvt share the cvt signal with bosch A/B so can reuse your common gearbox include file
  • canfd auto has the same gearbox signal starting position number, but the dbc has individual message bits in different spots, some of it overlapping so probably needs a separate dbc.

edit: seems you refactored the canfd pilot signal too. Guess it was fun endian stuff... I'll merge this into my combined Honda build in discord so there is early testing in case any issues arise.

@jyoung8607 jyoung8607 moved this from In progress to In review in Honda: The Power of Dreams Jul 14, 2025
@mvl-boston
Copy link
Contributor

I'll merge this into my combined Honda build in discord so there is early testing in case any issues arise.

Taking longer than anticipated. vanillagorilla didn't build canfd long control safety tests. Unseen until now since safety coverage ci used to timeout on forks. Will fix (target Wednesday evening) in combined-honda before asking people to test.

@jyoung8607
Copy link
Collaborator Author

You don't have to worry about this one too much. It's mostly good but not quite finished or quite fully tested yet, and I'll just add the CI test routes from your new PRs to the validation notebook in commaai/openpilot#35682.

@jyoung8607 jyoung8607 marked this pull request as ready for review July 22, 2025 17:09
@jyoung8607
Copy link
Collaborator Author

@adeebshihadeh @sshane This is large change to the most crufty part of the Honda port, and touches ALL Hondas. I'm reasonably confident in my testing, but be on the lookout in case I missed something.

Expect process_replay to see a diff on CP.transmissionType, but not for actual gear positions.

***** results for segment regenB328FF8BA0A|2025-04-08--22-57-45--0 *****
    controlsd
    card
        ref: https://commadataci.blob.core.windows.net/openpilotci/regenB328FF8BA0A|2025-04-08--22-57-45--0_card_a6dd96e4a23b0af5b5bebec07c8ab1d2703f91ab.zst
        new: /tmp/openpilot/openpilot/selfdrive/test/process_replay/fakedata/regenB328FF8BA0A|2025-04-08--22-57-45--0_card_006a2e3fccd8a143b1f960386994f51836f55f10.zst

        carParams.transmissionType: 2
    lagd
***** results for segment regen6170C8C9A35|2025-04-08--22-57-46--0 *****
    controlsd
    card
        ref: https://commadataci.blob.core.windows.net/openpilotci/regen6170C8C9A35|2025-04-08--22-57-46--0_card_a6dd96e4a23b0af5b5bebec07c8ab1d2703f91ab.zst
        new: /tmp/openpilot/openpilot/selfdrive/test/process_replay/fakedata/regen6170C8C9A35|2025-04-08--22-57-46--0_card_006a2e3fccd8a143b1f960386994f51836f55f10.zst

        carParams.transmissionType: 2
    lagd

@jyoung8607 jyoung8607 merged commit 9ef0b1f into master Jul 22, 2025
8 checks passed
@jyoung8607 jyoung8607 deleted the honda-gearshift-cleanup branch July 22, 2025 17:18
@github-project-automation github-project-automation bot moved this from In review to Done in Honda: The Power of Dreams Jul 22, 2025
@adeebshihadeh
Copy link
Contributor

Looks like one of the fuzzing tests failed: https://github.com/commaai/opendbc/actions/runs/16451100901/job/46496300736

@jyoung8607
Copy link
Collaborator Author

Looks like one of the fuzzing tests failed: https://github.com/commaai/opendbc/actions/runs/16451100901/job/46496300736

#2525, hopefully. I'm not fully clear on what the fuzzer was changing and it doesn't seem to happen every run.

@mvl-boston
Copy link
Contributor

Integra CVT uses the auto message 0x191. Seems to cause no problems, but the label isn't 100% precise.

ad9840558640c31d/0000011e--89f22afa86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

car related to opendbc/car/ DBC signals honda

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants