nRF24 transmission/reception problems

I'm having an issue with this sketch. Everything works just fine. Wake and sleep works well, reading my dip switch using the 165 shift register works great (finally) and the radio works just fine.

However, I get random values on the receiving end. Sometimes I'll get 2 or 3 radio broadcasts back to back and sometimes things will work as intended (1 broadcast per state change). The only thing I can think of needing is a debounce circuit for the reed switch. Does this sound about right?

The working function is, sleep -> pin change interrupt -> read state of that pin -> write the pin value, current battery voltage, and decimal value of 8 position dip switch -> sleep.

Here is my current project layout.

Here is my current working version of the code, still in testing so ignore any comments.

#include <nRF24L01.h>
#include <RF24Network.h>
#include <RF24.h>
#include <RF24_config.h>
#include <avr/sleep.h>
#include <avr/interrupt.h>

#define CE_PIN  8
#define CSN_PIN 7
#define SW 3
#define SH_LD  0  // SH/LD (Pin 1) of 74165 is connected to the digital pin 3 of Arduino UNO.
#define CLK  1    // CLK (Pin 2) of 74165 is connected to the digital pin 4 of Arduino UNO.
#define SO  2     // SO-Serial output (Pin 9) of 74165 is connected to the digital pin 2 of Arduino UNO.

struct nodeInfo {
  int SWState;
  unsigned long volt;
  byte addy;
} nodeInfo;

int SWPos;
int SWState;
int lastSWState;
const uint64_t Pipe = 0x3A3A3A3AD2LL;
RF24 radio(CE_PIN, CSN_PIN);

void setup() {
  pinMode(SW, INPUT);
  pinMode(SH_LD, OUTPUT);
  pinMode(CLK, OUTPUT);
  pinMode(SO, INPUT);
  digitalWrite(CLK, HIGH);
  digitalWrite(SH_LD, HIGH);
}

void loop() {
  sleep();
}

void awake() {
  radio.powerUp();
  radio.begin();
  radio.openWritingPipe(Pipe);
  delay(250);
  SWState = digitalRead(SW);
  SWPos = digitalRead(SW);
  if (SWState != lastSWState) {
    if (SWPos == HIGH) {
      nodeInfo.SWState = 1;
      lastSWState = 1;
      digitalWrite(SW, HIGH);
    }
    if (SWPos == LOW) {
    nodeInfo.SWState = 0;
    lastSWState = 0;
    digitalWrite(SW, LOW);
    }
  }
readVcc();
address();
radio.write( &nodeInfo, sizeof(nodeInfo));
radio.powerDown();
}

void sleep() {
  GIMSK |= _BV(PCIE0);                    // Enable Pin Change Interrupts
  PCMSK0 |= _BV(PCINT7);                  // Use PA7 as interrupt pin
  ADCSRA &= ~_BV(ADEN);                   // ADC off
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);    // replaces above statement

  sleep_enable();                         // Sets the Sleep Enable bit in the MCUCR Register (SE BIT)
  sei();                                  // Enable interrupts
  sleep_cpu();                            // sleep

  cli();                                  // Disable interrupts
  PCMSK0 &= ~_BV(PCINT7);                 // Turn off PA7 as interrupt pin
  sleep_disable();                        // Clear SE bit
  ADCSRA |= _BV(ADEN);                    // ADC on

  sei();                                  // Enable interrupts
} // sleep


unsigned long readVcc() {
  // Read 1.1V reference against AVcc
  // set the reference to Vcc and the measurement to the internal 1.1V reference
#if defined (__AVR_ATtiny24__) || defined(__AVR_ATtiny44__) || defined(__AVR_ATtiny84__)
  ADMUX = _BV(MUX5) | _BV(MUX0);
#endif

  delay(2); // Wait for Vref to settle
  ADCSRA |= _BV(ADSC); // Start conversion
  while (bit_is_set(ADCSRA, ADSC)); // measuring

  uint8_t low  = ADCL; // must read ADCL first - it then locks ADCH
  uint8_t high = ADCH; // unlocks both

  unsigned long result = (high << 8) | low;

  nodeInfo.volt = 1125300L / result; // Calculate Vcc (in mV); 1125300 = 1.1*1023*1000
}

