help needed with code not doing as expected

Projects General Guidance
1

I have an Arcade Machiene that I have made, and I wanted to get all fancy and do some things with an arduino, I have planned to interface some rgb strips etc later, but for now I have it setup with three switches on a control panel that are wired to the arduino. the idea of the switches is to control the tv but also allow multiple functions on the same switch.

The switches are defined in the code as:
//switches on the front panel conencteed to these pins, pulled to low when pressed.
int up = 5;
int power = 4;
int down = 6;

The code is my attempt at a state machiene, using simple 1 and 0 flags to trigger events in non blocking code.

The power button is basically debounced, and then toggles the tv power on or off if held for more than 3 seconds,
during the run up to those 3 seconds it acts as a "shift" button allowing secondary functions to be achieved by pressing the up and down buttons,

there are four relays connected to the arduino are represented in the code by out1 to out4, the relays always do as expected when expected. (apart from needing to reverse the logic to trigger them)

there are then three wires soldered to the TV push buttons for power and volume up and down.

when i want to trigger a button press i briefly set them as outputs then back to inputs,

so the conundrum I would like some help with is:

the volume doesnt work as expected when first powered up.
the volume up actually reduces the volume, and the vol down apparently does nothing, the power button works as expected, and when power is used as shift, both up and down vol buttons activate the relays.

there is a lot of serial debugging in the code, which suggests to me that the output to the tv has been toggled as expected but the TV doesnt react.

THEN

once I power cycle the TV by holding the power button 3 seconds, waiting briefly then doing it again, then the vol buttons react as expected for as long as the arduino stays powered up.

once I reset power to the arduino it all goes pants again.

I have gone through the code time and time again, looking for flags that are not set, or double set, but my serial debugging is oly sent right as the output is sent to the TV, so if i can read it, then it shoudl have been sent,

I cant for the life of me work out whats going on.

any help would be greatly appreciated, Ive done my best to annotate the code and give it meaningful names, so hopefully it can be understood.

m00se

cabinet_2.2_keypad_lockout.ino (11.9 KB)

2

Please post your code using code tags. Many forum members use phones & tablets and cannot open your attachment. See the forum sticky post to find out how, and other tips for getting the most out of the forum.

3

it is too long,
i had to attach it or it goes over the character count allowed.
sorry

4

spruce_m00se:
it is too long,
i had to attach it or it goes over the character count allowed.
sorry

Break it into pieces, or take out extraneous stuff, and just show a short program with the same problem. You won't get the best help if you don't follow the rules here.

5

theres the variables, setup and main loop.

//switches on the front panel conencteed to these pins, pulled to low when pressed.
int up = 5;
int power = 4;
int down = 6;

//to keep track of the state of the buttons on the front panel, LOW is pressed HIGH is released.
int uppress = HIGH;
int uppressprev = HIGH;
int powerpress = HIGH;
int powerpressprev = HIGH;
int downpress = HIGH;
int downpressprev = HIGH;


int pi = A2;// output to pi

//variables to control output to relays
boolean lightson1 = 0;  // LEFT RIGHT
boolean lightson2 = 0;// FRONT REAR
boolean lightson3 = 1;// CONTROL DCK on by default
boolean fanon = 1 ; // fan on by default

// pins connected to relays
int out1 = 7; // control deck
int out2 = 8; // fan
int out3 = 9; //LEFT RIGHT
int out4 = 10; // front and rear

//pins connected to the TV push buttons and LED
int voldown = 12;
int volup = 2;
int menu = A4;
int chdown = A5;
int chup = 1;
int tvpwr = 3;
int tvledpin = A3;

//variables to control button push routines, set these to one to initiate a simulated button push on the TV
boolean tvpwrbutton = 0;
boolean volupbutton = 0;
boolean voldownbutton = 0;
boolean chupbutton = 0;
boolean chdownbutton = 0;

