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: DBC signals for manual trans gear position #1034

Merged
merged 7 commits into from
Oct 12, 2024

Conversation

vanillagorillaa
Copy link
Contributor

@vanillagorillaa vanillagorillaa commented Apr 13, 2024

Adds a manual transmission gear signal on new ID.

I have a test user with a CVT integra and one with a MT Integra. The CVT does have 0x1DD on CAN bus, but this signal is always 0. On the MT Integra the signal changes based on gear.

Verification : 61e73b2b0226d1a7/0000002a--c9db9c1967 (Integra) and 2d33198e4ff0ee67/00000006--9852f1fdf9 (Civic Type R)

This reverts commit d3489b5.
@vanillagorillaa vanillagorillaa marked this pull request as ready for review July 6, 2024 16:57
@vanillagorillaa
Copy link
Contributor Author

@sshane This is ready. Will be used for my Integra car port PR and a Civc Type R PR i'll be opening soon.

@vanillagorillaa vanillagorillaa changed the title WIP: Manual Gear msg Honda Manual Gear msg Jul 6, 2024
@@ -50,6 +50,11 @@ BO_ 467 CRUISE_FAULT_STATUS: 8 XXX
SG_ CHECKSUM : 59|4@0+ (1,0) [0|15] "" XXX
SG_ COUNTER : 61|2@0+ (1,0) [0|3] "" XXX

BO_ 477 GEARBOX_BOH: 8 XXX
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give this a better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GEARBOX_ALT_2, perhaps? Or could do GEARBOX_ALT_A and GEARBOX_ALT_B?

Copy link
Contributor

@sshane sshane Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on where this comes from, Toyota calls a similar message CLUTCH. This can be GEARBOX_MANUAL or GEARBOX_MT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to do gearbox_manual or gearbox_mt as this message is also found on the CVT transmission vehicles. The GEAR_MT signal only seems to change on MT vehicles while it's always 0 on CVT transmission vehicles. I will go with ALT_2 in order to prevent additional changes needed to support the renaming

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in c1e1823

Copy link
Contributor

@sshane sshane Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it exists but is truly unused, those names seem fine to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I recall, on the CVT vehicles, the message itself has data in other bytes, however only on the MT vehicles does GEAR_MT actually have data. On CVT vehicles GEAR_MT is always zero. I'm working on getting access again to the CVT integra device that I can confirm this.

You are saying though that even if there is data in other bytes on the CVT vehicles that you are fine naming the message GEARBOX_MT or GEARBOX_MANUAL ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this route on a CVT integra you can see that there is other data on this message, but the GEAR_MT signal is always 0. 3f8ae015ce70365f/000000fe--b691f954c2

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sshane any more comments on this?

@beckej13820
Copy link

@sshane Can this pull request be reopened. I believe that VanillaGorillaa is no longer contributing to OpenPilot, which is why he closed the PR, but there are users who would benefit from this change.

If additional routes are needed, I can try to provide and coordinate with other members of the Honda Acura channel.

(2024 Integra CVT Owner)

@BareNakedSlayer
Copy link

I would also ask the same as beckej13820. I have a Civic Type R and I'm using a dev branch that was custom set for my car, based on VanillaGorillaa's work. I can report it works pretty much great. It would be nice to have the manuals supported natively, at least the current gens of the MTs out there.

@jyoung8607 jyoung8607 reopened this Oct 11, 2024
@jyoung8607 jyoung8607 self-assigned this Oct 11, 2024
@jyoung8607 jyoung8607 changed the title Honda Manual Gear msg Honda: DBC signals for manual trans gear position Oct 11, 2024
@jyoung8607
Copy link
Collaborator

Re-opening and self-assigning, but not merging right away. In concept I'm happy to bring this in, but I want to take a closer look at the Integra port first, commaai/openpilot#32056.

@funnybonez
Copy link

I would also ask the same as beckej13820. I have a Civic Type R and I'm using a dev branch that was custom set for my car, based on VanillaGorillaa's work. I can report it works pretty much great. It would be nice to have the manuals supported natively, at least the current gens of the MTs out there.

How does one reach VanillaGorillaa?

I have a 2019 Honda Civic Sport MT
Do you have a link to the repo?

@BareNakedSlayer
Copy link

I would also ask the same as beckej13820. I have a Civic Type R and I'm using a dev branch that was custom set for my car, based on VanillaGorillaa's work. I can report it works pretty much great. It would be nice to have the manuals supported natively, at least the current gens of the MTs out there.

How does one reach VanillaGorillaa?

I have a 2019 Honda Civic Sport MT Do you have a link to the repo?

I don't know how to get in contact with anyone, sorry. I was on the SunnyPilot discord and the original requestor replied there when I asked about the plans to merge it, saying that VanillaGorillaa left and closed the pull request. I don't know if what I have would work with what you are driving. I had a 2019 sport, and it was pretty much awesome, so your car is a sweet ride. One thing I missed is the 10th gen design, it was the best looking Honda ever, IMO. I would actually suggest that that you go to the Comma AI reddit and do some searching there. From what I understand, what I'm using basically fools the system into thinking it's actually a 2022-2023 Civic Sedan. And since the security, LKAS, and ACC are all the same, it works. If it works for a 2024, a similar thing would likely work for a 2019 Sport, or maybe VanillaGorillaa found a way that this can be made compatible with earlier model MTs. I'm not a developer, so my knowledge around this is very limited. Sorry I can't do more.

@BareNakedSlayer
Copy link

Re-opening and self-assigning, but not merging right away. In concept I'm happy to bring this in, but I want to take a closer look at the Integra port first, commaai/openpilot#32056.

That is amazing! Thank you so much for doing that. I'm very excited for some native support. I'm on a dev branch with SP right now, but I'm afraid that won't get updated when big updates are done in OP. I'm not super knowledgeable about how all this works.

@jyoung8607
Copy link
Collaborator

It's very likely the D1 through D6 values are inferred from transmission input/output RPM ratios rather than any sort of gearshift position sensing, which means a value of 0 might mean gear-unknown rather than an actual indicator of clutch-pressed.

We'll need to verify that when working on the Integra port. For now, merging as-is.

@jyoung8607 jyoung8607 merged commit 6375fd6 into commaai:master Oct 12, 2024
4 checks passed
cydia2020 pushed a commit to cydia2020/opendbc that referenced this pull request Oct 13, 2024
* add integra MT gear message

* update gearn msg

* apply pr 998

* Revert "apply pr 998"

This reverts commit d3489b5.

* update gearbox name

---------

Co-authored-by: Jason Young <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants