Error when sending and receiving mesage using Mirf library

Using Arduino Programming Questions
1

Hello i have 2x ardiono Mega 2560 with LCD and keypad connected using NRF24L01 module. It is working only sometimes so i dont know where is problem in code or in hardware? Please look at my code and tell me if it is functional. Thanks for help.

//klávesnice
#include <Keypad.h>
const byte ROWS = 4; //four rows
const byte COLS = 4; //three columns
char keys[ROWS][COLS] = {
  {
    '1','2','3','A'                                  }
  ,
  {
    '4','5','6','B'                                  }
  ,
  {
    '7','8','9','C'                                  }
  ,
  {
    '*','0','#','D'                                  }
};

char rec;


byte rowPins[ROWS] = {
  34, 36, 38, 40}; //connect to the row pinouts of the keypad
byte colPins[COLS] = {
  42, 44, 46, 48};//connect to the column pinouts of the keypad
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

//RF
#include <SPI.h>
#include <Mirf.h>
#include <nRF24L01.h>
#include <MirfHardwareSpiDriver.h>

//LCD
#include <Wire.h>
#include <LCD.h>
#include <LiquidCrystal_I2C.h> 
#define I2C_ADDR    0x20  

#define BACKLIGHT_PIN  7
#define En_pin  4
#define Rw_pin  5
#define Rs_pin  6
#define D4_pin  0
#define D5_pin  1
#define D6_pin  2
#define D7_pin  3
#define  LED_OFF  0
#define  LED_ON  1

//String count;

LiquidCrystal_I2C  lcd(I2C_ADDR,En_pin,Rw_pin,Rs_pin,D4_pin,D5_pin,D6_pin,D7_pin);

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

  //LCD
  {
    lcd.begin (16,2);
    lcd.setBacklightPin(BACKLIGHT_PIN,NEGATIVE);
    lcd.setBacklight(LED_ON);
    lcd.backlight();
  }



  Mirf.cePin = 9;
  Mirf.csnPin = 10;

  Mirf.spi = &MirfHardwareSpi;
  Mirf.init();


  String rec = setAddress("Receiving address");
  String tar = setAddress("Target address");

  Mirf.setRADDR((byte *)&rec);
  Mirf.setTADDR((byte *)&tar);

  //Mirf.setRADDR((byte *)"00002");
  //Mirf.setTADDR((byte *)"00001");

  Mirf.payload = sizeof(char);

  Mirf.config();

  lcd.setCursor(0,0);
  lcd.print("Send:");
  lcd.setCursor(0,1);
  lcd.print("Rec:");
}

void loop(void)
{
  char key = keypad.getKey();
  if (key != NO_KEY){
    Mirf.send((byte *) &key);
    lcd.setCursor(5,0);
    lcd.print(key);
    while(Mirf.isSending()){
    }
  }

  if(Mirf.dataReady()){
    char get; 
    lcd.setCursor(5,1);
    Mirf.getData((byte *) &get);
    lcd.print(get);
  }
}

String setAddress(String rec){
  int pos = 0;
  String add = "";
  lcd.setCursor(0,0);
  lcd.print(rec);
  char key = keypad.getKey();
  char keyOld;

  while(pos<5){
    if (key != NO_KEY){
      lcd.setCursor(pos,1);
      pos++;
      add +=key;        
      lcd.print(key);
    }
    key = keypad.getKey();
  }
  while(true){
    if(key == 'D'){
      lcd.clear();
      return add;
    }
    if(key == 'C'){
      lcd.clear();
      return setAddress(rec);
    }
    key = keypad.getKey();
  }
}
2 
  String add = "";

This needs to be global and not local to setAddress() as you are returning it.

Mark

3

This needs to be global and not local to setAddress() as you are returning it.

Why? It is perfectly OK for add to be defined in setAddress(), and returned by setAddress(). Returning a global variable is silly.

4

Setting address is ok but my real problem is then from one device everithing what i send is registred on second device. But for example only 1 of 10 message is recieved when sending from second to first.

5

Quote
This needs to be global and not local to setAddress() as you are returning it.
Why? It is perfectly OK for add to be defined in setAddress(), and returned by setAddress(). Returning a global variable is silly.

As I understand it an object, declared locally is created completely on the stack no part of the object is created on the heap!. Thus

a. no explicit destructor is required.

b. its an object, so a pointer to it is returned not a copy of the object

After the return from setAddress() the point gives the address of part of the stack which will be over written by later function calls.

If you want to return an object from a function the it should be created with new().

Retuning something declared/defined globally is far from silly, its a bog standard thing to do. Eg ON/OFF declared as constants.

@OP

If i'm right an add is on the stack and then gets over written in a later function call then you'll get strange results which may explain 1 in 10 of your messages going missing

Mark

6

Retuning something declared/defined globally is far from silly, its a bog standard thing to do.

Standard does not mean the same as not silly.

If the thing being returned is global, there is no reason to return it. The caller has direct access to what the function would return.

7

So i tried to avoid this problematic code so i set Recieving and Target address in code manually for both devices. But still same problem occures. Any other thoughts where problem could be?

8 
String setAddress(String rec) {
...
while (true) {
 if ( key == 'C' )
    return setAddress(rec);
 }
}

means for every 'C' without 'D' you go one level deeper in the setAddress() recursion ?

Looks suspicious to me, but didn't analyse completely
( ... because I hate Strings, which look like they could be used like ordinary variables, e.g. returned by value without fear to get a pointer to invalid stack variable.)

@ holmes4: Assumed it's implemented properly, setAddress returns a valid copy of it's local variable add. Guess how many constructors and destructors have to be called to make it work ...

9

michael_x:

String setAddress(String rec) {

...
while (true) {
if ( key == 'C' )
    return setAddress(rec);
}
}




means for every 'C' without 'D' you go one level deeper in the setAddress() recursion ?

Looks suspicious to me, but didn't analyse completely
<sub>( ... because I hate Strings, which look like they could be used like ordinary variables, e.g. returned by value without fear to get a pointer to invalid stack variable.)</sub>

@ holmes4: Assumed it's implemented properly, setAddress returns a valid copy of it's local variable add. Guess how many constructors and destructors have to be called to make it work ...

It looks like real problem was one broken cable.