Time and if statement

Using Arduino Programming Questions
1

Hi,

I am having such an annoying problem and I can't seem to figure out why it's not working. I am working with a Nokia 5110 screen.
This is the code:

String getFormattedTime(byte format) {
  String timeMsg;
  
  if (format == 0x00) {
    timeMsg = prefixZero(hour()) + ":" + prefixZero(minute());
  } else if (format == 0x01) {
    timeMsg = prefixZero(hour()) + ":" + prefixZero(minute()) + ":" + prefixZero(second());
  } else if (format == 0x02) {
    timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + " " + (isAM() ? "am" : "pm");
  } else if (format == 0x03) {
    timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + ":" + prefixZero(second()) + " " + (isAM() ? "am" : "pm");
  }
  
  return timeMsg;
}


String prefixZero(int num) {
  if (num < 10)  return "0" + String(num);
  
  return String(num);
}

I call this function as follows, display.println(getFormattedTime(0x01)); // Displays the current time in a certain format
Problem is, when I call this function is doesn't return anything, timeMsg doesn't get set with any data, just an empty string. I have inserted Serial.println()'s just to check each condition to make sure that only the 0x01 condition is being run and it is. No other part of the if statement is run except the 0x01 condition, as it should. The serial monitor simply fills up with empty lines.
When I do this:

String getFormattedTime(byte format) {
  String timeMsg = prefixZero(hour()) + ":" + prefixZero(minute()) + ":" + prefixZero(second());
  
  return timeMsg;
}

the function returns the formatted time.
Only without the if statement does it seem to get the values for day(), month() and year(). As soon as I include the if statement (even if it's just one condition and I delete the others) the function fails and returns an empty string.

I have been looking at this code for days and can't for the life of me understand why it's not working. It doesn't make any sense!

Please if someone could point something out or tell me what's happening I would be very grateful.

Thanks!

2

Hard to tell without seeing the rest of the code...
Are you sure you're passing a "format" value between 0 and 3 to the function?

You could try this instead:

String getFormattedTime(byte format) {
  String timeMsg;
  
  switch(format)
  {
    case 0x01:
      timeMsg = prefixZero(hour()) + ":" + prefixZero(minute()) + ":" + prefixZero(second());
      break;

    case 0x02:
      timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + " " + (isAM() ? "am" : "pm");
      break;

    case 0x03:
      timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + ":" + prefixZero(second()) + " " + (isAM() ? "am" : "pm");
      break;

    default:
      timeMsg = prefixZero(hour()) + ":" + prefixZero(minute());
      break;
  }
  
  return timeMsg;
}
3

I will give it a go when I get home. Never bothered using switch statement as I didn't see why it would be different to if, but I will certainly try it! Definitely passing 0x01 to the function.

Thank you for the reply, will let you know.

4

Two problems.
timeMsg is declared to be local to the getFormattedTime function and therefore doesn't exist as soon as you return from it. So you are passing a pointer to a non-existant string. Declare timeMsg globally and that will fix one problem.

Second problem is using the String class. It is not a good idea to use it on a processor that has very little static ram, like 328-based Arduinos.

Pete

5

Thank you. I will take this on board.
Doesn't explain why the code I pasted without the if statement works though. It's still using local variable timeMsg, which returns a value and works?

6

Um..I thought that in C++, Objects are returned by value. ie. The return value of type String, which is an Object, will be a copy of the local timeMsg. Some compilers may optimize it to give the same memory location to avoid the overhead of copying. Not sure what Arduino compiler does. But in any case, I don't think that it is a problem.

My suggestion is to initialize the timeMsg variable to some value.

String timeMsg = "Unexpected format?";

So if none of your if conditions are met then it atleast returns the junk value as opposed to empty string. The switch-case suggested by in2str is doing something similar in "default:" case. Only thing is it is returning some time. But I'd rather be certain by returning something catchy that will let you know of the mistake.

Another way to do the same thing is to put in another simple "else" clause right at the end that catches it.

7

Thanks for the suggestion but i've already tried that. I know the if statement is definitely catching it at the right condition because i have previously put Serial.println()'s into each condition to see what happens. The serial monitor tells me that each time the correct condition is being caught. For example (if i didn't explain myself well), i did (including the idea to initialise the timeMsg variable):

String getFormattedTime(byte format) {
  String timeMsg = "hi";
 
  if (format == 0x00) {
    timeMsg = prefixZero(hour()) + ":" + prefixZero(minute());
    Serial.println("0");
  } else if (format == 0x01) {
    timeMsg = prefixZero(hour()) + ":" + prefixZero(minute()) + ":" + prefixZero(second());
    Serial.println("1");
  } else if (format == 0x02) {
    timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + " " + (isAM() ? "am" : "pm");
    Serial.println("2");
  } else if (format == 0x03) {
    timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + ":" + prefixZero(second()) + " " + (isAM() ? "am" : "pm");
    Serial.println("3");
  }
 
  return timeMsg;
}

Each time it will continually output "1" to the serial monitor, so timeMsg = prefixZero(hour()) + ":" + prefixZero(minute()) + ":" + prefixZero(second()); is definitely being executed and only that line; yet timeMsg will output "" constantly.... lol confusing..

8

Show the rest of the code....

9

Here's the code:

// EEPROM addresses
#define EEPROM_FIRST_TIME_SETUP_ADDR         0
#define EEPROM_LCD_BACKLIGHT_TIMER_ADDR      1
#define EEPROM_DATE_FORMAT_ADDR              2
#define EEPROM_TIME_FORMAT_ADDR              3

#define I2C_CONTROL_BOX_ADDR                 4
#define I2C_ATTINY85_ADDR                    2#define NO_PIN_STATE        // to indicate that you don't need the pinState

#define DISPLAY_MODE_IDLE                    0
#define DISPLAY_MODE_MENU                    1
#define DISPLAY_MODE_ALARM                   2


#include <Adafruit_GFX.h>
#include <Adafruit_PCD8544.h>
#include <Wire.h>
#include <EEPROM.h>
#include <Time.h>


const String MONTHS_STRING[12] = { "Jan", "Feb", "March", "April", "May", "June", "July", "Aug", "Sep", "Oct", "Nov", "Dec" };

Adafruit_PCD8544 display = Adafruit_PCD8544(7, 6, 5, 4, 3);

byte DATE_FORMAT, TIME_FORMAT, displayMode, displayMenu, displaySubMenu, numTriggers;



void setup() {
  Serial.begin(9600);
  
  display.begin();
  display.setContrast(50);
  
  numTriggers = displayMenu, displaySubMenu = 0x00;
  
  Wire.begin();
  
  DATE_FORMAT = 0x02;//EEPROM.read(EEPROM_DATE_FORMAT_ADDR);
  TIME_FORMAT = 0x01;//EEPROM.read(EEPROM_TIME_FORMAT_ADDR);
  
  
  Wire.beginTransmission(I2C_ATTINY85_ADDR);
  Wire.write(0x00);
  Wire.endTransmission();
  
  setTime(1, 45, 0, 29, 4, 2013);
  
  display.clearDisplay();
  display.setTextSize(1);
  display.setTextColor(BLACK);
  
  delay(2000); // Splash
  
  displayMode = DISPLAY_MODE_IDLE;
}


String getFormattedDate(byte format) {
  String dateMsg;
  
  if (format == 0x00) {
    dateMsg = String(day()) + "/" + String(month()) + "/" + String(year());
  } else if (format == 0x01) {
    dateMsg = String(month()) + "/" + String(day()) + "/" + String(year());
  } else if (format == 0x02) {
    dateMsg = String(day()) + " " + MONTHS_STRING[month() - 1] + " " + String(year());
  } else if (format == 0x03) {
    dateMsg = MONTHS_STRING[month() - 1] + " " + String(day()) + " " + String(year());
  }
  
  return dateMsg;
}


String getFormattedTime(byte format) {
  String timeMsg;
  
  if (format == 0x00) {
    timeMsg = prefixZero(hour()) + ":" + prefixZero(minute());
  } else if (format == 0x01) {
    timeMsg = prefixZero(hour()) + ":" + prefixZero(minute()) + ":" + prefixZero(second());
  } else if (format == 0x02) {
    timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + " " + (isAM() ? "am" : "pm");
  } else if (format == 0x03) {
    timeMsg = prefixZero(hourFormat12()) + ":" + prefixZero(minute()) + ":" + prefixZero(second()) + " " + (isAM() ? "am" : "pm");
  }
  
  return timeMsg;
}


String prefixZero(int num) {
  if (num < 10)  return "0" + String(num);
  
  return String(num);
}


void displayModeIdle() {
  display.clearDisplay();
  display.setCursor(0, 0);
  //display.println(getFormattedDate(DATE_FORMAT));
  display.println(getFormattedTime(TIME_FORMAT));
  display.println();
  display.print("0 new alerts.");
  display.display();
}


void displayModeMenu() {
  
}


void displayModeAlarm() {
  
}


void loop() {
  if (displayMode == DISPLAY_MODE_IDLE) {
    displayModeIdle();
  } else if (displayMode == DISPLAY_MODE_MENU) {
    displayModeMenu();
  } else if (displayMode == DISPLAY_MODE_ALARM) {
    displayModeAlarm();
  }

  delay(5);
}
10

Thank you. I will take this on board.

But did you try it?

Pete

11

Yeah, still the same Pete :frowning:

12

Which arduino are you using?

Pete

13

Uno Rev 3

14

If you did have the extra "else" clause then why did you remove it?
It is still a good practice to leave it in and handle the error condition appropriately.

So if you have tried so many print to serial statements, have you tried printing the return value of timeMsg just prior to returning it and then just after getting it back in the calling function?

15

@skippy.... Just as a matter of interest, and not related to your problem, are you using level shifting to get the logic down to 3.3v from 5? I've seen circuits which (apart from the 3.3v supply to the screen) shoot the Uno's 5v from SCK, MOSI etc straight into the screen, and 5v power into the backlight through 330R. Can you let me know how you've hooked yours up please?