byte address()
{
  byte shift;  // An 8 bit number to carry each bit value of D7 - D0

  // Trigger loading the state of the A-H data lines into the shift register
  digitalWrite(SH_LD, LOW);
  delayMicroseconds(5); // Requires a delay here according to the datasheet timing diagram
  digitalWrite(SH_LD, HIGH);
  delayMicroseconds(5);
  digitalWrite(CLK, HIGH); // Required initial states of these two pins according to the datasheet timing diagram
  shift = shiftIn(SO, CLK, MSBFIRST);
  nodeInfo.addy = shift;
}

ISR(PCINT0_vect) {
  awake();
}

Your picture is artistic but not informative in the engineering sense. Just make a simple pencil drawing showing all the connections with the chips and pins clearly labelled and post a photo of the drawing.

...R

What Arduino are you using? On an Uno pin10 should be set as OUTPUT (so it acts as an SPI master) and I wonder if it is an INPUT after it wakes up.

Does it work properly if you take all the sleep/wake stuff out of it?

You don't seem to set any of the acknowledgement or retries features?

What version of the RF24 library are you using? The older Maniac Bug version does not like long intervals between usage (though I have never tried it with sleep) and the TMRh20 version of the RF24 library solves that.

Also, post the receiver code.

...R
Simple nRF24L01+ Tutorial

The interrupt service routine ISR(PCINT0_vect) is quite heavy because the call it makes to awake() contains a delay() and possibly blocking code (depending on the nrf24l01 library behaviour). This could cause instability. It is usually better to set a flag in the interrupt service routine which you test, act on, then clear, in the loop()

Robin2:
Your picture is artistic but not informative in the engineering sense. Just make a simple pencil drawing showing all the connections with the chips and pins clearly labelled and post a photo of the drawing.

...R

Here is a schematic that I hope helps.

Robin2:
What Arduino are you using? On an Uno pin10 should be set as OUTPUT (so it acts as an SPI master) and I wonder if it is an INPUT after it wakes up.

Does it work properly if you take all the sleep/wake stuff out of it?

You don't seem to set any of the acknowledgement or retries features?

What version of the RF24 library are you using? The older Maniac Bug version does not like long intervals between usage (though I have never tried it with sleep) and the TMRh20 version of the RF24 library solves that.

Also, post the receiver code.

...R
Simple nRF24L01+ Tutorial

I have tested an earlier version without the sleep functions and everything works the same as it does now. I'm not using any ACK's right now, as I'm not sure I want to, simply to reduce the time the radio is powered up. But, if ACK's could be used to help with the duplicate transmissions, I can include them. I'm using an ATTiny84 and the newest version of TMRh20's RF24 library and it works fantastically.

Here's the receiver code:

#include <nRF24L01.h>
#include <RF24.h>
#include <RF24_config.h>
#include <SPI.h>

RF24 radio(7, 8);

const uint64_t pipe = 0x3A3A3A3AD2LL;

struct nodeInfo {
  int swState;
  unsigned long volt;
  byte addy;
} nodeInfo;

void setup(void) {
  Serial.begin(9600);
  radio.begin();
  radio.openReadingPipe(1, pipe);
  radio.startListening();
}
void loop(void) {
  if (radio.available()) {
    radio.read(&nodeInfo, sizeof(nodeInfo));
    Serial.println(nodeInfo.swState);
    Serial.println(nodeInfo.volt);
    Serial.println(nodeInfo.addy);
  }
}

6v6gt:
The interrupt service routine ISR(PCINT0_vect) is quite heavy because the call it makes to awake() contains a delay() and possibly blocking code (depending on the nrf24l01 library behaviour). This could cause instability. It is usually better to set a flag in the interrupt service routine which you test, act on, then clear, in the loop()

That delay was supposed to be removed before I uploaded the code. It's not intended to be there and I was experimenting with some delay() functions to allow the reed switch to settle.

Ok, I think I've narrowed down the problem. I've modified the existing code and added some software debounce in the awake() function.

When my switch goes HIGH, I get 1 transmission. However, when it goes LOW I get 2 duplicate transmissions, every time. What am I missing here?

//#include <nRF24L01.h>
//#include <RF24Network.h>
#include <RF24.h>
//#include <RF24_config.h>
#include <avr/sleep.h>
#include <avr/interrupt.h>