//variables to track the state of a few bits n bobs
int pwrmenu = 0;
int tvled = 0;
boolean tvpwrstate = 1;
boolean tvpwrswitchreleased = 0;

int state = LOW;      // the current state of the output pin
int reading;           // the current reading from the input pin
int previous = LOW;    // the previous reading from the input pin

// variables for the debounce function
unsigned 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);
  //set low because for some reason it lights up.
  pinMode(13, INPUT);
  digitalWrite(13, LOW);
  pinMode(11,INPUT);
  digitalWrite(11,LOW);

  // relay outputs, When set LOW the relays activate. try pullups instead of digital write high.
  pinMode(out1, OUTPUT);
  digitalWrite(out1, HIGH);
  pinMode(out2, OUTPUT);
  digitalWrite(out2, HIGH);
  pinMode(out3, OUTPUT);
  digitalWrite(out3, HIGH);
  pinMode(out4, OUTPUT);
  digitalWrite(out4, HIGH);

  pinMode(Pi,OUTPUT);
  digitalWrite(Pi,HIGH);

  //buttons on front panel
  pinMode(power, INPUT_PULLUP);
  //digitalWrite(power,HIGH);
  pinMode(up, INPUT_PULLUP);
  //digitalWrite(up,HIGH);
  pinMode(down, INPUT_PULLUP);
  //digitalWrite(down,HIGH);

  //original TV inputs
  pinMode(tvpwr, INPUT_PULLUP);
  ///digitalWrite(tvpwr,HIGH);
  pinMode(voldown, INPUT_PULLUP);
  //digitalWrite(voldown,HIGH);
  pinMode(volup, INPUT_PULLUP);
  //digitalWrite(volup,HIGH);
  pinMode(menu, INPUT_PULLUP);
  //digitalWrite(menu,HIGH);
  pinMode(chdown, INPUT_PULLUP);
  ///digitalWrite(chdown,HIGH);
  pinMode(chup, INPUT_PULLUP);

  pinMode(tvledpin, INPUT_PULLUP);

  /*Serial.println("tvpwrstate");
  Serial.println(tvpwrstate);*/
}

