Repeater node crashes

rejoe2

Thank you for adding some more info, now there's only the code missing if you follwo the instructions here.

Sounds like you did not do a "non-blocking-loop()" but used some kind of wait/delay functionality instead. I'd highly recommend using non-blocking methods for repeaters and not use wait/delay (with the exeption of short periodes needed for the DS18B20 to do their measurements).

mimaret

The code is inspired by MySensors sketchs :

/******************
  MM Octobre 2017
*******************
 1.5 remplacé Delay() par Wait()
     Update intervalle 5 min
     readDHT() : wait(1100)
*/
#define MY_REPEATER_FEATURE
// Enable debug prints
//#define MY_DEBUG

// Enable and select radio type attached
#define MY_RADIO_NRF24

#define MY_RF24_PA_LEVEL RF24_PA_MAX

#define MY_NODE_ID 111
#define SketchName "Buanderie"
#define SketchVer "1.5"

#include <SPI.h>
#include <MyConfig.h>
#include <MySensors.h>
#include <DHT.h>
#include <OneWire.h>
#include <DallasTemperature.h>

#define ONE_WIRE_BUS 5
#define DHT_DATA_PIN 3
#define DOOR_PIN 2
#define DHTTYPE DHT22

// Set this offset if the sensor has a permanent small offset to the real temperatures
#define SENSOR_TEMP_OFFSET 0

// Sleep time between sensor updates (in milliseconds)
// Must be >1000ms for DHT22 and >2000ms for DHT11
static const uint64_t UPDATE_INTERVAL = 5 * 60000;

// Force sending an update of the temperature after n sensor reads, so a controller showing the
// timestamp of the last update doesn't show something like 3 hours in the unlikely case, that
// the value didn't change since;
// i.e. the sensor would force sending an update every UPDATE_INTERVAL*FORCE_UPDATE_N_READS [ms]
static const uint8_t FORCE_UPDATE_N_READS = 10;

#define CHILD_ID_HUM 0
#define CHILD_ID_TEMP 1
#define CHILD_ID_DOOR 2
#define CHILD_ID_1WIRE 3

float lastTemp;
float lastHum;
float lastTemp1W;
uint8_t nNoUpdatesTemp;
uint8_t nNoUpdatesHum;
uint8_t nNoUpdatesTemp1W;

MyMessage msgHum(CHILD_ID_HUM, V_HUM);
MyMessage msgTemp(CHILD_ID_TEMP, V_TEMP);
MyMessage msgDoor(CHILD_ID_DOOR, V_TRIPPED);
MyMessage msg1W(CHILD_ID_1WIRE, V_TEMP);

DHT dht;
OneWire oneWire(ONE_WIRE_BUS); // Setup a oneWire instance to communicate with any OneWire devices (not just Maxim/Dallas temperature ICs)
DallasTemperature sensors(&oneWire); // Pass our oneWire reference to Dallas Temperature.
DeviceAddress insideThermometer;

void presentation() {
  sendSketchInfo(SketchName, SketchVer);

  // Register all sensors to gw (they will be created as child devices)
  present(CHILD_ID_HUM, S_HUM);
  present(CHILD_ID_TEMP, S_TEMP);
  present(CHILD_ID_DOOR, S_DOOR);
  present(CHILD_ID_1WIRE, S_TEMP);
}

void setup() {
  analogReference(INTERNAL);
  wait(500);
  pinMode(DOOR_PIN, INPUT_PULLUP);
  digitalWrite(DOOR_PIN, HIGH);

  oneWire.reset_search();
  // assigns the first address found to insideThermometer
  if (!oneWire.search(insideThermometer)) {
  }
  sensors.setResolution(insideThermometer, 9);

  dht.setup(DHT_DATA_PIN); // set data pin of DHT sensor
  sleep(dht.getMinimumSamplingPeriod());
  attachInterrupt(digitalPinToInterrupt(DOOR_PIN), DoorMoving, CHANGE);
}

void DoorMoving() {
    Serial.println("------> Door");
    send(msgDoor.set(digitalRead(DOOR_PIN) == HIGH ? 1 : 0));
}

