DigitalRead need to be 1, but after some hours is 0...

Hi to all,
yes, title say all.

Basically I talk PI and Arduino via NRF.
Arduino acts a LED on or off, receiving instruction from a PI (it's heater control).

All works as a charme. And I use the ACK to send the PIN state to the PI. PI asks for the PIN state every 3 seconds.

The issue: if LED is on by several hour (1,5h /2h), suddenly ACK (digitalRead) will be forever 0. But Led is ON. I can test because in Serial print I read "read of pin is 0".

If I shutdown the led (via NRF command) or shutup nothing change.

Do you have some idea? In future project will be powered on battery, maybe with the sleep function I will bypass the problem, but I want to understand because this happen.

Thank you

This is the Arduino sketch:

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

/* Hardware configuration: Set up nRF24L01 radio on SPI bus plus pins 7 & 8 */
RF24 radio(7,8);

// address of this 'duino
const byte heater[6] = {'H','E','A','T','E','R'};
const byte pipe = 1;

const int pin = 6;

void setup(){

  pinMode(pin,OUTPUT);
  digitalWrite(pin,LOW);

  unsigned short ack_start = digitalRead(pin);

  Serial.begin(115200);
  printf_begin();
  // Setup and configure radio

  radio.begin();

  radio.enableAckPayload();                     // Allow optional ack payloads
  radio.enableDynamicPayloads();                // Ack payloads are dynamic payloads
  
  radio.openReadingPipe(1, heater);
  radio.startListening();                       // Start listening

  radio.writeAckPayload(pipe,&ack_start,1);          // Pre-load an ack-paylod into the FIFO buffer for pipe 1
  radio.printDetails();
  
}

void loop() {

  if (radio.available()) {
    
    char text[32] = {0};
    radio.read(text, radio.getDynamicPayloadSize());

    /*if (!strcmp((char*)text, "get")) {
      // do nothing here. 
    }*/

    if (!strcmp((char*)text, "1")) {
      digitalWrite(pin,HIGH);
    }

    if (!strcmp((char*)text, "0")) {
      digitalWrite(pin,LOW);
    }

    unsigned short pin_state = digitalRead(pin);
    Serial.println("Read of pin is ");
    Serial.println(pin_state);
    radio.writeAckPayload(pipe,&pin_state,sizeof(pin_state));
    
  }
}

how is this wired and powered?

Led is "direct" from pin 6 (plus resistor 220K) and to the GND of the 'duino.

Powered from USB PC, to have the Serial open.

Just now, swapped the resistor for a 330K.

Led is RED, superbright type. So, maybe, a couple of hours is too time with only 220K ?

220K ? or 220 Ohms?

    radio.read(text, radio.getDynamicPayloadSize()); how do you make sure that what you get is a null terminated string? (to be able to use strcmp() later?

220 Ohm. Sorry for mistake. But now is 330 Ohm.

I think that I didn't understand your question, but I try to explain.

PI has a running webserver. A cron every 1 minute start a C++ script that sends via NRF "1" if heater needs to be on, or "0" if heater needs to be off (based on temperature recorded by DS18B20 on PI). Arduino answers with the new state, via ACK.

A page of webapp check every 3 seconds if heater is on or off (ajax based that calls a PI self-written API that calls a C++ script that sends the "get" string). You can note that this string is only part "without" code. In effect code sends (via ACK) the pin state.

No human act. I'm sure of things that "fly" on "wireless".

And I can assure you that all works (except for the don't-anymore-digitalRead-stuff-functions-after-several-hours).

(Native language Italian, sorry for my mystake. LED is a Led wired to the PI).

An Arduino "short" is 16 bits, why not use the more common and recognizable "byte", 8 bits, to hold a 1 bit value (1 or 0)?
Also, I would make "pin_state" a global byte variable declared before setup():

byte pin_state;

And change line 57 from:

unsigned short pin_state = digitalRead(pin);

To;

pin_state = digitalRead(pin);

And a short delay between writing the pin and reading it may be helpful, insert before line 57:

delay(5);

I want update. After changing the resistor from 220 Ohm to 330 Ohm and +9 hours the issue is disappeared.

I think that LED did drain too much current.

@outsider : thank you for your hints, code edited like your hints.

Thank you to all

There is no reason on earth why pinState should be a global... definitely better as a uint8_t or byte indeed than a short but as its use is limited to then loop, it should be declared in the loop (Usually best to do so at the begining of the function).

Well, if it's in loop doesn't the processor have to recreate and push it on the stack every time through loop()? Or does the clever compiler optimize that out? :confused:
I understand (mostly) the downsides of globals in a large, complex program but after all, this is a 63 line microcontroller sketch, not a 100 file "Global Thermonuclear War" game. :slight_smile:

doesn't the processor have to recreate and push it on the stack every time through loop

Not really.

if you create a variable as a global, then the compiler allocates memory in the "global space" and everywhere you use this variable, it uses a static pointer towards the heap address to read or write the data (possibly optimizing with a register in between)

if you create a local variable, then the compiler knows it will live on the stack and everywhere you use this variable, it uses a relative pointer towards the stack address

In both cases there are really no "creation" process/cost involved, it's sorted out at compile time

As you can see in the image:

The standard RAM layout is to place .data variables first, from the beginning of the internal RAM, followed by .bss. The stack is started from the top of internal RAM, growing downwards. The so-called "heap" available for the dynamic memory allocator will be placed beyond the end of .bss. There is still a risk that the heap and stack could collide if there are large requirements for either dynamic memory or stack space. The former can even happen if the allocations aren't all that large but dynamic memory allocations get fragmented over time such that new requests don't quite fit into the "holes" of previously freed regions

The fragmentation they refer to is something that can happen if you use a lot the String class for example, hence many here recommend sticking to char buffers rather than using that class

@J-M-L and @outsider ... so, what are your hints to edit code to perform at the best possible?

@J-M-L the "don't-read-anymore-bug" is diseappeared. Are you in accord with me that probably LED did drain too much current?

well what does your final code look like? we can make comments if you post again. (the global or local is not a huge deal)

for the LED and resistor, I don't know... what's your LED spec / datasheet? that will tell you the current it can draw...