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

Questions and ISO 14230-2 KWP development #11

Closed
mjs513 opened this issue Jul 12, 2018 · 49 comments · Fixed by #12
Closed

Questions and ISO 14230-2 KWP development #11

mjs513 opened this issue Jul 12, 2018 · 49 comments · Fixed by #12
Labels

Comments

@mjs513
Copy link

mjs513 commented Jul 12, 2018

First I have to tell your library makes it easy. So now the questions. I don't have one of the ICs readily available (they are order be about a week to get) but I found another circuit that is floating around out there on the web, see attached:
obdii_avr

In looking through the lib I know that it can't really be used directly to drive the circuit. But if I wanted to use this circuit instead of the chip I would have to change several functions. Any suggestions?

BTW I am using the inti_kwp_fast fork.

Thanks
Mike

@iwanders
Copy link
Owner

But if I wanted to use this circuit instead of the chip I would have to change several functions.

You shouldn't have to change anything, for the library it doesn't matter what type of transceiver chip or circuit is used. As long as the Tx and Rx pins both support digitalWrite and can function as a Serial port at 10400 baud it should all just work.

That being said, the schematic you have it a bit problematic, since the K-line is a 12v data line, it is connected to the Rx pin on your microcontroller through just a 10k resistor. That means that there's still 12v connected to your microcontroller, (although at low current,) it will likely exceed the microcontroller's maximum rating. At the very minimum I would use a voltage divider to drop the K-line from 12v to your microcontrollers level. The bus is a low speed one, so you can get away with a simple voltage divider to drop the voltage, I also used a voltage divider to record the logic analyzer traces.

BTW I am using the inti_kwp_fast fork.

Okay, remember that that is untested code, see #5. The branch contains quite some debugging statements intended to diagnose problem, please report back on your findings and output from those. If it works I'd love to hear about it because I can then merge it into the master branch. If it doesn't work, please also get back with your findings, we may be able to fix the issues you encounter by inspecting the code and output. What microcontroller are you intending to use?

@mjs513
Copy link
Author

mjs513 commented Jul 13, 2018

Realy appreciate you getting back to me on this. Anyway,

You shouldn't have to change anything, for the library it doesn't matter what type of transceiver chip or circuit is used.

Was looking at the code a little closer after I posted actually didn't see anything I needed to change so not planning on any changes.

As long as the Tx and Rx pins both support digitalWrite and can function as a Serial port at 10400 baud it should all just work.

I using a Teensy 3.2 so it should work based on the read me. The only change I made was using init to Serial3 and RXpin = 7 with TXpin = 8;

the schematic you have it a bit problematic, since the K-line is a 12v data line, it is connected to the Rx pin on your microcontroller through just a 10k resistor.

Thanks for the explanation - not very good with circuits as you can tell, that's why I posted the schematic :) I will try a simple voltage divider and give it a try tomorrow. Its a little late here now to try it. Do I still need the rest of the circuit? Actually probably not. Just looked at the other circuit I found:
capture

Okay, remember that that is untested code, see #5. The branch contains quite some debugging statements intended to diagnose problem, please report back on your findings and output from those.

I will definitely get back to you on the results of my testing. Like I said I have one of chips coming and should be here next week so at least I am using the same circuit as you have. If it works with a simple voltage divider I will let you know as well.

Thanks for your help.
Mike

@iwanders
Copy link
Owner

Do I still need the rest of the circuit? Actually probably not. Just looked at the other circuit I found

Looks much better, that's a voltage divider that drops 12v to 4.95v. I'd make it drop to 3.3 such that you don't risk exceeding the 5v input tolerance on the pin if there's some ripple on the line because your car alternator produces above 12v, just to be sure. But yeah, I think this should work. Look forward to hearing about your findings.

@mjs513
Copy link
Author

mjs513 commented Jul 14, 2018

I tried the circuit this morning but no luck. The following is what showed on the serial monitor:

Looping
Before 25 ms / 25 ms startup.
Enable port.
w: 193
w: 51
w: 241
w: 129
w: 102
Timeout on reading bytes.
init_success:0

