nRF24L01 transmission fails but succeeds?

Hello,

following up my last thread here, I finally managed to get the radio communication between my two Arduinos to work.

The Transmitter code:

#include <arduino.h>
#include <SPI.h>
#include "RF24.h"

// Initialize Array with Radio Node addresses
byte addresses[][6] = {"0xB48","0x5c2"};

// INITIALIZING THE RADIO COMMUNICATION
RF24 radio(6,7);

void setup() {
  Serial.begin(115200);
  radio.begin();
  radio.setPALevel(RF24_PA_LOW);
  radio.openWritingPipe(addresses[0]);
  radio.openReadingPipe(1,addresses[1]);
  radio.startListening();
}

void loop() {
  String command = ""; // Storing the latest command received
  
	if(command.startsWith("setFlag") && command.endsWith("#")) {
	  // If this is the case, the control unit got the command to send a new flag color

	  radio.stopListening();
	  /* Parse the given command into the Flag Unit number and the flag color */
	  int firstSeparator = command.indexOf('|');
	  String sendingString = command.substring(firstSeparator + 1);
	  char sendingArray[6];
	  sendingString.toCharArray(sendingArray, 6);

	  radio.stopListening(); 

	  /* Send Flag Change Command with the important information to every Flag Unit */
	  if(!radio.write(&sendingArray, sizeof(sendingArray))) {
		throwError("FailedSending");
	  }

	  /* Wait for the response and watch out for a timeout */
	  radio.startListening();

	  unsigned long startingTime = micros();
	  bool timeout = false;
	  while(!radio.available()) {
		if(micros() - startingTime > 200000) {
		  throwError("Timeout");
		  timeout = true;
		  break;
		}
	  }
	}
}

void throwError(String errorType) {
  Serial.print(STX);
  Serial.print(errorType);
  Serial.print(ETX);
}

The Receiver code:

#include <arduino.h>
#include <SPI.h>
#include "RF24.h"

// Flag Unit Number
#define FLAGUNIT 01

// Initialize Array with Radio Node addresses
byte addresses[][6] = {"0xB48","0x5c2"};

// Initializing Radio Communication
RF24 radio(9,10);

// Flag Color Command Buffer and set the Default Flag Color
char command[6];
String commandString;
int unitFlagColor = 00;

// Global Variables for parsing the Flag Command
int separator;
int unitNo;
int flagColor;

void setup() {
  Serial.begin(115200);

  radio.begin();
  radio.setPALevel(RF24_PA_LOW);
  radio.openWritingPipe(addresses[1]);
  radio.openReadingPipe(1,addresses[0]);

  radio.startListening();
}

void loop() {
  if(radio.available()) {
    while(radio.available()) {
      radio.read(&command, sizeof(command));
    }

    radio.stopListening();

    commandString = "";
    for(int i = 0; i < sizeof(command); i++) {
      commandString += command[i];
    }

    separator = commandString.indexOf('|');
    unitNo = commandString.substring(0, separator).toInt();
    flagColor = commandString.substring(separator + 1).toInt();;

    if(unitNo == FLAGUNIT) {
      unitFlagColor = flagColor;
      Serial.print("ChangedFlagColor_");
      Serial.println(unitFlagColor);
    }

    radio.write("1", 1);
    radio.startListening();
  }
 
  changeFlag(unitFlagColor);
}

void changeFlag(int flagType) {
	...
}

This works just perfect and as expected. With one little flaw. After transmitting three messages to the receiver, the following messages I send always return "false" for the radio.write() method and therefore a "FailedSending" message.

And here comes the strange thing: The message did not fail to send. It is received successfully and properly by the Receiver and it does exactly what I want it to do. But for some reason, it always shows me that error from the fourth message onward.

This is of course not a critical issue, as I could just escape the case that the write() method returns false, but I want to keep the possibility to do some error handling or at least logging, for the case that the transmission really fails.

Does anyone have an idea what's going on there?

You may be interested in this Simple nRF24L01+ Tutorial

A transmission and an acknowledgement requires an message in both directions. It is perfectly possible for the outgoing message to arrive correctly and for the acknowledgement message to fail. The sender has no way of knowing which part failed.

I don't understand why you have such long addresses. The max size of an address is 5 bytes.

...R

Thank you Robin. I actually know your tutorial by now :wink:

Thing is, if you look at my code, I do everything as you did there. And the first three times, the outgoing message works as well as the acknowledgment. But from there something goes wrong and I don't know what because neither the messages itself nor anything else have changed in the meantime.

Regarding the addresses, I changed them. Didn't seem to matter much though.

I can't find the documentation for the function toCharArray() that you are using to convert a String to a cstring. I suspect that is where your problem lies. A cstring needs one more element than the length of the data in order to hold the terminating 0. My guess is that you should be doing

char sendingArray[7];
sendingString.toCharArray(sendingArray, 6);

But it would be very much better not to use the String class at all as it can cause memory corruption in the small memory on an Arduino.

...R

Thank you for your reply.

I guess the toCharArray() function does exactly what it sounds like, it copies the contents of a String step for step into an char array. See here.

I applied your code, allocating an additional byte to the array both on the sender and on the receiver side, didn't change anything though. And I would like to keep the Strings tbh, because of useful methods like substring(). Or are they good alternatives?

Are you really sure that the problem lies within in the String/char array? Wouldn't explain to me why it works three times before somehow starting to behave strange.

200000 should be 200000L if its a long constant.

Thank you, Mark, changed that.

The problem still exists, unfortunately.

It had not occurred to me that toCharArray() might be an Arduino-specific function.

I never use the String class in my own code so I can't quickly see whether there is a problem in your program. And. sorry, but I don't plan to learn all about it just to debug your program.

There have been many Threads here where problems were caused by the String class. You can do anything with cstrings than can be done with the String class, although it is not as convenient to use.

I can't figure out from your program what it is supposed to be doing. For example why you need if(command.startsWith("setFlag") or where the value "setFlag" might come from. Is there any reason why you could not simply use 'S' in place of "setFlag" ?

...R

Thanks Robin for your answer.

I now completely rewrote my code ditching any Strings and using char-arrays instead.

The Sender code:

#include <arduino.h>
#include <SPI.h>
#include "RF24.h"

char command[] = ""; // Storing the latest command received

// Initialize Array with Radio Node addresses
byte addresses[][6] = {"0xB48","0x5c2"};

// INITIALIZING THE RADIO COMMUNICATION
RF24 radio(6,7);

void setup() {
  Serial.begin(115200);
  radio.begin();
  radio.setPALevel(RF24_PA_LOW);
  radio.openWritingPipe(addresses[0]);
  radio.openReadingPipe(1,addresses[1]);
  radio.startListening();
}

void loop() {
  char command[] = ""; // Storing the latest command received
  int serialResult = 0; // The answer code of the command received
  
  serialResult = readSerialInputCommand(command);
  if(serialResult == 0) {
    if(command[0] == 's' && command[1] == '|' && command[7] == '#') {
      // If this is the case, the control unit got the command to send a new flag color
      radio.stopListening();
      
      /* Send Flag Change Command with the important information to every Flag Unit */
      if(!radio.write(&command, 8)) {
        throwError("FailedSending");
      }

      /* Wait for the response and watch out for a timeout */
      radio.startListening();

      unsigned long startingTime = micros();
      bool timeout = false;
      while(!radio.available()) {
        if(micros() - startingTime > 200000L) {
          throwError("Timeout");
          timeout = true;
          break;
        }
      }
    }
  }
}

...

The Receiver code:

#include <arduino.h>
#include <Adafruit_NeoPixel.h>
#include <SPI.h>
#include "RF24.h"

// Flag Unit Number
#define FLAGUNIT 01

...

// Initialize Array with Radio Node addresses
byte addresses[][6] = {"0xB48","0x5c2"};

// Initializing Radio Communication
RF24 radio(9,10);

...

// Flag Color Command Buffer and set the Default Flag Color
char command[8];
int unitFlagColor = 00;

// Global Variables for parsing the Flag Command
int unitNo;
int flagColor;

void setup() {
  Serial.begin(115200);
  flag.begin();
  flag.show();

  radio.begin();
  radio.setPALevel(RF24_PA_LOW);
  radio.openWritingPipe(addresses[1]);
  radio.openReadingPipe(1,addresses[0]);

  radio.startListening();
}

void loop() {
  if(radio.available()) {
    // Receive command
    while(radio.available()) {
      radio.read(&command, 8);
    }
    radio.stopListening();

    // Determine unit number
    if(command[2] == '1') {
      unitNo = command[3] + 10 - '0';
    } else {
      unitNo = command[3] - '0';
    }

    //Determine flag color code
    if(command[5] == '1') {
      flagColor = command[6] + 10 - '0';
    } else{
      flagColor = command[6] - '0';
    }

    // Apply new flag color
    if(unitNo == FLAGUNIT) {
      unitFlagColor = flagColor;
      Serial.print("ChangedFlagColor_");
      Serial.println(unitFlagColor);
    }

    // Ping back
    radio.write("0", 2);
    radio.startListening();
  }

  changeFlag(unitFlagColor);
}

void changeFlag(int flagType) {
	...
}

But the error still occurs. Interesting thing I noted is that the receiving unit seems to have nothing to do with it. When I reset the sending unit, I get my three "free shots" again while not touching the receiver.

Btw, the command (formely "setFlag", now "s"), is read via serial.

You are not posting complete programs. The problem is ALWAYS in the other part :slight_smile:

...R

The parts I left out have nothing to do with the transmission so I thought I won't include them for better readability. But here you go:

Sender:

#include <arduino.h>
#include <SPI.h>
#include "RF24.h"

// Contants and variables for serial communication
#define STX "\x02"    //ASCII-Code 02, text representation of the STX code
#define ETX "\x03"    //ASCII-Code 03, text representation of the ETX code
#define RS  "$"       //Used as RS code
char command[] = ""; // Storing the latest command received
int serialResult = 0; // The answer code of the command received

// Initialize Array with Radio Node addresses
byte addresses[][6] = {"0xB48","0x5c2"};

// INITIALIZING THE RADIO COMMUNICATION
RF24 radio(6,7);

void setup() {
  Serial.begin(115200);
  radio.begin();
  radio.setPALevel(RF24_PA_LOW);
  radio.openWritingPipe(addresses[0]);
  radio.openReadingPipe(1,addresses[1]);
  radio.startListening();
}

void loop() {
  char command[] = ""; // Storing the latest command received
  int serialResult = 0; // The answer code of the command received
  
  serialResult = readSerialInputCommand(command);
  if(serialResult == 0) {
    if(command[0] == 'c' && command[1] == '#') {
      returnPing();
    }
    if(command[0] == 's' && command[1] == '|' && command[7] == '#') {
      // If this is the case, the control unit got the command to send a new flag color
      radio.stopListening();
      
      /* Send Flag Change Command with the important information to every Flag Unit */
      if(!radio.write(&command, 8)) {
        throwError("FailedSending");
      }

      /* Wait for the response and watch out for a timeout */
      radio.startListening();

      unsigned long startingTime = micros();
      bool timeout = false;
      while(!radio.available()) {
        if(micros() - startingTime > 200000L) {
          throwError("Timeout");
          timeout = true;
          break;
        }
      }
    }
  }
}

void returnPing() {
  Serial.print(STX);
  Serial.print("success");
  Serial.print(ETX);
}

void throwError(char errorType[]) {
  Serial.print(STX);
  Serial.print(errorType);
  Serial.print(ETX);
}

int readSerialInputCommand(char *command) {
  int operationStatus = 0;
  if (Serial.available()) {
    char serialInByte;//temporary variable to hold the last serial input buffer character
  
    // Run the following until the terminating character comes up or no data is available.
  
    // Counter for position in char array
    int i = 0;
    do{
      while((serialInByte = Serial.read()) == -1);
        command[i] = serialInByte; //Add last read serial input buffer byte to command
      i++;
    } while (serialInByte != '#');
  
    // If command is not terminated by '#', return an error.
    if(serialInByte != '#') {
      operationStatus = -1;
    }
  }
  else{
    // If not serial input buffer data is available, operationStatus becomes WRG_NO_SERIAL_DATA_AVAIBLE
    operationStatus = 250;
  }
    
  return operationStatus;
}

Receiver:

#include <arduino.h>
#include <Adafruit_NeoPixel.h>
#include <SPI.h>
#include "RF24.h"

// Flag Unit Number
#define FLAGUNIT 01

// Display PIN & Number of LEDs
#define PIN 6
#define LED 16

// Initialize Array with Radio Node addresses
byte addresses[][6] = {"0xB48","0x5c2"};

// Initializing Radio Communication
RF24 radio(9,10);

// Intializing the Flag
Adafruit_NeoPixel flag = Adafruit_NeoPixel(LED, PIN, NEO_GRB + NEO_KHZ800);

// Defining Flag Colors and standard delay
uint32_t reset = flag.Color(0, 0, 0);
uint32_t red = flag.Color(255, 0, 0);
uint32_t yellow = flag.Color(255, 125, 0);
uint32_t green = flag.Color(0, 255, 0);
uint32_t blue = flag.Color(0, 0, 255);
uint32_t white = flag.Color(255, 255, 255);
#define STDDEL 250

// Flag Color Command Buffer and set the Default Flag Color
char command[8];
int unitFlagColor = 00;

// Global Variables for parsing the Flag Command
int unitNo;
int flagColor;

void setup() {
  Serial.begin(115200);
  flag.begin();
  flag.show();

  radio.begin();
  radio.setPALevel(RF24_PA_LOW);
  radio.openWritingPipe(addresses[1]);
  radio.openReadingPipe(1,addresses[0]);

  radio.startListening();
}

void loop() {
  if(radio.available()) {
    // Receive command
    while(radio.available()) {
      radio.read(&command, 8);
    }
    radio.stopListening();

    // Determine unit number
    if(command[2] == '1') {
      unitNo = command[3] + 10 - '0';
    } else {
      unitNo = command[3] - '0';
    }

    //Determine flag color code
    if(command[5] == '1') {
      flagColor = command[6] + 10 - '0';
    } else{
      flagColor = command[6] - '0';
    }

    // Apply new flag color
    if(unitNo == FLAGUNIT) {
      unitFlagColor = flagColor;
      Serial.print("ChangedFlagColor_");
      Serial.println(unitFlagColor);
    }

    // Ping back
    radio.write("0", 2);
    radio.startListening();
  }

  changeFlag(unitFlagColor);
}

void changeFlag(int flagType) {
   ...
}

I left out the last method of the receiver because all it does it setting new colors to the LED strip. And with including it, I actually exceeded this forums' message length limit. :smiley:

char command[] = ""; // Storing the latest command received

This can only store one char, the trailing zero.
You have that error in at least two places.

Why don't you read the responses, but only wait for their reception?

This will only work three times... does that sound familiar?

It is amazing that it works at all with command[] defined the way it is. You must create it with enough space for the largest message plus the terminating 0.

Also, you seem to have defined an array called command[] in two places.

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

...R

Whandall:
char command[] = ""; // Storing the latest command received

This can only store one char, the trailing zero.
You have that error in at least two places.

Robin2:
It is amazing that it works at all with command[] defined the way it is. You must create it with enough space for the largest message plus the terminating 0.

You both are right, this was actually a leftover from the String stuff, I changed that now.

Robin2:
Also, you seem to have defined an array called command[] in two places.

Yup, changed that too.

Whandall:
Why don't you read the responses, but only wait for their reception?

This will only work three times... does that sound familiar?

It does! :o What exactly do you mean by the responses? The "0" it is sending back?

TheEpicSnowWolf:
It does! :o What exactly do you mean by the responses? The "0" it is sending back?

I had not spotted that. Full marks to @Whandall. The nRF24 has buffers that can hold 3 responses. As you are not reading the data that it receives those three buffers fill up.

...R

Great, thank you you two! Seems like we're getting close.

How exactly do you take care of clearing that buffer? I've added the following after the while-loop that handles the time out in the transmitter:

      if(timeout == false) {
        char responseCode[2];
        radio.read(&responseCode, 2);
        if (responseCode == 0 - '0') {
            Serial.println("success");
        }
      }

That doesn't do anything though, as I'm still getting the errors (now I can say: "as the buffer still fills up") and the "success" message does not appear.

        if (responseCode == 0 - '0') {

Where did you see such a crap?

How about

        if (responseCode == '0') {

Oops, yes, my bad, I got it mixed up with the code from before, where I needed that to parse the char into a correct int. But that's not needed here, you're completely right.

So what can I say, it works now. Thank you so much to both of you, @Whandall and @Robin2!

0 -'0'

does not convert from ASCII to binary.

True, but for my case, it actually does.
https://forum.arduino.cc/index.php?topic=438980.msg3024108#msg3024108
Or is there a better/smoother way?