Button with EEPROM

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

Hello guys, previously I was able to create a bluetooth controlled switch for the light of my room but I have noticed that it missed a thing that whenever the whole device is switched ON/OFF i.e. in case of a power cut the output turns OFF no big deal as I have coded them to behave that way but I thought of a feature that what ever output is ON should remain ON and hence I experimented with avr eeprom but after so many trials I could get it to working can some one examine my code and indicate me what I am doing wrong?

I am using atmega8 clocked at 8MHz and buttons are nicely debounced and there is not a problem with the UART(Bluetooth functions).

The thing that I want to achieve is that when ever I turn ON a switch it should remember that it was turned ON and then turn it ON after the device has recovered from the power cut.

Any advice would be highly appreciated.

Here is the code that I have written:-

#include <avr/io.h>
#include <avr/eeprom.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include "debounce.h"
uint16_t state_saver = 0;//state saver
const uint16_t EEMEM memory_saver = 10;//setting up the initial value

ISR(TIMER0_OVF_vect)
{
	debounce();//calling the debounce routines in the isr
}
int main(void)
{
	int output_state = 0;
	DDRB = 0x00;
	PORTB = 0xff;

	DDRC = 0xff;
	PORTC = 0x00;
	TCCR0 = 1<<CS02;//starting the timer_1 in normal mode
	TIMSK = 1<<TOIE0;//starting the prescaler of 128
    /* Replace with your application code */
	debounce_init();
	sei();

    while (1)
    {
		output_state = eeprom_read_word(&memory_saver);
		if((button_down(BUTTON1_MASK))  || (output_state == 1))
		{
			state_saver++;
			if(state_saver == 1)//if button is clicked for the first time
			{
				output_state = 0x01;
				eeprom_update_byte((uint16_t*)10, output_state);//updating the mcu
			    PORTC |= (1<<PC1);
			}
			if(state_saver == 2)//button is clicked second time
			{
				output_state = 0;
				eeprom_update_byte((uint16_t*)10, output_state);//updating the mcu
				PORTC &= ~(1<<PC1);
				state_saver = 0;
			}
		}
    }
}

 

This topic has a solution.

anshumaan kumar

Last Edited: Wed. Feb 26, 2020 - 07:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why are you using uint16_t to hold 0 / 1?

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

Be consistent:

const uint16_t EEMEM memory_saver = 10;//setting up the initial value

 

    output_state = eeprom_read_word(&memory_saver);

 

    eeprom_update_byte((uint16_t*)10, output_state);//updating the mcu

 

This is no doubt your bug.

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

clawson wrote:

Why are you using uint16_t to hold 0 / 1?

Sorry to say that I know what are you saying I have allocated more space than what is required because whenever I have used uint8_t the compiler issued me a warning of using wrong data types although that shouldn't be the case I have reverted this thing no big issue here thanks for indicating me that .smiley

N.Winterbottom wrote:

Be consistent:

const uint16_t EEMEM memory_saver = 10;//setting up the initial value

 

    output_state = eeprom_read_word(&memory_saver);

 

    eeprom_update_byte((uint16_t*)10, output_state);//updating the mcu

 

This is no doubt your bug.

Yes this is the part that is freaking out I examined carefully to see that I have been using the eeprom_read_byte function incorrectly. Also I think you are indicating that I was unable to assign a value to a address of the eeprom using the EEMEM function I think I should see the datasheet more carefully.Thanks good sir for your precious time.

Sorry I got busy I should have replied earlier.smiley

anshumaan kumar

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

you read a word, but write a byte to the eeprom. 

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

Oh.This means that I got confused on what I have to do well this is what I want to achieve :-

1. avr is powered ON 

eeprom is at it's default (zero)

2. then someone presses a button

output associated to that button turns on and  a value is saved at the eeprom.

3. In case of a power cut avr gets turned off.

4. avr is turned on then the mcu will see the saved value in the eeprom and lights up the output automatically which was turned ON before the power went down.

This is precisely what I want but seems that I am confused although I tried and still I am trying. Thanks Kartmansmiley I will change the code and show it as soon as possible.

anshumaan kumar

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

eeprom is at it's default (zero) - are you sure? I thought the default was 255.

 

Write your code to do precisely what you want! AS7 has a simulator - use that to step through your code and verify it does what you want.

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

Kartman wrote:
eeprom is at it's default (zero) - are you sure? I thought the default was 255.
Well i thought I saw that somewhere I need to read the datasheet a few more times.
Kartman wrote:
AS7 has a simulator
Wow. Didn't knew that I'll give that a try.