Going to do two things this weekend:(1) rewire the circuit and (2) bring out my multimeter with me to the car to see what I am getting when connected. I read somewhere on line that the timing on startup is 25ms/50ms as opposed to the 25/25 that you have. I tried that too but same results. If I can't get it working I will have the SN65HVDA100 on Wednesday. To be honest, would rather used that rather than the voltage divider, but I can't resist.

The reason I am using init_kwp_fast is that is the protocol that my ODBlink/mx indicated. From what I was reading that seems to be what my car is using (Hyundai Sonata 2006).

I did find another circuit on line that I thought you might find interesting
obd2_iso9141-2_cable

@iwanders
Copy link
Owner

The reason I am using init_kwp_fast is that is the protocol that my ODBlink/mx indicated. From what I was reading that seems to be what my car is using (Hyundai Sonata 2006).

Two quick comments, please also try the normal initialisation, some cars respond to both initialisations. Also, I don't quite remember of the top of my head if the actual protocol for KWP is the same as for 9141-2, we may have to do some more work.

Secondly, because you have a working OBDlink on hand, it may be helpful to test the Rx side of your circuitry by trying to listen to the working OBD reader you have while it does communication with the bus. Not only will it help verify the Rx side of your circuit, but it will also provide us with information on how we should be communicating with the bus. A small program that just blocks on Serial3.available() and and then print a timestamp and the byte itself would be helpful.

Testing the Tx side of your circuit should be fairly straightforward, just check if the K-line goes low if you set the teensy pin low.

@mjs513
Copy link
Author

mjs513 commented Jul 14, 2018

Thanks will work on it and let you know how it goes

@mjs513
Copy link
Author

mjs513 commented Jul 14, 2018

Just as a quick update:

  1. I redid the circuit and checked the voltages when key off, key on (not acc) and engine on. Voltages at RX and TX look good, think the max voltage was with engine off and that read 5.03 at rx and just about 0 at tx (wasn't transmitting though).
  2. I reran with kwp_fast and kwp_slow (5baud) and nothing, would not connect.
  3. I did a benchtest of driving the tx low and it looks like that is working ok.

This after noon I will test with the ODBLink connected and see if I get anything with just serial reads.

UPDATE: Using a splitter odb cable I attached the teensy to one side and the odblink to the other and just using serial3.read() odblink kept losing connection with the Arduino attached - going to try on my windows 7 laptop and just try looking at the port but I have the feeling the same thing is going to happen.

Oh, btw you might find this instructable interesting: https://www.instructables.com/id/Low-Cost-OBD2-Communications-on-K-line-ISO-9141-2-/

@iwanders
Copy link
Owner

Using a splitter odb cable I attached the teensy to one side and the odblink to the other and just using serial3.read() odblink kept losing connection with the Arduino attached - going to try on my windows 7 laptop and just try looking at the port but I have the feeling the same thing is going to happen.

Using a different OS is likely not going to make a difference. If you are unable to sniff the data, it sounds like the interface circuitry doesn't work as it should. Maybe it always pulls the bus low? You may have to remove the 510 pullup to 12v from your circuit, if another device on the K-line provides the pullup. Though I doubt this would be as impactful as losing the connection. May be best / easiest to wait for the transceiver chip, that should rule out the circuitry issues.

@mjs513
Copy link
Author

mjs513 commented Jul 14, 2018

I agree. Estimated date is Thursday - just hate waiting :)

UPDATE: Ok. For the heck of it since I had finished putting the third circuit together I finally got it to read the k-line. While I don't think the circuit is right it finally not only wrote the init but it got a message back. Most of the time it was all zeros since init was not successful but periodically it would return:

246, 0, 0, 0, 0, 0, 0, 0.

just some added info just in case.

@mjs513
Copy link
Author

mjs513 commented Jul 16, 2018

Hi. Nothing new yet. But I just found your thread on the Teensy forum: https://forum.pjrc.com/threads/24549-OBD-K-Line-Buffer-Circuit/page2.

@mjs513
Copy link
Author

mjs513 commented Jul 28, 2018

Ok. Back again. I finally got the sn65 chip. Wired it up and gave it a test and no go. Pretty sure the wiring is correct. Did a bench test of voltages.

  1. From kline to rx its was 4.5V
  2. Cycling the TX line high and low it went from 11.9v to 8.6v
    Anything I can check?

