Code crashing after a few days

I'm an Arduino n00b but not an absolute beginner.

The issue is that I can have this code running for days or sometimes just hours.

I've attached it as mainCodelisting which I've run this on the Arduino Uno with no issues. Then I switched to just the atmega328p. I've attached the schematic.

The power to it is coming off of another Arduino connected to the PC USB which I've read will give at a minimum 500mA.

The AltSoftSerial is to send the temperatures to another microcontroller, and the hardware serial to debug on the PC.

I'm using 3 instances of the OneWire library since getting them all (several meters away) to report one one pin has been a nightmare, so I made the code as it is to see if I can get 4 DS28EA00U probes to work on 3 pins.

I'm just wondering if I'm having any sort of Memory leak.

In another variant, used a character array since everyone that mentions String is instantly pelted with rotten vegetables. I've attached that as charArrayCnippet.

To be able to use printf with floats I followed some tutorial that said to edit boards.txt and add this

diecimila.menu.printf.default=Default printf
diecimila.menu.printf.default.compiler.c.elf.extra_flags=
diecimila.menu.printf.full=Full printf
diecimila.menu.printf.full.compiler.c.elf.extra_flags=-Wl,-u,vfprintf -lprintf_flt
diecimila.menu.printf.minimal=Minimal printf
diecimila.menu.printf.minimal.compiler.c.elf.extra_flags=-Wl,-u,vfprintf -lprintf_min

One last thing I've tried is to print free memory using

#ifdef __arm__
// should use uinstd.h to define sbrk but Due causes a conflict
extern "C" char* sbrk(int incr);
#else  // __ARM__
extern char *__brkval;
#endif  // __arm__

int freeMemory() {
  char top;
#ifdef __arm__
  return &top - reinterpret_cast<char*>(sbrk(0));
#elif defined(CORE_TEENSY) || (ARDUINO > 103 && ARDUINO != 151)
  return &top - __brkval;
#else  // __arm__
  return __brkval ? &top - __brkval : &top - __malloc_heap_start;
#endif  // __arm__
}

Then I would print out the free memory before and after the calls processOneWire.
In both instances I didn't see any dramatic decline in free memory.

I've ordered an Atmel JTAGICE, but it won't be here for some time.

Any help is appreciated.

mainCodelisting.ino (4.9 KB)

charArraySnippet.cpp (719 Bytes)

Besides Strings, another thing that can cause the behavior that you describe is reading or writing outside the bounds of an array. Examine all operations on arrays to make sure that none are exceeding array bounds.

Your "schematic" does not show the required decoupling caps on Vcc to ground and AVcc to ground. And, optional, Aref to ground if using the ADC.

More members will see your code if you post the code in accordance with the forum guidelines.

Thank you very much groundFungus. I've added the caps and will see how long it runs. I'll also try to put the code up. It's long, so I attached it as How to get the best out of this forum - Programming Questions - Arduino Forum suggested:

Here's the main code:

#include <OneWire.h>// http://www.pjrc.com/teensy/td_libs_OneWire.html
#include <AltSoftSerial.h>
#include<Wire.h>

AltSoftSerial temperatureAltSerial;

int baudRate = 9600;

void setup(void) {
  Serial.begin(baudRate);
  temperatureAltSerial.begin(baudRate);
}

int oneWireSerialPinIndex = 0;
int countOfTemperaturePins = 4;
OneWire ds1(3);
OneWire ds2(4);
OneWire ds3(5);
float temps[] = {0, 0, 0, 0};
int currentTempIndex = 0;

bool showOneWireRawData = false;

void processOneWire(OneWire oneWire, int pinNumber){
  while(true){
    Serial.print("One Wire Pin: ");
    Serial.println(pinNumber);
  
    byte i;
    byte present = 0;
    byte type_s;
    byte data[12];
    byte addr[8];
    float celsius, fahrenheit;
    
    if (!oneWire.search(addr)) {
      Serial.println("No more addresses.");
      Serial.println();
      oneWire.reset_search();
      delay(250);
      
      break;
    }

    if(showOneWireRawData){
      Serial.print("ROM =");
      for( i = 0; i < 8; i++) {
        Serial.write(' ');
        Serial.print(addr[i], HEX);
      }
    }
    
    if (OneWire::crc8(addr, 7) != addr[7]) {
      if(showOneWireRawData){
        Serial.println("CRC is not valid!");
      }
        
      return;
    }
    Serial.println();
   
    // the first ROM byte indicates which chip
    switch (addr[0]) {
      case 0x10:
        Serial.println("  Chip = DS18S20");
        type_s = 1;
        break;
      case 0x28:
        Serial.println("  Chip = DS18B20");
        type_s = 0;
        break;
      case 0x22:
        Serial.println("  Chip = DS1822");
        type_s = 0;
        break;
      case 0x42:
        Serial.println("  Chip = DS28EA00U");
        type_s = 0;
        break;
      default:
        Serial.println("Device is not a DS18x20 family device.");
        return;
    } 
  
    oneWire.reset();
    oneWire.select(addr);
    oneWire.write(0x44, 1);        // start conversion, with parasite power on at the end
    
    delay(1000);     // maybe 750ms is enough, maybe not
    // we might do a ds.depower() here, but the reset will take care of it.
    
    present = oneWire.reset();
    oneWire.select(addr);    
    oneWire.write(0xBE);         // Read Scratchpad
    if(showOneWireRawData){
      Serial.print("  Data = ");
      Serial.print(present, HEX);
      Serial.print(" ");  
    }
    
    for ( i = 0; i < 9; i++) {           // we need 9 bytes
      data[i] = oneWire.read();
      Serial.print(data[i], HEX);
      Serial.print(" ");
    }
    Serial.print(" CRC=");
    Serial.print(OneWire::crc8(data, 8), HEX);
    Serial.println();
  
    // Convert the data to actual temperature
    // because the result is a 16 bit signed integer, it should
    // be stored to an "int16_t" type, which is always 16 bits
    // even when compiled on a 32 bit processor.
    int16_t raw = (data[1] << 8) | data[0];
    if (type_s) {
      raw = raw << 3; // 9 bit resolution default
      if (data[7] == 0x10) {
        // "count remain" gives full 12 bit resolution
        raw = (raw & 0xFFF0) + 12 - data[6];
      }
    } else {
      byte cfg = (data[4] & 0x60);
      // at lower res, the low bits are undefined, so let's zero them
      if (cfg == 0x00) raw = raw & ~7;  // 9 bit resolution, 93.75 ms
      else if (cfg == 0x20) raw = raw & ~3; // 10 bit res, 187.5 ms
      else if (cfg == 0x40) raw = raw & ~1; // 11 bit res, 375 ms
      //// default is 12 bit resolution, 750 ms conversion time
    }
    celsius = (float)raw / 16.0;
    temps[currentTempIndex] = celsius;
    currentTempIndex++;
    if(currentTempIndex >= 4){ // once the end of the index is reached, write the data and
      currentTempIndex = 0;
      temperatureAltSerial.write((char)50); // header
      temperatureAltSerial.write((char)60); // header
      temperatureAltSerial.write((char)74); // says that it is for temps
      String tempMsg = "[" + String(temps[0]) + "," + String(temps[1]) + "," + String(temps[2]) + "," + String(temps[3]) + "]";
      temperatureAltSerial.write((char)tempMsg.length());
      temperatureAltSerial.print(tempMsg);
      temperatureAltSerial.flush();
      for(int i = 0; i < 4; i++){
        temps[i] = 85; // reset and put in values that will indicate error  
      }
      
    }

    fahrenheit = celsius * 1.8 + 32.0;
    Serial.print("  Temperature = ");
    Serial.print(celsius);
    Serial.print(" Celsius, ");
    Serial.print(fahrenheit);
    Serial.println(" Fahrenheit");
    Serial.println("");  
  } // end while
  
}

unsigned long lastMillis;
int secondsToReadOneWire = 20;//120;

void loop(void) {
  if (millis() - lastMillis >= secondsToReadOneWire * 1000UL){
    Serial.println("Processing Temps line 254");
    processOneWire(ds1, 3);
    processOneWire(ds2, 4);
    processOneWire(ds3, 5);
    lastMillis = millis();
  }// end seconds checker 
}

I tried using a char array instead of String:

if(countOfTemperaturePins >= countOfTemperaturePins){ // once the end of the index is reached, write the data and
      currentTempIndex = 0;

      int tempReportLength = 40; // the max should be 30 even if negative and two digits on each side
      char tempReportBuffer[tempReportLength];
      int charactersToPrint = snprintf(tempReportBuffer, tempReportLength, "[%.2f,%.2f,%.2f,%.2f]", temps[0], temps[1], temps[2], temps[3]);

      Serial.print("experiment: ");
      Serial.write(tempReportBuffer, charactersToPrint);
      Serial.println("");

      for(int i = 0; i < countOfTemperaturePins; i++){
        temps[i] = 85; // reset and put in values that will indicate error  
      }      
    }

It still crashed.

 String tempMsg = "[" + String(temps[0]) + "," + String(temps[1]) + "," + String(temps[2]) + "," + String(temps[3]) + "]";

I thought that you weren't using Strings.

The Evils of Arduino Strings discusses why Strings can corrupt memory and cause weird hard to find bugs. That kind of concatenation can lead to holes in the heap.

I would use sprintf() to replace that.

I used strings at first, and then ran a separate one with the char array and snprintf to see if that would help. They both crashed.

Insert some serial prints at strategic places to find out where it is crashing.

abs0:
It still crashed.

How do you know?
"It crashed" doesn't tell us anything.

SteveMann:
How do you know?
"It crashed" doesn't tell us anything.

It stopped sending out any amount of serial data.
I've put added the decoupling caps and let it run over night, so I'm very hopeful of that being the solution. I'll let it run all week and over the weekend and see what happens. I also had a simple blinky sketch running on the atmel328p and even it stopped working without the capacitors.

Nothing wrong with using Strings, see my tutorial on Taming Arduino Strings

If you prefer to use char[]s then you can use my SafeString library which will give you the safety and ease of use of Strings but also output error messages if you try to access outside the bounds of the underlying char[] that would crash your program.

If you have a timing problem, Serial prints() can block see my tutorial on Arduino Serial I/O for the Real World

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.