Arduino pro mini - reboot loop after period of time

[ardwrt]

Hello, I hope someone is able to help

I'm having an issue with an arduino pro mini, being used for a very simple weather station transmitter. It reads some data from a bme280, transmits the data and then sleeps for 5 mins. The sleep is accomplished by setting an alarm on a ds3231, and then sleeping until the interrupt occurs. The above then loops.

This all seems to work fine for a while, but if you leave it running eventually it seems to end up in some kind of crash loop. The led on the Arduino blinks either brightly, or very dimly with an almost continuous flash.

I found this page a while back and used it to flash optiboot bootloader to pro mini but it still seems to encounter the same issue.

My code is as follows:

#include "LowPower.h"
#include "ds3231.h"
#include <time.h>
#include <Wire.h>
#include <RH_ASK.h>
#include <forcedClimate.h>
ForcedClimate climateSensor = ForcedClimate();

int i;
char msg[100];
const char *tempstr = "temp:";
const char *presstr = "pres:";
const char *humstr = "hum:";
const char *divstr = ":";
static const byte wakeUpPin = 2;

uint8_t sleep_period = 5;

RH_ASK driver(2000,4,3);

struct bmedata{
  char temp[5];
  char press[7];
  char hum[6];
};

bmedata bmedataStr;

void getBmeVals(bmedata *bmedata){
    climateSensor.takeForcedMeasurement();
    dtostrf(climateSensor.getTemperatureCelcius(), 4, 1, bmedata->temp);
    dtostrf(climateSensor.getPressure(), 6, 1, bmedata->press);
    dtostrf(climateSensor.getRelativeHumidity(), 4, 1, bmedata->hum);
}


// the setup function runs once when you press reset or power the board
void setup() {
  //Serial.begin(9600);
  climateSensor.begin();
  pinMode(LED_BUILTIN, OUTPUT);

  int i;
  for(i=0;i<5;i++){
    digitalWrite(LED_BUILTIN, HIGH);
    delay(200);
    digitalWrite(LED_BUILTIN, LOW);
    delay(200);
  }

  driver.init();
  DS3231_init(DS3231_CONTROL_INTCN);
  DS3231_clear_a2f();

  // Configure Interrupt Pin
  pinMode(wakeUpPin, INPUT_PULLUP);
  digitalWrite(wakeUpPin, HIGH);

}

void wakeUp(){
}

void set_next_alarm(void)
{
    struct ts t;
    unsigned char wakeup_min;

    DS3231_get(&t);

    // calculate the minute when the next alarm will be triggered
    wakeup_min = (t.min / sleep_period + 1) * sleep_period;
    if (wakeup_min > 59) {
        wakeup_min -= 60;
    }

    uint8_t flags[4] = { 0, 1, 1, 1 };

    // set Alarm2. only the minute is set since we ignore the hour and day component
    DS3231_set_a2(wakeup_min, 0, 0, flags);

    // activate Alarm2
    DS3231_set_creg(DS3231_CONTROL_INTCN | DS3231_CONTROL_A2IE| DS3231_CONTROL_BBSQW);
}

// the loop function runs over and over again forever
void loop() {
  getBmeVals(&bmedataStr);
  sprintf(msg, "%s%s%s%s%s%s%s%s", tempstr, bmedataStr.temp, divstr, presstr, bmedataStr.press, divstr, humstr, bmedataStr.hum);
  driver.send((uint8_t *)msg, strlen(msg));
  driver.waitPacketSent();

  set_next_alarm();
  attachInterrupt(0, wakeUp, FALLING);
  delay(500);
  LowPower.powerDown(SLEEP_FOREVER, ADC_OFF, BOD_OFF);
  DS3231_clear_a2f();
  detachInterrupt(0);
}

Is there anything obvious in my code which eventually crash the pro mini?

Perhaps a memory leak? I've seen some people raise concerns around sprintf crashing some arduino's, which I'm using.

As this takes hours to crash (maybe even more than 24 hours) it's not easy to debug. Perhaps I need to get it to write out to an SD card as a means of saving logs to work out at which point it's crashing?

Thanks in advance for any help.

[RayLivingston]

You are probably over-running one of the many char arrays with dtostrf, sprintf or similar functions. That will randomly clobber other data. You need to make 100% certain the arrays are large enough to hold the longest possible string they make encounter, not just the length you EXPECT the valid strings to be. snprintf can be used in place of printf to ensure it won't over-run the target array, but I'm not sure if there are similar replacement for the other string functions you use, like dtostrf.

[jremington]

There is really no reason to send floating point values.

It is simpler, safer and takes up less message space to send integers instead (send fixed length binary data rather than formatted ASCII) and do the scaling at the receiver.

[david_2018]

Conversely, sending the floats as binary data instead of converting to ASCII would be another possibility, since the data is coming off the sensor library as floats.

[ardwrt]

Thank you for your responses.

Buffers are:

  char temp[5];
  char press[7];
  char hum[6];

If I increase them all to [15] and print the data back after doing the dtostrf I get:

temp = 18.7
press = 1017.3
hum = 82.2

Which means so long as temp never gets to 100c or some other crazy number:

temp = 4 chars + null = [5]
press = 6 chars + null = [7]
hum = 4 chars + null = [5] (however with 100% humidity that would need [6]).

So I think that my sizes are ok, so long as no invalid data is returned. Which I accept is possible.... I might try setting them to [15] and see how it goes for a while.

I also accept that just sending the raw values would be preferable but I think the lib I'm using is giving me floats back which I need to convert.

[ardwrt]

Thanks @david_2018, I might try just sending the binary data. I've only ever tried sending strings though up until this point. Need to look into how to convert it back into the correct data at the other end. Much easier with a string.

[jremington]

@ardwrt

I think the lib I'm using is giving me floats back which I need to convert.

You can always get the raw values, but another option is to multiply the float value by 100, round and truncate, then send the integer value as binary. Scale back down at the receiver.

[ardwrt]

Thanks @jremington I will give this a try.

Regarding failure modes, how should the pro mini deal with crashes? Would it be expected that it would recover and begin program execution from the beginning again? (I assume by running the code in setup()).

Is it normal for it to crash and then be stuck in some kind of infinite loop?

[ardwrt]

I realise that even though I switched to using the optiboot as a bootloader, I'm not actually using the watchdog timer as part of my code..........

So if I add a:

  wdt_enable(WDTO_8S);

During setup, disable it before deep sleeping/re-enable post:

  wdt_disable();
  LowPower.powerDown(SLEEP_FOREVER, ADC_OFF, BOD_OFF);
  wdt_enable(WDTO_8S);

I can at least get the pro mini to recover if a crash happens.....

[jremington]

@ardwrt

Is it normal for it to crash and then be stuck in some kind of infinite loop?

No. If that behavior is frequent, it is almost always due to a programming problem or unstable power supply -- problems that can be diagnosed and fixed.

But rare crashes can happen, for example for example when a cosmic ray corrupts a memory location. In those cases the watchdog timer could reboot the MCU.

The WDT is normally not used, but if it is, the code usually resets it at regular intervals.

[cdr_xavier]

You could just .. oh well. Yeah.
You could write your own "float to string" function or use some other printing method. Basically so you can clamp down the max string length.

This is year 2022. What is sprintf and dtostrv doing here.

To aid debug, you can set the sleep and wakeup period to be .. well, 5 second. This will make things 60x faster.

Although my primary concern/interest is in the overflow of time keeping variables. Although the symptoms (e.g. LEDs dimming/flashing) seems inconsistent.

[RayLivingston]

@ardwrt

Regarding failure modes, how should the pro mini deal with crashes? Would it be expected that it would recover and begin program execution from the beginning again? (I assume by running the code in setup()).

The very nature of a "crash" is it can do pretty much anything. There is no predicting, and not necessarily any recovering. When the program stomps on memory, anything can happen. By FAR the best way to deal with "crashes" is to write proper code that does not do stupid things that can lead to a crash. It is NOT an inherent characteristic of the processor. 99.99% of the time it is due to bugs in the software.

[ardwrt]

I've switched this now to send a struct:

struct bmedata{
  int temp;
  int press;
  int hum;
  int valid;
};

Valid just contains some expected data so can validate on the other end everything looks good....

This is then sent using:

  driver.send((uint8_t *)&bmedataStr, sizeof(struct bmedata));

Amazingly the Raspberry Pico on the receiving end seems to be able to decode this data using int16_t to represent the ints, mind blown

I think though perhaps the wiring on my breadboard is a little flakey, which might be causing it to behave weirdly sometimes. Lets see how it runs like this. Using ints definitely makes it easier to work with the data though.

Thanks for all who replied !

[cdr_xavier]

@ardwrt

int16_t

Well, you can always explicitly give the variables (in the struct) a type int16_t.

You could also, instead of using a struct, use a array.

The best way to handle printing characters, integers and floats (for 3rd-party libraries) is to overload the print function. Which can be a very big pain.

I wrote this

code

int8_t printNum
(unsigned int n, int8_t color)
{
  boolean FP = true;
  unsigned int TMP = n;
  unsigned short Digit = 1;
  while (FP) {
    TMP = TMP / 10;
    if (TMP > 0) {
      Digit = Digit + 1;
    } else {
      FP = false;
    }
  }
  char bufr[Digit + 1];

  char *str = &bufr[sizeof(bufr) - 1];

  *str = '\0'; //null terminated string

  // prevent crash if called with base == 1
  //if (base < 2) base = 10;
  do {
    char c = n % 10;
    n /= 10;
    *--str = c < 10 ? c + '0' : c + 'A' - 10;
  } while (n);
  return print(bufr, color);
}

Which works on arduino, but it decide to die on another mcu. Maybe it's the entire "null-terminated string" thing.