anshumaan kumar

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

Well I have done this. This code here is free from any of the warnings that I was facing earlier and I think it's working well (the other one didn't work at all)

What I have observed here is that after hitting the button the lights blinks 

Well it's good to have something than nothing but I think that the condition that I am using in the IF statement seems to be messing this time can someone advice me what is required still.

I am thinking about the SWITCH statement now however.


#include <avr/io.h>
#include <avr/eeprom.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include "debounce.h"
uint16_t state_saver = 0;//state saver

ISR(TIMER0_OVF_vect)
{
	debounce();//calling the debounce routines in the isr
}
int main(void)
{
	uint8_t output_state = 0;//variable to store output state
	uint8_t memory_state = 0;//variable to store the output state into memory
	DDRB = 0x00;
	PORTB = 0xff;

	DDRC = 0xff;
	PORTC = 0x00;
	TCCR0 = 1<<CS02;//starting the timer_1 in normal mode
	TIMSK = 1<<TOIE0;//starting the prescaler of 128
    /* Replace with your application code */
	debounce_init();
	sei();
	memory_state = eeprom_read_byte((const uint8_t*)10);//reading the value at the start

    while (1)
    {
		if( (button_down(BUTTON1_MASK)) || (memory_state == 1))//either button or via memory trigger
		{
			state_saver++;
			if(state_saver == 1)//if button is clicked for the first time
			{
				output_state = 1;//saving the ON state
				eeprom_update_byte((uint8_t*)10, output_state);//updating the mcu
			    PORTC |= (1<<PC1);
			}
			if(state_saver == 2)//button is clicked second time
			{
				output_state = 0;//saving the OFF state
				eeprom_update_byte((uint8_t*)10, output_state);//updating the mcu
				PORTC &= ~(1<<PC1);
				state_saver = 0;
			}
		}
    }
}

 

anshumaan kumar

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

Going back to the code in #1 I am not entirely sure what the thing in EEPROM is being used for anyway. Even if you got the access to be consistent (all uint8_t):

const uint8_t EEMEM memory_saver = 10;//setting up the initial value
	uint8_t output_state = 0;
		output_state = eeprom_read_word(&memory_saver);
		if((button_down(BUTTON1_MASK))  || (output_state == 1))
				output_state = 0x01;
				output_state = 0;

then what on Earth is the default setting to 10 for anyway? It seems that in use the variable only ever has the value 0 or 1. Yet you create the initial copy to hold 10. 

 

Most curious of all is when you do:

				eeprom_update_byte((uint16_t*)10, output_state);//updating the mcu

this is quite, quite wrong. The first parameter to eeprom_update_byte() is supposed to be the address of the variable in EEMEM. But you are passing a hard coded 10. So did you think that when you wrote:

const uint16_t EEMEM memory_saver = 10;//setting up the initial value

you were creating a variable in EEPROM called memory_saver and rather than setting the initial value to be 10 this was supposed to say "this variable should live at EEPROM address 10" ? That is not how this works. 

 

Usually you do't need to care what actual addresses in EEPROM are used because (just like variables in RAM) you let the linker in the build system pick an address for you. So when you come to update it, it should probably look more like:

eeprom_update_byte(&memory_saver, output_state);//updating the mcu

which says "write whatever is currently in the output_state variable to the EEPROM location called memory_saver". The & operation says "pass the (EEPROM) address of memory_saver to the writing routine".

 

So perhaps wind back and tell us what all the "10"s in your code are supposed to be achieving?

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

Your state saver logic is suspect. I gather you want to detect when the button is pressed - this is called the 'rising edge'. 

 

For this the logic is:

 

while(1)

{

press = button_pressed();

if press is true AND state_saver is false then do something (rising edge)

state_saver = press;

}

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

