Serial Interrupt - No parsing data

Hi guys, I´m trying to receive some serial data at any time, so I found a way to do it using interrupts (credits: My code is bug free.: Arduino : Serial communication with interrupt). In my own tests, I can parse only the first arrangement of data sent over serial, the rest of the data is received with some variations.

My code is:

// To use this example, you have to connect Rx pin (digital pin 0) to interrupt 0 pin (digital pin 2).

int value = 1;
boolean Message_Completed = false;
String Message = "";

void setup()
{
  Serial.begin(9600);
  attachInterrupt(0, serialInterrupt, CHANGE);
}

void loop()
{
  Serial.print(value);
  value = value + 1;
  delay(1000);
  
  Message = "";
  Message_Completed = false;
}

// Volatile, since it is modified in an ISR.
volatile boolean inService = false;

void serialInterrupt()
{
  if (inService) return;
  inService = true;
  interrupts();

  while(Serial.available())
  {
    char ReadChar = (char)Serial.read();
  
    if(ReadChar == '>')  // ">" indica el final del string
    { 
      Message_Completed = true;
    }
    else { Message += ReadChar; }
  }
  
  if(Message_Completed)
  {
    parsedata();
  }

  inService = false;
  value = 1;
}

void parsedata()
{
  Serial.println();
  Serial.print("Mensaje: ");
  Serial.println(Message);
  
  int SP1 = Message.indexOf(',');
  Serial.print("SP1: "); Serial.println(SP1);
  int SP2 = Message.length();
  Serial.print("SP2: "); Serial.println(SP2);


//  String Variable = Message.substring(0, SP1);
//  Serial.print("Variable: "); Serial.println(Variable);
//  String ValorString = Message.substring(SP1+1, SP2);
//  Serial.print("ValorString: "); Serial.println(ValorString);
//  int Valor = ValorString.toInt();
}

If you want to test it, send RESET,1> over serial, the first try will give this answer:

123
Mensaje: RESET,1
SP1: 5
SP2: 7

Which is OK, the next time the same command is sent, the SPs increment in 2 units:

Mensaje:
RESET,1
SP1: 7
SP2: 9

Which is not OK.

My second problem is that the debug information is shown twice even when in code I only call it once.

Does anybody have an idea what is going on?

You know when we say not to use serial I/O in interrupt context?
We really mean it.

Debug printing twice?
Once for rising edge, once for falling?

