pires / obd-java-api

OBD-II Java API
Apache License 2.0
594 stars 294 forks source link

Error running the VinObdCommand #54

Closed joshiparth1000 closed 8 years ago

joshiparth1000 commented 8 years ago

Hi,

I am getting the following error when running the VinObdCommand. 07-22 19:52:15.905 31668-31723/org.obdclient E/AndroidRuntime﹕ FATAL EXCEPTION: Thread-391 Process: org.obdclient, PID: 31668 com.github.pires.obd.exceptions.NonNumericResponseException: Error reading response: 0140:4902013248471:464731323832362:48353334333135 at com.github.pires.obd.commands.ObdCommand.fillBuffer(ObdCommand.java:168) at com.github.pires.obd.commands.ObdCommand.readResult(ObdCommand.java:150) at com.github.pires.obd.commands.PersistentObdCommand.readResult(PersistentObdCommand.java:45) at com.github.pires.obd.commands.ObdCommand.run(ObdCommand.java:96) at com.github.pires.obd.commands.PersistentObdCommand.run(PersistentObdCommand.java:59) at org.obdclient.OBDStatsFetcher.run(OBDStatsFetcher.java:280) at java.lang.Thread.run(Thread.java:818)

Looks like the fillbuffer method in ObdCommand class doesn't like the ':' charecter.

pires commented 8 years ago

@anti43 to the rescue?

anti43 commented 8 years ago

Do you get those colons for other commands as well? seems like the carriage return is replaced by those here. what commands do you issue before?

joshiparth1000 commented 8 years ago

nope i get it only for VinObdCommand. The only commands i run before are LIneFeedOff, EchoOff and AutoProtocol.

I have run the '09 02' command from other ELM327 clients as well and it seems like ':' is the part of response sent.

Got this explanation from a site(http://tunertools.com/prodimages/obdpros/manuals/obdpro_datasheet.pdf):

CAN systems will display this information in a format shown below

0902 014
0: 49 02 01 59 56 31
1: 4D 53 33 38 32 32 36
2: 32 31 36 36 35 36 33

anti43 commented 8 years ago

Probably the protocol used does allow this. Probably its normal and shall be implemented then (regex remove all non-numeric and line numbers). but then we have to deal with the 014 header aswell

2015-07-23 20:55 GMT+02:00 joshiparth1000 notifications@github.com:

nope i get it only for VinObdCommand. The only commands i run before are LIneFeedOff, EchoOff and AutoProtocol.

I have run the '09 02' command from other ELM327 clients as well and it seems like ':' is the part of response sent.

— Reply to this email directly or view it on GitHub https://github.com/pires/obd-java-api/issues/54#issuecomment-124210763.

nomwerp commented 8 years ago

found this somewhere:

Multi-frame responses may be returned by the vehicle with ISO 15765 and SAE J1939. To make these more readable, the Auto Formatting mode will extract the total data length and print it on one line, then show each line of data with the segment number followed by a colon (‘:’), and then the data bytes.

nomwerp commented 8 years ago

the 014 is the number of bytes (in hex)

joshiparth1000 commented 8 years ago

i made the following change in the fillbuffer method in ObdCommand to fix this issue:

if (!rawData.matches("([0-9A-F])+")) {
            if(!(this instanceof VinObdCommand))
                throw new NonNumericResponseException(rawData);
            else {
                rawData=rawData.substring(4);
                String []frames=rawData.split(":");
                rawData=frames[1].substring(2,frames[1].length()-1)+frames[2].substring(0,frames[2].length()-1)+frames[3];
            }
        }
anti43 commented 8 years ago

sorry, but this code is not acceptable. What if there are more frames or less frames. all kinds of exceptions (IndexOutOfBounds, Nullpointer ..) will appear. Plus you do not count the case where there are NO colons (as in most cases). You have to regex match the response and decide then what to do. Hard-coded count is a no go in 99% of solving programming tasks! Plus the colons only appear in certain kinds of protocols, so you have to take this into account. Plus there are other commands where colons may appear, depending on the protocol.

joshiparth1000 commented 8 years ago

This works only if VinObdCommand has something not in the regular which in this case is ':'. The VIN can only be from 17-20 charecters in length so there will always be 3 frames.

anti43 commented 8 years ago

Thats not a valid argument in the context of interface programming (which this is, an interface to a serial obd2 board). The colons are regular characters in some of the protocol possible, with some of the available options being ON. It is not irregular, its a valid use case which the library may or may not handle. Either the library does not support multi frame responses whit colons (then throw an Exception) or the library properly handles "multi frame responses with colons as delimiter and a header as defined for ISO 15765 and SAE J1939" (the find the specifications for it and write code to handle this properly for all commands). Just putting workarounds here and there and making "it work for special case xy" will lead to a bad library which is not reliable and will utterly fail for many other cases.

You don't even check the rawdata for ":" before you assume that the no-match for "([0-9A-F])+" is caused by your specially formatted response! The device may have responded with anything from "FF FF FF" to "7 FA 56 C0" or "HELLO;TEST" whatever

anti43 commented 8 years ago

@joshiparth1000 is that a volvo S40 ?

joshiparth1000 commented 8 years ago

nope its 8th gen honda civic

anti43 commented 8 years ago

and thats the output of the honda?

0902 014

0: 49 02 01 59 56 31

1: 4D 53 33 38 32 32 36

2: 32 31 36 36 35 36 33

?

joshiparth1000 commented 8 years ago

yes

anti43 commented 8 years ago

what VIN do you get for it?

anti43 commented 8 years ago

See:

https://github.com/anti43/obd-java-api/blob/master/src/main/java/com/github/pires/obd/commands/ObdCommand.java#L165

which is NOT the good solution, too - a "MultiframeObdCommand" shall be implemented for this, however its less error prone using regex replace and it shall work for those VINs.

Again, this is just a quick and dirty workaround and not really acceptable, we need an implementation for multi frame responses in general.

pires commented 8 years ago

@anti43 thank you for your effort here.

pires commented 8 years ago

Any idea about this @valens254?

valens254 commented 8 years ago

and thats the output of the honda?

0902 014

0: 49 02 01 59 56 31

1: 4D 53 33 38 32 32 36

2: 32 31 36 36 35 36 33

?

This is format for CAN (ISO-15765) protocol. It is necessary to add a scan into VinCommand and parse different protocols.

pires commented 8 years ago

@valens254 are you aware of other commands that need differentiated protocol support?

valens254 commented 8 years ago

@pires no, while only VinCommand and TroubleCodesCommand.

pires commented 8 years ago

Fixed by #86 thanks to @valens254.