Go Down

Topic: NeoHWSerial/interrupts (Read 1 time) previous topic - next topic

gospod

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.

pylon

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.

-dev

#2
Feb 15, 2018, 05:03 pm Last Edit: Feb 15, 2018, 05:04 pm by -dev
Quote from: 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.
Really, I used to be /dev.  :(

gospod

#3
Feb 15, 2018, 11:54 pm Last Edit: Feb 16, 2018, 12:09 am by gospod
    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;
[/list]

-dev

Quote
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:

Code: [Select]
#include <NMEAGPS.h>
#include <NeoHWSerial.h>
#include <OBD2UART.h>
#include <LiquidCrystal_I2C.h>
#include <avr/sleep.h>

Define the gpsPort to use NeoSerial1:

Code: [Select]
#define gpsPort NeoSerial1
Attach an interrupt function to NeoSerial1:

Code: [Select]
void setup()
{
  gpsPort.begin(115200);
  gpsPort.attachInterrupt( GPSisr );

Make that interrupt function call NeoGPS to handle each character:

Code: [Select]
void GPSisr( uint8_t c )
{
  gps.handle( c );

}

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

Code: [Select]
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):

Code: [Select]
#include <NeoHWSerial.h>
#define OBDUART NeoSerial

That should do it!

Cheers,
/dev
Really, I used to be /dev.  :(

gospod

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.

pylon

Quote
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.

gospod

I have made a "workaround" with:
Code: [Select]
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 ツ

-dev

Quote from: 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.

Quote
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).

Quote
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.
Really, I used to be /dev.  :(

-dev

Quote
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


Quote
I have made a "workaround" with [smartDelay]
Oh please don't do that.  smartDelay is really stupid.  :P  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:

Code: [Select]
//#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:

Code: [Select]
//#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:

Code: [Select]
//#define NMEAGPS_STATS

//#define NMEAGPS_COMMA_NEEDED

//#define NMEAGPS_RECOGNIZE_ALL

This will make your sketch quite a bit smaller and faster.
Really, I used to be /dev.  :(

gospod

I will try, thanks!

Oh please don't do that.  smartDelay is really stupid.  :P  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.

-dev

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

Code: [Select]
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).
Really, I used to be /dev.  :(

gospod

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

-dev

Quote
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.
Really, I used to be /dev.  :(

gospod

#14
Feb 17, 2018, 08:57 pm Last Edit: Feb 17, 2018, 09:30 pm by gospod
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.

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?

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.

Go Up