duplicate printing

I have created the following code and it is not doing what I expected it to do >:(

Header file:

#ifndef test_H
#define test_H

#include <Arduino.h>
#include <SoftwareSerial.h>
#include <XBee.h>

int *processData(XBee *myXBee, ZBRxResponse *myRx, SoftwareSerial *myNSS);

#endif

CPP file:

#include "test.h"

int *processData(XBee *myXBee, ZBRxResponse *myRx, SoftwareSerial *myNSS){	
  int *dataVar = (int*)malloc(sizeof(int)*6);
  if (dataVar == 0)
  {
	  myNSS->println("ERROR: Out of memory");
  }
  int casenum = -1;
  int incomingByte;
  char xbv[5];
  int jj = 0;
  int cnt = 0;
  
  myXBee->readPacket();

    if (myXBee->getResponse().isAvailable()) {
      // got something
           
      if (myXBee->getResponse().getApiId() == ZB_RX_RESPONSE) {
        // got a zb rx packet
        
        // now fill our zb rx class
        myXBee->getResponse().getZBRxResponse(*myRx);
            
        if (myRx->getOption() == ZB_PACKET_ACKNOWLEDGED) {
            // the sender got an ACK
            myNSS->println("packet acknowledged:");
        } else {
          myNSS->println("packet not acknowledged");
        }
	
        for(cnt = 0; cnt < myRx->getDataLength(); cnt++) {
		incomingByte = myRx->getData()[cnt];
		if(incomingByte == ','){      // Go to the next array slot
	 	        xbv[jj] = '\0';
			casenum++; // Increment for next letter
			dataVar[casenum] = atoi(xbv);
			jj = -1;
		} else {
			xbv[jj] = incomingByte;
		}
		jj++;
	}
      }
    } else if (myXBee->getResponse().isError()) {
      myNSS->print("oh no!!! error code:");
      myNSS->println(myXBee->getResponse().getErrorCode());
    }
  return dataVar;
}

Sketch File:

#include "test.h"
#include <SoftwareSerial.h>
#include <XBee.h>

XBee xbee = XBee();
ZBRxResponse rx = ZBRxResponse();


// Define NewSoftSerial TX/RX pins
// Connect Arduino pin 8 to TX of usb-serial device
uint8_t ssRX = 8;
// Connect Arduino pin 9 to RX of usb-serial device
uint8_t ssTX = 9;
SoftwareSerial nss(ssRX, ssTX);


void setup() {
  Serial.begin(57600);
  xbee.setSerial(Serial);
  nss.begin(57600);
  nss.println("Starting Up!");
}

void loop() {
  if(Serial.available()) {
	  xBeeReport = processData(&xbee, &rx, &nss);
	  for (int ii=0; ii<6; ii++) {
		  nss.print(xBeeReport[ii]);
		  nss.print(", ");
	  }
	  free(xBeeReport);
	  xBeeReport = NULL;
	  nss.println();
  }
}

I am sending the following message three time to my XBee in API2 mode:
1,-12,2,
31 2C 2D 31 32 2C 32 2C in hex

After receiving the message three times, it looks like this on my terminal:

Starting Up!
0, 0, 0, 0, 0, 0,
packet acknowledged:
1, -12, 2, 0, 0, 0,
0, -12, 2, 0, 0, 0,
packet acknowledged:
1, -12, 2, 0, 0, 0,
0, -12, 2, 0, 0, 0,
packet acknowledged:
1, -12, 2, 0, 0, 0,

But I expected to see the following message:

Starting Up!
packet acknowledged:
1, -12, 2, 0, 0, 0,
packet acknowledged:
1, -12, 2, 0, 0, 0,
packet acknowledged:
1, -12, 2, 0, 0, 0,

I can’t figure out what I am doing wrong! :confused:

Can somebody point out my mistake?

Thanks in advance

XBee xbee = XBee();
ZBRxResponse rx = ZBRxResponse();

Explicitly calling the constructor is wrong.

The correct way:

XBee xbee;
ZBRxResponse rx;
	  xBeeReport = processData(&xbee, &rx, &nss);
	  for (int ii=0; ii<6; ii++) {
		  nss.print(xBeeReport[ii]);
		  nss.print(", ");
	  }
	  free(xBeeReport);
	  xBeeReport = NULL;

ASSuming that the allocation succeeded is a really bad idea.

You are not passing anything back from processData() that indicates whether there as data for the XBee to read, or not. You are not passing anything back that indicates whether the data formed a complete, valid packet.

It would be far better to use a global array to hold the data, and avoid the malloc() and free() calls (and misuse that you have now), and have processData() return a boolean indicating whether or not it was successful.

Thanks PaulS

So I have made the changes you recommended and now I am noticing that whenever a message is sent to my XBee, the “if(Serial.available())” loop is activated twice! first time with no data, second time with the correct data! Is that typical, or am I still doing something wrong?!
I am also amazed when the first time no data is present, I get the ‘false’ return but not the myNSS prints inside the same loop!!! :o Please have a look at my code to see what I mean. Thanks

Here is my edited code…

Header File:

#ifndef test_H
#define test_H

#include <Arduino.h>
#include <SoftwareSerial.h>
#include <XBee.h>

extern int xBeeData[6];

boolean processData(XBee *myXBee, ZBRxResponse *myRx, SoftwareSerial *myNSS);

#endif

CPP File:

#include "test.h"

int xBeeData[6];

boolean processData(XBee *myXBee, ZBRxResponse *myRx, SoftwareSerial *myNSS){	
  int casenum = -1;
  int incomingByte;
  char xbv[5];
  int jj = 0;
  int cnt = 0;
  
  myXBee->readPacket();

    if (myXBee->getResponse().isAvailable()) {
      // got something
           
      if (myXBee->getResponse().getApiId() == ZB_RX_RESPONSE) {
        // got a zb rx packet
        
        // now fill our zb rx class
        myXBee->getResponse().getZBRxResponse(*myRx);
            
        if (myRx->getOption() == ZB_PACKET_ACKNOWLEDGED) {
            // the sender got an ACK
            myNSS->println("packet acknowledged:");
        } else {
          myNSS->println("packet not acknowledged");
        }
	
        for(cnt = 0; cnt < myRx->getDataLength(); cnt++) {
		incomingByte = myRx->getData()[cnt];
		if(incomingByte == ','){      // Go to the next array slot
	 	        xbv[jj] = '\0';
			casenum++; // Increment for next letter
			xBeeData[casenum] = atoi(xbv);
			jj = -1;
		} else {
			xbv[jj] = incomingByte;
		}
		jj++;
	}
      }
      return true;
    } else if (myXBee->getResponse().isError()) {
      myNSS->print("oh no!!! error code:");
      myNSS->println(myXBee->getResponse().getErrorCode());
      return false;
    }
}

Sketch File:

#include "test.h"
#include <SoftwareSerial.h>
#include <XBee.h>

boolean processStatus = false;

XBee xbee;
ZBRxResponse rx;


// Define NewSoftSerial TX/RX pins
// Connect Arduino pin 8 to TX of usb-serial device
uint8_t ssRX = 8;
// Connect Arduino pin 9 to RX of usb-serial device
uint8_t ssTX = 9;
SoftwareSerial nss(ssRX, ssTX);


void setup() {
  Serial.begin(57600);
  xbee.setSerial(Serial);
  nss.begin(57600);
  nss.println("Starting Up!");
}

void loop() {
  if(Serial.available()) {
	  processStatus = processData(&xbee, &rx, &nss);
	  if(processStatus == true){
		  for (int ii=0; ii<6; ii++) {
			  nss.print(xBeeData[ii]);
			  nss.print(", ");
		  }
		  nss.println();
	  } else {
		  nss.println("process status = false");
	  }
  }
}

Now after receiving the message three times, it looks like this on my terminal:

Starting Up!
process status = false
packet acknowledged:
1, -12, 2, 0, 0, 0,
process status = false
packet acknowledged:
1, -12, 2, 0, 0, 0,
process status = false
packet acknowledged:
1, -12, 2, 0, 0, 0,

I am noticing that whenever a message is sent to my XBee, the "if(Serial.available())" loop is activated twice!

There is NO if loop. There is an if STATEMENT. If there is serial data to be read, the condition will be true. If there is not, the condition will be false. If the condition is true, the body of the statement is executed. If the condition is false, the body will not be executed.

boolean processData(XBee *myXBee, ZBRxResponse *myRx, SoftwareSerial *myNSS)
{
  if (myXBee->getResponse().isAvailable())
  {
    return true;
  }
  else if (myXBee->getResponse().isError())
  {
    return false;
  }
}

If no response is available, that is not necessarily an error, is it? What does the function return in that case? EVERY branch needs a return statement. It's even better if there is only ONE return statement, and every branch influences what to return.

Sorry, you are right PaulS, I meant if STATEMENT, not LOOP.

So I edited the code and added the else statement and looks like this:

boolean processData(XBee *myXBee, ZBRxResponse *myRx, SoftwareSerial *myNSS)
{
  boolean status;
  if (myXBee->getResponse().isAvailable())
  {
    status = true;
  }
  else if (myXBee->getResponse().isError())
  {
    status = false;
  }
  else
  {
    status = false;
  }
  return status;
}

And you are right again :slight_smile: every time I send a message to my XBee, there's no response available first time, and then there is a response the second time!

So in the void loop(), once there is serial data to be read, the "if(Serial.available())" STATEMENT is executed, BUT "if(myXBee->getResponse().isAvailable())" is not executed the first time because it says nothing is available! In other words, at first, there is serial data available, but no response available. Confusing. I wonder what that captured data is the first time.

Do you consider this a problem? If yes, is there a way to fix it?

Thanks alot PaulS for your help :smiley:

So in the void loop(), once there is serial data to be read, the "if(Serial.available())" STATEMENT is executed, BUT "if(myXBee->getResponse().isAvailable())" is not executed the first time because it says nothing is available!

Serial.available() returns a value greater than 0 when there is one or more bytes to be read.

myXBee->getResponse().isAvailable() returns true only when there is a complete packet.

So, the behavior you are seeing is perfectly reasonable.