@iwanders
Copy link
Owner

First thoughts, did you ensure the EN pin on the Sn65 is pulled high and that nwake is connected to ground?

About 1. and 2.; I'm not sure how you measured this, always measure between measuring point and ground. K line should idle at 12v and when active is should go to ground. Setting Tx to high should make the Kline ground, I thought the sn65 had some timeout mechanism in case the mcu locks up or something, it may not hold it at ground indefinitely.

If K-line does not idle at 12v, you need the pullup (R1 in the readme). If it doesn't go to ground it may already be pulled up quite a lot, you may want to remove R1 if you had it. Though I'm pretty sure the Sn65 chip can sink a sufficient amount, even with two pullups.

It can also be that the KWP init is not working (most likely the issue), in which case I'd try sniff that other OBD reader you have. Just connect the K-line (add no resistors) to the Sn65 and hook the Sn65 up to the serial port of your teensy. Initialiser the serial port at a baudrate of 10400 and try to listen in. I expect that to work 100%, since the SN65 is only acting as a level converter then.

@mjs513
Copy link
Author

mjs513 commented Jul 28, 2018

After your last set of comments rechecked the wiring and had one issue. Also forgot to change something back in the sketch. Anyway I did get a response back over the K-line after the initialization message was sent:

Looping
Before 25 ms / 25 ms startup.
Enable port.
w: 193
w: 51
w: 241
w: 129
w: 102
R: 131 241 17 193 239 143 196 0 
init_success:0

Haven't checked the message yet. Have you seen this before?

EDIT: Question. If I look at ISO1423-2 the fast initialization response message format is

image
Your read8() function is looking at element 5 which would correspond to one of the key bytes. Shouldn't it be buffer[4] instead of buffer[5]? Then it would be success since 193 is c1.

@iwanders
Copy link
Owner

The bytes after R: are the raw bytes as read.

R: 131 241 17 193 239 143 196 0 

Checksum is addition of all bytes before the checksum. We expected one byte too many, it is correct if we discard the last 0 byte:

(131 + 241 + 17 + 193 + 239 + 143) & 0xFF = 196

From this webpage I get that the length byte may be omitted in case the format byte specifies the length. So in that care the length byte from your table drops out, and we would indeed see 193 on position 4.

So, that's pretty exciting, the physical layer is confirmed good and the KWP init may even be working.

We need to fix the incorrect expected response length, either by changing the 7 into a 6 here or by rebasing the branch and using the variable length response method I added for the DTC support. Alternatively we could write a KWP request that actually handles the length byte correctly and reads that from the first byte, that would be the ideal situation. Shouldn't be too hard to do it properly, I'll likely take a stab at it.

Do we know whether the protocol after the initialisation is identical to 9141-2? Or are the requests a different format than the obd ones? Best way to figure this out is probably to listen in on the working OBD device you have. Or just assuming it's identical and giving it a go.

@iwanders
Copy link
Owner

Please pull the latest version of the kwp2000_fast_init branch. In 6be33b3 I've done the following:

  • Rebased on the latest commit.
  • Rename init_kwp_fast to initKWP, so you'll have to update your sketch.
  • Introduce requestKWP(void*, uint8_t request_len) to read the correct number of response bytes as specified by the first byte of the response. This method is only used from initKWP at the moment. If the protocol is different than 9141-2 we will need to use this everywhere.
  • Ensured that the initKWP method checks the correct byte for 0xC1.

Since I don't have the hardware to test this, could you give it a go and report back?

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Best way to figure this out is probably to listen in on the working OBD device you have. Or just assuming it's identical and giving it a go.

Decided to just give it a go until we get past initialization. Results were the same for some reason it still shows initialization success: 0 and just keeps looping. Response is the same. Going to add some more debug statements but just wanted to post this for now.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Update; Added some debugging statements to your readKWP function and think the problem might be in the checksum:

Looping
Before 25 ms / 25 ms startup.
Enable port.
w: 193
w: 51
w: 241
w: 129
w: 102
RfmtB: 131
msg_len: 3
remainder: 6
ret_len: 7

R: 131 241 17 193 239 143 196 0 
Cks: 136 0
init_success:0

