saving leds to epprom

Hi
I am currently trying to save led states from a button push to an eeprom address and read the state back with a different button with a push

here is the code so far

/* switch
 * 
 * Each time the input pin goes from LOW to HIGH (e.g. because of a push-button
 * press), the output pin is toggled from LOW to HIGH or HIGH to LOW.  There's
 * a minimum delay between toggles to debounce the circuit (i.e. to ignore
 * noise).  
 *
 * David A. Mellis
 * 21 November 2006
 */
#include <EEPROM.h>;
int led14 = 14;
int storebutton = 4;
int rotarybut = 6;
int storestate = 0;         // the number of the input pin
int outPin = 13;       // the number of the output pin
int butpin = 3;
int outpin2 = 10;
int state[3] = { HIGH, HIGH, HIGH };      // the current state of the output pin
int reading [3];          // the current reading from the input pin
int previous [3] = { LOW, LOW, LOW };    // the previous reading from the input pin
int buttons [] = { 2, 3, 7 };
int leds [] = { 10, 13, 11 };
int location = 0;
// the follow variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long time = 0;         // the last time the output pin was toggled
long debounce = 200;   // the debounce time, increase if the output flickers

void setup()
{
	Serial.begin(9600);
	for(int i=0; i<3; i++) pinMode(leds[i], OUTPUT);
		for(int i=0; i<3; i++) pinMode(buttons[i], INPUT);
			// pinMode(inPin, INPUT);
	pinMode(led14, OUTPUT);
	digitalWrite(led14, LOW);
}

void loop()
{
	for(int i=0; i<3; i++){
		reading[i] = digitalRead(buttons[i]);
		
		// if the input just went from LOW and HIGH and we've waited long enough
		// to ignore any noise on the circuit, toggle the output pin and remember
		// the time
		if (reading[i] == HIGH && previous[i] == LOW && millis() - time > debounce) {
			if (state[i] == HIGH)
				state[i] = LOW;
			else
				state[i] = HIGH;
			
			time = millis();    
		}
		
		digitalWrite(leds[i], state[i]);
		
		
		
		previous[i] = reading[i];
		
		
	}
	delay(50);
	
	
	storestate = digitalRead(storebutton);      // check storebutton
	
	
	if (storestate == HIGH){
		//if any of the fxbuttons and the storebutton have been pressed together
		unsigned char lights = 0;
		int i=0;
		for (i=0; i<3; i++) {
			lights = digitalRead(leds[i]);
		}
		EEPROM.write(location, lights);
		
	}	
}

if(digitalRead(rotarybut) == LOW)
{
	
	lights = EEPROM.read(location);
	Serial.println(EEPROM.read(location));
	int i =0;
	for (i=0; i<3; i++) {
		digitalWrite(i, lights);
		
		
	}		
	
}

this is the part of the code where the problem is

storestate = digitalRead(storebutton);      // check storebutton
	
	
	if (storestate == HIGH){
		
		unsigned char lights = 0;
		int i=0;
		for (i=0; i<3; i++) {
			lights = digitalRead(leds[i]);
		}
		EEPROM.write(location, lights);
		
	}	
}

