mysensors / MySensors

MySensors library and examples
https://www.mysensors.org
1.31k stars 892 forks source link

NRF51822 hangs with heavy traffic and smartsleep #1371

Closed slorenm closed 3 years ago

slorenm commented 4 years ago

I've noticed using smartsleep with nrf51 platform, when heavy traffic is applied to the system (i.e. a lot of messages during switching groups), nrf51 hangs in Timer interrupt 'NRF5_RADIO_TIMER_IRQ_HANDLER' of Radio_ESB.cpp, with never cleared status of 'NRF5_RADIO_TIMER->EVENTS_COMPARE[1]'. This results in interrupt being handled over and over again i blocks execution. What I found in the code, this status should be handled using PPI channel. Clearing this status in debug resolves the issue until next such case occurs. I found a solution in 'static bool NRF5_ESB_initialize()' changing radio and timer intterrupt priorities (basically swapping them) .With this, problem never occurs anymore. Could someone take a look at this and verify the solution?

petewill commented 4 years ago

@slorenm I believe I am having this same problem. Can you please be more specific about what exactly you changed in the code? Are you saying you set NVIC_SetPriority(RADIO_IRQn, 1); to NVIC_SetPriority(RADIO_IRQn, 2); and NVIC_SetPriority(NRF5_RADIO_TIMER_IRQN, 2); to NVIC_SetPriority(NRF5_RADIO_TIMER_IRQN, 1); in Radio_ESB.cpp? Thanks, Pete

slorenm commented 4 years ago

Yes so basically timer interrupt has higher priority than the radio with the change. It's kind of major change so it would be nice to confirm it's a real fix rather than just a cover up for some other issue. Btw. sorry for my previous blurry explanation

petewill commented 4 years ago

@slorenm Thanks! I just made the change and I'm testing it now. In my experience the node will run fine for a few days then crash so I'll report back in a few days with my results.

petewill commented 4 years ago

@slorenm Thank you for this change. My device has been working flawlessly for a few weeks since I made the change.

fonitzu commented 4 years ago

@slorenm I took a look at the code last night, due to my #1410 .

For me, the code looks wrong.

EVENTS[1] seems to be used to power off the radio, when no ACK is received after waiting NRF5_ESB_ACK_WAIT microseconds. This feature is only activated when NRF5_ESB_USE_PREDEFINED_PPI is defined. In this case, the radio will be powered down after ACK's timeout until retransmission, saving some power, otherwise the radio will actually wait for an ACK until retransmission.

EVENTS[3] is the retransmission timer, obvious from timer's interrupt handler.

For this, to work as expected:

Reference: https://github.com/mysensors/MySensors/blob/8103c6cf6bcb3e271df08b17d0fb2fee3b444c86/hal/transport/NRF5_ESB/driver/Radio_ESB.cpp#L195

https://github.com/mysensors/MySensors/blob/8103c6cf6bcb3e271df08b17d0fb2fee3b444c86/hal/transport/NRF5_ESB/driver/Radio_ESB.cpp#L700

nikolac commented 4 years ago

I'm having the same issue with both my NRF51822 & NRF52832 nodes. At some point, they freeze, consume huge amounts of power. Sometimes triggering brownout which brings them back. Other nodes require manual intervention.

NRF51 Sketch

#define SKETCH_NAME "SolarSoil04"
#define SENSOR_NAME SKETCH_NAME
#define SKETCH_VERSION "3.1"
#define MY_BAUD_RATE 115200
#define ADC_BITS 1024.0
#define MY_DEBUG

#define MY_NODE_ID 49
#define MY_PASSIVE_NODE

#define MY_PARENT_NODE_ID 0
#define MY_PARENT_NODE_IS_STATIC
#define MY_PASSIVE_NODE

#define MY_TRANSPORT_UPLINK_CHECK_DISABLED
#define MY_TRANSPORT_CHKUPL_INTERVAL_MS 2000
#define MY_TRANSPORT_WAIT_READY_MS  1000
#define MY_SLEEP_TRANSPORT_RECONNECT_TIMEOUT_MS 2000

#define MY_SKIP_PRESENT 
#define MY_RADIO_NRF5_ESB

#include <MySensors.h>

