Debugging OBD II flow switching between queries

I have a OBD II project the uses the sparkfun UART OBD II (https://www.sparkfun.com/products/9555) device, a 1.8" tft (similar to the one sold at by adafruit), a button and an accelerometer. Everything is configured via a file in an SD card.

A bit of warning, the code block is long and I don't like how it is organized (one large loop of reading through the config file), but that part works for the moment.

Here is the larger, graphical 1.8" tft version: https://github.com/stirobot/arduinoModularTFTgauges/blob/master/modularTFTgauge.ino

I also have a smaller more readable version of this project that uses 3 buttons, the OBD II UART device and a serial 7 segment display (https://www.sparkfun.com/products/11442).

Here is the code for that: https://github.com/stirobot/arduinoModularTFTgauges/blob/master/brzclockgauge.ino

Some other notes: -Both sketches use roughly the same OBD II code. I've included the second one so you don't have to wade through the long project code listing of the first version. It has a lot of graphics code and stuff for slogging through the config file (in an inefficient manner...it is like I did the whole thing forgetting to use arrays :astonished:). -The BRZ (also called the FRS/GT86) is a car I own that use a special OBD II CAN header to access the 2101 pid that gives oil temperature. -Even though some of the code applies the header right before getting an oil temp reading, it isn't necessary. Resetting when switching back to standard PIDs is also not necessary.

So, what is my issue... -I have something wrong with the OBD II serial flow of things. I will oftentimes get no data or partial data from some PID calls. And this is dependent on what PID I query for first -I suspect this has something to do with stuff still left in the rxData[] array or maybe with how I initialized the connection to the UART device and make the first call.

There are two versions of getResponse() that are responsible for getting the serial stuff back from the UART device. I've gotten them from others in other forums. What are your thoughts on the two of them?

//from:  https://forum.sparkfun.com/viewtopic.php?f=14&t=32457&start=60 and https://forum.sparkfun.com/viewtopic.php?f=14&t=38253
/*void getResponse(void){
  char c;
 // int start=millis();
  //If nothing is currently available do nothing and break after 3 seconds
  //while(Serial1.available()==0){if(millis()-start>3000){break;}}
  do {
    if (Serial1.available() > 0)
    {
      c = Serial1.read();
      if ((c != '>') && (c != '\r') && (c != '\n')) //Keep these out of our buffer
      {
        rxData[rxIndex++] = c; //Add whatever we receive to the buffer
      }
    }
  } 
  while (c != '>'); //The ELM327 ends its response with this char so when we get it we exit out.
  rxData[rxIndex++] = '\0';  //Converts the array into a string
  Serial.print("rxData(in getResponse): ");
  Serial.println(rxData);
  rxIndex = 0; //Set this to 0 so next time we call the read we get a "clean buffer
}

and

void getResponse(void){
  char obdIn=0;
  int i=0;
  int start=millis();
  //If nothing is currently available do nothing and break after 3 seconds
  while(Serial1.available()==0){if(millis()-start>3000){break;}}
  while(Serial1.available()){
    //check to see if end of line/message
    if (Serial1.peek()=='\r'){
      obdIn=Serial1.read();
      rxData[i]='\0';
      Serial.println(rxData);
      i=0;
    }
    // The prompt is sometimes the only thing recieved so this needs to be taken care of
    else if(Serial1.peek()=='>'){
      obdIn=Serial1.read();
      Serial.write(obdIn);
    }
    // Add next character to string
    else{
      obdIn=Serial1.read();
      rxData[i++]=obdIn;
    }
  }
  Serial.print("rxData(in getResponse): ");
  Serial.println(rxData);
  rxIndex=0;
}

Sorry for the word fort/wall of text. It isn't the most straightforward question and I don't expect a silver bullet answer (though that would be pretty neat). I would like to know what to try next. I'm stumped.

I slightly prefer the second version. The code in the demo in Reply #4 here is similar.

What I don't like about both versions is the use of WHILE which ties up the Arduino waiting for very slow serial data to arrive. And in the case of while(Serial1.available()==0){if(millis()-start>3000){break;}} it is just waiting for up to 3 seconds doing nothing at all. Both should be replaced by non-blocking code.

What I can't do is relate this piece of code to the bigger problem you have. I wonder if the 3 second delay is part of the problem, but that's only because I haven't seen anything else to wonder about.

Edit to add ...

I have now had a look at the shorter of the two codes you linked to. (It would be much more convenient if you just added them as attachments). It's a bit of a dog's breakfast !

You seem extraordinarily attached to the use of the delay() function.

What Arduino are you using? I'm guessing it's an Uno because you use SoftwareSerial. If SoftwareSerial can read the ODB interface I would always start with a program that just writes stuff to the Serial Monitor and only go for another display interface when the technical part of the program works properly.

In any case can you write a short sketch that just requests the different responses from the OBD system and work on that until it behaves properly. Hopefully without needing to use delay() :)

You have a huge amount of code with various conditions in loop(). That makes it very difficult to see at a glance what is supposed to happen. I much prefer it when code is split out into small single-purpose functions so you can concentrate on one piece without being distracted by other stuff. For example I would have a function to read the buttons and save their values and then other functions that would do things depending on those values rather than string everything together in a series of IF statements.

...R

https://github.com/stirobot/arduinoModularTFTgauges/commit/d29fae5a3ed9c86136833c38ec6fd00e4247274e

Works, with the exception of the Coolant PID....which I put in right after all of the initializations and it works there. So, I'm baffled as to why it doesn't work in the display code.

Thanks for the links and all. There really is a lot of room for improvement in stuff like this. Some of the pseudo multi-threading and non-blocking examples are pretty neat pieces of code. (I should have got that CS degree and not that Cognitive Science degree).

The previous code I looked at has about 300 lines of code. That new link seems to have 1400.

Sorry, but I'm too lazy to work my way through all that.

...R