Arduino Reboots after one loop - Formatting of Date Time from DS1307

The post exceeded the 9500 character limit so I broke it over 2 posts. (1 of 2)

I am using an Arduino to control a clock/light that I am building. I am using an ATMEGA328P with the "Aduino Duemilanove w/ ATmega328" bootloader. I am also using the DS1307 Real Time Clock breakout board kit -ID: 264 from Adafruit. (DS1307 Real Time Clock breakout board kit : ID 264 : $7.95 : Adafruit Industries, Unique & fun DIY electronics and kits)

I was using the String object to format the date and time to display on an LCD but I was having issues with it not working and I assumed that it may be due to the use of the String object so I rewrote the code to use char arrays and it is working a bit better but now I notice that if the output string is 34 characters or more, the Arduino reboots.

I did go a bit overboard with the formatting which is more than I really need but I wanted to make something that others could easily reuse. It has been over 15 years since I did anything with pointers so I believe I did something wrong in the code which is causing some memory to overflow which is causing the Arduino to reboot.

Attached is the complete code. Below is the code listing for the Beta00A.ino and rtclock.ino files for your quick reference.

Beta00A.ino

#include <avr/io.h>
#include <avr/interrupt.h>

#define DISP_DEBUG true
#define DISP_DEBUG2 true

// Keypad
#define NUM_BUTTONS (sizeof(button_pins)/sizeof(uint8_t)) //array size
#define LED_PIN 8
#define BUTTON_PWR_IDX 0
#define BUTTON_DIM_IDX 1
#define BUTTON_BRITE_IDX 2
#define BUTTON_PREVIOUS_IDX 3
#define BUTTON_NEXT_IDX 4

uint8_t button_pins[] = {
  2, 3, 4, 5, 7};
bool buttons_touched[] = {
  false, false, false, false, false};
String buttons_function[] = {
  "POWER ", "DIM   ", "BRIGHT", "MINUS ", "PLUS  "};

// Neopixels
#include <Adafruit_NeoPixel.h>
#define NEOPIXEL_PIN 6
#define BRIGHTSTEP 25
#define MAX_PATTERN 3

// Parameter 1 = number of pixels in strip
// Parameter 2 = pin number (most are valid)
// Parameter 3 = pixel type flags, add together as needed:
//   NEO_KHZ800  800 KHz bitstream (most NeoPixel products w/WS2812 LEDs)
//   NEO_KHZ400  400 KHz (classic 'v1' (not v2) FLORA pixels, WS2811 drivers)
//   NEO_GRB     Pixels are wired for GRB bitstream (most NeoPixel products)
//   NEO_RGB     Pixels are wired for RGB bitstream (v1 FLORA pixels, not v2)
Adafruit_NeoPixel strip = Adafruit_NeoPixel(116, NEOPIXEL_PIN, NEO_GRB + NEO_KHZ800);

uint8_t nextLEDdelay = 100;
uint8_t nextPatternDelay = 500;
int neopixel_brightness = 60;

int pattern = 0;
bool powerOn = true;
//Flags from keypad
bool changePattern = false;
bool changeBrightness = false;
bool changePower = false;


// *** SETUP ***
void setup() {  
  if(DISP_DEBUG || DISP_DEBUG2) {
    Serial.begin(9600);
  }

  neopixel_setup();

  pinMode(LED_PIN, OUTPUT);

  buttons_setup();

  rtclock_setup();

  // Flash LED to let us know we are ready
  for(uint8_t i=0; i<4; i++){
    toggle_LED();
    delay(500);
  }
  digitalWrite(LED_PIN, LOW);
  Serial.print("COMPLETED SETUP");
  Serial.println();
}

// *** MAIN LOOP ****
void loop() {
  char myTimeFormat[] = "dddd d/M MM d yyyy h:mm:ss tt";
  getTime(myTimeFormat);
  Serial.println(myTimeFormat);
  delay(3000);
}

void toggle_LED() {
  digitalWrite(LED_PIN, !digitalRead(LED_PIN));
}

(Continued on next post)

Beta00A.zip (4.55 KB)

The post exceeded the 9500 character limit so I broke it over 2 posts. (2 of 2)
(Continued from previous post)

rtclock.ino

#include <malloc.h>
#include <Wire.h>
#include "RTClib.h"

#define MAXDTSTRINGLEN 100
#define DEFAULTDATEFMT "dddd d MMMM yyyy h:mm:ss tt"

RTC_DS1307 rtc;


char daynamesfull[7][10] = {
  "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};
char daynamesabbr[7][4] = {
  "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
char monthnamesfull[12][10] = {
  "January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"};
char monthnamesabbr[12][4] = {
  "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};

void getTime(char *dateformat);
void getTimePart(char *timepart);
void strcatchar(char *str1, char char1);
char *substring(char *string, int position, int length);

void rtclock_setup() {
#ifdef AVR
  Wire.begin();
#else
  Wire1.begin(); // Shield I2C pins connect to alt I2C bus on Arduino Due
#endif
  rtc.begin();

  // following line sets the RTC to the date & time this sketch was compiled
  rtc.adjust(DateTime(__DATE__, __TIME__));
}

void getTime(char *formatedDateTime) {
  char displayDateTimeFormat[MAXDTSTRINGLEN];
  strcpy(displayDateTimeFormat, formatedDateTime);

  if(strlen(displayDateTimeFormat)==0) {
    strcpy(displayDateTimeFormat, DEFAULTDATEFMT);
  }

  // Initialize formatedDateTime
  strcpy(formatedDateTime, "");

  char lastChar = displayDateTimeFormat[0];
  char thisChar = '\0';
  char currentPart[MAXDTSTRINGLEN];
  // Initialize currentPart
  strcpy(currentPart, "");
  strcatchar(currentPart, lastChar);
  for(size_t pos=1; pos<strlen(displayDateTimeFormat); pos++) {
    thisChar = displayDateTimeFormat[pos];

    if(thisChar != lastChar || pos==strlen(displayDateTimeFormat)-1) {
      if(pos==strlen(displayDateTimeFormat)-1 && thisChar==lastChar) {
        strcatchar(currentPart, thisChar);
      }
      getTimePart(currentPart);
      strcat(formatedDateTime, currentPart);

      strcpy(currentPart, "");
      strcatchar(currentPart, thisChar);

      if(pos==strlen(displayDateTimeFormat)-1 && thisChar!=lastChar) {
        getTimePart(currentPart);
        strcat(formatedDateTime, currentPart);
      }
    }
    else {
      strcatchar(currentPart, thisChar);
    }
    lastChar=thisChar;
  }
}

void getTimePart(char *timepart) {
  // REF: http://msdn.microsoft.com/en-us/library/8kb3ddd4(v=vs.110).aspx
  // Not supported are f, F, g, K, z
  char validfromatchars[] = "dhHmMsty";
  DateTime now = rtc.now();

  char *q = strchr(validfromatchars, timepart[0]);
  if(!q) {
    return;
  }

  // Handle day formats
  if(timepart[0]=='d') {
    switch(strlen(timepart)) {
    case 4:
      strcpy(timepart, daynamesfull[now.dayOfWeek()]);
      break;
    case 3:
      strcpy(timepart, daynamesabbr[now.dayOfWeek()]);
      break;
    case 2:
      if(now.day() < 10) {
        sprintf(timepart, "0%d", now.day());
        break;
      }
    case 1:
      sprintf(timepart, "%d", now.day());
      break;
    default:
      return;
      break;
    }
  }
  // Handle 12 hour formats
  else if(timepart[0]=='h') {
    switch(strlen(timepart)) {
    case 2:
      if(now.hour() == 0) {
        strcpy(timepart, "12");
        break;
      }
      else if(now.hour() < 10) {
        sprintf(timepart, "0%d", now.hour());
        break;
      }
    case 1:
      if(now.hour() == 0) {
        strcpy(timepart, "12");
      }
      else if(now.hour() > 12) {
        sprintf(timepart, "%d", now.hour()-12);
      }
      else {
        sprintf(timepart, "%d", now.hour());
      }
      break;
    default:
      return;
      break;
    }
  }
  // Handle 24 hour formats
  else if(timepart[0]=='H') {
    switch(strlen(timepart)) {
    case 2:
      if(now.hour() < 10) {
        sprintf(timepart, "0%d", now.hour());
        break;
      }
    case 1:
      sprintf(timepart, "%d", now.hour());
      break;
    default:
      return;
      break;
    }
  }
  // Handle minute formats
  else if(timepart[0]=='m') {
    switch(strlen(timepart)) {
    case 2:
      if(now.minute() < 10) {
        sprintf(timepart, "0%d", now.minute());
        break;
      }
    case 1:
      sprintf(timepart, "%d", now.minute());
      break;
    default:
      return;
      break;
    }
  }
  // Handle month formats
  else if(timepart[0]=='M') {
    switch(strlen(timepart)) {
    case 4:
      strcpy(timepart, monthnamesfull[now.month()-1]);
      break;
    case 3:
      strcpy(timepart, monthnamesabbr[now.month()-1]);
      break;
    case 2:
      if(now.month() < 10) {
        sprintf(timepart, "0%d", now.month());
        break;
      }
    case 1:
      sprintf(timepart, "%d", now.month());
      break;
    default:
      return;
      break;
    }
  }
  // Handle second formats
  else if(timepart[0]=='s') {
    switch(strlen(timepart)) {
    case 2:
      if(now.second() < 10) {
        sprintf(timepart, "0%d", now.second());
        break;
      }
    case 1:
      sprintf(timepart, "%d", now.second());
      break;
    default:
      return;
      break;
    }
  }
  // Handle AM/PM designator formats
  else if(timepart[0]=='t') {
    switch(strlen(timepart)) {
    case 2:
      if(now.hour() < 12) {
        strcpy(timepart, "AM");
      }
      else{
        strcpy(timepart, "PM");
      }
      break;
    case 1:
      if(now.hour() < 12) {
        strcpy(timepart, "A");
      }
      else{
        strcpy(timepart, "P");
      }
      break;
    default:
      return;
      break;
    }
  }
  // Handle year formats
  else if(timepart[0]=='y') {
    char tempq[10] = "";
    sprintf(tempq, "%d", now.year());
    switch(strlen(timepart)) {
    case 5:
      if(now.year()>9999) { 
        sprintf(timepart, "%d", now.year()); 
      }
      else if(now.year()>999) { 
        sprintf(timepart, "0%d", now.year()); 
      }
      else if(now.year()>99) { 
        sprintf(timepart, "00%d", now.year()); 
      }
      else if(now.year()>9) { 
        sprintf(timepart, "000%d", now.year()); 
      }
      else { 
        sprintf(timepart, "0000%d", now.year()); 
      }
      break;
    case 4:
      if(now.year()>999) { 
        sprintf(timepart, "%d", now.year()); 
      }
      else if(now.year()>99) { 
        sprintf(timepart, "0%d", now.year()); 
      }
      else if(now.year()>9) { 
        sprintf(timepart, "00%d", now.year()); 
      }
      else { 
        sprintf(timepart, "000%d", now.year()); 
      }
      break;
    case 3:
      if(now.year()>99) { 
        sprintf(timepart, "%d", now.year()); 
      }
      else if(now.year()>9) { 
        sprintf(timepart, "0%d", now.year()); 
      }
      else { 
        sprintf(timepart, "00%d", now.year()); 
      }
      break;
    case 2:
      if(strlen(tempq) < 2) {
        sprintf(timepart, "0%s", tempq);
      }
      else if(strlen(tempq) == 2) {
        sprintf(timepart, "%s", tempq);
      }
      else {
        strcpy(timepart, substring(tempq, strlen(tempq)-1, 2));
      }
      break;
    case 1:
      strcpy(timepart, substring(tempq, strlen(tempq), 1));
      break;
    default:
      break;
    }
  }
}

void strcatchar(char *str1, char char1) {
  strcat(str1, " ");
  str1[strlen(str1)-1] = char1;
}

char *substring(char *string, int position, int length) {
  char *pointer;
  int c;

  pointer = (char*) malloc(length+1);

  if (pointer == NULL)
  {
    return pointer;
  }

  for (c = 0 ; c < position -1 ; c++) 
    string++; 

  for (c = 0 ; c < length ; c++)
  {
    *(pointer+c) = *string;      
    string++;   
  }

  *(pointer+c) = '\0';

  return pointer;
}

Here is the serial output with it running on 26 January 2014 at 11:24 AM.

COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM
COMPLETED SETUP
Sunday 26/1 01 26 2014 11:24:30 AM

As you can see, it is running one time and rebooting the Arduino. I have done several tests and found that it will only reboot if the output is 34 characters or more.

Thank you in advance for pointing out any issues that I may have with the code. I would also appreciate any information on a better way to solve the same problem.

String buttons_function[] = {
  "POWER ", "DIM   ", "BRIGHT", "MINUS ", "PLUS  "};

NOTHING here warrants pissing away resources on the String class.

#define MAXDTSTRINGLEN 100

Get real. No reasonable combination of date and time takes 100 characters.

How much memory do you have available?
http://playground.arduino.cc/Code/AvailableMemory
Not enough is my guess.

PaulS,

You are correct on your comment regarding "NOTHING here warrants pissing away resources on the String class." This was some debugging stuff that will be removed.

You are also correct regarding "Get real. No reasonable combination of date and time takes 100 characters." Again this is for some unit testing that I am doing. I did lower it much lower and it did not make a difference.

I believe I did find the problem. In the loop, I did not define a length for myTimeFormat. Once I defined a length for it, I was able to do as I pleased with it. In hind site this does make sense as this is the char array that the result will be written to and displayed.

Hopefully this helps someone in the future. Attached is the updated code with the fix for reference. You can also see that I did several other things to try to free up memory but they did not work as that was not the root of the problem.

Updated Loop

// *** MAIN LOOP ****
void loop() {
  char myTimeFormat[255] = "dddd d MMMM yyyy h:mm:ss tt MMMM MMMM MMMM MMMM MMMM MMMM MMMM MMMM MMMM yyyy yyyy yyyy yyyy";
  getTime(myTimeFormat);
  Serial.println(myTimeFormat);
  delay(3000);
  toggle_LED();
}

Updated Result

COMPLETED SETUP
Sunday 26 January 2014 3:10:12 PM January January January January January January January January January 2014 2014 2014 2014
Sunday 26 January 2014 3:10:15 PM January January January January January January January January January 2014 2014 2014 2014
Sunday 26 January 2014 3:10:18 PM January January January January January January January January January 2014 2014 2014 2014
Sunday 26 January 2014 3:10:21 PM January January January January January January January January January 2014 2014 2014 2014
Sunday 26 January 2014 3:10:24 PM January January January January January January January January January 2014 2014 2014 2014
Sunday 26 January 2014 3:10:27 PM January January January January January January January January January 2014 2014 2014 2014
Sunday 26 January 2014 3:10:30 PM January January January January January January January January January 2014 2014 2014 2014

Beta00A.zip (5.77 KB)

Attached are library files and test app. This requires the Real Time Clock hardware connected to I2C and the RTClib library from JeeLabs. (GitHub - adafruit/RTClib: A fork of Jeelab's fantastic RTC Arduino library)

Test001_RTC.zip (4.01 KB)

PaulS:

String buttons_function[] = {

"POWER ", "DIM   ", "BRIGHT", "MINUS ", "PLUS  "};



NOTHING here warrants pissing away resources on the String class.



#define MAXDTSTRINGLEN 100



Get real. No reasonable combination of date and time takes 100 characters.

How much memory do you have available?
http://playground.arduino.cc/Code/AvailableMemory
Not enough is my guess.

Considering that he's sprintf'ing to char pointers and not buffers of sufficient size, I'm amazed it even works as far as it does.

Considering that he's sprintf'ing to char pointers and not buffers of sufficient size, I'm amazed it even works as far as it does.

No, he's not. In the function, the memory location is referenced by a pointer. The caller passes the address of an array that is properly allocated (although on the stack, so it might go away when the function ends, so care in its use is important).

PaulS:

Considering that he's sprintf'ing to char pointers and not buffers of sufficient size, I'm amazed it even works as far as it does.

No, he's not. In the function, the memory location is referenced by a pointer. The caller passes the address of an array that is properly allocated (although on the stack, so it might go away when the function ends, so care in its use is important).

I'll take your word for it, because I'm not seeing it...

Besides, what's wrong with a simple:

char buffer [32];
sprintf (buffer, "Time: %02u:%02u:02u\r\n", hours, mins, secs);
Serial.print (buffer);

It amazes me how UNBELIEVABLY complex people write code.

I'll take your word for it, because I'm not seeing it...

sprintf is called in getTimePart:

        sprintf(timepart, "0%d", now.day());

The signature for getTimePart() looks like:

void getTimePart(char *timepart) {

where I can see that you think timepart is a pointer. It is, but it points to something because the caller passed a pointer to the function. The function is called in getTime():

      getTimePart(currentPart);

The argument is defined earlier:

  char currentPart[MAXDTSTRINGLEN];

So, a pointer to real space is passed to the function, so the function can safely write to the memory reserved for the array.

It amazes me how UNBELIEVABLY complex people write code.

No argument from me.

PaulS:

I'll take your word for it, because I'm not seeing it...

sprintf is called in getTimePart:

        sprintf(timepart, "0%d", now.day());

The signature for getTimePart() looks like:

void getTimePart(char *timepart) {

where I can see that you think timepart is a pointer. It is, but it points to something because the caller passed a pointer to the function. The function is called in getTime():

      getTimePart(currentPart);

The argument is defined earlier:

  char currentPart[MAXDTSTRINGLEN];

So, a pointer to real space is passed to the function, so the function can safely write to the memory reserved for the array.

It amazes me how UNBELIEVABLY complex people write code.

No argument from me.

OK NOW I see it. Thanks for pointing it out.

Now just to be sure I understand, what he's doing is more or less like this:

char buffer [100]; // defined buffer space
char *ptr; // just a pointer
ptr = buffer; // ptr now points to the 100 byte space
sprintf (ptr, ......); // sprintf to buffer via ptr

Right?