#define SLEEP_MINS 5
#define SLEEP    SLEEP_MINS * 60 * 1000
#define SLEEP_BETWEEN_SEND 1000 * 10

#define POWER_WAIT 1000
#define POWER_PIN 2

#define CHILD_ID_VOLT 1
#define CHILD_ID_SOIL 2

#define SOIL_PIN 3
#define SOIL_MIN 30.0
#define SOIL_MAX 73.0

MyMessage msgBattery(CHILD_ID_VOLT, V_VOLTAGE);
MyMessage msgSoil(CHILD_ID_SOIL, V_LEVEL);

// setup
void setup() {
  pinMode(SOIL_PIN, INPUT);
  pinMode(POWER_PIN, OUTPUT);
}

void present() {

}

// loop
void loop() {
  float internalVoltage = getInternalVoltage();

  send(msgBattery.set(internalVoltage, 3));

  sleep(SLEEP_BETWEEN_SEND);

  digitalWrite(POWER_PIN, HIGH);
  wait(POWER_WAIT);
  float soilLevel = getSoilLevel(readLevel(SOIL_PIN));
  send(msgSoil.set(soilLevel, 2));
  digitalWrite(POWER_PIN, LOW);

  sleep(SLEEP, false);
}

float getInternalVoltage(){
  return ((float)hwCPUVoltage())/1000.0;
}

float getSoilLevel(float rValue){
  return map(rValue, SOIL_MIN, SOIL_MAX, 100.0, 0.0);
}

float readLevel(int pin){
  uint16_t adc = analogRead(pin);

  return 100 * adc / ADC_BITS ;
}

NRF52 Sketch

#define SKETCH_NAME "Weather Station"
#define SKETCH_VERSION "1.4"
#define MY_DEBUG
#define MY_BAUD_RATE 115200
#define MY_RADIO_NRF5_ESB

#define MY_NODE_ID 29
#define MY_PARENT_NODE_ID 13
#define MY_PARENT_NODE_IS_STATIC
#define MY_PASSIVE_NODE

#define MY_TRANSPORT_UPLINK_CHECK_DISABLED
#define MY_TRANSPORT_CHKUPL_INTERVAL_MS 2000
#define MY_TRANSPORT_WAIT_READY_MS  1000
#define MY_SLEEP_TRANSPORT_RECONNECT_TIMEOUT_MS 2000

#define POWER_PIN 25
#define SLEEP 60000
#define SLEEP_BETWEEN_SEND 1000
#define ADC_BITS 1024.0

#include <MySensors.h>

#define   WIND_SPEED_CHILD_ID 0
#define   WIND_SPEED_PIN      2
#define   SPEED_PER_SEC_REV   1.492
uint32_t  lastWindReadingMs = millis();
float     lastWindSpeed     = 0;
float     maxWindSpeed      = 0;
MyMessage msgWindSpeed(WIND_SPEED_CHILD_ID, V_WIND);

#define   WIND_VANE_CHILD_ID 3
#define   WIND_VANE_ANALOG_PIN 30
#define   WIND_VANE_OFFSET     0
MyMessage msgWindDir(WIND_VANE_CHILD_ID, V_DIRECTION);

#define   CHILD_ID_VOLT 1
MyMessage msgBattery(CHILD_ID_VOLT, V_VOLTAGE);

#define   RAIN_GAUGE_CHILD_ID   2
#define   RAIN_GAUGE_PIN        23
#define   RAIN_INCH_PER_PULSE 0.011
uint8_t   rainPulseCount =      0;
uint32_t  lastRainMs =          millis();
MyMessage msgRainGauge(RAIN_GAUGE_CHILD_ID, V_RAIN);

#define LIGHT_LEVEL_CHILD_ID 4
#define LIGHT_LEVEL_PIN 31
MyMessage msgLightLevel(LIGHT_LEVEL_CHILD_ID, V_LIGHT_LEVEL);

void setup(){
  pinMode(WIND_VANE_ANALOG_PIN, INPUT);
  pinMode(POWER_PIN, OUTPUT);
  //analogReadResolution(10);
  analogReference(AR_VDD4);
}

