NeoHWSerial/interrupts

Hello,
can I please for your help?

I am reading GPS and OBD data, on Mega2560 Pro board. I am using NeoGPS and ArduinoOBD libraries. If I read data just from one device OBD or GPS, it works great. When I read data from both of them, becomes reading slow. For example, GPS speed is no longer flowing but takes more than a second to refresh. I got advice to use interrupt. I spent hours for searching examples, how to use NeoHWSerial, but unsuccessful. Can someone please send me working code/library which uses interrupts and whether HWSerial? If i need both of them of course.

Sorry but I don't understand NeoGPS and NeoHWSerial examples. Too much sources and examples, I don't now which one to use and what all i need to (re)configure.

P.S. If someone know why is happening written above and how to solve/fix it, please let me know.

multiair_12_GPS_2_NeoGPS.ino (2.92 KB)

The problem you has nothing to do with interrupts but with hardware configuration. If you use the libraries as provided on github you have configured both the GPS and the OBD adapter to be on Serial1. That cannot work. I guess if you have sometimes success with it that you have connected both devices to the same serial interface. Please post a wiring diagram of your setup.

pylon:
you have configured both the GPS and the OBD adapter to be on Serial1. That cannot work.

+1

Move one of them to Serial2 or Serial3.

Thank you for reply!

Wiring "diagram":
Freematics is connected to Serial (D0,D1)
GPS is connected to Serial1 (D18,D19).
mega2560 PINOUT

I got data from OBD and GPS, but they are not flowing/continuously. Otherwise I think, it wouldn't show any data?

I have changed in OBD library:

  • OBD2UART.h

#define OBDUART Serial1 -> #define OBDUART Serial

  • OBD2UART.cpp

extern HardwareSerial Serial1; -> extern HardwareSerial Serial;

Freematics is connected to Serial (D0,D1)
GPS is connected to Serial1 (D18,D19).

Why don't you connect the Freematics to Serial2 or Serial3? You will have to disconnect D0 to upload new sketches.

The problem is that sendCommand "blocks" while it waits for a reply. But the GPS device continues to send characters during that time. Eventually, the Arduino loses some of the GPS data. This prevents the Arduino from receiving complete, error-free information.

One solution is to use NeoHWSerial to process the characters during an interrupt. This is not affected by the OBD sendCommand routine.

The NeoGPS example NMEA_isr.ino shows how this is done. In your case, you must replace all uses of Serial with NeoSerial, and all uses of Serial1 with NeoSerial1. You cannot have any code in your sketch or in a library that uses Serial at the same time as NeoSerial.

Include NeoHWSerial.h at the top of your sketch:

#include <NMEAGPS.h>
#include <NeoHWSerial.h>
#include <OBD2UART.h>
#include <LiquidCrystal_I2C.h>
#include <avr/sleep.h>

Define the gpsPort to use NeoSerial1:

#define gpsPort NeoSerial1

Attach an interrupt function to NeoSerial1:

