OBD MPGuino gauge

[gone.s2]

I think I've narrowed down almost all my issues to the RPM polling failing every other pass on certain screens. After some investigation it appears the ECU is becoming "lethargic" if it is not polled every so often, and this is causing rpm polling in verifyECUAlive() to timeout, resulting in ECUconnection toggling back and forth (the next polling of RPM succeeds). This is happening for me on screen 3 which are all calculated PIDs, and screen 1 in certain instances. This may be an idiosyncracy of the VW ECU, but it causes numerous problems, most intrusive of which is saving trip data every other tick of loop(). I also think it's why reinit from inside the main loop fails when it otherwise succeeds in setup().

I was wondering if anyone has ever seen this or would have any documentation that might reference communication timeouts. That, or really any good reading on 9141-2 would be appreciated - the last URL I saw in this thread gives me a 404.

One other item I ran into that I wonder if worth investigating. In iso_read_data() there's a delay happening to ensure the program waits a sufficient amount of time before polling the ECU again after a read.

  // we send only one command, so result start at buf[4] Actually, result starts at buf[5], buf[4] is pid requested...
  memcpy(data, buf+5, len);

  delay(ISORequestDelay);    //guarantee 55 ms pause between requests

  return dataSize - 6; // return payload length

Wouldn't it be a little more efficient to use a timeout mechanism here so the program can be doing other things while the delay expires? Something like:

  // we send only one command, so result start at buf[4] Actually, result starts at buf[5], buf[4] is pid requested...
  memcpy(data, buf+5, len);

  static long lastRcv=millis();

  return dataSize - 6; // return payload length

and then in get_pid() something like...

  cmd[0]=0x01;    // ISO cmd 1, get PID
  cmd[1]=pid;
  long nowms=millis();
  while ((nowms - ISORequestDelay) < lastRcv)
  {
    delay(1);
    nowms=millis();
  }
  // send command, length 2
  iso_write_data(cmd, 2);

Lastly, is my understanding of PID caching correct to state that it's really only useful for PIDs that the program will be polling for the fuel calculations and displaying simultaneously (i.e. MAF or VSS)? If so, assuming that no-one would want to display the same PID more than once on a screen, why not just store the ones that are going to be polled each tick in accu_trip() and verifyECUAlive(), i.e. RPM, VSS, MAF, etc. Then add a boolean parameter to get_pid() to signify whether to force ECU polling or just return the cached value. This is what I've tried and it seems to work well.