@iwanders
Copy link
Owner

I'm not sure how your sketch came up with 136, but there was indeed an error in it. I included the checksum on the wire in the checksum calculation. I copied the print statements you added and fixed it in 8c698a4, could you give that a go?

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

This is what I used to get the checksum

    if (this->serial->readBytes(&(this->buffer[1]), remainder)){
        OBD9141print("R: ");
        for (uint8_t i=0; i< (ret_len+1); i++){
            OBD9141print(this->buffer[i]);OBD9141print(" ");
        };OBD9141println();
        
        OBD9141print("Cks: ");
        OBD9141print(checksum(&(this->buffer[0]), ret_len));
        OBD9141print(" "); OBD9141print(this->buffer[ret_len]);
        OBD9141println();
        
        if (this->checksum(&(this->buffer[0]), ret_len) == this->buffer[ret_len])
        {
          return ret_len; // have data; return whether it is valid.
        }
    } else {
        OBD9141println("Timeout on reading bytes.");
        return 0; // failed getting data.
    }

I will give the update a go and let you know.

@iwanders
Copy link
Owner

Yeah, give it a go, the mistake was here:

if (this->checksum(&(this->buffer[0]), ret_len) == this->buffer[ret_len])

That should've been

if (this->checksum(&(this->buffer[0]), ret_len - 1) == this->buffer[ret_len])

Otherwise we include the checksum in the checksum calculation.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Think you may have to do the same with the buffer[ret_len] . buf cs is coming back as 0 while the checksum is 196:

Looping
Before 25 ms / 25 ms startup.
Enable port.
w: 193
w: 51
w: 241
w: 129
w: 102
Rem: 6
ret_len: 7
R: 131 241 17 193 239 143 196 0 
calc cs: 196
buf cs: 0
init_success:0

EDIT: Have to go out in a little while. So any changes I will test when I get back.

@iwanders
Copy link
Owner

You are completely right, I've pushed a fix for that.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Ok. Made the fix, it passed initialization but messages just timed out. Checking another this: https://www.instructables.com/id/Low-Cost-OBD2-Communications-on-K-line-ISO-9141-2-/ I noticed that the request header to get a PID had to change if using 14230. I made the change and immediately got results;

Looping
Before 25 ms / 25 ms startup.
Enable port.
w: 193
w: 51
w: 241
w: 129
w: 102
Rem: 6
ret_len: 7
R: 131 241 17 193 239 143 196 0 
calc cs: 196
buf cs: 0
init_success:1
w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
R: 131 241 17 65 17 36 251 
Result 0x11 (throttle): 36
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
R: 132 241 17 65 12 12 76 43 
Result 0x0C (RPM): 787
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244
R: 131 241 17 65 13 0 211 
Result 0x0D (speed): 0

In the getPID function the message had to be:

//uint8_t message[5] = {0x68, 0x6A, 0xF1, mode, pid}; //For ISO 9141-2 
   uint8_t message[5] = {0xc2, 0x33, 0xf1, mode, pid};  For ISO 14230

Not sure how you want to handle making the difference - a different function or maybe a define for 9140 vs 14230?

@iwanders
Copy link
Owner

iwanders commented Jul 29, 2018

Awesome, good find! Amazing that it's all coming together now!

Not sure how you want to handle making the difference - a different function or maybe a define for 9140 vs 14230?

Lets store a boolean in the class that gets set in both init methods. Then in the request method we can select the appropriate header. I see the header we transmit also has the length of the payload in the first byte. Lets see if we can generalize that with the handshake. I'll whip up something in a bit.

Benefit of a boolean over a define is that this way one can write a sketch that support both protocols, which is of course advantageous if we are to try this on a car we don't know what protocol to speak to.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Benefit of a boolean over a define is that this way one can write a sketch that support both protocols, which is of course advantageous if we are to try this on a car we don't know what protocol to speak to.

There you go. Let me know when its ready and I will give another test.

@iwanders
Copy link
Owner

Shame the architecture of this class isn't as pretty for multiple protocols.I didn't really write it with that in mind, so adding this KWP support is a bit hacky since it requires a different header. I chose to rename the request method to request9141 and use the original request method to dispatch based on whether or not we have a use_kwp_ that's true. It didn't break the API and it's only one extra conditional.