if(digitalRead(rotarybut) == LOW)
{
	
	lights = EEPROM.read(location);
	Serial.println(EEPROM.read(location));
	int i =0;
	for (i=0; i<3; i++) {
		digitalWrite(i, lights);
		
		
	}

any help would be greatly appreciated.

Gre

what type of arduino do you have? a Mega? something else? (I noticed pin14)

and your code above does not even check the parser stage. did you copy everything?

when I see this

    if (reading[i] == HIGH &&@previous[i] == LOW && millis() - time > debounce) {
      if (state[i] == HIGH)
        state[i] = LOW;
      else
        state[i] = HIGH;

      time = millis();
    }

    digitalWrite(leds[i], state[i]);
    previous[i] = reading[i];

don’t you think you have a debounce issue since you are storing in previous the value of the reading even if that was a “bounce”? might be better to only take action with the if section?

also what do you think this does? (I added comments as a hint)

   int i = 0; // <-- no need to initialize as you do that in the for loop
    for (i = 0; i < 3; i++) {
      lights = digitalRead(leds[i]); // here you overwrite the lights variable 3 times
    }
    EEPROM.write(location, lights); // here you only save the last lights value in address 0

thanks for your reply

   int i = 0; // <-- no need to initialize as you do that in the for loop
    for (i = 0; i < 3; i++) {
      lights = digitalRead(leds[i]); // here you overwrite the lights variable 3 times
    }
    EEPROM.write(location, lights); // here you only save the last lights value in address 0

What im trying to do is save the leds state to the eeprom with a button press called storebutton

and read the state back when another button called rotary with each button press it moves the eeprom read by one.

im not really sure what the code should be so i was trying different examples to try to make it work.

this is what i thought it does read the 3 leds (lights) and stores them in the eeprom with an address and a value.

How is this actually done?

well what you do in plain english is this.

Read led 0 and save states in variable lights
Read led 1 and save states in variable lights
Read led 2 and save states in variable lights
save lights value to position 0 in EEPROM

so you only saved the value of led 2 in EEPROM position 0

what you might want to do is

Read led 0 and save states in variable lights
save lights value to position 0 in EEPROM

Read led 1 and save states in variable lights
save lights value to position 1 in EEPROM

Read led 2 and save states in variable lights
save lights value to position 2 in EEPROM

is it?

Thanks for explaining that and thanks for your patience, i am experimenting how to put this in a statement.
the way i was hoping to do it was to save the leds state into one eeprom address press the button and it saves the next led state into the next eeprom address.

example:

button press = led 123 on to save eeprom address 0
button press 2 = leds 1 on and 3 on 2 off save to eeprom address 1 ect.

so you need to come up with a representation of the 3 LEDs states into 1 byte

a byte has 8 bits so technically can represent on/off state of 8 elements. if bit 0 is 0 then it means LED0 was off and if bit 0 is 1 then it means that LED0 was on.

do the same for LED1 in bit #1 and do the same for LED3 in bit #2

for that you will need to study bitwise operators

Once you understand how to play with bits, then do something like this

——— initialize a variable position to first address in EEPROM you want to write to repeat the code below each time a you want to save the LEDs (button press in your case I guess) initialize state to zero for each LED - read LED status - add LED status to a state variable in the correct bit position for that LED store state in an EEPROM position and move to next position ———

Another way to save bits into a byte is with the bitWrite, bitSet and bitClear functions.

https://www.arduino.cc/en/Reference/BitWrite https://www.arduino.cc/en/Reference/BitSet https://www.arduino.cc/en/Reference/BitClear

@groundfungus

yes this is true.

I think it is important to understand the binary representation and the basic operators.

Once you have this clear in you head, then you can indeed use higher level macros.

Note that there is a catch there though. This is how they are defined in Arduino.h

(on my Mac located at /Applications/Arduino.app/Contents/Java/hardware/arduino/avr/cores/arduino/Arduino.h)

#define bitRead(value, bit) (((value) >> (bit)) & 0x01)
#define bitSet(value, bit) ((value) |= (1UL << (bit)))
#define bitClear(value, bit) ((value) &= ~(1UL << (bit)))
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))

So it is very important to understand they operate on UL which are 32bit. if you use 64 bits representation such as uint64_t then the macros will fail.

The doc should mention that bit should be < 31 - but does not.

Cheers for the explanation, this is a whole new learning curve other than eeprom.read eeprom.write

I read the bitwise documentation and bit shifting i am still confused how to get the led states into bits and read them.

a byte has 8 bits so technically can represent on/off state of 8 elements. if bit 0 is 0 then it means LED0 was off and if bit 0 is 1 then it means that LED0 was on.

do the same for LED1 in bit #1 and do the same for LED3 in bit #2

can you show me an example of what the statement would be.

Assume you have a byte allLEDSstatus;

a byte in Arduino language can be represented by B00000000 to B11111111

each 0 or 1 represents a position in the byte (base 2 representation)

  • assume led1Status is a variable that is LOW
  • assume led2Status is a variable that is LOW
  • assume led3Status is a variable that is HIGH

you want int allLEDSstatus is B00000100 the 1 in the 3rd position from the right representing the HIGH status of LED3

  • assume led1Status is a variable that is HIGH
  • assume led2Status is a variable that is LOW
  • assume led3Status is a variable that is HIGH