void loop() {

  Read();
  process();
  Tv();
  Relays();
  Pi();

  // Serial.println("TVLED");
  // Serial.println(tvled);
  // Serial.println(tvpwrstate);


  /*if (tvpwrstate == 1) {
    lightson1 = 1;
    }
    if (tvpwrstate == 0) {
    lightson1 = 0;
    }*/
}
6 
void Read() {
  //read the TV LED pin to see if its powered on
  tvled = analogRead(tvledpin);
  delay(15);
  //determine if the up button is down or up, LOW is down HIGH is up
  uppress = digitalRead(up);
  delay(15);
  //read down button
  downpress = digitalRead(down);
  delay(15);
  //read power button
  powerpress = digitalRead(power);
  //Serial.println(uppress);
  //Serial.println(downpress);
  // Serial.println(lightson1);
  delay(15);


}
7 
void process() {
  //update the tvpwrstate so that the code knows if the TV is powered or not.
  // the code will operate differently when the tv is on and off.
  /*if (tvled >= 500) {
    tvpwrstate = 0;
    }
    if (tvled < 500) {
    tvpwrstate = 1;
    }*/

  if (pwrmenu == 1) { // these actions are for when the secondary button set is selected. in Theory
    //after ten seconds of inactivity, go back to the default power menu which is ZERO
    if (millis() - Time >= 10000) {
      pwrmenu = 0;
      Serial.println("power menu set to zero");
    }
    if (downpress == LOW && downpressprev == HIGH) { /* && millis() - Time > debounce) {*/
      chdownbutton = 1;
      Serial.println("channel down button pressed");

    }
    if (downpress == HIGH && downpressprev == LOW) { /* && millis() - Time > debounce) {*/
      chdownbutton = 0;
      Serial.println("channel down button released");
      Time = millis();
    }
    if (uppress == LOW && uppressprev == HIGH) { /* && millis() - Time > debounce) {*/
      chupbutton = 1;
      Serial.println("channel up button pressed");

    }
    if (uppress == HIGH && uppressprev == LOW) { /* && millis() - Time > debounce) {*/
      chupbutton = 0;
      Serial.println("channel up button released");
      Time = millis();
    }
  }



  if (powerpress != powerpressprev) {
    Time = millis();
    Serial.println("millis updated");
  }


  if (pwrmenu == 0) {// normal power menu
    if (tvpwrstate == 0) { // when TV is off according to the LED on the TV
      Serial.println("tv power LED reading off");
      //when tv is off down button toggles front and rear lights
      if (downpress == LOW && downpressprev == HIGH) { /* && millis() - Time > debounce) {*/
        lightson2 = !lightson2;
        Serial.println("power off, lights 2 toggled");
      }
      // when tv is off, up button toggles left and right lights on and off.
      if (uppress == LOW && uppressprev == HIGH) { /* && millis() - Time > debounce) {*/
        lightson1 = !lightson1;
        Serial.println("power off, lights 1 toggled");
      }
    }

    //when the Tv is on, but the power button isnt being pressed. this allows seperate functions for up and down buttons when the power button is pressed as opposed to when its not.
    if (tvpwrstate == 1 && powerpress == HIGH) {
      if (uppress == LOW && uppressprev == HIGH) { /* && millis() - Time > debounce) {*/
        if (downpress == HIGH) { //only the up button is pressed
          volupbutton = 1;
          Serial.println("up button pressed");
        }
        if (downpress == LOW) { // the up button and down button are pressed.
          lightson3 = !lightson3;
          //pwrmenu = 1; //POWER MENU WAS TOGGLED HERE BEFORE CONTROL DCK LOCKOUT WAS IMPLEMENTED
          Serial.println("CONTROLDCK TOGGLED");
          // why is the following line here?
          voldownbutton = 0;
          //Time = millis();
        }

      }
      if (uppress == HIGH && uppressprev == LOW) { /* && millis() - Time > debounce) {*/

        volupbutton = 0;
        ///lightson1=0;
        //lightson2=0;
        Serial.println("up button released");
      }

      if (downpress == LOW && downpressprev == HIGH) {/* && millis() - Time > debounce) {*/ // DOWN BUTTON HAS JUST GONE FROM NOT BEING PRESSED TO BEING PRESSED
        if (uppress == HIGH) { // THE UP BUTTON IS NOT BEING PRESSED

          voldownbutton = 1; // SET FLAG FOR VOLUME DOWN TO HIGH
          Serial.println("down button pressed");
        }
        if (uppress == LOW) { //UP BUTTON IS BEING PRESSED AT THE SAME TIME AS THE DOWN BUTTON
          fanon = !fanon; //TOGGLE THE FAN RELAY
          Serial.println("fan toggled");
        }
      }
      if (downpress == HIGH && downpressprev == LOW) {/* && millis() - Time > debounce) {*/ // DOWN BUTTON HAS GONE FROM BEING PRESSED TO BEING RELEASED
        //if (powerpress == HIGH) {
        voldownbutton = 0; // SET THE FLAG FOR VOLUME DOWN TO LOW

        Serial.println("down button released");
        //}
      }
    } // THE END OF THE SEPERATE FUNCTIONS FOR WHEN THE POWER BUTTON IS NOT BEING PRESSED.



    if (powerpress == LOW && powerpressprev == HIGH) {/* && millis() - Time > debounce && millis() - Time < 2900) {*/ // POWER BUTTON HAS JUST BEEN PRESSED AND IS STILL HELD LOW
      Serial.println("power button down");
      Time = millis(); // UPDATE TIME TO DETERMINE HOW LONG THE BUTTON HAS BEEN PRESSED FOR LATER
      Serial.println("power button down timer started");
    }
    if (powerpress == LOW && millis() - Time > 200 && millis() - Time < 2900) { // DEBOUNCE THE POWER BUTTON AND REACT IF ITS HELD FOR LESS THAN 3 SECONDS.
      if (downpress == LOW && downpressprev == HIGH) {/* && millis() - Time > debounce) {*/ //THE DOWN BUTTON HAS BEEN PRESSED AND IS STILL LOW
        Serial.println("simultaneous down pressed");
        lightson2 = !lightson2;

      }

      if (uppress == LOW && uppressprev == HIGH) {/* && millis() - Time > debounce) {*/ // THE UP BUTTON HAS BEEN PRESSED AND IS STILL LOW
        Serial.println("simltaneous up pressed");
        lightson1 = !lightson1;
        //Time = millis();
      }
    } // END OF SIMULTANEOUS UP DOWN AND POWER COMBOS
    if (tvpwrstate == 1 && tvpwrswitchreleased == 0) {
      /* if (powerpress == LOW && powerpressprev == HIGH){
        Time=millis();
        Serial.println("power button down");
        Serial.println("power button timer refreshed");
        }*/

      if (powerpress == LOW /*&& powerpressprev == HIGH */ && millis() - Time > 3000) { // WHEN THE POWER BUTTON HAS BEEN HELD LOW FOR OVER 3 SECONDS.
        tvpwrbutton = 1;
        tvpwrswitchreleased = 1;
        //lightson1 = 0;
        // lightson2 = 0;
        Serial.println("power off FLAG RAISED");
        //Serial.print("tvpwrstate");
        //Serial.println(tvpwrstate);
      }
    }
    /* if (tvpwrstate == 0 && tvpwrswitchreleased == 0) {
        if (powerpress == LOW && powerpressprev == HIGH){
       Time=millis();
       }
       if (powerpress == LOW && millis() - Time > 50 ) {

         if (uppress == HIGH && downpress == HIGH) {
           tvpwrbutton = 1;
           tvpwrswitchreleased = 1;
           lightson2 = 1;
           Serial.println("power on actioned");
           //// Serial.print("tvpwrstate");
           //Serial.println(tvpwrstate);
         }
       }
      }*/

    if (powerpress == HIGH && powerpressprev == LOW ) {
      tvpwrswitchreleased = 0;
      Serial.println("power button released");
      //Serial.print("tvpwrstate");
      //Serial.println(tvpwrstate);
    }




  }
  powerpressprev = powerpress;
  uppressprev = uppress;
  downpressprev = downpress;
  // Serial.println("buttons read");
}
8 
void Pi() {

}
void Tv() {

  if (tvpwrbutton == 1) {
    pinMode(tvpwr, OUTPUT);
    digitalWrite(tvpwr, LOW);
    delay (500);
    pinMode(tvpwr, INPUT);
    tvpwrbutton = 0;
    //if (tvpwrstate == 1) {
    // lightson1 = 0;
    // lightson2 = 0;
    // }
    // tvpwrstate = !tvpwrstate;

    Serial.println("power command sent to TV");
  }
  if (volupbutton == 1) {
    pinMode(volup, OUTPUT);
    digitalWrite(volup, LOW);
    delay (150);
    pinMode(volup, INPUT);
    // volupbutton=0;
    //// lightson1=1;
    Serial.println("vol up command sent to TV");
  }
  if (voldownbutton == 1) {
    pinMode(voldown, OUTPUT);
    digitalWrite(voldown, LOW);
    delay (150);
    pinMode(voldown, INPUT);
    // voldownbutton=0;
    // lightson1=1;
    Serial.println("vol down command sent to TV");
  }
  /*if (chdownbutton == 1) {
    pinMode(chdown, OUTPUT);
    digitalWrite(chdown, LOW);
    delay (100);
    pinMode(chdown, INPUT);
    // voldownbutton=0;
    // lightson1=1;
    Serial.println("ch down command sent to TV");
    }
    if (chupbutton == 1) {
    pinMode(chup, OUTPUT);
    digitalWrite(chup, LOW);
    delay (100);
    pinMode(chup, INPUT);
    // voldownbutton=0;
    // lightson1=1;
    Serial.println("ch up command sent to TV");
    }*/
}
void Relays() {
  if (fanon == 1) { //Fan
    digitalWrite(out2, 0);
  }
  if (fanon == 0) { // Fan
    digitalWrite(out2, 1);
  }

  if (lightson1 == 1) { //LEFT RIGHT

    digitalWrite(out3, 0);

  }
  if (lightson1 == 0) { //LEFT RIGHT
    digitalWrite(out3, 1);

  }
  if (lightson2 == 1) {//front rear


    digitalWrite(out4, 0);

  }
  if (lightson2 == 0) {//front rear
   
    digitalWrite(out4, 1);
    
  }
  if (lightson3 == 1) {//CONTROL DCK

    digitalWrite(out1, 0);

  }
  if (lightson3 == 0) {//CONTROL DCK

    digitalWrite(out1, 1);

  }
}
9

I don't think @ChrisTenone intended you to post your code in several pieces. Joining pieces together just introduces the risk of adding errors.

I believe he intended that you should just write a shortened version of your program that illustrates the problem and excludes all the irrelevant stuff.

...R

10

I have looked through the code attached to your Original Post and I immediately ran into a problem. One of your key variables is called "up" but when I try to see all instances of that variable by highlighting it I discover that the same pair of letters appears all over the place so I can't pick out the instances that only have "up".

If you had called it upButtonPin there would be no problem finding the instances. But I can't do a search-and-replace because it will change all the other names that use the two letters "up"

...R

11

I may rewrite it and try to cut it down, but I dont have time at the moment, i wrote it with the family out of town, and its a different story when you only have an hour a night once the kids are in bed :slight_smile:

I see what you mean about "UP" that could be quicker to sort out

12

Just change the name of "up" to something better where it is first declared. Then the compiler will find all the other uses for you.

13

I don't want to be teaching granma to suck eggs and certainly don't want to be insulting with a stupid suggestion (given I am NOT a software engineer) but it seems to me that you are taking the hard road with your state machine. I build them in one fashion exactly because it saves me the headache of trying to keep a bunch of combinatorial logic clear in my head. I hobbled this together quickly to demonstrate so it may have an error or two, but here goes:

void setup() {
  // put your setup code here, to run once:

}

#define SM_IDLE           0
#define SM_PWR_BTN        1
#define SM_SHIFT          2
#define SM_UP             3
#define SM_DWN            4
#define SM_SHIFT_UP       5
#define SM_SHIFT_DWN      6
#define SM_RESET          7
#define SM_TURN_PWR_OFF   8
#define SM_TURN_PWR_ON    9

void loop() {
  // put your main code here, to run repeatedly:

  byte StateVar = SM_IDLE;
  unsigned long BtnTmr;
  bool PwrOn;

  switch (StateVar) {
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case Idle:
      if (PwrBtnPressed) SateVar = SM_PWR_BTN;
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_PWR_BTN:
      BtnTmr = (3000 + millis());    // start the 3 second timer
      StateVar = SM_SHIFT;
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_SHIFT:
      if (BtnTmr < millis())
        if (PwrOn)
          StateVar = SM_TURN_PWR_OFF;
        else
          StateVar = SM_TURN_PWR_ON;
      if (UpBtnPressed)
        StateVar = SM_SHIFT_UP;
      if (DwnBtnPressed)
        StateVar = SM_SHIFT_DWN;
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_UP:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_DWN:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_SHIFT_UP:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_SHIFT_DWN:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_TURN_PWR_ON:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_TURN_PWR_OFF:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_RESET:
    default:
      break;
      //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  } //switch
  //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
}

At each state I have only to consider the factors relevant to that state. Nothing else is relevant because nothing else is being executed. I have turned nightmare problems into clear and simple solutions using this type of state machine. And the only cost, a state variable of 1 byte. :slight_smile:
I hope this helps in some way and isn't just proof I should keep my none software mouth shut.

PE.

14

PaulRB:
Just change the name of "up" to something better where it is first declared. Then the compiler will find all the other uses for you.

if i change the name "up" to "uppp" at the start of the sketch then click verify, when it says compiling sketch, it then says "up" is not declared.

is that what you are suggesting to do ?

15

PedantEngineer:
I don't want to be teaching granma to suck eggs and certainly don't want to be insulting with a stupid suggestion (given I am NOT a software engineer) but it seems to me that you are taking the hard road with your state machine. I build them in one fashion exactly because it saves me the headache of trying to keep a bunch of combinatorial logic clear in my head. I hobbled this together quickly to demonstrate so it may have an error or two, but here goes:

void setup() {

// put your setup code here, to run once:

}

#define SM_IDLE          0
#define SM_PWR_BTN        1
#define SM_SHIFT          2
#define SM_UP            3
#define SM_DWN            4
#define SM_SHIFT_UP      5
#define SM_SHIFT_DWN      6
#define SM_RESET          7
#define SM_TURN_PWR_OFF  8
#define SM_TURN_PWR_ON    9

void loop() {
  // put your main code here, to run repeatedly:

byte StateVar = SM_IDLE;
  unsigned long BtnTmr;
  bool PwrOn;

switch (StateVar) {
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case Idle:
      if (PwrBtnPressed) SateVar = SM_PWR_BTN;
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_PWR_BTN:
      BtnTmr = (3000 + millis());    // start the 3 second timer
      StateVar = SM_SHIFT;
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_SHIFT:
      if (BtnTmr < millis())
        if (PwrOn)
          StateVar = SM_TURN_PWR_OFF;
        else
          StateVar = SM_TURN_PWR_ON;
      if (UpBtnPressed)
        StateVar = SM_SHIFT_UP;
      if (DwnBtnPressed)
        StateVar = SM_SHIFT_DWN;
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_UP:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_DWN:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_SHIFT_UP:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_SHIFT_DWN:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_TURN_PWR_ON:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_TURN_PWR_OFF:
      break;
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    case SM_RESET:
    default:
      break;
      //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  } //switch
  //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
}



At each state I have only to consider the factors relevant to that state. Nothing else is relevant because nothing else is being executed. I have turned nightmare problems into clear and simple solutions using this type of state machine. And the only cost, a state variable of 1 byte. :-)
I hope this helps in some way and isn't just proof I should keep my none software mouth shut.

PE.

I will have a look at this, thanks,
is it able to tell when a button is pressed and when it is released?

16

spruce_m00se:
if i change the name "up" to "uppp" at the start of the sketch then click verify, when it says compiling sketch, it then says "up" is not declared.

is that what you are suggesting to do ?

The compiler error will list each line the variable "up" occurs on. But don't name it uppp! Use a name that describes what it represents. That way, when you go back to work on the program in a few months, the names will remind you what they are for!

17

the name "up" reminds me exactly what its for, as do all of the names, when combined with the annotations. I have actually spent a lot of time on this making sure i can pick it back up late as i plan to use the code again for something else later down the road.

Uppp would also remind me what it is, and be easier to do a CTRL+F on

18

The joy of the state machine is exactly this:
When (in what states) are you interested in a button being pressed - because it is in those states that you test: if (ButtonPressed) then State changes to xxxxx and "break".
In my example code the up button is checked for being pressed in the idle state only. If it is pressed then the state is changed from SM_IDLE to SM_UP. And in state SM_UP I will look for the up button released.

A better example is the power button. Again, only checked in SM_IDLE for being pressed and if it is the state is changed to SM_PWR_BTN. In SM_PWR_BTN a timer is set for the 3 second "shift" function you mentioned and the state is changed to SM_SHIFT because for the next 3 seconds that's what it is, a shift button. Now in "shift" the up and down buttons do something different (I'm not sure what) but because the state is SM_SHIFT you know that if up or down is pressed you do something different. You also know that if the 3 second timer expires that the power is toggled because the power button is only a 'shift' button for 3 seconds.