Did the same in the variable length one, modify the request and call the requestKWP method. This means that all functionality that was supported (DTC's as well) is expected to work. I considered writing into the void*, but people may have assumed that that was unmodified in their sketches. So I just make a copy of the header.

Please try the latest, I've also added a readerKWP example that should work out of the box now.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Works part way. Getting data but results not printed.

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 39 254 0 0 1 0 245 14 1 0 0 0 
calc cs: 196
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 16 124 95 0 
calc cs: 95
buf cs: 95
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 39 254 0 0 1 0 8 17 1 0 0 0 
calc cs: 218
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 16 124 95 0 
calc cs: 95
buf cs: 95
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

Think where ever you use the message construct uint8_t message[5] = {0x68, 0x6A, 0xF1, mode, pid}; It needs to be changed to:

if(use_kwp_) {
    uint8_t message[5] = {0xc2, 0x33, 0xf1, mode, pid};
} else {
   uint8_t message[5] = {0x68, 0x6A, 0xF1, mode, pid}; 
}

@iwanders
Copy link
Owner

If you compare the w: sequences from your recording with those of your previous comment you see that the request is actually not the problem, the bytes are correct. However I included the checksum byte in the length, so by return length check didn't pass. I've tried to correct this in the latest commit. (3505bce)

I spotted another issue in

R: 211 194 51 241 1 17 248 131 241 17 65 17 39 254 0 0 1 0 8 17 1 0 0 0 

If we failed on the reading there was no delay between the requests and we probably went out of sync here. I've added an extra delay to compensate for this.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Ok. Just tried the latest. Worked sometimes - maybe something still out of sync. Most of the time just got the RPM using the example sketch. Here's part of the run:

Before 25 ms / 25 ms startup.
Enable port.
w: 193
w: 51
w: 241
w: 129
w: 102
Rem: 6
ret_len: 7
R: 131 241 17 193 239 143 196 0 
calc cs: 196
buf cs: 196
init_success:1
w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 52
ret_len: 53
R: 241 1 12 243 131 241 17 65 17 34 249 0 0 0 0 0 1 0 125 22 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 194 1 0 0 0 0 0 
calc cs: 58
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 34 249 0 0 1 0 71 24 0 0 0 0 
calc cs: 21
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244
Rem: 7
ret_len: 8
R: 132 241 17 65 12 10 134 99 0 
calc cs: 99
buf cs: 99

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 6
ret_len: 7
R: 131 241 17 65 17 34 249 0 
calc cs: 249
buf cs: 249
Result 0x11 (throttle): 34
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244
Rem: 4
ret_len: 5
R: 1 13 244 132 241 0 
calc cs: 134
buf cs: 241

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 5
ret_len: 6
R: 194 51 241 1 17 248 0 
calc cs: 248
buf cs: 248
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 6
ret_len: 7
R: 131 241 17 65 17 34 249 0 
calc cs: 249
buf cs: 249
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 34 249 0 0 1 0 102 29 0 0 0 0 
calc cs: 57
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 10 126 91 0 
calc cs: 91
buf cs: 91
Result 0x0C (RPM): 671
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 34 249 0 0 1 0 121 31 0 0 0 0 
calc cs: 78
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 10 150 115 0 
calc cs: 115
buf cs: 115
Result 0x0C (RPM): 677
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 34 249 0 0 1 0 140 33 0 0 0 0 
calc cs: 99
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 10 105 70 0 
calc cs: 70
buf cs: 70
Result 0x0C (RPM): 666
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 34 249 0 0 1 0 160 35 0 0 0 0 
calc cs: 121
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 10 98 63 0 
calc cs: 63
buf cs: 63
Result 0x0C (RPM): 664
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 22
ret_len: 23
R: 211 194 51 241 1 17 248 131 241 17 65 17 34 249 0 0 1 0 179 37 0 0 0 0 
calc cs: 142
buf cs: 0
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 10 112 77 0 
calc cs: 77
buf cs: 77
Result 0x0C (RPM): 668

@iwanders
Copy link
Owner

Ok, I think this issue is timing related. I've made some small changes, and added a delay to the readerKWP sketch.

However, I think it's the most likely that you'll have to tune the OBD9141_WAIT_FOR_REQUEST_ANSWER_TIMEOUT parameter in the header file, you likely need to increase it. Or, thinking about it, increase the time between each request, try that first, the readme specifies that the time between each request is up to the user. From what I can see in the data is that we time out on the first request, then start the second request and while we are doing that the answer from the ECU comes in, which we discard as the perceived echo, then we write the request to the bus, which we read as a response. Can you try increasing the said define from the header file and the delay between requests?

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018 via email

@iwanders
Copy link
Owner

Funny though, in original working version where I juse changed the mesage
heaDer we didn't have that problem.

Yeah, that's kinda peculiar I agree... You could try to go back to that version and see if that really worked with the example test? Or just copy the init method and change the header, see if that works... I feel like the current version should be better, but maybe it isn't? :O If just the current init and header change solves all the issues we can go down that route? Thing is that I think we need to populate the first header byte with the length, and the old header didn't do that did it?

@iwanders
Copy link
Owner

Maybe my assumption is incorrect? And we shouldn't specify the correct length in the request header?

Can you change this line to 0xc2? That's the only change we made to that byte since changing the header. If that works we'll just put it at that with a fixed value.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

You mean just make rbuf[0] = 0xc2 ?

@iwanders
Copy link
Owner

Yes, in the old requests we sent that value was fixed. I assumed that it should've been just like the response where the lower 5 bits of that byte represent the length of the data. But in our previous working requests that value was always 0xc2 instead of also encoding the length.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Ok. Glad you gave me a couple of things to try.

  1. Just changing rbuf[0] to 0xc2 did not work - got the same results as before.

  2. Leave rbuf[0] = 0xC2 and changing to 30+15ms on the define
    #define OBD9141_WAIT_FOR_REQUEST_ANSWER_TIMEOUT (30 + 15)

  3. Leaving the define the same I changed rbuf[0] back to rbuf[0] = (0b11<<6) | (request_len - 3); that seemed to work better - I think.

So I think the solution is just to change the timeout and it will work. Looks like just some cleanup on the additional debug prints and your good to go to update the main branch :) Probably should let everyone know that on the forum that you updated the lib and it was also tested on a T3.5 as well