you want int allLEDSstatus is B00000101 the 1 in the 1st position from the right representing the HIGH status of LED1 the 1 in the 3rd position from the right representing the HIGH status of LED3

using groundfungus high level macros (you don't work on 64 bits so they are fine)

if you do

allLEDSstatus = 0; // representation B00000000
if (led1Status == HIGH)  bitSet(allLEDSstatus, 0) ;  // the 1st bit in the byte is at index 0
if (led2Status == HIGH)  bitSet(allLEDSstatus, 1) ;  // the 2nd bit in the byte is at index 1
if (led3Status == HIGH)  bitSet(allLEDSstatus, 2) ;  // the third bit in the byte is at index 2

then your allLEDSstatus has the bits sets as needed and you can write that in the EEPROM

get it?

i think im getting this corect me if im wrong

allLEDSstatus = 0; // representation B00000000
if (led1Status == HIGH)  bitSet(allLEDSstatus, 0) ;  // the 1st bit in the byte is at index 0
if (led2Status == HIGH)  bitSet(allLEDSstatus, 1) ;  // the 2nd bit in the byte is at index 1
if (led3Status == HIGH)  bitSet(allLEDSstatus, 2) ;  // the third bit in the byte is at index 2

the above would be B00000111

if all leds are off it would be B00000000 if i had 8 leds and they were all on would be B11111111

so would it be

storeleds[i] = digitalread(leds[i]);
allLEDSstatus = 0; // representation B00000000
if (storeleds[1] == HIGH)  bitSet(allLEDSstatus, 0) ;  // the 1st bit in the byte is at index 0
if (storeleds[2] == HIGH)  bitSet(allLEDSstatus, 1) ;  // the 2nd bit in the byte is at index 1
if (storeleds[3] == HIGH)  bitSet(allLEDSstatus, 2) ;  // the third bit in the byte is at index 2

is this right so far

almost

if (storeleds[1] == HIGH)  bitSet(allLEDSstatus, 0) ;  // the 1st bit in the byte is at index 0

because it’s better to ensure your arrays starts at zero.

you could do

byte allLEDSstatus = 0; // representation B00000000

for (int i=0; i<NBLEDS;i++) { // only works if NBLEDS is less than 8 because we store in a BYTE
  storeleds[i] = digitalread(leds[i]);
  if (storeleds[i] == HIGH)  bitSet(allLEDSstatus, i) ;
}

or if you don’t need to memorize the value for later in your code

byte allLEDSstatus = 0; // representation B00000000

for (int i=0; i<NBLEDS;i++) {
  if (digitalread(leds[i]) == HIGH)  bitSet(allLEDSstatus, i) ;
}

or even shorter (because HIGH is equiv. to true, LOW to false)

byte allLEDSstatus = 0; // representation B00000000

for (int i=0; i<NBLEDS;i++) {
  if (digitalread(leds[i]))  bitSet(allLEDSstatus, i) ;
}

I think it’s better to put the == HIGH though because the code is more readable and compiler should be able to get good performance out of it because HIGH is a constant, so would probably both compile to the same code… (may be)

Thanks again, your explaining this great.

storeleds[i] = digitalread(leds[i]);
allLEDSstatus = 0; // representation B00000000
if (storeleds[0] == HIGH)  bitSet(allLEDSstatus, 0) ;  // the 1st bit in the byte is at index 0
if (storeleds[1] == HIGH)  bitSet(allLEDSstatus, 1) ;  // the 2nd bit in the byte is at index 1
if (storeleds[2] == HIGH)  bitSet(allLEDSstatus, 2) ;  // the third bit in the byte is at index 2[/code ]


So how do you save this into the EEprom write im a bit confused about how that is done? do you put all this in a value?

}

Would this be right?