Is this making sense to you?

I find that a state machine coded this way can make a huge and complex thing easily handled and without those annoying logical errors that come from having a bunch of booleans that somehow, in combination can represent the states in a state machine but they also represent a vast number of illegal or impossible states that can happen all too easily. For example, if I have 3 booleans then there are 2^3 = 8 possible states and any or all can happen if there is an error in the coding or an unexpected input signal for example. A state machine though, cannot get into an illegal state (assuming no coding errors) because the illegal states are not defined and therefore cannot be assigned. I really hope that makes sense to you because that is the heart of the thing. The 'reason for being' for a state machine and why they are so fantastic. :slight_smile:

19

spruce_m00se:
the name "up" reminds me exactly what its for, as do all of the names, when combined with the annotations. I have actually spent a lot of time on this making sure i can pick it back up late as i plan to use the code again for something else later down the road.

Uppp would also remind me what it is, and be easier to do a CTRL+F on

For what it is worth, I'd use "UpButton" or "UpBtn" but I tend to use long names etc. That's just my style. My code often reads like english as a result. Wordy and a lot more typing and a lot less time debugging.

20

spruce_m00se:
I have an Arcade Machiene that I have made, and I wanted to get all fancy and do some things with an arduino, I have planned to interface some rgb strips etc later, but for now I have it setup with three switches on a control panel that are wired to the arduino. the idea of the switches is to control the tv but also allow multiple functions on the same switch.

