How to use eeprom to save alarm hour?

Go To Last Post
29 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Guys,

I want to set alarm from my EEPROM and compare it to current time on RTC .
How can I save 23 into address 00 ? ---> hour 23, 23:00
I used ATMEGA128
Should I use printf for displaying alarm_hour to LCD ?

Is the address increasing by itself by the time I saved?
because I want to use address 00 only for hour...
the content for address 00 will be 00 to 23.

Hour 00 to 23.

I've done :

 case SET_ALARM_HOUR: //Set alarm hour
			  //set hour alarm
			  lcd_xy(0,0);//put on 0,0
			  lcd_string("SET HOUR ALARM ");
			  lcd_cmd(0xC0);
			  lcd_string(alarm_hour);
			  lcd_string("  ");
			  button_input = button_is_pressed();//read button
			  
			  if (button_input==1)
			  {
				  tone(50);
				  _delay_ms(50);
				  tone(0);
				  state_var = DISPLAY_TIME;
			  }
			  
			   if (button_input==2) // //  if button 2 pressed ( button up)
			   {
				   alarm_hour++;
				   tone(50);
				   _delay_ms(50);
				   tone(0);
				   if (alarm_hour >= 23)
				   {
					   alarm_hour = 0;
				   }
				   
				   
				   uint8_t ByteOfData ;
				   ByteOfData = alarm_hour ;
				   eeprom_update_byte (( uint8_t *) 00, ByteOfData );
				   state_var = SET_ALARM_HOUR;
			   }
			   
			    if (button_input==3) // //  if button 3 pressed ( button up)
			    {
				    tone(50);
				    _delay_ms(50);
				    tone(0);
				    state_var = BATTERY_STATUS;
			    }
			  
			  break;

Thanks for reading and helping

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How many times did i mention not to put delays in?

So what is the problem? You write an hour value to eeprom. Have you tested it yet? What have you done to test it?

Should you use printf? Does it do what you want? Are there any specific reasons not to use it?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see on eeprom it's 01 on address 00 but can't increase...does alarm_hour++ do the task?

I don't think I need printf, it's already "char"...
or I need alarm_hour[3] ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why do you askthese questions before actually trying? Are you scared of failure? Besides, regardless of what we tell you, you always seem to do your own thing.

What does alarm_hour++ got to do with not being able to write to eeprom? How do you inow it doesn't work - you don't seem to read the eeprom, so where's the evidence?

Why guess with alarm_hour[3] ? We've been through displaying stuff on the lcd some time ago. Would you like me to repeat myself? I think you've been given the answers to these questions before, so go and review the previous posts and answer the questions that have been answered before.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can I suggest you don't use absolute addressing in the EEPROM. You don't do it for RAM so why do it for EEPROM. Let the linker place your variables for you:

#include 

uint8_t hour_ee EEMEM;
uint8_t minute_ee EEMEM;
uint8_t second_ee EEMEM;

...
  eeprom_update_byte(&hour_ee, 17);
  eeprom_update_byte(&minute_ee, 23);
  eeprom_update_byte(&second_ee, 56);

thaht writes 127:23:56 to three locations in the EEPROM. You don't know where they are located but that doesn't matter because when you want to read them back you just:

uint8_t hour;
uint8_t minute;
uint8_t second;

  hour = eeprom_read_byte(&hour_ee);
  minute = eeprom_read_byte(&minute_ee);
  second = eeprom_read_byte(&second_ee);

So it doesn't really matter if the linker chose to put hour_ee at 0x0000, minute_ee at 0x0001 and second_ee at 0x0002. It could just as easily be at 0x01F3, 0x276 and 0x001B and the code would still work as you refer to the locations by name (just as you do for the hour/minute/second variables in RAM - you never really care what locations they are held in either).

BTW one of your previous threads talked about using mktime()/localtime() and so on. That was actually a very good idea. Your alarm in EEPROM is then just a single uint32_t and the test of whether the current time is close to the alarm time is something like:

if ((current_time >= alarm_time) && (current_time <= (alarm_time+60))) {

that checks whether the current time is within 60 seconds of the alarm time.

trying to do the same with individual h/m/s uint8_t variables is FAR more complex than making simple uint32_t comparisons!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OTOH, most modern RTC chips have Alarm registers. You can let the hardware look after the radio / kettle / toaster / bomb / light ...

Actually, it looks as if you are beginning to organise yourself!

Print out your program on paper.
You will see how you can simplify the logic and tidy up your structure.

You can either let the Compiler look after EEPROM variables or use fixed locations. Do NOT mix the two methods.

Calculating a Unix time_t is the easiest way to do Maths. OTOH, if you read all the DS1307 registers every second, you know all the year, month, day, ...
So you can compare the current time with an Alarm time relatively easily.

Personally, I would just work in BCD. You may be happier with converting every field to regular Binary.

Hint. You want to make a tone() with each button_is_pressed(). There is little point in duplicating the call for each case.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the opinion guys, let me understand them first..

Thanks clawson for the review, so unix time is a better solution for this case...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Thanks clawson for the review, so unix time is a better solution for this case...

Read David's reply - it depends. It's certainly true that if you want to make time comparisons in the AVR itself or calculations such as "what is 83 days and 5 hours from now?" then Unixtime is the way to go. However if you are using something like DS1307 then as David says it works in BCD and it has alarm registers. So you just set h/m/s alarm registers to 17, 23, 56 and it alarms at 17:23:56.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

the issue is, DS1307 doesn't have alarm register...
for current experiment, I want to save a time in EEPROM (set alarm) then compare it with RTC, if it's match, turn the beep on...

next step, with unix time so, on in a certain ammount of time....

I'll do it one step at the time....

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

make sure you also have a case that handles when the alarm has already passed.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Guys I tried, when I press the button,
it does the job, since I check the content of eeprom,
it's changing and increasing it's on address 00, and I followed clawson suggestion to put the address on variable..., but why isn't it increasing on LCD,
did I put alarm_hour++;
wrongly ?
my experiment :

case SET_ALARM_HOUR: //Set alarm hour
			  //set hour alarm
			  lcd_xy(0,0);//put on 0,0
			  lcd_string("SET HOUR ALARM ");
			  sprintf(alarm_hour_char,"%.0f",alarm_hour);
			  lcd_cmd(0xC0);
			  lcd_string(alarm_hour_char);
			  lcd_string("  ");
			  //alarm_hour = 0;
			  button_input = button_is_pressed();//read button
			  
			  if (button_input==1)
			  {
				  tone(50);
				  _delay_ms(50);
				  tone(0);
				  state_var = DISPLAY_TIME;
			  }
			  
			   if (button_input==2) // //  if button 2 pressed ( button up)
			   {
				   alarm_hour++;
				   tone(50);
				   _delay_ms(50);
				   tone(0);
				   if (alarm_hour >= 23)
				   {
					   alarm_hour = 0;
				   }
				   
				   
				   
				   //eeprom_update_byte (( uint8_t *) 00, alarm_hour );
                   eeprom_update_byte (hour_ee, alarm_hour );
				   
				   state_var = SET_ALARM_HOUR;
			   }
			   
			    if (button_input==3) // //  if button 3 pressed ( button up)
			    {
				    tone(50);
				    _delay_ms(50);
				    tone(0);
				    state_var = BATTERY_STATUS;
			    }
			  
			  break;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
case SET_ALARM_HOUR: 
           lcd_xy(0,0);    //put on 0,0
           lcd_string("SET HOUR ALARM ");
           sprintf(alarm_hour_char,"%.0f",alarm_hour);
           lcd_xy(0,1);    // same as lcd_cmd(0xC0);
           lcd_string(alarm_hour_char);
           lcd_string("  ");
           button_input = button_is_pressed();//read button
           tone(50);
           _delay_ms(50);
           tone(0);
          
           switch (button_input)
           {
              case 1:
                 state_var = DISPLAY_TIME;
                 break;
              case 2:
                 state_var = SET_ALARM_HOUR;
                 if (++alarm_hour > 23) alarm_hour = 0;
                 eeprom_update_byte (hour_ee, alarm_hour );
                 break;
              case 3:
                 state_var = BATTERY_STATUS;
                 break;
           }
   
           break; 

I have just done a little 'tidying up'.
I have removed superflous comments. e.g. case SET_ALARM_HOUR is self-explanatory.
lcd_cmd(0xC0) is only another way of saying lcd_xy()
I have moved the tone().
I have used another switch() statement.
I have changed a >=23 to >23

Untested. I hope that it shows how you can make code easier to understand.

Do you really use a f-p variable for 'hour' ?

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

David,
I tried your code up there, it's not increasing as we want, I haven't touched the button, it repeats itself already and not increasing alarm_hour, perhaps you have an idea why ?
thanks

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I can see the value on the eeprom is changing,
but why can't I display it on LCD ??

sprintf(alarm_hour_char,"%.0f",alarm_hour);
lcd_string(alarm_hour_char);

Did I miss something here ?

thanks

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Nowhere have you shown the definition of alarm_hour_char.

Is it really a float? If not, then why are you using "%.0f" as your format string in the printf?

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Are you really using a float variable for alarm_hour ?

Most normal people would use a uint8_t or possibly an int8_t.
Sprintf() knows nothing about variable types. If you give it an 'f' format, it assumes you will give it a float argument.
I can assure you that mis-matched formats and arguments will create havoc with the printf() family.

Sorry. I never saw your post from 22 Oct. I never tested my 'tidied' code. That is your job.
I was only trying to make the point that "concise neatly formatted code is easier to read and maintain".

Of course, I could have introduced errors myself.
I still find that 'smaller' code is less daunting.

I suggest that you review your variable names too.
e.g. lcd_alarm_char does not suggest a string to normal people. Call it lcd_buf or lcd_alarm_string or something that is intuitive to Aussie (or Italian) people.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What do you see on the LCD? Not a question mark by any chance?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

no any questions mark at all...

here's the definition :

char alarm_hour_char[3];
int alarm_hour;

is it supposed to be

sprintf(alarm_hour_char,"%.0i",alarm_hour); 

Yes I'm the one who will test it, thanks for helping David..

Thanks

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would say:

char alarm_hour_string[3];
int alarm_hour;
    ...
    sprintf(alarm_hour_string, "%02d", alarm_hour);
    ...

Note that [3] is a bit small for an integer that could be -32768 .. 32767
If you tried to print -32768 it needs buffer[7] to hold the minus, the digits, the NUL.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried to write the minute

char alarm_hour_char[7],alarm_minute_char[7];
int alarm_hour,alarm_minute;
uint8_t hour_ee EEMEM, minute_ee EEMEM;

 case 2:
				  	  alarm_minute++;
				  	  tone(50);
				  	  _delay_ms(50);
				  	  tone(0);
				  	  if (alarm_minute >= 60)
				  	  {
					  	  alarm_minute = 0;
				  	  }
				  	  eeprom_update_byte (minute_ee, alarm_minute );
				  	  
				  	  state_var = SET_ALARM_MINUTE;
				  	  break;

but I didn't see it saved properly on the EEPROM, what do I miss here ?

thank you

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
                   eeprom_update_byte (minute_ee, alarm_minute );

Doesn't your Compiler whinge about this statement?

Look at the library documentation and examples.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

no it didn't whinge,

should I write like this :
I thought that the library will decide the address itself? As follow the suggestion in the previous post ?

# include 
void main ( void )
{
uint16_t WordOfData ;
WordOfData = 0 x1232 ;
eeprom_update_word (( uint16_t *) 46, WordOfData );
}
uint8_t eeprom_read_byte ( const uint8_t * addr )
void eeprom_write_byte ( uint8_t *addr , uint8_t value )
void eeprom_update_byte ( uint8_t *addr , uint8_t value )
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Explore C's & operator!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This uses absolute addresses. If you use this method, you must never have any EEMEM variables

#include 

void main ( void )
{
    uint16_t WordOfData;
    WordOfData = 0x1234;
    eeprom_update_word (( uint16_t *) 46, WordOfData )
} 

Or you use regular EEMEM variables as suggested by Cliff:

#include 

uint16_t EEMEM eeprom_var = 0x5678;

void main ( void )
{
    uint16_t WordOfData;
    WordOfData = 0x1234;
    eeprom_update_word (&eeprom_var, WordOfData );
} 

Note that you need to pass the 'address' i.e. the & operator.

Of course, other compilers can treat eeprom natively. e.g. CodeVision:

#include 
#include 

uint16_t __eeprom eeprom_var = 0x5678;

void main ( void )
{
    uint16_t WordOfData;
    WordOfData = 0x1234;
    eeprom_var = WordOfData;   // like any other variable
} 

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thanks guys,
it can be saved now,
How can I compare it with RTC value ?
since it's on byte

uint8_t eeprom_read_byte ( const uint8_t * addr )
char ds1307_addr[7];

is it already the same type with RTC value ?

hour = ds1307_addr[2]
minute = ds1307_addr[1]

Thanks

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

could it be like this ?

	alarm_hour   =  eeprom_read_byte(&hour_ee );
			alarm_minute =  eeprom_read_byte(&minute_ee );
			
				if ((hour == alarm_hour) && (minute ==alarm_minute) && (second <= 60))
				{
					tone(14);
					_delay_ms(500);
					tone(0);
					
				}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes. That looks fine to me. We are not going to physically test every snippet that you post here.

As can be seen by my recent blunders, I sometimes post erroneous untested calculations and examples.

It is your job to design your own code, design your own tests, execute the tests. You will learn from your own experience.

When you first start with a completely unknown subject like C or microcontrollers, you will be wary of the type / edit / compile / burn cycle. I can understand a certin amount of 'hand holding' is required.

Later on, you have an idea. You can try it for yourself. By all means ask questions when you don't understand why it did or did not work.

Incidentally, your DS1307 has got battery-backed RAM in locations 8 .. 63. Why not store your alarm values in this non-volatile area? e.g. 8 and 9. Then you simply read 10 registers instead of 8 registers every second.

Personally, I would use the same BCD format for 'alarm' registers as the time registers. If you ever change to a DS1337, the code would be virtually the same.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That's good idea David about RAM location and read 10 registers, I read about it but don't know what it's for...
==============
from DS1307 datasheet :
56-Byte, Battery-Backed, General-Purpose RAM
with Unlimited Writes

....
The RAM registers are located in address locations 08h to 3Fh

I can use for example RAM registers at address 08h for hour and 09h for minute...
================
If I want to make LED on for 10 minutes from 16:10....may be it's better using Unix time as already discussed, do you reckon ?

or with EEPROM / RAM inside DS1307 is better ? or depends on what I need ?

Is it the same way writing 08h register and 07 register ?

Yes, I tested the code I wrote, and it worked,
I can set the alarm from the range of 00-23 and 00-59...

I have another idea on making the LED on at a certain time...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ah-ha. You are trying ideas by yourself now! Well done.

You can try out "alarm 10 minutes from now"
Or "wake me up in 8 hours"
Or "parturition is due 281 days from insemination"
Or "oestrus is due 21 days"
Or "TV licence is due 1 year"
...

You will see what is involved. You will choose the best method.

Incidentally, it is not difficult to calculate Unix time. Hint. Zellers Congruence.
I used to calculate days in 16-bit maths using 6502 ASM with my own algorithm. It is only the days that are complicated. Seconds in the day is trivial.
Nowadays, you can just google and you get a ready-made C function.

David.