Also if this is about detecting edges and keeping state while the power is off how do you know at next power on when you read "last edge state" (if that's what it is?) from EEPROM that there haven't been 57 intervening edges that weren't seen while the power was off?

 

IOW what is the point in storing "last state" in EEPROM when it could be "out of date" when you re-read it at next power on ??

 

As I say so often here: I'd maybe wind back a bit - state what the overall system design is supposed to achieve and then concentrate on what you need to do to accomplish that design. I can't help thinking this isn't it !

Last Edited: Tue. Feb 25, 2020 - 11:59 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Cliff I am so sorry that I decided to hastily read the datasheet and write some crap and then complain that nothing is working now leave this come to the point.

I saw somewhere that

 

Anshumaan wrote:

const uint16_t EEMEM memory_saver = 10;//setting up the initial valu

this is used to save a value to the eeprom address but then I realized when I am tracing problems on a paper with internet at my help, that this is used to start a variable to the stated eeprom address. That comment is deceptive, sorry it made you see that I have used EEMEM function to store a initial value.

 

Then let's come to the 10. This is the eeprom address that I am using to store the 0 and 1.

 

Now as I have said in #6 what I actually want to do is that after clicking the button the MCU should save a value so that when the MCU is turned off and then turned ON it can use that value to turn ON the output automatically. I did this because I couldn't come up with something else.Hope you got what I am trying to do.

Kartman wrote:
Your state saver logic is suspect. I gather you want to detect when the button is pressed - this is called the 'rising edge'

Yes I want to detect button presses I was missing this keyword and hence came up with this. This could look suspicious but believe me this works whatsoeverlaugh.

But as you have stated I will be taking a look at the rising edge now.And thanks for the example.

clawson wrote:
Also if this is about detecting edges and keeping state while the power is off how do you know at next power on when you read "last edge state" (if that's what it is?) from EEPROM that there haven't been 57 intervening edges that weren't seen while the power was off?

Precisely you have nailed it but I can't come up with something else how would you tackle this situation? Any suggestion for me ???devil

anshumaan kumar

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

Oh Kartman I realized that I made a mistake while reading your comment this code here perfectly debounces and hence avr registers only one press each time button is pressed. What I have done here is that when button is pressed I incremented a variable from zero when it's 1 I instruct the MCU to do something as turning ON the led and also writing to the eeprom then when the button is pressed next time the variable state_saver is incremented again so that I can turn OFF the led and also instruct the MCU to write to the eeprom. In very simple words I have created a toggle switch (This can be done by using just the ^ operator if I had to toggle the outputs only)But here I have to write to the eeprom so that MCU can store the led state that is why I came up with this.Hope you understood. 

 

anshumaan kumar

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

Your best bet is not to let the micro ever power off. Just keep it asleep and either wake up from  time to time to sense edges or better is to have the edges act as the wakeup trigger.

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I’m pretty sure I get the gist of what you want to achieve:
1. Push button toggles the led on/off
2. Eeprom stores the current on/off state
3. On power up, the previous current state is restored.
The above basically describes three functions.

The first function ‘toggleLed()’ doesn’t care who or what calls it. In your case you would have some like ‘if buttonpress then toggleLed()’
We’ve separated the action of buttonpress from toggleLed. If you wanted to have a uart command toggle the led, then that would call toggleLed.
ToggleLed would call storeLedState(). On power up you would call restoreLedState().
Three simple functions. Three easy to test functions. Simple rule - one function, one job - properly. If it takes more than a couple of lines to explain the function, it is most likely too complex.

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

clawson wrote:

Your best bet is not to let the micro ever power off. Just keep it asleep and either wake up from  time to time to sense edges or better is to have the edges act as the wakeup trigger.

well you know what If I was unable to keep the micro in asleep state even this state requires some sort of power (battery) which I don't want to implement If I were to use the battery power I would have not let the avr shut down but even then thanks for your valuable advice. 

Kartman wrote:
I’m pretty sure I get the gist of what you want to achieve

Ah.. You always does good sir. This pretty much sums it all this is the only thing which I want to achieve you even knew somehow that I want to use this system in a bluetooth controlled switching device very clever you got this right too.You appears to be either highly experienced or some sort of magician to me laugh.

Kartman wrote:
Three easy to test functions. Simple rule - one function, one job - properly.

Thanks for this I am going to write a new code on paper first keeping this mind.

 

Also One more thing What should I store in the eeprom to save the led state, My mind is still leaning towards 0 and 1 . Can I do that to save the led state .?

anshumaan kumar

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

I don't know what is with me I have employed the most simple tactic but this time it works thanks for helping me guys and specially Kartman you always get what I am trying to say. Thanks really appreciate your dedication keep up the good work.smiley

Kartman wrote:
The first function ‘toggleLed()’ doesn’t care who or what calls it. In your case you would have some like ‘if buttonpress then toggleLed()’
We’ve separated the action of buttonpress from toggleLed. If you wanted to have a uart command toggle the led, then that would call toggleLed.
ToggleLed would call storeLedState(). On power up you would call restoreLedState().
Three simple functions. Three easy to test functions. Simple rule - one function, one job - properly. If it takes more than a couple of lines to explain the function, it is most likely too complex.

This works really.! I can't believe it.

anshumaan kumar