if (storestate == HIGH){
		//if any of the fxbuttons and the storebutton have been pressed together
		counter++;
		byte allLEDSstatus = 0; // representation B00000000
		
		for (int i=0; i<NBLEDS;i++) {
			if (digitalread(leds[i]) == HIGH)  bitSet(allLEDSstatus, i) ;
				EEPROM.write(addr, bitSet(allLEDSstatus, i);[code]
}
}

I can't help feeling that there is some confusion here.

The status of each of 8 LEDs could be stored in an 8 element array of individual bytes with a byte value of 0 representing off and a byte value of 1 representing on. Each of the 8 bytes would need to be saved to 8 different EEPROM locations and read back when required. The value of individual bytes in the array would be set and read using the normal methods of setting the value of a variable and testing its value when required.

or

The status of each of 8 LEDs could be stored in the 8 bits of a single byte with a bit value of 0 representing off and a bit value of 1 representing on. The single byte would need to be saved to a single EEPROM location and read back when required. Each bit of the single byte would be set using bitSet() and read using bitRead() (yes, I know that there are "smarter" ways to do it but let's keep it simple)

The code in an earlier post uses both an array and a single status byte but I don't know why.

thanks for the reply

The status of each of 8 LEDs could be stored in the 8 bits of a single byte with a bit value of 0 representing off and a bit value of 1 representing on. The single byte would need to be saved to a single EEPROM location and read back when required. Each bit of the single byte would be set using bitSet() and read using bitRead() (yes, I know that there are "smarter" ways to do it but let's keep it simple)

Am i right, that in the led array the led high would represent a bit value of 1 in a byte and led low would have The value of 0 in a bit i can run 8 leds in a byte (B 00000011 would have a value of B LLLLLLHH).

so does this not set the 8 bits within the byte (bitSet(allLEDSstatus, i)) ?

well your code is formatted in a weird way above

but

		for (int i=0; i<NBLEDS;i++) {
			if (digitalread(leds[i]) == HIGH)  bitSet(allLEDSstatus, i) ;
				EEPROM.write(addr, bitSet(allLEDSstatus, i);

is a pb because you write the EEPROM each time in the for loop.

you need to complete the for loop to set all the individual bits in allLEDSstatus and once this is done, perform the EEPROM.write

if (storestate == HIGH){
    //if any of the fxbuttons and the storebutton have been pressed together
    counter++;
    byte allLEDSstatus = 0; // representation B00000000
	
    // set the bits for each of our LED in allLEDSstatus
    for (int i=0; i<NBLEDS;i++) {
        if (digitalread(leds[i]) == HIGH)  bitSet(allLEDSstatus, i) ;
    }  // close the for loop

    EEPROM.write(addr, bitSet(allLEDSstatus, i);
} // close if storestate == HIGH

Cheers for that

yes i have to work on ending loops properly.

i was just checking that i am learning this properly.

am i right if i want to write each byte to an eeprom adddress from 0-255

it would read

if (storestate == HIGH){
    //if any of the fxbuttons and the storebutton have been pressed together
    counter++;
    byte allLEDSstatus = 0; // representation B00000000
	
    // set the bits for each of our LED in allLEDSstatus
    for (int i=0; i<NBLEDS;i++) {
        if (digitalread(leds[i]) == HIGH)  bitSet(allLEDSstatus, i) ;
    }  // close the for loop
for(int addr=0; addr<255; addr++){
counter ++
    EEPROM.write(addr, bitSet(allLEDSstatus, i);
}
} // close if storestate == HIGH

so does this not set the 8 bits within the byte (bitSet(allLEDSstatus, i)) ?

No.

As its name implies the bitSet() function sets one bit in a byte to 1 and leaves the other bits as they are.

Try this

byte aByte = 0;

void setup()
{
  Serial.begin(115200);

  for (int bit = 7; bit >= 0; bit-- )
  {
    bitSet(aByte, bit);
    Serial.println(aByte, BIN);
  }
}

void loop(){}

    counter ++; // you were missing a semi colon at the end of this, and not sure this is used…

    EEPROM.write(addr, bitSet(allLEDSstatus, i);

you are missing a parenthesis
you need to pass the value to save in the EEPROM address as second parameter

you want to do

    EEPROM.write(addr, allLEDSstatus);

so

for(int addr=0; addr<255; addr++){
    counter ++; // NOT SURE WHY YOU DO THIS
    EEPROM.write(addr, allLEDSstatus); // ALL ADDRESSES ARE NOW WRITTEN WITH THE SAME DATA
}