boolean get_pid(byte pid, boolean noncrit, char *retbuf, long *ret)
{
  <code removed for brevity>
  long tmpVal;
  boolean cache_hit=false;

  <code removed for brevity>

  if(delta_time>1000)
  {
    nbpid_per_second=nbpid;
    nbpid=0;
    getpid_time=time_now;
  }

  if (noncrit)
  {
    // if true return previously read values for display
    switch(pid)
    {
    case ENGINE_RPM:
      // disclaimer: i've modified the program to store the full RPM value in a long
      sprintf_P(retbuf, PSTR("%ld RPM"), rpm);
      cache_hit=true;
      break;

    case MAF_AIR_FLOW:
      long_to_dec_str(maf, decs, 2);
      sprintf_P(retbuf, PSTR("%s g/s"), decs);
      cache_hit=true;
      break;

    case VEHICLE_SPEED:
      tmpVal=vss;
      if(!params.use_metric)
        tmpVal=(tmpVal*1000U)/1609U;
      sprintf_P(retbuf, pctldpcts, tmpVal, params.use_metric?"\003\004":"\006\004");
      cache_hit=true;
      break;
    }
    if (cache_hit=true;)
      return true;
  }

Then, anywhere we want to force ECU polling for fresh data such as in accu_trip() change the call to: get_pid(PID_NAME, false, ....) and make all calls from the display() routine use get_pid(PID_NAME, true, ....)

I don't know if either of these will significantly effect program size, but I do think they'll free up some processor time that was otherwise spent stepping through the pid cache once cell at a time or counting sheep in delay().

I'm very rusty with avr_gcc, and an Arduino newb so I might be making some really erroneous assumptions. Please don't hesitate to correct me - I'm here to learn.

[Magister]

IIRC after a timeout (was it 3s or 5s?) without communication, the ECU shutdown its obd2 connection, but obviously accu_trip() is called more often, so maybe it's a VW thing?

[gone.s2]

Originally Posted by Magister

IIRC after a timeout (was it 3s or 5s?) without communication, the ECU shutdown its obd2 connection, but obviously accu_trip() is called more often, so maybe it's a VW thing?

I've seen that somewhere also (5s) but I know the delay was significantly less than that, possibly less than 1s. Whatever the case, I just tried out the timeout mechanism I posted about and that seems to have resolved the issue (sped up refresh significantly also). So that chunk of code is definitely a candidate for inclusion in trunk IMO.

BTW, all that in my previous post is based on r187. Sorry, I've been working on it for days and never noticed the two new updates until earlier today. I'll try to port to current trunk after I test out reinit.

[gone.s2]

Sorry to spam the topic, but I've got many questions and a poor memory.

What's the reason for the 3 second delay at the beginning of the iso_init() routine(s)? Wouldn't it save a few bytes to do that once in loop(), possibly starting with a short delay at first and then ramping up after a few failed attempts (thinking forward to power management)? LOL, there goes the few bytes it would've saved... but power management is worth it.

Edit: N/M, that's all conditional so it wouldn't save anything. Nevertheless, what's the reason for 3s? I've got it at 300ms and it seems fine.

[eimix]

Originally Posted by spfautsch

Whatever the case, I just tried out the timeout mechanism I posted about and that seems to have resolved the issue (sped up refresh significantly also).

Are you talking about this part of code:

  
while ((nowms - ISORequestDelay) < lastRcv)
  {
    delay(1);
    nowms=millis();
  }

How this changes anything?

And 3s delay before ISO init is something written in specification of that protocol. But as you mentioned that shorter also works, i noticed same "delay ramping" in most of VAG-COM cable software: 0s, 0.5s, 1s ... up to 5s.
This could speed up on ECU that supports it.

But for a while i am unable to test code in real (until last tank becomes empty).

[gone.s2]

Originally Posted by eimix

How this changes anything?

Because there are other program instructions between the last byte received and the first byte sent (i.e. checksumming, formatting data, sending to lcd, handling button presses, etc.) and those instructions take time to execute. IIRC the spec is 55ms between last byte transmitted by ECU to next request byte received. The way it is in trunk at the end of the last byte received the program calls delay(x), which does nothing but wait x milliseconds except possibly interrupt handlers. A timeout mechanism like this lets the iso_read_data() function return so those other tasks can be performed. Then if execution gets back to sending another request to the ECU too quickly, this loop waits until the timeout has expired.

It appears with VW ECUs of this vintage, the faster the better.

Originally Posted by eimix

And 3s delay before ISO init is something written in specification of that protocol. But as you mentioned that shorter also works, i noticed same "delay ramping" in most of VAG-COM cable software: 0s, 0.5s, 1s ... up to 5s.
This could speed up on ECU that supports it.

With standards, it all comes down to wording. I don't have anything to go by, but I would imagine there's language in there along the lines of "as much as" or "no more than" or something to that effect. Probably the best way to handle reinit as quickly as possible would be to add a self-learning routine, but that would be a lot of program space for little realized utility.

Originally Posted by eimix

But for a while i am unable to test code in real (until last tank becomes empty).

I'm sure that deters a lot of ppl from testing. Unfortunately it seems there's an awful lot of code in there than could stand optimization like this. Building an application for a microcontroller that has to handle time-sensitive communication AND user input seamlessly is always difficult. The methods used differ wildly from the way PC applications are written, where CPU time is abundant.

What build version are you currently using, and have you ever gotten reinit to work? I almost had it working last night, but something is failing around the end where supported pids are queried.

[eimix]

Originally Posted by spfautsch

...checksumming, formatting data, sending to lcd, handling button presses, etc...

make sense - i will try to put in next release.

Originally Posted by spfautsch

With standards, it all comes down to wording. I don't have anything to go by, but I would imagine there's language in there along the lines of "as much as" or "no more than" or something to that effect. Probably the best way to handle reinit as quickly as possible would be to add a self-learning routine, but that would be a lot of program space for little realized utility.

this will not take much space, i will attemt to make this too, next week.

Originally Posted by spfautsch

What build version are you currently using, and have you ever gotten reinit to work? I almost had it working last night, but something is failing around the end where supported pids are queried.

In my car (VW jetta 2001 1.8t) reinit works normaly, i do not know why do u get problems
I use 190 (it will be 190 some day)
and if you get errors while requesting first pid (about supported pids) is your "lastRcv" initialized? (dumb question, but i have to ask it)

[Bix]

Has anyone had a look at the vishay SI9241? http://www.datasheetcatalog.org/data...shay/70013.pdf

Would this chip be compatible with this project, or perhaps some slight mod to get it working?

[gone.s2]

Originally Posted by eimix

In my car (VW jetta 2001 1.8t) reinit works normaly, i do not know why do u get problems
I use 190 (it will be 190 some day)
and if you get errors while requesting first pid (about supported pids) is your "lastRcv" initialized? (dumb question, but i have to ask it)

Thanks, I remembered you had problems with it from page 56 and just curious what resolved it for you.

I've only tested with 187, reinit will not work at all for me. I'll try updating my diesel changes into a newer version.

lastRcv is initialized, no idea what could be wrong but I've made many changes like disabling security screen, so it could be one of the other changes I've made causing me problems now.

[gone.s2]

Nevermind about the reinit issues. I suspected the same lethargy problem I was seeing previously was causing the call to get_pid(PID_SUPPORT00...) to fail, and that proved to be the case. Adding a retry loop in check_supported_pids() to this call resolved my issue.

void check_supported_pids(void)
{
  char str[STRLEN];

#ifdef DEBUG
  pid01to20_support=0xBE1FA812;
#else
  byte i=0; << ADDED
  while (i<3 && !pid01to20_support) << ADDED
  { << ADDED
    i++; << ADDED
    pid01to20_support  = (get_pid(PID_SUPPORT00, false, str, &tempLong)) ? tempLong : 0;
  } << ADDED
#endif

Also, my statement about 300ms working for time between init sequences doesn't seem to stand in re-init.

EDIT: Actually, this works better because the pid01to20_support doesn't need to be zeroed out.

  byte i=0;
  do
  {
    i++;
    pid01to20_support  = (get_pid(PID_SUPPORT00, false, str, &tempLong)) ? tempLong : 0;
  } while (i<3 && !pid01to20_support);