-
Notifications
You must be signed in to change notification settings - Fork 791
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
IsoTpMessage: support ISO-TP w/ CAN FD #1524
base: master
Are you sure you want to change the base?
Conversation
Working on my F150 Lightning fork as well. |
I have been running with this change in this PR, as is, for a few weeks - it fingerprints correctly every time - 2023 F-150 ICE |
@adeebshihadeh we have a lot of users on the Ford CAN-FD test branch that @coffee-cake-isaac created and continue to see successful fingerprints using this fix. |
@sshane, this is working for C3X users. However, we do have one Mach-E user who missed a FP and was returning Mazda ECUs. This UDS fix works to get all the responses, but there may need to be some work on the FW version query on the OP side.
|
Also seem to have a bit of an issue with some HKG users no longer fingerprinting properly, either. |
@coffee-cake-isaac what is the issue related to HKG? |
Do you have a dongle ID for that user? |
I don't actually know if this was related or not; Sunny's users were having issues when introducing this with Sunnypilot, but I think it was unrelated as we haven't heard anything since. EDIT: I just confirmed this is still in SP and we haven't had fingerprint issues aside from the overlapping ones like myself and another Mach E have experienced. |
Actually, since I've had some ECU upgrades done for a recall mine is doing it now, as well. Since most of my routes recently have been on a fork without this panda change, I have a route that shows this as well.
|
That route looks fine as long as we get everything back from the Ford queries, if ECUs happen to respond to Mazda queries, that's okay Edit: same with 83a4e056c7072678, EPS responds to both ford and mazda so during fingerprinting it will only look at Mazda |
Strange, not sure why they're failing to fingerprint then. Those FW versions are in Ford's listed FW versions. |
Yeah, they all come back but it says "Car Unrecognized". |
Which route? All recent mock routes (not many) for 83a4e056c7072678 fingerprint on master. |
elif rx_data[0] >> 4 == ISOTP_FRAME_TYPE.FIRST: | ||
# should work with CAN FD. maybe a check for max 62 bytes from the spec? |
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.
@sheaduncan can you get a route where you cause a query longer than the single frame can hold, so we can see the first and consecutive frame action?
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.
This is with a fresh copy of master and pointing to this Panda Branch (isotp-canfd-support):
09136c309ba9461d/2024-02-07--08-51-50
This is with a fresh copy of master with current Panda master:
09136c309ba9461d/2024-02-07--14-42-56
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.
2 more routes:
Red Panda in Aux port of C3 and USB Power into Main
09136c309ba9461d/2024-02-07--17-31-57
Red Panda in Aux port and OBD2 Power in Main
09136c309ba9461d/2024-02-07--17-36-38
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.
@sheaduncan can you try this branch and upload the rlog? It won't fingerprint. https://github.com/commaai/openpilot/tree/ford-test
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.
@sshane None of these fingerprinted for that branch. Getting Hyundai responses and getting new FW extended versions.
09136c309ba9461d/00000000--7340e1444b
09136c309ba9461d/00000001--578b507ed2
09136c309ba9461d/00000002--b138639015
09136c309ba9461d/00000003--7e95bc66d8
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.
Thanks, that's what I was looking for. I'll check it out when I have time.
Revert "handle can fd first frames" This reverts commit 0dcc048.
What's needed to help get this PR merged? |
status update? |
When we officially support CAN FD Ford (we are looking into a vehicle to purchase), we will put the time into verifying and merging this! |
very much a draft just to make it work, wanted to have a CAN FD user test.