Saving a preset, choosing address

Projects Programming
1

Hello everyone,

First off, amazing community you all have here. It has inspired me to begin a design I've thought of for quite awhile. It is a guitar effects switching system with a few twists. Right now I am having some trouble. I have a save button that reads the current state of relays, and then what I am trying to do is save it to eitherpreset button 1, 2, or 3, depending on which button I press. I've spent a few days reading over source material and re-educating myself, but I need to reach out and ask for some guidance. Am I on target with else if, is a switch case more appropriate?

This is my first "complex" code, I'm sure it's filled with rookie mistakes and repetitiveness. Are there any issues making separate void programs (helps keep me personally organized), am I going to screw up my code somewhere?

//Save button
  if (digitalRead(Save) == HIGH)
  {
    SavePressed = true;
  }
  else {
    SavePressed = false;
    SaveToggle = false;
  }
  if (SavePressed == true && SaveToggle == false) {
    SaveToggle = true;
    save();

//Save program
void save() {
  int var = digitalRead(Preset1) && digitalRead(Preset2) && digitalRead(Preset3);

  unsigned char relay = 0;

  if (Preset1 == HIGH) {
    for (int r = 14; r < 17; r++)
    {
      bitWrite(relay, r - 14, digitalRead(r));
    }
    EEPROM.write (Address1, relay);
  }
  else if (Preset2 == HIGH) {
    for (int r = 14; r < 17; r++)
    {
      bitWrite(relay, r - 14, digitalRead(r));
    }
    EEPROM.write(Address2, relay);
  }
  else (Preset3 == HIGH); {
    for (int r = 14; r < 17; r++)
    {
      bitWrite(relay, r - 14, digitalRead(r));
    }
    EEPROM.write(Address3, relay);
  }
}

void preset1() {

  unsigned char relay = 0;
  relay = EEPROM.read(Address1);
  for (int r = 14; r < 17; r++) {
    digitalWrite(r, bitRead(relay, r - 14));
  }
  lcd.setCursor(16, 1);
  lcd.print("Preset 1");

}

void preset2() {
  unsigned char relay = 0;
  relay = EEPROM.read(Address2);
  for (int r = 14; r < 17; r++) {
    digitalWrite(r, bitRead(relay, r - 14));
    lcd.setCursor(16, 1);
    lcd.print("Preset 2");
  }
} void preset3() {
  unsigned char relay = 0;
  relay = EEPROM.read(Address3);
  for (int r = 14; r < 17; r++) {
    digitalWrite(r, bitRead(relay, r - 14));
    lcd.setCursor(16, 1);
    lcd.print("Preset 3");
  }
}

Here is the full code as it is now. I'm writing the code for a nano, then once I know what I'm doing, I'll adapt it for a much larger Mega project. Thank you guys for any help or pointers in how to do better, it's all very appreciated. Keep up the great work and community!

#include <EEPROM.h>

#include <LiquidCrystal.h>

LiquidCrystal lcd(5, 6, 7, 8, 9, 10);

const int Save = 11;
const int Preset1 = 12;
const int Preset2 = 13;
const int Preset3 = 20;

int Address1 = 30;
int Address2 = 31;
int Address3 = 32;

int RelayPins [] = {14, 15, 16};
int RelayPinCount = 3;
int LedPins [] = {17, 18, 19};
int LedPinCount = 3;
int SwitchPins [] = {2, 3, 4};
int SwitchPinCount = 3;


boolean Switch1Pressed = false;
boolean Toggle1 = false;
boolean Switch2Pressed = false;
boolean Toggle2 = false;
boolean Switch3Pressed = false;
boolean Toggle3 = false;

//for save and presets
boolean SavePressed = false;
boolean SaveToggle = false;

boolean Address1Pressed = false;
boolean Address1Toggle = false;
boolean Address2Pressed = false;
boolean Address2Toggle = false;
boolean Address3Pressed = false;
boolean Address3Toggle = false;

void setup()
{
  int r;
  for (int r = 0; r < 2; r++) {
    pinMode(RelayPins[r], OUTPUT);
  }
  pinMode(Switch1, INPUT);
  pinMode(Switch2, INPUT);
  pinMode(Switch3, INPUT);
  pinMode(Relay1, OUTPUT);
  pinMode(Relay2, OUTPUT);
  pinMode(Relay3, OUTPUT);
  pinMode(Led1, OUTPUT);
  pinMode(Led2, OUTPUT);
  pinMode(Led3, OUTPUT);
}

void loop()
{
  lcd.begin(16, 2);
  lcd.print("Barnett Bypass");

  //LED and Relay tracking
  if (digitalRead(Relay1) == HIGH) {
    digitalWrite(Led1, HIGH);
  }
  else {
    digitalWrite(Led1, LOW);
  }
  if (digitalRead(Relay2) == HIGH) {
    digitalWrite(Led2, HIGH);
  }
  else {
    digitalWrite(Led2, LOW);
  }
  if (digitalRead(Relay3) == HIGH) {
    digitalWrite(Led3, HIGH);
  }
  else {
    digitalWrite(Led3, LOW);
  }

  //Save button
  if (digitalRead(Save) == HIGH)
  {
    SavePressed = true;
  }
  else {
    SavePressed = false;
    SaveToggle = false;
  }
  if (SavePressed == true && SaveToggle == false) {
    SaveToggle = true;
    save();
  }

  //Preset 1 button
  if (digitalRead(Preset1) == HIGH)
  {
    Address1Pressed = true;
  }
  else {
    Address1Pressed = false;
    Address1Toggle = false;
  }
  if (Address1Pressed == true && Address1Toggle == false) {
    Address1Toggle = true;
    preset1();
  }

  //Switch 1
  if (digitalRead(Switch1) == HIGH)
  {
    Switch1Pressed = true;
  }
  else {
    Switch1Pressed = false;
    Toggle1 = false;
  }
  if (Switch1Pressed == true && Toggle1 == false) {
    Toggle1 = true;
    toggle1();
  }

  //Switch 2
  if (digitalRead(Switch2) == HIGH)
  {
    Switch2Pressed = true;
  }
  else {
    Switch2Pressed = false;
    Toggle2 = false;
  }
  if (Switch2Pressed == true && Toggle2 == false) {
    Toggle2 = true;
    toggle2();
  }

  //Switch 3
  if (digitalRead(Switch3) == HIGH)
  {
    Switch3Pressed = true;
  }
  else {
    Switch3Pressed = false;
    Toggle3 = false;
  }
  if (Switch3Pressed == true && Toggle3 == false) {
    Toggle3 = true;
    toggle3();
  }
  delay(50);
}




void save() {
  int var = digitalRead(Preset1) && digitalRead(Preset2) && digitalRead(Preset3);

  unsigned char relay = 0;

  if (Preset1 == HIGH) {
    for (int r = 14; r < 17; r++)
    {
      bitWrite(relay, r - 14, digitalRead(r));
    }
    EEPROM.write (Address1, relay);
  }
  else if (Preset2 == HIGH) {
    for (int r = 14; r < 17; r++)
    {
      bitWrite(relay, r - 14, digitalRead(r));
    }
    EEPROM.write(Address2, relay);
  }
  else (Preset3 == HIGH); {
    for (int r = 14; r < 17; r++)
    {
      bitWrite(relay, r - 14, digitalRead(r));
    }
    EEPROM.write(Address3, relay);
  }
}

void preset1() {

  unsigned char relay = 0;
  relay = EEPROM.read(Address1);
  for (int r = 14; r < 17; r++) {
    digitalWrite(r, bitRead(relay, r - 14));
  }
  lcd.setCursor(16, 1);
  lcd.print("Preset 1");

}

void preset2() {
  unsigned char relay = 0;
  relay = EEPROM.read(Address2);
  for (int r = 14; r < 17; r++) {
    digitalWrite(r, bitRead(relay, r - 14));
    lcd.setCursor(16, 1);
    lcd.print("Preset 2");
  }
} void preset3() {
  unsigned char relay = 0;
  relay = EEPROM.read(Address3);
  for (int r = 14; r < 17; r++) {
    digitalWrite(r, bitRead(relay, r - 14));
    lcd.setCursor(16, 1);
    lcd.print("Preset 3");
  }
}



void toggle1()
{ digitalWrite(Led1, !digitalRead(Led1));
  digitalWrite(Relay1, !digitalRead(Relay1));
  lcd.setCursor(0, 1);
  lcd.print("Loop 1");


}
void toggle2()
{ digitalWrite(Led2, !digitalRead(Led2));
  digitalWrite(Relay2, !digitalRead(Relay2));
  lcd.setCursor(0, 1);


}
void toggle3()
{ digitalWrite(Led3, !digitalRead(Led3));
  digitalWrite(Relay3, !digitalRead(Relay3));
  lcd.setCursor(0, 1);
  lcd.print("Loop 3");


}
2

Are there any issues making separate void programs

I assume that you mean functions when you say "void programs". Used properly they will cause no problems and can help divide a program into logical sections with meaningful names.

if/else and switch/case are functionally identical but I find that switch/case is easier to write and maintain as long as the controlling condition can be reduced to an integer as required by switch/case.

My first question would be how are the switches wired ? Any pullup or pulldown resistors in place ? Have you consider what will happen if the switch contacts bounce as they make or break and produce multiple changes of state ? Would it be better to detect when a switch becomes pressed rather than when it is pressed thus avoiding the program acting multiple times rather than once ?

3 
  else (Preset3 == HIGH); {

That ; does not belong there.

 unsigned char relay = 0;
  relay = EEPROM.read(Address1);

as opposed to

 unsigned char relay = EEPROM.read(Address1);

? Why?

} void preset3() {

Ugly. Get your enter key fixed.

preset1(), preset2(), and preset3() do exactly the same things, except for the address that they read from.

Imagine if there where digitalRead1(), digitalRead2(), digitalRead3(), etc. functions. How much harder would that make reading from digital pins?

4

PaulS:
Get your enter key fixed.

That's a new one... funny!

5

The switches are wired up with a pulldown resistor to ground and digital, other to power. I must have misnterpreted the purpose of the debounce.

6

From the code you've posted...

[...]
  int var = digitalRead(Preset1) && digitalRead(Preset2) && digitalRead(Preset3);

  unsigned char relay = 0;

  if (Preset1 == HIGH) {
[...]
  else if (Preset2 == HIGH) {
[...]
  else (Preset3 == HIGH); {

First you don't use 'var' and it doesn't seem to make sense anyways.

Second you need to understand the difference between a pin number and the results of a digitalRead() call. In your If/Else statement it seems you're comparing actual pin numbers to the results of calls to digitalRead() using those pin numbers.

So that won't work.

And if you're playing with buttons it seems important to make sure you've checked out the State Change example. It might help.

Finally... if you ever find yourself creating variables like sw1, sw2, sw3 it might be time to learn how arrays work.

Sorry I didn't get any further with your code.

Regards,

Brad.

7

Thank you for pointing me in the right direction, Brad. I am going to read more into arrays and right the preset buttons into there.

My original intention and interest began by happening upon this instructable, it was my first window into Arduino world: http://www.instructables.com/id/arduino-programable-5-pedal-switcher/

This was only 3 months ago, so I am still very new. I got my hands on a few kits and went through all the tutorials as I was breadboarding that design. Once I became more familiar with the language and platform, I began altering it to fit my own design, and writing my own code as I learned segments that I thought could fit. I've revisited all the original guides, tutorials, exercises, and lectures that I found on YouTube, google, etc. While I'm re-educating myself, I'm going back and updating as I can when something else makes sense.

It will be an extended project that grows as I learn more. I am a touring gear tech and stage manager for bands, and I've taken a long absence in order to do this. I am not looking for any shortcuts, copy and past programmings, or anything less than what is needed to make it a reliable unit. My ultimate goal is to design and construct equipment that is built for touring, and the only way it can be used in this environment is if I get all of this 100% confident.

With that said, I have found the Arduino.cc host of material to be a bit too vague for in depth understanding. I have frequented the Jeremy Blum guides and other private ones of the sort. My have a strong background in math (up until calculus) and design (10 years+ woodworker/guitar builder), so I know I will be perfectly capable of honing this skill as I go. I do not mind putting in the time to study and practice, but there is a lot of things to sift through, and some teachers are definitely better than others. What are some of the best guides and in depth explanations that you could point me towards?

8

I did some more reading on debounce and using pullup resistors, looks like I'm overcomplicating things code wise. Here is a new thought I had to approach my situation:

If I wanted to create a "wait" scenario, where I press and hold save, and it waits for the next command, would I be looking at something along the lines of:

  {

  unsigned char relay = 0;
  while (digitalRead(Save) == HIGH) {
    if (digitalRead(Preset1) == HIGH) {
      for (int r = 14; r < 17; r++) {
        digitalWrite(r, bitRead(relay, r - 14));
        EEPROM.write(Address1, relay);
      }
    }    else if (digitalRead(Preset2) == HIGH) {
      for (int r = 14; r < 17; r++) {
        digitalWrite(r, bitRead(relay, r - 14));
        EEPROM.write(Address2, relay);
      }
    }    else (digitalRead(Preset3) == HIGH) {
      for (int r = 14; r < 17; r++); {
        digitalWrite(r, bitRead(relay, r - 14));
        EEPROM.write(Address3, relay);
      }
    }
  }
}

Would this be more of the direction I need to head in? It isn't functional right now, just trying to fill in the pieces with experimentation.

I do need to go and clean up a lot of the code, but my current strategy is (correct me if it's wrong), to get the results first by whatever means, then once I know it's functional and have a clearer image of what had to be done, now having a better understanding, go backwards and rework it.

9 
      EEPROM.write(Address1);

Write what to that address?

Using Tools + Auto Format would fix your horrid indenting. Compiling before you post code would be a good idea, too.

10

Alright, so I spent a lot of time studying up and exploring different libraries. I have the final code for what I want to do, but I am getting an error within my Switch/Case statement.

QBlibraryButtonV2.ino: In function 'void button1()':
QBlibraryButtonV2:93: error: jump to case label [-fpermissive]
QBlibraryButtonV2:82: error: crosses initialization of 'unsigned char relays'
QBlibraryButtonV2:97: error: name lookup of 'r' changed for ISO 'for' scoping [-fpermissive]
QBlibraryButtonV2.ino:97:12: note: (if you use '-fpermissive' G++ will accept your code)
jump to case label [-fpermissive]"

I included just the first button function, as the rest follow the same outline. I can't figure out where my error is thats causing this message. Thanks in advance for the help.

#include <EEPROM.h>
#include<Arduino.h>
#include<ButtonV2.h>
#include<LiquidCrystal.h>
ButtonV2 Button1;
ButtonV2 Button2;
ButtonV2 Button3;

const byte ButtonPin1 = 2;
const byte ButtonPin2 = 3;
const byte ButtonPin3 = 4;


const byte RelayPin[] = {A3, A4, A5};
const byte RelayPinCount = 3;
const byte LedPin[] = {A0, A1, A2};
const byte LedPinCount = 3;

const byte Preset1 = 30; // EEPROM addresses for presets
const byte Preset2 = 31;
const byte Preset3 = 32;

unsigned char relayState = 0; //For saving relay state to EEPROM

LiquidCrystal lcd (9, 10, 5, 6, 7, 8);

void setup()
{
  pinMode(ButtonPin1, INPUT);
  pinMode(ButtonPin2, INPUT); //Initialize button pins individually
  pinMode(ButtonPin3, INPUT); //How to use array with this library?

  int relay; //Initialize relay array
  for (relay = 0; relay < 3; relay = relay + 1)
  {
    pinMode(RelayPin[relay], OUTPUT);
  }

  int led; //Initialize led array
  for (led = 0; led < 3; led = led + 1)
  {
    pinMode(LedPin[led], OUTPUT);
  }

  Serial.begin(9600);

  //To initlaize LCD and print welcome message
  lcd.begin(16, 2);
  lcd.print("Barnett Bypass");

  Button1.SetStateAndTime(LOW); //Set button states and time
  Button2.SetStateAndTime(LOW); //as per library instructions
  Button3.SetStateAndTime(LOW);
}


void loop()
{
  ledrelaytracking();
  button1();
  button2();
  button3();
}

void button1 ()
{
  byte type = Button1.CheckButton(ButtonPin1); // current time and length of time to press the button as many times as you can ie. 1.5 seconds
  switch (type)
  {
    case WAITING:
      break;
    case PRESSED:

      lcd.setCursor(0, 1);
      lcd.print("Loop 1 toggled    ");
      digitalWrite(RelayPin[0], !digitalRead(RelayPin[0]));
      break;

    case DOUBLE_PRESSED:
      lcd.setCursor(0, 1);
      lcd.print("Preset 1         ");
      unsigned char relays = EEPROM.read(Preset1);
      for (int r = 0; r < 3; r++) 
      {
        digitalWrite(r, bitRead(relays, r));
      }
      break;

    /*case MULTI_PRESSED:
      Serial.println("Preset");
      break;*/

    case HELD:
      lcd.setCursor(0, 1);
      lcd.print("Saved to Preset1");
      unsigned char relayState = 0;
      for (r = 0; r < 3; r++)
      {
        bitWrite(relayState, r, digitalRead(r));
      }
      EEPROM.write(Preset1, relayState);
      break;

  }
}

  void ledrelaytracking () //To ensure led follows relay state 
  {
    if (digitalRead(RelayPin[0]) == HIGH) {
      digitalWrite(LedPin[0], HIGH);
    }
    else {
      digitalWrite(LedPin[0], LOW);
    }
    if (digitalRead(RelayPin[1]) == HIGH) {
      digitalWrite(LedPin[1], HIGH);
    }
    else {
      digitalWrite(LedPin[1], LOW);
    }
    if (digitalRead(RelayPin[2]) == HIGH) {
      digitalWrite(LedPin[2], HIGH);
    }
    else {
      digitalWrite(LedPin[2], LOW);
    }
  }
11 
    case DOUBLE_PRESSED:
      lcd.setCursor(0, 1);
      lcd.print("Preset 1         ");
      unsigned char relays = EEPROM.read(Preset1);
      for (int r = 0; r < 3; r++)
      {
        digitalWrite(r, bitRead(relays, r));
      }
      break;

You can't declare local variables in a case statement, unless the entire case is in curly braces:

    case DOUBLE_PRESSED:
    {
      lcd.setCursor(0, 1);
      lcd.print("Preset 1         ");
      unsigned char relays = EEPROM.read(Preset1);
      for (int r = 0; r < 3; r++)
      {
        digitalWrite(r, bitRead(relays, r));
      }
      break;
    }