void serialInterrupt()
{
if (inService) return;
inService = true;
interrupts();

Enabling interrupts in an interrupt service routine is something you should do only when you KNOW what you are doing. You don't, so don't enable interrupts.

And please, USE CODE TAGS

So..... You get an interrupt EVERY time Rx changes state, which means you're getting up to 20 PER character received! I would be hard-pressed to come up with a LESS efficient way of handling serial data.

Why on earth don't you simply take over the Rx interrupt, so you get just one interrupt, when a new character is actually ready to read??

This is perhaps the worst way to handle serial receive I've ever seen....

Regards,
Ray L.

maverick85:
Hi guys, I´m trying to receive some serial data at any time,

Why so complicated?

Have a look at the examples in Serial Input Basics - simple reliable ways to receive data.

...R

PS. please modify your post and use the code button </> so your code looks like this and is easy to copy to a text editor. See How to use the Forum Your code is too long for me to study quickly without copying to a text editor.

Thanks guys for your replays and advise!!

Well I've tried with HIGH, LOW, RISING, FALLING and CHANGE and there is no change in the ouptut, its always the same.

In my final project I´m using 2 serial comms, one of them being executed by the great example n° 5 of the (Serial Input Basis) examples, but it processes the information once the loop restarts, that is not to important due to its only information to be shown in a display.

The other serial comm has important information that requires to be executed as soon as I read it, that is why I´m using interrupts()

Any other advise?

The other serial comm has important information that requires to be executed as soon as I read it

If that "important information" is more than one byte, then using interrupts is not going to be faster than just reading the data in loop(), provided that you don't have any blocking code.

If you do, you MUST get rid of it.

maverick85:
The other serial comm has important information that requires to be executed as soon as I read it, that is why I´m using interrupts()

Give an example of the sort of message that arrives and has to be dealt with faster than my code could detect it.

My guess is that you just need a separate copy of my function (and its data elements) for the other serial comm.

What Arduino board are you using?

...R

maverick85:
Any other advise?

Interrupts from the serial port hardware are already handled for you. Let the Serial object do its job and buffer the incoming data. Serial.available() tells you when one or more characters have been buffered.

Get rid of your ISR.

Get rid of the delay() function in loop().

Don’t use the C++ String class.

Do use an array of char (aka C string).

Assemble your string in loop() character by character each time Serial.available() is true.

When the whole string has been assembled, parse it.

Use millis() to time when to print the ‘value’ variable.

Well I'm sending information from an android app over Bluetooth:

VARIABLE,VALOR>

Variable is a String. example: RESET, DEBUG, UNIT, PLACA (license plate)
and VALOR is an integer.

In my loop Im using a gps with TinyGPS library that has this "smartdelay()" function, and my GPS coord reading is each 5 seconds, so my loop takes some time to start all over again.

My board is an arduino mega (currently using 3 serial comms: for a wifi moxa comm, Bluetooth, and GSM module).

Regards.

Well I'm sending information from an android app over Bluetooth:

VARIABLE,VALOR>

Variable is a String. example: RESET, DEBUG, UNIT, PLACA (license plate)

Then convert to C string as soon as you get the info from Bluetooth. You're doing a lot of necessary C++ String processing with the 'message' variable.

In my loop Im using a gps with TinyGPS library that has this "smartdelay()" function, and my GPS coord reading is each 5 seconds, so my loop takes some time to start all over again.

It' almost always possible to rewrite things to be non-blocking. I looked at the TinyGPS Library on github. If I got the right one, then smartdelay() is NOT a part of the library. It's just a function defined in one of the EXAMPLE .ino files. Structure your loop properly and you won't need it.

maverick85:
Well I'm sending information from an android app over Bluetooth:

VARIABLE,VALOR>

I reckon for that sort of data my Serial Input Basics examples are ideal. It is certainly how I would deal with the problem.

...R

In my loop Im using a gps with TinyGPS library that has this "smartdelay()" function, and my GPS coord reading is each 5 seconds, so my loop takes some time to start all over again.

That is the stupidest possible way to read GPS data. Instead of blocking waiting for the end of the sentence, just read the GPS data that is available, on every pass through loop(), and pass the data to the encode() method. When encode() returns true, parse the sentence you just received.

Like everyone else said, the best approach is to make sure loop gets a chance to read the GPS characters.

* Don't use delay. Read through the "General Design" information on the Useful Links page.

* Don't use a while loop to wait for one special thing to happen. Use the same techniques described above for eliminating delay (millis() + timestamp + interval, and State Machines).

* Don't use a library that does one of the above. Look for "non-blocking" libraries. You have not said which libraries you are using, so we cannot make suggestions. GPS libraries are, in general, non-blocking. Unfortunately, the smartDelay is blocking, and therefore not so smart. Don't use it.

* Don't enable interrupts in an interrupt service routine. It is possible to manage this reliably, but I'm sure you haven't analyzed the system enough to know that whether is ok. This technique is rarely used.

* Dynamic memory is not interrupt-safe. The String class uses dynamic memory (i.e., malloc and free). The links that manage the free and used chunks of memory can become corrupted. Not going to be pretty.

* Don't use String™. There are many reasons to avoid dynamic memory in this limited-RAM environment. Here's a few.

Given this general guidance, you should do something like this:

char GSMdata[80];
int  GSMindex = 0;
#define GSMport Serial1

char BTdata[80];
int  BTindex = 0;
#define BTport Serial2

char WIFIdata[80];
int  WIFIindex = 0;
#define WIFIport Serial3

#define GPSport Serial1 // ??? what port?  GSM maybe?


void setup()
{
  Serial  .begin( 9600 );
  GSMport .begin( 9600 );
  BTport  .begin( 9600 );
  WIFIport.begin( 9600 );
  GPSport.begin( 9600 ); // ???
}

void loop()
{
  checkGSM();
  checkBT();
  checkWIFI();
  checkGPS();
}

void checkGSM()
{
  while(GSMport.available())
  {
    char ReadChar = (char) GSMport.read();
  
    if (ReadChar == '>')  // ">" indica el final del string
    {
      GSMdata[ GSMindex ] = '\0'; // C strings need a zero byte at the end
      parseGSMdata();
      GSMindex = 0; // reset for next message
    }
    else if ((ReadChar >= ' ') && (GSMindex < sizeof(GSMdata)-1))
    {         // Ignore CR and LF control characters.
                                              // Is there room in the array?
      // Save another character
      GSMdata[ GSMindex ] = ReadChar;
      GSMindex++;
    }
  }
} // checkGSM


void parseGSMdata()
{
  Serial.println();
  Serial.print( F("Mensaje: ") );  // F macro in prints saves RAM!
  Serial.println( GSMdata );

  char *Variable = strtok( GSMdata, "," );  // find the first comma
  Serial.print("Variable: ");
  Serial.println( Variable );
  
  if (strcmp( Variable, "RESET" ) == 0) {
    Serial.println( F("RESET!") );
  } else if (strcmp( Variable, "DEBUG" ) == 0) {
    Serial.println( F("DEBUG!") );
  } else {
    Serial.println( F("Invalid variable") );
  }
  
  char *ValorString = strtok( NULL, "," );  // returns everything after the comma
  Serial.print( F("ValorString: ") );
  Serial.println( ValorString );

  int valor = atoi( ValorString );
  Serial.print( F("Valor: ") );
  Serial.println( valor );
}

void checkBT()
{}

void checkWIFI()
{}

#include <NMEAGPS.h>
NMEAGPS gps;
gps_fix fix;

void checkGPS()
{
  if (gps.available( GPSport )) { // read and process some GPS characters
    fix = gps.read();  //  get the entire structure of GPS fields

    if (fix.valid.location) {
      Serial.print( fix.latitude(), 6 );
      Serial.print( ',' );
      Serial.print( fix.longitude(), 6 );
      Serial.println();
    } else {
      // No location yet
      Serial.println( '.' );
    }
  }
}

I do not know where the GPS characters are coming from, so this sketch assumes Serial1.

It uses character arrays (aka C strings) instead of the String class. There are lots of functions you can use to manipulate C strings. This sketch uses strtok and atoi.

Note that it uses my NeoGPS, which is smaller, faster, more accurate and more reliable than all other libraries. If you'd like to try it, NeoGPS is available from the Ardino IDE Library Manager, under the menu Sketch -> Include Library -> Manage Libraries.