void read1Wire() {
  sensors.requestTemperatures(); // Send the command to get temperatures
  float tempC = sensors.getTempC(insideThermometer);
  if (tempC != lastTemp1W  && tempC > -50.00 && tempC != 85.00 || nNoUpdatesTemp1W == FORCE_UPDATE_N_READS) {
    send(msg1W.set(tempC, 1));
    lastTemp1W = tempC;
    nNoUpdatesTemp1W = 0;
#ifdef MY_DEBUG
    Serial.print("Temp 1Wire C: "); Serial.println(tempC);
#endif
  } else
    nNoUpdatesTemp1W++;
}

void readDHT() {
  // Force reading sensor, so it works also after sleep()
  dht.readSensor(true);
  wait(1100);
  // Get temperature from DHT library
  float temperature = dht.getTemperature();
  if (isnan(temperature)) {
    Serial.println("T DHT!");
  } else if (temperature != lastTemp || nNoUpdatesTemp == FORCE_UPDATE_N_READS) {
    // Only send temperature if it changed since the last measurement or if we didn't send an update for n times
    lastTemp = temperature;
    // Reset no updates counter
    nNoUpdatesTemp = 0;
    temperature += SENSOR_TEMP_OFFSET;
    send(msgTemp.set(temperature, 1));

#ifdef MY_DEBUG
    Serial.print("T: "); Serial.println(temperature);
#endif
  } else {
    // Increase no update counter if the temperature stayed the same
    nNoUpdatesTemp++;
  }

  // Get humidity from DHT library
  float humidity = dht.getHumidity();
  if (isnan(humidity)) {
    Serial.println("H DHT");
  } else if (humidity != lastHum || nNoUpdatesHum == FORCE_UPDATE_N_READS) {
    // Only send humidity if it changed since the last measurement or if we didn't send an update for n times
    lastHum = humidity;
    // Reset no updates counter
    nNoUpdatesHum = 0;
    send(msgHum.set(humidity, 1));

#ifdef MY_DEBUG
    Serial.print("H: "); Serial.println(humidity);
#endif
  } else {
    // Increase no update counter if the humidity stayed the same
    nNoUpdatesHum++;
  }
}

void loop() {
  Serial.println("------> readDHT");
  readDHT();
  Serial.println("------> read1Wire");
  read1Wire();
  Serial.println("------> wait");
  wait(UPDATE_INTERVAL);
}

rejoe2

@mimaret Well organised code, congratulation.