The switches are defined in the code as:
//switches on the front panel conencteed to these pins, pulled to low when pressed.
int up = 5;
int power = 4;
int down = 6;

The code is my attempt at a state machiene, using simple 1 and 0 flags to trigger events in non blocking code.

The power button is basically debounced, and then toggles the tv power on or off if held for more than 3 seconds,
during the run up to those 3 seconds it acts as a "shift" button allowing secondary functions to be achieved by pressing the up and down buttons,

there are four relays connected to the arduino are represented in the code by out1 to out4, the relays always do as expected when expected. (apart from needing to reverse the logic to trigger them)

there are then three wires soldered to the TV push buttons for power and volume up and down.

when i want to trigger a button press i briefly set them as outputs then back to inputs,

so the conundrum I would like some help with is:

the volume doesnt work as expected when first powered up.
the volume up actually reduces the volume, and the vol down apparently does nothing, the power button works as expected, and when power is used as shift, both up and down vol buttons activate the relays.

there is a lot of serial debugging in the code, which suggests to me that the output to the tv has been toggled as expected but the TV doesnt react.

THEN

once I power cycle the TV by holding the power button 3 seconds, waiting briefly then doing it again, then the vol buttons react as expected for as long as the arduino stays powered up.

once I reset power to the arduino it all goes pants again.

I have gone through the code time and time again, looking for flags that are not set, or double set, but my serial debugging is oly sent right as the output is sent to the TV, so if i can read it, then it shoudl have been sent,

I cant for the life of me work out what's going on.

any help would be greatly appreciated, I've done my best to annotate the code and give it meaningful names, so hopefully it can be understood.

m00se

I've just read through this more closely and have a few questions:

  1. are you using the relays to operate the buttons?

  2. or are you operating the TV buttons directly with the arduino? (Wondering about electrical isolation)

  3. Why do you switch the button inputs to outputs albeit briefly?

  4. Do you have external pull up resistors on the button inputs or just using the internal pull ups?

  5. Your serial debugging tells you something was attempted, why are you so confident that the message sent to the TV was correctly done? (Not a criticism, just wondering what you know but haven't said. Probably something I missed or you take so much as assumed knowledge that it hasn't even occurred to you that it might not be assumable :wink:

PE