#define CE_PIN  8
#define CSN_PIN 7
#define SW 3
#define SH_LD  0
#define CLK  1
#define SO  2

struct nodeInfo {
  int SWState;
  unsigned long volt;
  int addy;
} nodeInfo;

int SWPos;
int SWVal;
int val;
int val2;
const uint64_t Pipe = 0x3A3A3A3AD2LL;
RF24 radio(CE_PIN, CSN_PIN);

void setup() {
  pinMode(SW, INPUT);
  pinMode(SH_LD, OUTPUT);
  pinMode(CLK, OUTPUT);
  pinMode(SO, INPUT);
  digitalWrite(CLK, HIGH);
  digitalWrite(SH_LD, HIGH);
}

void loop() {
  sleep();
}

void awake() {
  val = digitalRead(SW);
  delay(10);
  val2 = digitalRead(SW);
  if (val == val2) {
    SWPos = digitalRead(SW);
    if (SWPos == HIGH) {
      nodeInfo.SWState = 1;
      digitalWrite(SW, HIGH);
    }
    else {
      nodeInfo.SWState = 0;
      digitalWrite(SW, LOW);
    }
  }
  readVcc();
  address();
  radioAct();
}


void sleep() {
  GIMSK |= _BV(PCIE0);                    // Enable Pin Change Interrupts
  PCMSK0 |= _BV(PCINT7);                  // Use PA7 as interrupt pin
  ADCSRA &= ~_BV(ADEN);                   // ADC off
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);

  sleep_enable();                         // Sets the Sleep Enable bit in the MCUCR Register (SE BIT)
  sei();                                  // Enable interrupts
  sleep_cpu();                            // sleep

  cli();                                  // Disable interrupts
  PCMSK0 &= ~_BV(PCINT7);                 // Turn off PA7 as interrupt pin
  sleep_disable();                        // Clear SE bit
  ADCSRA |= _BV(ADEN);                    // ADC on

  sei();                                  // Enable interrupts
} // sleep


unsigned long readVcc() {
  // Read 1.1V reference against AVcc
  // set the reference to Vcc and the measurement to the internal 1.1V reference
#if defined (__AVR_ATtiny24__) || defined(__AVR_ATtiny44__) || defined(__AVR_ATtiny84__)
  ADMUX = _BV(MUX5) | _BV(MUX0);
#endif

  delay(2); // Wait for Vref to settle
  ADCSRA |= _BV(ADSC); // Start conversion
  while (bit_is_set(ADCSRA, ADSC)); // measuring

  uint8_t low  = ADCL; // must read ADCL first - it then locks ADCH
  uint8_t high = ADCH; // unlocks both

  unsigned long result = (high << 8) | low;

  nodeInfo.volt = 112530L / result; // Calculate Vcc (in mV); 112530 = 1.1*1023*100
}

void radioAct() {
  radio.powerUp();
  radio.begin();
  radio.openWritingPipe(Pipe);
  radio.write( &nodeInfo, sizeof(nodeInfo));
  radio.powerDown();
}

byte address()
{
  byte shift;  // An 8 bit number to carry each bit value of D7 - D0

  // Trigger loading the state of the A-H data lines into the shift register
  digitalWrite(SH_LD, LOW);
  delayMicroseconds(5); // Requires a delay here according to the datasheet timing diagram
  digitalWrite(SH_LD, HIGH);
  delayMicroseconds(5);
  digitalWrite(CLK, HIGH); // Required initial states of these two pins according to the datasheet timing diagram
  shift = shiftIn(SO, CLK, MSBFIRST);
  nodeInfo.addy = shift;
}

ISR(PCINT0_vect) {
  awake();
}

At least the global variables which are set indirectly by the interrupt service routine e.g. val should be declared as volatile.

Hi,
How are you powering the project?
The OPs pictures.


Tom.... :slight_smile:

TomGeorge:
Hi,
How are you powering the project?

3.3v breadboard power supply right now, 3v battery later.

@jschmall, there is no value in repeating the pictures. It just makes the Thread harder to read.

...R

Incase anybody's still curious, I seemed to have solved it by moving the awake() function call from the SR(PCINT0_vect) to the loop(). Now I'm getting one transmission for each pin change and proper wake from sleep.