void setup() 
{
  gpsPort.begin(115200);
  gpsPort.attachInterrupt( GPSisr );

Make that interrupt function call NeoGPS to handle each character:

void GPSisr( uint8_t c )
{
  gps.handle( c );

}

Modify your GPS() routine to call gps.available(), but DON'T pass in the gpsPort:

void GPS()
{
  //  Print new fixes when they arrive
  if (gps.available()) {     <--- no arguments!
    fix = gps.read();
    displayGPS();
  }
}

And modify OBD2UART.h to use NeoSerial instead of Serial (NeoSerial2 would be better):

#include <NeoHWSerial.h>
#define OBDUART NeoSerial

That should do it!

Cheers,
/dev

-dev:
Why don't you connect the Freematics to Serial2 or Serial3? You will have to disconnect D0 to upload new sketches.

Thank you for your explanation!
The reason is that the OBD Freematics adapter has too short wires. It's too much distance is between VIN and Serial2/3.

I have replace the code and works, but I don't notice improvements, better working.

The problem is that sendCommand "blocks" while it waits for a reply.

While this is true, the author implemented a call to idleTasks(). This is an empty method that may be overwritten by a own class inheriting from OBD2UART. That way you don't have to fidle with interrupts (which is quite dangerous as interrupt context is a bit different). Calling the lengthy method "handle()" inside interrupt context (where other interrupts are blocked) is definitely wrong because during that time the hardware serial interfaces are not handled correctly.

I have made a "workaround" with:

static void smartDelay(unsigned long ms)
{
  unsigned long start = millis();
  do 
  {
    GPS();
  } while (millis() - start < ms);
}

void loop() 
{ 
  MultiAirTemp();
  smartDelay(1000);
}

I know this is not a solution,but works ツ

pylon:
While this is true, the author implemented a call to allows overriding idleTasks(). This is an empty method that may be overwritten overriden by a own derived class inheriting from OBD2UART.

Fixed.

That is certainly another way to do it. This is similar to providing a yield function (when blocking functions/libraries bother to call it...). You should describe this technique to the OP.

That way you don't have to fidle with interrupts (which is quite dangerous as interrupt context is a bit different)

If by "fiddle" you mean that you wouldn't have to call attachInterrupt and provide an ISR, I guess that's true. However, the GPSisr function shown here is quite safe. If the OP starts adding print statements to the ISR, that would be "dangerous". But the same warnings apply to any ISR (e.g., Pin Change Interrupts, see notes here).

Calling the lengthy method "handle()" inside interrupt context (where other interrupts are blocked) is definitely wrong because during that time the hardware serial interfaces are not handled correctly.

While it's true that other interrupts are blocked during GPSisr (or any ISR), I'll have to ask you to clarify why you think "the hardware serial interfaces are not handled correctly." I have measurements that prove otherwise, but I'd like to know why you make this claim.

First, the "lengthy" handle method is highly optimized, spending an average of 10us per character. This will not disturb other ISRs nor the handling of the hardware serial interface. Even at 115200, it would be ~175us before any characters would be lost. NeoGPS can process an entire sentence in 800us.

Second, handling each character during the ISR actually uses less total CPU time, because the character does not get queued into the RX buffer, polled by loop (or idleTasks) and then dequeued by read(). A secondary benefit is that GPS processing is immune to blocking libraries (there are many), as suggested to the OP elsewhere.

Perhaps your experience is with other libraries? I would not suggest calling other libraries' parse functions during an ISR. Unlike other libraries, NeoGPS completely avoids floating-point operations; they are expensive and should not be performed in any ISR.

I have always been a little surprised that the Arduino community shies away from handling the RX char interrupt. In previous versions of the Arduino core, the serialEvent functions were called from the ISR, but now they are called from the invisible main. Oh well.

but I don't notice improvements

I forgot to mention that you also have to enable interrupt-style processing. Uncomment line 144 in NeoGPS/src/NMEAGPS_cfg.h:

    #define NMEAGPS_INTERRUPT_PROCESSING

I have made a "workaround" with [smartDelay]

Oh please don't do that. smartDelay is really stupid. :stuck_out_tongue: It keeps your sketch from doing anything else for a whole second, and you may not get complete GPS data. You might get a lat/lon but no speed. If you ever see the speed not update (freeze), smartDelay caused that.

You should also reduce the NeoGPS configuration items down to just the items you use. In NeoGPS/src/GPSfix_cfg.h, disable everything but the speed:

//#define GPS_FIX_DATE
//#define GPS_FIX_TIME
//#define GPS_FIX_LOCATION
//#define GPS_FIX_LOCATION_DMS
//#define GPS_FIX_ALTITUDE
#define GPS_FIX_SPEED
//#define GPS_FIX_HEADING
//#define GPS_FIX_SATELLITES
//#define GPS_FIX_HDOP
//#define GPS_FIX_VDOP
//#define GPS_FIX_PDOP
//#define GPS_FIX_LAT_ERR
//#define GPS_FIX_LON_ERR
//#define GPS_FIX_ALT_ERR
//#define GPS_FIX_GEOID_HEIGHT

In NMEAGPS_cfg.h, also disable all sentences except RMC:

//#define NMEAGPS_PARSE_GGA
//#define NMEAGPS_PARSE_GLL
//#define NMEAGPS_PARSE_GSA
//#define NMEAGPS_PARSE_GSV
//#define NMEAGPS_PARSE_GST
#define NMEAGPS_PARSE_RMC
//#define NMEAGPS_PARSE_VTG
//#define NMEAGPS_PARSE_ZDA

#define LAST_SENTENCE_IN_INTERVAL NMEAGPS::NMEA_RMC

You could also disable these items, since you don't need them:

//#define NMEAGPS_STATS

//#define NMEAGPS_COMMA_NEEDED

//#define NMEAGPS_RECOGNIZE_ALL

This will make your sketch quite a bit smaller and faster.

I will try, thanks!

-dev:
Oh please don't do that. smartDelay is really stupid. :stuck_out_tongue: It keeps your sketch from doing anything else for a whole second, and you may not get complete GPS data. You might get a lat/lon but no speed. If you ever see the speed not update (freeze), smartDelay caused that.

I have seen not update (freeze) before I added smartDelay. One moment was 0kph, next was 40kph. Now it freeze for just a fraction of a second, when read OBD.

Actually, I bet this would also work without interrupt-style processing (like your original sketch):

void loop() 
{ 
  //  Print new fixes when they arrive
  if (gps.available( gpsPort )) {
    fix = gps.read();
    displayGPS();
    MultiAirTemp();
  }
}

Delete the GPS() function, because loop can do all of it very simply.

This loop structure waits for the GPS update to arrive (once per second). When a fix is available, the GPS will not send any more data for almost 1 second. That should be plenty of time for the OBD functions to complete (two 300ms timeouts = 600ms maximum delay).

This will update the display once per second with GPS and OBD data, at the same time.

If you want to try this, start with your original sketch and make sure this is commented out in NMEAGPS_cfg.h:

    //#define NMEAGPS_INTERRUPT_PROCESSING  <-- commented out

Re-ordering the loop to run off the GPS updates is a good way to balance the processing load of the Arduino (read more here).

GPS works with 5HZ, will this not affect on update once per second?

GPS works with 5HZ, will this not affect on update once per second?

Maybe.

The polling style (NOT interrupt style) program above might work at 5Hz. Or it may lose GPS updates if the OBD functions take longer than ~180ms.

But I have to ask: Are you sending a command to set the update rate to 5Hz? I don't see that in your sketch.

You should send a command that turns off all sentences except the RMC, but I don't know what kind of GPS you are using.

But why are you running at 5Hz? You can only run the OBD at 1.67Hz (with timeouts). You seemed to be happy using "smartDelay" to slow down the display to 1Hz.

You could run reliably at 5Hz if the GPS is configured correctly (with commands), and if loop is structured correctly. You may need to use the interrupt processing style, depending on the OBD response time (vs. timeout).

Too many questions.

-dev:
Are you sending a command to set the update rate to 5Hz? I don't see that in your sketch.

No, I don't send and I don't need update rate at 5Hz.

-dev:
You should send a command that turns off all sentences except the RMC, but I don't know what kind of GPS you are using.

I am using this GPS. Probably I can do with U-Center?
If I turns off all sentences except the RMC, what impact will it have? What and why is RMC?

-dev:
But why are you running at 5Hz? You can only run the OBD at 1.67Hz (with timeouts). You seemed to be happy using "smartDelay" to slow down the display to 1Hz.

How to configure that will run at 1.67Hz?

I've tried without interrupt-style processing and withoud smartDelay. It works pretty well. But still there are some "wrong" readings from OBD. This was start happening when I included the NeoGPS library. The same is happening with TinyGPS / TinyGPS ++ library.

P.S. Sorry for noobie questions.

What and why is RMC?

The GPS devices sends information in "sentences".

Different sentences contain different fields (see this table). Since you are only using speed, you only need one of the sentences that contain speed: RMC or VTG.

If I turns off all sentences except the RMC, what impact will it have?

To reduce the number of RX char interrupts the Arduino has to handle, tell the GPS to send the fewest sentences possible.

To reduce the time that NeoGPS spends in recognizing different sentences, minimize the NMEAGPS_cfg.h configuration file as described above. This also reduces RAM and program space.

To reduce the time that NeoGPS spends parsing fields that you don't use, minimize the GPSfix_cfg.h configuration file as described above. This also reduces RAM and program space.

To configure the GPS to send only the RMC sentence, add these lines of code to your setup function:

  gps.send_P( &gpsPort, F("PUBX,40,GLL,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,GSV,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,GSA,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,VTG,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,ZDA,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,RMC,0,1,0,0,0,0") );  // <-- only enabled sentence
  gps.send_P( &gpsPort, F("PUBX,40,GGA,0,0,0,0,0,0") );

This matches the config files above.

To be even more efficient, you could try using just the VTG sentence. Make these changes to NMEAGPS_cfg.h:

//#define NMEAGPS_PARSE_GGA
//#define NMEAGPS_PARSE_GLL
//#define NMEAGPS_PARSE_GSA
//#define NMEAGPS_PARSE_GSV
//#define NMEAGPS_PARSE_GST
//#define NMEAGPS_PARSE_RMC
#define NMEAGPS_PARSE_VTG
//#define NMEAGPS_PARSE_ZDA

#define LAST_SENTENCE_IN_INTERVAL NMEAGPS::NMEA_VTG

And send these commands in setup:

  gps.send_P( &gpsPort, F("PUBX,40,GLL,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,GSV,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,GSA,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,VTG,0,1,0,0,0,0") );  // <-- only enabled sentence
  gps.send_P( &gpsPort, F("PUBX,40,ZDA,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,RMC,0,0,0,0,0,0") );
  gps.send_P( &gpsPort, F("PUBX,40,GGA,0,0,0,0,0,0") );

How to configure that will run at 1.67Hz?

Start with these changes at 1Hz, as it will give the maximum amount of time for the OBD functions to complete. It will also coordinate the OBD functions with the GPS quiet time. No RX char interrupts will occur during the OBD functions.

Likewise, the OBD functions will have completed by the time a new GPS update begins (RX char interrupts).

If this works very reliably, post your code and I can comment on changing the display update rate.

But still there are some "wrong" readings from OBD.

Explain this in more detail.

-dev:
To reduce the time that NeoGPS spends in recognizing different sentences, minimize the NMEAGPS_cfg.h configuration file as described above. This also reduces RAM and program space.

To reduce the time that NeoGPS spends parsing fields that you don't use, minimize the GPSfix_cfg.h configuration file as described above. This also reduces RAM and program space.

To be even more efficient, you could try using just the VTG sentence.

I have minimized the NMEAGPS_cfg.h and GPSfix_cfg.h configuration.
If use just RMC/VTG sentence, this will not affect from which satellites receive signal. Galileo or GPS, will use all of them?

-dev:
Explain this in more detail.

LCDs sometimes show values other than the actual oil temperature. For example shows the value of 70 °C, next time show "random" value (negative, or more than 1000), and then next time it's right again 70 °C.

gospod:
If use just RMC/VTG sentence, this will not affect from which satellites receive signal. Galileo or GPS, will use all of them?

Correct. From the M8 Protocol Specification:

u-blox receivers generally try to combine information from
all available GNSS to create the best possible navigation information.

LCDs sometimes show values other than the actual oil temperature. For example shows the value of 70 °C, next time show "random" value (negative, or more than 1000), and then next time it's right again 70 °C.

I would suggest printing the complete response from the "22 19 4F" command. Try something like:

Serial.println( prebralo_bo_sem );  // show the complete response buffer

    int temperatura = (readMultiAir(prebralo_bo_sem) - 40);

Then you can see why the temperature is not calculated correctly from those characters.

I have record how looks reading when is not correct (different of: 62 19 4F ")Dropbox - IMG_5453_2.mov - Simplify your life

I have tried with Serial.println( prebralo_bo_sem ); and didn't work. Of course, I need to use NeoSerial. If I'm not wrong, USB works on (Neo)Serial? So I changed OBD on (Neo)Serial2. When I have changed OBD port from (Neo)Serial to (Neo)Serial2 the problem disappeared.

Question is why?

I have record how looks reading...

Just select the text in the Serial Monitor Window with your mouse, copy it (ctrl-C), and then paste it (ctrl-V) into your post, in code tags. In the Serial Monitor Window, uncheck the Autoscroll box to make it easier to control what you select.

BTW, most people will not download from external sites, especially dropbox. Don't post a screen image of the Serial Monitor window, either.

Of course, I need to use NeoSerial.

Only for the interrupt processing style. You probably don't need to do that, but it's ok to try it.

USB works on (Neo)Serial?

Yes, on the Mega, the USB interface (an FTDI chip) is connected to pins 0 & 1.

When I have changed OBD port from (Neo)Serial to (Neo)Serial2 the problem disappeared. Question is why?

Maybe the OBD adapter does not drive RX pin 0 "hard enough" to override the USB connection. Sending data from the Serial Monitor window would also interfere with receiving the OBD response.

Be sure to post (or attach) your code, so others can benefit from your final solution.