Imo, you should just change your loop() (btw: that's really cool, just "straight forward") to be non-blocking. The principle is described here: https://www.arduino.cc/en/tutorial/BlinkWithoutDelay.
If you need a working implementation of the blink-without-delay function within MySensors, see the https://www.mysensors.org/build/pulse_power example.

Additional remarks: As mysensors may use interrupts in the future, (if possible) connecting Pin2 to nRF interrupt, door sensor to Pin3 and use one of the remaining Pins to connect the DHT may be a good idea.

mimaret

@rejoe2 Thank you for your reply.
I seemed to have read that for a repeater it was necessary to replace delay () by wait (). . .
I will adapt my program according to your recommendations and keep you informed.

mfalkvidd

@mimaret could you post the debug output from before and during the crash?
What you're doing should work. The debug output usually gives useful info on why things don't work the way they are supposed to.

Also, check the power problem flowchart on the page @rejoe2 linked earlier. PA_MAX puts very high stress on the power supply. My guess is that this is related to insufficient power stability.

mimaret

@mfalkvidd Thank you for your reply.
I do not think about power problem. The power supply can deliver 1A and I mounted a capacitor of 470 uF on the 3.3v and a 47 uF close to the radio module. I had doubts about hardware and I set up a second repeater.
On the first repeater, radio module nrf24L01 + PA + LNA. On the second repeater, radio module nrf24L01 +. Same problem.
When the repeater crashes, the interrupt is executed again. The led pin 13 that I do not manage is ON during normal operation and goes OFF when the repeater crashes.
I noticed that by disabling the interrupt (// attachInterrupt (....), the repeater does not crash.
I will look how to check the state of memory, stack, etc.
I also include the debug output and I do not see anything particular.

mimaret

OOPS :

19820560 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:0
------> Door
19820560 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:0
------> Door
19825700 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:1
------> Door
19826745 !TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=NACK:0
------> Door
19826745 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=1,st=OK:0
19826747 TSF:MSG:READ,255-255-255,s=255,c=7,t=255,pt=7,l=25,sg=1: 0
19826755 !TSF:MSG:LEN,7!=32
------> readDHT
20105310 TSF:MSG:SEND,111-111-0-0,s=0,c=1,t=1,pt=7,l=5,sg=0,ft=0,st=OK:42.4
H: 42.40
------> read1Wire
20105426
------> readDHT
20405309 TSF:MSG:SEND,111-111-0-0,s=1,c=1,t=0,pt=7,l=5,sg=0,ft=0,st=OK:21.8
T: 21.80
20405323 TSF:MSG:SEND,111-111-0-0,s=0,c=1,t=1,pt=7,l=5,sg=0,ft=0,st=OK:42.1
H: 42.10
------> read1Wire
20405440
20700045 TSF:SAN:OK
------> readDHT
20705310 TSF:MSG:SEND,111-111-0-0,s=0,c=1,t=1,pt=7,l=5,sg=0,ft=0,st=OK:41.9
H: 41.90
------> read1Wire
20705427
------> Door
20707352 !TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=NACK:1
------> Door
20707352 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=1,st=OK:0
------> Door
20723965 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:1
------> Door
20725862 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:0
------> Door
20725862 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:0
<------------------------------------CRASH-----------------
------> Door
20729143 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:1
------> Door
20730232 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:0
------> Door
20730232 TSF:MSG:SEND,111-111-0-0,s=2,c=1,t=16,pt=2,l=2,sg=0,ft=0,st=OK:0

mfalkvidd

@mimaret could you describe what you mean ny "crash"? I thought you meant that the node rebooted, but I see now that my assumption was wrong.

mimaret

In fact, the node no longer executes the program and the only solution is to restart it with a reset or switch off the power supply.

mfalkvidd

@mimaret strange. The (re)start should be visible in the debug output. Did you cut it out?

mimaret

There is no restart. Only the interrupt executes when I open the door contact. The Loop () does not continue. I did not cut anything, I just added the crash line as soon as the program stopped.

mfalkvidd

@mimaret thanks for explaining.

I think the problem lies in calling send from within the interrupt handler. Interrupt handlers are supposed to execute very quickly. Usually they just set a flag that there is work to be done, and return.

From https://learn.adafruit.com/multi-tasking-the-arduino-part-2/interrupt-etiquette

Code in the ISR should not call anything that requires interrupts to be active (e.g. delay() or anything that uses the i2c bus). This will result in hanging your program.

From the debug output, send fails. That means the node must have tried to send, waited for acknowledgement until timeout, tried to send again and waited and so on until the max number of attempts is reached. That's way too much time to spend inside an interrupt handler. And the radio use

Try to only set a variable inside the interrupt handler, and perform the send from inside loop instead.

Also see https://arduino.stackexchange.com/questions/30968/how-do-interrupts-work-on-the-arduino-uno-and-similar-boards/30969

mimaret

A big thank you for your contribution. I modified the program according to your proposals. Everything is working. I am attaching you the pieces of code relating to the door contact.
My problem is solved.

.
.
.
  attachInterrupt(digitalPinToInterrupt(DOOR_PIN), DoorMoving, CHANGE);
  DoorChange = 1;
  LastDoor = 0;
}
void DoorMoving() {
    DoorChange = 1;
}

void readDoor() {
  if (DoorChange) {
    boolean Door = (digitalRead(DOOR_PIN));
    if (Door != LastDoor) {
      send(msgDoor.set((Door) == HIGH ? 1 : 0));
      LastDoor = Door;
      DoorChange = 0;
    }
  }
}
.
.
.
void loop() {
  unsigned long currentMillis = millis();
  readDoor();
  if (currentMillis - previousMillis >= UPDATE_INTERVAL) {
    previousMillis = currentMillis;
    readDHT();
    read1Wire();
  }
}

mfalkvidd

@mimaret great work! Thanks for posting your solution.

Repeater node crashes

27

11.7k

11.2k

113.1k