void presentation(){
  sendSketchInfo(SKETCH_NAME, SKETCH_VERSION);

  present(WIND_VANE_CHILD_ID, S_WIND, "Wind Direction");

  wait(100);

  present(WIND_SPEED_CHILD_ID, S_WIND, "Wind Speed");

  wait(100);

  present(CHILD_ID_VOLT, S_MULTIMETER, "Weather Battery");

  wait(100);

  present(RAIN_GAUGE_CHILD_ID, S_RAIN, "Rain Meter");

  wait(100);

  present(LIGHT_LEVEL_CHILD_ID, S_LIGHT_LEVEL, "Light Level");
}

void loop()
{
  disableInterrupts();

  digitalWrite(POWER_PIN, HIGH);
  wait(10);
  uint16_t windDirection = readWindDirection();
  float lightLevel = readLightLevel();
  send(msgWindDir.set(windDirection, 1));
  digitalWrite(POWER_PIN, LOW);

  sleep(SLEEP_BETWEEN_SEND);

  send(msgWindSpeed.set(maxWindSpeed, 1));
  lastWindSpeed = 0;
  maxWindSpeed = 0;

  sleep(SLEEP_BETWEEN_SEND);

  float internalVoltage = getInternalVoltage();
  send(msgBattery.set(internalVoltage, 3));

  sleep(SLEEP_BETWEEN_SEND);

  float rainAmount = rainPulseCount * RAIN_INCH_PER_PULSE;
  send(msgRainGauge.set(rainAmount,3));
  rainPulseCount = 0;

  sleep(SLEEP_BETWEEN_SEND);

  send(msgLightLevel.set(lightLevel, 1));

  enableInterrupts();
  sleep(SLEEP);
}

void countRainTip(void){
  uint32_t currentMs = millis();
  uint32_t msSinceLast = currentMs - lastRainMs;

  if(msSinceLast < 500) return;

  lastRainMs = currentMs;
  rainPulseCount++;
}

void clockWindSpeed(void){
  uint32_t currentMs = millis();
  uint32_t msSinceLastRev = currentMs - lastWindReadingMs;

  if(msSinceLastRev < 10) return;

  lastWindReadingMs = currentMs;
  lastWindSpeed = SPEED_PER_SEC_REV/(msSinceLastRev / 1000.0);
  maxWindSpeed = lastWindSpeed > maxWindSpeed ? lastWindSpeed : maxWindSpeed;
}

uint16_t getHeading(int direction){
  if (direction < 155)
    return 90;
  else if (direction >= 155  && direction < 263)
    return 135;
  else if (direction >= 263 && direction < 411)
    return 180;
  else if (direction >= 411 && direction < 594)
    return 45;
  else if (direction >= 594 && direction < 771)
    return 225;
  else if (direction >= 771 && direction < 909)
    return 0;
  else if (direction >= 909 && direction < 992)
    return 315;
  else if (direction >= 992)
    return 270;
}

void enableInterrupts(){
  attachInterrupt(digitalPinToInterrupt(WIND_SPEED_PIN), clockWindSpeed, RISING);
  attachInterrupt(digitalPinToInterrupt(RAIN_GAUGE_PIN), countRainTip, RISING);
}

void disableInterrupts(){
  detachInterrupt(digitalPinToInterrupt(WIND_SPEED_PIN));
  detachInterrupt(digitalPinToInterrupt(RAIN_GAUGE_PIN));
}

int readWindDirection(){
  uint16_t vaneAdc = analogRead(WIND_VANE_ANALOG_PIN);
  uint16_t heading = getHeading(vaneAdc);

  return (heading + WIND_VANE_OFFSET) % 360;
}

float readLightLevel(){
  uint16_t adc = analogRead(LIGHT_LEVEL_PIN);

  return 100 * adc / ADC_BITS;
}

float getInternalVoltage(){
  return ((float)hwCPUVoltage())/1000.0;
}
d00616 commented 3 years ago

There is a fix for the ESB radio https://github.com/mysensors/MySensors/pull/1445

rikki78 commented 2 years ago

I also experienced the NRF to hang after sending. The fix as described by @slorenm , solved this as far as I see now. However when I tried to use the development branch (to include the deadlock fix as mentioned by @d00616), the NRF5 doesn't find its parent. What is the right fix?