w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 6
ret_len: 7
R: 131 241 17 65 17 33 248 0 
calc cs: 248
buf cs: 248
Result 0x11 (throttle): 33
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 9 209 173 0 
calc cs: 173
buf cs: 173
Result 0x0C (RPM): 628
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244
Rem: 6
ret_len: 7
R: 131 241 17 65 13 0 211 0 
calc cs: 211
buf cs: 211
Result 0x0D (speed): 0

w: 194
w: 51
w: 241
w: 1
w: 17
w: 248
Rem: 6
ret_len: 7
R: 131 241 17 65 17 33 248 0 
calc cs: 248
buf cs: 248
Result 0x11 (throttle): 33
w: 194
w: 51
w: 241
w: 1
w: 12
w: 243
Rem: 7
ret_len: 8
R: 132 241 17 65 12 9 185 149 0 
calc cs: 149
buf cs: 149
Result 0x0C (RPM): 622
w: 194
w: 51
w: 241
w: 1
w: 13
w: 244
Rem: 6
ret_len: 7
R: 131 241 17 65 13 0 211 0 
calc cs: 211
buf cs: 211
Result 0x0D (speed): 0

@iwanders
Copy link
Owner

Cool!

  1. Just changing rbuf[0] to 0xc2 did not work - got the same results as before

Interesting, that tells us that the ECU doesn't care about the first byte, and they always accept a variable length, instead of reading the value from the first byte. I'll leave the rbuf[0] as is then.

#define OBD9141_WAIT_FOR_REQUEST_ANSWER_TIMEOUT (30 + 15)

I'll update that for both protocols, and make it 20 for good measure. Increasing it doesn't have any adverse effects as we only block in readBytes up to the point where we have received all our bytes. So for normal operation this timeout can be pretty large without problems.

I'll update the branch, hopefully the last time.

@iwanders
Copy link
Owner

Disabled the debug prints (they can be enabled again with the define in the header file) and changed the timeout to 30 + 20. Let me know if this works out of the box now.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Ok. Works like a charm:

Result 0x11 (throttle): 37
Result 0x0C (RPM): 1007
Result 0x0D (speed): 0

Result 0x11 (throttle): 37
Result 0x0C (RPM): 985
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 946
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 949
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 924
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 926
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 920
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 908
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 896
Result 0x0D (speed): 0

Result 0x11 (throttle): 37
Result 0x0C (RPM): 902
Result 0x0D (speed): 0

Think you need to test it on a car that just uses 9140 and not KWP?

@iwanders
Copy link
Owner

Nice! 🎉

Think you need to test it on a car that just uses 9140 and not KWP?

Probably the best to do that yes, given that we touched some of that functionality as well. I'll be sure to test it before merging this. Could be a few weeks, my hardware is at a friend at the moment. I'll also update the readme and point towards this issue when I get around to it.

@mjs513
Copy link
Author

mjs513 commented Jul 29, 2018

Thanks for all the help in getting this working so quickly. Will be a nice addition to your library :)

BTW going to give the display sketch a try when I get a chance. I will go ahead and close this and if something else pops up will let you know.

Mike

@mjs513 mjs513 closed this as completed Jul 29, 2018
@mjs513
Copy link
Author

mjs513 commented Jul 30, 2018

Just a quick note - I ran the display example and it worked out of the box once I changed the obd.init to odb.initKWP no problem.

@iwanders
Copy link
Owner

Just a quick note - I ran the display example and it worked out of the box once I changed the obd.init to odb.initKWP no problem.

Great! Thanks for letting me know everything is working smoothly now.

@iwanders iwanders changed the title Not Really Issue But Questions Questions and ISO 14230-2 KWP development Aug 1, 2018
@zakirsheik
Copy link

Hi,

I am trying to connect to my vehicle network with Arduino and a OBD interface board with TJA1027 transceiver. I tried to run the readerKWP.ino from the examples and the initialization with ECU is failing
My Vehicle uses KWPFAST Protocol, any idea on how to resolve this issue ?

PS : I took the code from master branch.

@iwanders
Copy link
Owner

iwanders commented Jan 7, 2019

@zakirsheik , the master branch is the right branch, all other branches are stale feature branches that have been merged.

Try enabling this define and check if there is even any response coming back from the ECU, that should provide some information as to where the problem lies. In the ideal case you'd sniff communication (with a logic analyzer, or serial port on the transceiver chip) from an OBD reader that is working on your car and compare that to the data from your own PCB and your car.

Please don't use this (old) issue for extended discussion, make a new issue if you can't figure it out, I'm not sure I'll be able to help, but others may see the issue if it's unclosed and chime in.

@TeensyDucM
Copy link

hi iwanders,

thank's for your library, i want to know if it's compatible with iso 14230-4 ( KWP 2000)?

A lot of software uses an ELM because it makes it possible to get rid of all headers, checksum. we just do make the request. does it possible with it. Thank's for your answer

@iwanders
Copy link
Owner

@TeensyDucM , I'm not sure how different 14230-4 and 14230-2 are. Support for normal KWP and 14230-2 was added under #12 and this issue, I'm not sure how 14230-4 differs from this unfortunately.

@TeensyDucM
Copy link

TeensyDucM commented Nov 26, 2019 via email

@Ashok12698
Copy link

Ok. Works like a charm:

Result 0x11 (throttle): 37
Result 0x0C (RPM): 1007
Result 0x0D (speed): 0

Result 0x11 (throttle): 37
Result 0x0C (RPM): 985
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 946
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 949
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 924
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 926
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 920
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 908
Result 0x0D (speed): 0

Result 0x11 (throttle): 36
Result 0x0C (RPM): 896
Result 0x0D (speed): 0

Result 0x11 (throttle): 37
Result 0x0C (RPM): 902
Result 0x0D (speed): 0

Think you need to test it on a car that just uses 9140 and not KWP?

Hi @mjs513 Great it works for you. Could you please let me know your working configuration like....

  1. What message request you sent?
  2. Response log R:?
  3. Checksum: ?
  4. msg_len when it initiates?
  5. what is your buffer[0]=?
  6. Ddi you use requestKWP( ) or else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants