Setting the time with Time library

roli · March 24, 2010, 3:35pm

I have a little troube with the Arduino Time library:
http://www.arduino.cc/playground/Code/Time

When I set the time on the the device only the minute display shows the correct value - If I set 02:02, the device will display 00:02. If I set 02:59, the device will display 00:59 and after a minute it will display 01:00. So my display function that I have written is probably working. The problem seems to be in the time setting function. But I can't find it.

This is the function that sets the time:

void setMyTime()
{
  writeToDisplay('x', displayPin1); //clear display 1
  writeToDisplay('x', displayPin2); //clear display 2 
  writeToDisplay('x', displayPin3); //clear display 3
  writeToDisplay('x', displayPin4); //clear display 4
  
  int onScreen = 0;
  char temp[1];
  char hours[2];
  char minutes[2];
  
  writeToDisplay('_', displayPin1);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 3)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin1);
      onScreen++;
      delay(100);
    }
  }
  hours[0] = temp[0];
  writeToDisplay('x', displayPin1);
  onScreen = 0;
  delay(400);
  
  writeToDisplay('_', displayPin2);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 10)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin2);
      onScreen++;
      delay(100);
    }
  }
  hours[1] = temp[0];
  writeToDisplay('x', displayPin2);
  onScreen = 0;
  delay(400);
  
  writeToDisplay('_', displayPin3);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 6)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin3);
      onScreen++;
      delay(100);
    }
  }
  minutes[0] = temp[0];
  writeToDisplay('x', displayPin3);
  onScreen = 0;
  delay(400);
  
  writeToDisplay('_', displayPin4);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 10)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin4);
      onScreen++;
      delay(100);
    }
  }
  minutes[1] = temp[0];
  writeToDisplay('x', displayPin4);
  delay(400);
  
  int hrs = atoi(hours);
  int mins = atoi(minutes);
  
  setTime(hrs,mins,0,1,1,2010);
   
}

And this is the function that displays time:

void printTime()
{
  MsTimer2::stop();
  time_t curr_time = now();
  
  int hours = hour(curr_time);
  int minutes = minute(curr_time);
  
  char hours_temp[2];
  char minutes_temp[2];
  
  itoa (hours, hours_temp, 10);
  itoa (minutes, minutes_temp, 10);
  
   
  if (hours < 10)
  {
    char temp = hours_temp[0];
    hours_temp[0] = '0';
    hours_temp[1] = temp;
  }
  
  if (minutes < 10)
  {
    char temp = minutes_temp[0];
    minutes_temp[0] = '0';
    minutes_temp[1] = temp;
  }
  
  
  writeToDisplay(hours_temp[0], displayPin1);
  writeToDisplay(hours_temp[1], displayPin2);
  
  writeToDisplay(minutes_temp[0], displayPin3);
  writeToDisplay(minutes_temp[1], displayPin4);
  MsTimer2::start();
  
}

system · March 24, 2010, 4:47pm

  char temp[1];
  char hours[2];
  char minutes[2];

Why is temp an array of length 1? Why not just:

char temp;

The hours and minutes arrays are character arrays.

  int hrs = atoi(hours);
  int mins = atoi(minutes);

The atoi function takes a string.

The difference between an array of characters and a string is that a string is NULL terminated. You are not NULL terminating your arrays. Therefore, they are not strings. Therefore, the output from atoi is probably not what you think it is. Therefore, you are not actually setting the time that you think you are.

roli · March 24, 2010, 6:53pm

Why is temp an array of length 1? Why not just:

Because of this: itoa(onScreen, temp, 10);

The hours and minutes arrays are character arrays.

Code:

int hrs = atoi(hours);
int mins = atoi(minutes);

The atoi function takes a string.

The difference between an array of characters and a string is that a string is NULL terminated. You are not NULL terminating your arrays. Therefore, they are not strings. Therefore, the output from atoi is probably not what you think it is. Therefore, you are not actually setting the time that you think you are.

I am such an idiot! You were right! Only strange thing here is that it worked for minutes for some reason.

Coding_Badly · March 24, 2010, 9:18pm

Why is temp an array of length 1? Why not just:

Because of this: itoa(onScreen, temp, 10);

In that case, temp is too small. It needs to have one element for each digit plus a terminating null. From what I can tell, the largest value is 10 so temp needs to be...

  char temp[3];  // "10" + null = 2 characters + terminator = 3

roli · March 24, 2010, 10:40pm

max digit possible in temp is 9. And it doesn't need terminating character because I only use the temp variable to send single character to my "print on display function" and to store single character into a bigger C style string.

roli · March 24, 2010, 11:28pm

I have just found another problem. The library is very inacurate for some reason. I have used the original DateTime library a year or two ago and never had this problem. If I set the time to 0:0 at 0:0:0, when the time reaches 0:1:0 - the Time library still shows 0:0 and when the time is 0:1:10, the library switches to 0:1. When the time is 0:2:00, arduino still shows 0:1:0 and it stays that way until 0:3:0 - at which point it switches to 0:2:00. And this delay gets bigger every time.

I am using MsTimer2 library for refreshing the displays every 1ms and a few delays but that shouldn't do anything to Time library (at least it worked with the original).

Could it be that I am using custom built board with Atmega8 with 8MHz crystal? Before I was using Atmega168 with 16MHz crystal.

Coding_Badly · March 25, 2010, 3:44am

max digit possible in temp is 9

Ah, so it is.

And it doesn't need terminating character because I

How you use it is irrelevant. itoa always stores the terminator. If you do not provide space for it, the byte afterwards is overwritten.

system · March 25, 2010, 9:13am

Only strange thing here is that it worked for minutes for some reason.

Luck. That is the reason.

The atoi function looks at memory locations starting at the first one specified, and moving to the next, and the next, etc., until it finds a NULL. Then, it knows it has reached the end of the string. It does not know anything about the length of the array. So, if you pass it an array of length 2, all it knows is where the array starts. It will happily march along that array, until it finds a NULL in position 14, and then try to convert that string to an integer.

With minutes array, the first location in memory after the array just happened to contain a 0, which looks like a NULL.

roli · March 25, 2010, 4:29pm

Ok so I corrected that. It couldn't store the null character into the variable but that shouldn't be that big of a problem.

The main problem now is the accuracy - I posted about it a few posts higher. I can't find a reason for time getting so inacurate after just one minute.

EDIT: Yust tryed the same code with Atmega168 - the code has accuracy problems here too - but the time here is too fast. Atmega8 - too slow, Atmega168 - too fast. Does anyone have an idea what is wrong? And after a few minutes a minute is 60s. At least on Atmega168. On Atmega8 the problem is there forewer.

system · March 25, 2010, 4:56pm

I'm relatively sure that the library is not the source of your problem. Post your code, so we can see what other problems you might have.

roli · March 25, 2010, 4:58pm

Here is a complete code (still work in progress):

/*
pin 2 - segment C
pin 3 - segment D
pin 4 - segment E
pin 5 - speaker
pin 6 - segment F
pin 7 - segment A
pin 8 - segment B
pin 9 - segment G
pin 10 - display 1
pin 11 - display 2
pin 12 - display 3
pin 13 - display 4
pin 14 (analog 0) - temp_data
pin 15 (analog 1) - temp_clk
pin 16 (analog 2) - LED
pin 17 (analog 3) - SW3
pin 18 (analog 4) - SW2
pin 19 (analog 5) - SW1
*/

#include <MsTimer2.h>
#include <Time.h>


#define seg_A 7
#define seg_B 8
#define seg_C 2
#define seg_D 3
#define seg_E 4
#define seg_F 6
#define seg_G 9 

#define displayPin1 10
#define displayPin2 11
#define displayPin3 12
#define displayPin4 13

#define SW1 19
#define SW2 18
#define SW3 17

#define LED 16

char display1_char = 'x';
char display2_char = 'x';
char display3_char = 'x';
char display4_char = 'x';

int lastRefreshed = 1;
int lastPin;

boolean freshBoot = true;



void loading()
{
  writeToDisplay('x', displayPin1); //clear display 1
  writeToDisplay('x', displayPin2); //clear display 2 
  writeToDisplay('x', displayPin3); //clear display 3
  writeToDisplay('x', displayPin4); //clear display 4
  
  writeToDisplay('-', displayPin1);
  delay(1500);
  writeToDisplay('-', displayPin2);
  delay(1500);
  writeToDisplay('-', displayPin3);
  delay(1500);
  writeToDisplay('-', displayPin4);
  delay(1000);
}





//Funkcija nastavi uro
void setMyTime()
{
  writeToDisplay('x', displayPin1); //clear display 1
  writeToDisplay('x', displayPin2); //clear display 2 
  writeToDisplay('x', displayPin3); //clear display 3
  writeToDisplay('x', displayPin4); //clear display 4
  
  int onScreen = 0;
  char temp[2];
  char hours[3];
  char minutes[3];
  
  writeToDisplay('_', displayPin1);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 3)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin1);
      onScreen++;
      delay(100);
    }
  }
  hours[0] = temp[0];
  writeToDisplay('x', displayPin1);
  onScreen = 0;
  delay(400);
  
  writeToDisplay('_', displayPin2);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 10)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin2);
      onScreen++;
      delay(100);
    }
  }
  hours[1] = temp[0];
  writeToDisplay('x', displayPin2);
  onScreen = 0;
  delay(400);
  
  writeToDisplay('_', displayPin3);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 6)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin3);
      onScreen++;
      delay(100);
    }
  }
  minutes[0] = temp[0];
  writeToDisplay('x', displayPin3);
  onScreen = 0;
  delay(400);
  
  writeToDisplay('_', displayPin4);
  while (digitalRead(SW1) != HIGH)
  {
    if (onScreen == 10)
    {
      onScreen = 0;
    }
    
    if (digitalRead(SW2) == HIGH)
    {
      itoa(onScreen, temp, 10);
      writeToDisplay(temp[0], displayPin4);
      onScreen++;
      delay(100);
    }
  }
  minutes[1] = temp[0];
  writeToDisplay('x', displayPin4);
  delay(400);
  
  hours[3] = '\0';
  minutes[3] = '\0';
  int hrs = atoi(hours);
  int mins = atoi(minutes);
  
  setTime(hrs,mins,0,1,1,2010);
   
}



void printTime()
{
  MsTimer2::stop();
  
  time_t curr_time = now();
  
  int hours = hour(curr_time);
  int minutes = minute(curr_time);
  
  char hours_temp[2];
  char minutes_temp[2];
  
  itoa (hours, hours_temp, 10);
  itoa (minutes, minutes_temp, 10);
  
   
  if (hours < 10)
  {
    char temp = hours_temp[0];
    hours_temp[0] = '0';
    hours_temp[1] = temp;
  }
  
  if (minutes < 10)
  {
    char temp = minutes_temp[0];
    minutes_temp[0] = '0';
    minutes_temp[1] = temp;
  }
  
  
  writeToDisplay(hours_temp[0], displayPin1);
  writeToDisplay(hours_temp[1], displayPin2);
  
  writeToDisplay(minutes_temp[0], displayPin3);
  writeToDisplay(minutes_temp[1], displayPin4);
  
  MsTimer2::start();
}




void displayRefresh()
{
  MsTimer2::stop();
   switch (lastRefreshed)
  {
    case 1: writeToDisplay(display1_char, displayPin1); break;
    case 2: writeToDisplay(display2_char, displayPin2); break;
    case 3: writeToDisplay(display3_char, displayPin3); break;
    case 4: writeToDisplay(display4_char, displayPin4); break;
  }
  
  if (lastRefreshed < 4)
  {
    lastRefreshed++;
  }
  else 
  {
    lastRefreshed = 1;
  }
  MsTimer2::start();
}






void writeToDisplay (char character, int displayPin)
{
  switch (displayPin)
  {
    case displayPin1: display1_char = character; break;
    case displayPin2: display2_char = character; break;
    case displayPin3: display3_char = character; break;
    case displayPin4: display4_char = character; break;
  }
    
  digitalWrite(lastPin, LOW);
  lastPin = displayPin;
  digitalWrite(displayPin, HIGH);
  digitalWrite(seg_A, LOW);
  digitalWrite(seg_B, LOW);
  digitalWrite(seg_C, LOW);
  digitalWrite(seg_D, LOW);
  digitalWrite(seg_E, LOW);
  digitalWrite(seg_F, LOW);
  digitalWrite(seg_G, LOW);
  
  if (character == '1')
  {
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
  }
  else if (character == '2')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_G, HIGH);
    digitalWrite(seg_E, HIGH);
    digitalWrite(seg_D, HIGH);
  }
  else if (character == '3')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_D, HIGH);
    digitalWrite(seg_G, HIGH);
  }
  else if (character == '4')
  {
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_F, HIGH);
    digitalWrite(seg_G, HIGH);
  }
  else if (character == '5')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_D, HIGH);
    digitalWrite(seg_F, HIGH);
    digitalWrite(seg_G, HIGH);
  }
  else if (character == '6')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_D, HIGH);
    digitalWrite(seg_F, HIGH);
    digitalWrite(seg_G, HIGH);
    digitalWrite(seg_E, HIGH);
  }
  else if (character == '7')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
  }
  else if (character == '8')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_D, HIGH);
    digitalWrite(seg_E, HIGH);
    digitalWrite(seg_F, HIGH);
    digitalWrite(seg_G, HIGH);
  }
  else if (character == '9')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_D, HIGH);
    digitalWrite(seg_F, HIGH);
    digitalWrite(seg_G, HIGH);
  }
  else if (character == '0')
  {
    digitalWrite(seg_A, HIGH);
    digitalWrite(seg_B, HIGH);
    digitalWrite(seg_C, HIGH);
    digitalWrite(seg_D, HIGH);
    digitalWrite(seg_E, HIGH);
    digitalWrite(seg_F, HIGH);
  }
  else if (character == 'x')
  {
    digitalWrite(seg_A, LOW);
    digitalWrite(seg_B, LOW);
    digitalWrite(seg_C, LOW);
    digitalWrite(seg_D, LOW);
    digitalWrite(seg_E, LOW);
    digitalWrite(seg_F, LOW);
    digitalWrite(seg_G, LOW);
  }
  else if (character == '-')
  {
    digitalWrite(seg_G, HIGH);
  }
  else if (character == '_')
  {
    digitalWrite(seg_D, HIGH);
  }

}


void setup()
{
  pinMode(seg_A, OUTPUT);
  pinMode(seg_B, OUTPUT);
  pinMode(seg_C, OUTPUT);
  pinMode(seg_D, OUTPUT);
  pinMode(seg_E, OUTPUT);
  pinMode(seg_F, OUTPUT);
  pinMode(seg_G, OUTPUT);
  pinMode(displayPin1, OUTPUT);
  pinMode(displayPin2, OUTPUT);
  pinMode(displayPin3, OUTPUT);
  pinMode(displayPin4, OUTPUT);
  
  pinMode(SW1, INPUT);
  pinMode(SW2, INPUT);
  pinMode(SW3, INPUT);
  
  pinMode(LED, OUTPUT);
  
  
  MsTimer2::set(1, displayRefresh);
  MsTimer2::start();

}

void loop()
{

  if (freshBoot == true)
  {
    freshBoot = false;
    writeToDisplay('8', displayPin1);
    writeToDisplay('8', displayPin2);
    writeToDisplay('8', displayPin3);
    writeToDisplay('8', displayPin4);
    digitalWrite(LED, HIGH);
    
    delay(1000);
    digitalWrite(LED, LOW);
  
    loading();
    setMyTime();
  }
  

  
  printTime();
  delay(1000);
  
  
}

system · March 25, 2010, 8:00pm

What is MsTimer2 doing for you? Why do you stop it while you collect and display the time?

roli · March 25, 2010, 8:14pm

MsTimer2 is refreshing the displays (I have 4 7 segment displays in my setup). I stop it during the "display print" to prevent flickering. If I don't do that the displays flicker too much.

system · March 25, 2010, 10:14pm

How long does it take to update the display of the time? I suspect that the timer is interfering with the time library's use of timers.

roli · March 25, 2010, 10:32pm

Do you mean what is the delay between a real minute and "arduino minute"? If you mean that - I wrote it in one of my posts in this topic (at first the delay is about 10s but after a while this time increases - minute, two minutes...).

EDIT: Did a fast test - I disabled the MsTimer (commented out every line - including include) and tryed the board without it. I couldn't see much but I could see that the time was still wrong.

Coding_Badly · March 25, 2010, 10:46pm

No. What he means is comment out all "MsTimer2::start();" calls and run the Sketch. Does the problem disappear?

system · March 25, 2010, 10:46pm

No. What I meant was how long does the function that updates the display of the time take? If it takes 1 millisecond, that could throw the clock off 0.1%. If it takes 50 milliseconds, that could throw the clock off 5%. Not would. Just could. I don't know what timer the time library uses, so I don't know whether stopping the timer you are using matters.

roli · March 26, 2010, 1:30pm

I did a quick test. I printed out millis at the start and the end of printTime function. Here are the results:

Start: 
13346
End: 
13361
Start: 
14376
End: 
14391
Start: 
15407
End: 
15421
Start: 
16436
End: 
16450

roli · March 27, 2010, 2:36pm

I have gone over the code a few times and I still can't find the problem. Something is wrong but I can't figure out what. The old library doesn't work, disabling the MsTimer doesn't work...

mem · March 27, 2010, 5:12pm

You can try this sketch to verify that the hardware and library are working correctly

#include <Time.h>

void printTime()
{
   
  time_t curr_time = now();
  
  int hours = hour(curr_time);
  int minutes = minute(curr_time);

  int seconds = second(curr_time); // added just for testing using serial

  Serial.print(hours);
  Serial.print(':');
  Serial.print(minutes);
  Serial.print(':');
  Serial.println(seconds);
}


void setup()
{  
   Serial.begin(9600); 
}

void loop()
{
  printTime();
  delay(1000);   
}

I removed everything but the timekeeping code and output this to the serial port. If this code does not lose time then you look into your application code to find the problem. Perhaps try adding the mstimer code first to see it that is upsetting things. If that works ok then add progressively more of your application logic back in until you find what causes the incorrect display of time. I think it will be easier to debug your logic if you use the serial output for the display until you have everything working and then switch over to the LEDs.

Have fun!

Topic		Replies	Views
time and date library Programming	10	2244	May 5, 2021
New to using Time library trying to serial print current hour Programming	4	2257	May 5, 2021
Question about the time library Programming	6	1197	May 5, 2021
Problem with time Library Programming	9	3373	May 5, 2021
How to Output Time Programming	6	1386	May 5, 2021

Setting the time with Time library

Related topics