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

Possible bad parsing of DTC. #78

Closed
jbmokuz opened this issue Jun 18, 2019 · 3 comments
Closed

Possible bad parsing of DTC. #78

jbmokuz opened this issue Jun 18, 2019 · 3 comments
Labels

Comments

@jbmokuz
Copy link

jbmokuz commented Jun 18, 2019

Describe the bug
Possible bad parsing of DTC.
Not 100% sure if this is a bug.

Here is an example below

OBD Service: 0->7
ELM rx:'00A' (07)
Status change: ECU detected->Connected
ELM rx:'0:470401180122' (07)
ELM rx:'1:02232610000000' (07)
+DFC: 0401: P0401 Exhaust Gas Recirculation Flow Insufficient Detected
+DFC: 1801: P1801 Signal buzzer activationShort circuit to B+
+DFC: 2202: P2202 NOx Sensor Circuit Low Bank 1
+DFC: 2326: P2326 Ignition Coil I Secondary Circuit
+DFC: 1000: P1000 Injector Circuit/Open Cylinder 9

There seem to be two ways to parse the response (47040118012202232610000000) to service 07.

Number 1
The ELM327 returned bytes of information in total to be 10. If we only take those bytes we would get.
00A (returned bytes are 10)
47 (pending DTC)
04 (Length)
0118(P0118)
0122(P0122)
0223(P0223)
2610(P2610)
000000 (over 10 bytes so padding)

Number 2
How ever if we just read the number of returned bytes (13) and chose to ignore the ELM327 "returns bytes of information in total" (10) and not use byte 2 as a length field (because buffer.length % 4 equals 0) we get
00A (ignored?)
47 (pending DTC)
0401 (P0401)
1801 (P1801)
2202 (P2202)
2326 (P2326)
1000 (P1000)
0000 (Padding?)

To Reproduce
Got to OBD fault codes (in this example we use pending).
Find one where the information bytes returned from the ELM327%2 does not equal buffer.length % 4

Expected behavior
Perhaps use of length field if protocol requires it and or checking the information bytes returned from the ELM327? Again, not fully sure if this is a bug.

AndrOBD Debug log files
Please attach AndrOBD debug log files.

AndrOBD stores log files in a subdirectory on your primary storage device. (com.fr3ts0n.ecu.gui.androbd/log/AndrOBD.log.*)
AndrOBD.log.0.txt

Log detail level
The detail level of the logged debug information can be adjusted in the Settings > Development options > Logging level.

By default the detail level is set to INFO.
Preferred detail level for issue analysis is DEBUG. (Since V1.5.0 this level is called FINE)

Important details on your environment:

Additional context
Other android apps that I have used disagree with with the parsing that AndrOBD is providing (Number 2) and agree with Number 1. I have not completely read all the specs of each protocol the ELM327 supports, so perhaps AndrOBD is parsing this correctly. I can't seem to find something that confirms it should work like parsing method Number 2, so this could be a bug.

@fr3ts0n
Copy link
Owner

fr3ts0n commented Jun 23, 2019

Hello @jbmokuz ,
Thank you for your perfect analysis.

You are absolutely correct.
This is a mis-interpretation of the DFCs which occurs because the overall number of bytes of this DFC response is padded to a even number of bytes.
Androbd currently uses the overall response length to determine if the DFC response contains a implicit number of DFCs, or if it doesn't. Since this response parameter is optional within the standard, it's hard to know up front.

The hard part now is to determine which part of the response is data, and which is padding...
I will take a look if there are any valid DFC's, which have a 00 in the low byte. If there is none I can just cut off the trailing 0's as padding.

I will take your data sample as a new test step to verify the fix.

@fr3ts0n fr3ts0n added the bug label Jun 23, 2019
@fr3ts0n fr3ts0n reopened this Jun 23, 2019
@fr3ts0n
Copy link
Owner

fr3ts0n commented Jun 23, 2019

Sorry, didn't mean to close it yet ... (I pushed a commit stating a fix for this issue)

I have a test version ready for you to verify the fix for this issue:
https://t.me/AndrOBD_dev/216

I use your DFC pattern now in the demo for permanent DFC's

Please let me know if this solves the problem for you.

@jbmokuz
Copy link
Author

jbmokuz commented Jun 24, 2019

Works perfectly, thank you so much!

@jbmokuz jbmokuz closed this as completed Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants