Unable to switch off LEDs separately by using different pushbuttons

Hi there I'm currently creating a wind powered race track system for my school project,that requires a 4D systems ULCD-43PT-AR touchscreen to communicate with the Arduino UNO.I have to program them to send Arduino IOs signal in bidirectional ways.There is a START winbutton on my touchscreen and when I activate it,2 LEDs on the breadboard which is connected to the Arduino UNO and LED digits on the touchscreen are supposed to light up and run at the same time.Now,i have already achieved that.Next,there are also 2 pushbuttons on the breadboard.When pushbutton1 is activated,it is only meant to switch off LED1 and timer1(LED digit1) at the same time.When pushbutton2 is activated,it will then switch off LED2 and timer2(LED digit2) at the same time.However,right now I couldn't even get the pushbuttons to switch off either 1 of the LEDs on the breadboard.It just keeps remounting the ULCD-43PT-AR touchscreen whenever I activate either 1 of the pushbuttons.I can't seem to find a solution to this problem please help it's really urgent :cry:

#include <genieArduino.h>

Genie genie;

int buttonInput1 = 4;
int buttonInput2 = 5;      // the number of the pushbutton pin


int ledOutput1 =  12; 
int ledOutput2 =  11;      // the number of the LED pin

int buttonState = 0;
int buttonState1 = 0;     // variable for reading the pushbutton status


void setup() 
{

 Serial.begin(9600);
 genie.Begin(Serial);
 genie.AttachEventHandler(myGenieEventHandler); 
 pinMode (ledOutput1, OUTPUT);
 pinMode (ledOutput2, OUTPUT) ;
 pinMode (buttonInput1, INPUT);
 pinMode (buttonInput2, INPUT);
 genie.WriteStr(0,  GENIE_VERSION);

}

void loop() 
{

genie.DoEvents();
delay(10);
   
}

void myGenieEventHandler() 
{
 genieFrame Event;
 int zone1;                              //the START winbutton on the screen is named zone1
 static int zonestate = 0;            //initial state is 0               
 buttonState = digitalRead(buttonInput1);
 buttonState1 = digitalRead(buttonInput2); 
 
 char charbuf[50];     //to convert into char for showing state of winbutton in string0

 genie.DequeueEvent(&Event);    //Read from Button2 for both a Reported Message from Display  
                 
 if (genie.EventIs(&Event, GENIE_REPORT_EVENT, GENIE_OBJ_WINBUTTON, 0) || genie.EventIs(&Event, GENIE_REPORT_OBJ, GENIE_OBJ_WINBUTTON, 0))
 {                                                                                                                                      
    zone1 = genie.GetEventData(&Event);   //Receive the event data from the zone1

    sprintf(charbuf, "%d", zone1);       //convert zone1 int value to char value to show in string0

    genie.WriteStr(0, charbuf);      //write char to string0 --> Strings0 will display the status of zone1

    if(zone1 == 0) // check the status of the led
    {
      if(zonestate == 0)
      {
        
          zonestate = 1;
          
      } 
    
      else 
      zonestate = 1;

     if (buttonState == HIGH) 
 {     
   
   digitalWrite(ledOutput1, LOW); 
   
 } 
 else 
{
   
   digitalWrite(ledOutput1, HIGH); 
   
}

if (buttonState1 == HIGH) 
  {     
     
        digitalWrite(ledOutput2, LOW); 
  } 

 else 
{
   
   digitalWrite(ledOutput2, HIGH);   // LED remain on:
 }
          
}

    digitalWrite(12, zonestate);
    digitalWrite(11, zonestate);           
 
    timer();
    
 }
}
    
  void timer()  //Start Timer
{
 genie.WriteStr(3, "Leddigits at\nvarious states");

 for(int i =0;i<10000;i++)
 {  
  genie.WriteObject(GENIE_OBJ_LED_DIGITS, 0x00,i);
  genie.WriteObject(GENIE_OBJ_LED_DIGITS, 0x01,i);
 }
}

need to evaluaate your first if()

if(zone1 == 0) // check the status of the led
    {
      if(zonestate == 0)
      {
        
          zonestate = 1;
          
      } 
    
      else 
      zonestate = 1;

we know you hammered that zonestate=1 thing.
but the right bracket for
if(zone1 == 0) // check the status of the led

is not found for like 20 lines. includes 3 if/else statements.

      if(zonestate == 0)
      {
        
          zonestate = 1;
          
      } 
    
      else 
      zonestate = 1;

If the value is 0, make it 1. Otherwise, make it 1. The if test seems useless, somehow. Did I miss something?

buttonState and buttonState1 are poor names. I suppose that they are fine if your switches are actually labeled Button and Button1. But if I were viewing such a product on display in a store, I'd think "What moron built this?", and move on.

Give these variables that mean something, so we don't have to guess.

Get rid of

the useless

amounts of

white space

in your code.

Use Tools +
Auto Format to
fix the crappy
indenting.

Try explaining again what your problem is.

Personally, I get exhausted reading run-on sentences that don't even have space between them. I gave up before I was half way through. Hit the enter key once in while, too, like you do in your code...

this is one long if()

    if(zone1 == 0) // check the status of the led
    {
      if(zonestate == 0){
        zonestate = 1;
      }  
      else                                                   // no left bracket ?
      zonestate = 1;
      if (buttonState == HIGH)       {     
        digitalWrite(ledOutput1, LOW); 
      }       
      else       {
        digitalWrite(ledOutput1, HIGH); 
      }
      if (buttonState1 == HIGH)       {     
        digitalWrite(ledOutput2, LOW); 
      }       
      else       {
        digitalWrite(ledOutput2, HIGH);   // LED remain on:
      }

    }

at this point, I would offer that you should take a run through your code and just make sure the formatting is how you intended.

PaulS is dead-on. use auto formatting, it will help show how things are linked.

 if(zonestate == 0){
        zonestate = 1;
      }  
      else    
      zonestate = 0;

reduces to:

zonestate = !zonestate;

I tried modifying my code and this is how it looks like now.I hope this gives a better understanding.The start winbutton on the ULCD-43PT-AR touchscreen will activate the timer on the screen as well as turning on both of the hardware LEDs at the same time.Next,I need the hardware pushbuttons to switch off individual LEDs one at a time.However,the LEDs are not responding to the pushbuttons,they remained on.In need of a solution,I would really appreciate anyone's help!

#include <genieArduino.h>

Genie genie;
int buttonInput1 = 6;
int buttonInput2 = 5;      // the number of the pushbutton pin
int ledOutput1 =  11; 
int ledOutput2 =  12;      // the number of the LED pin
int buttonState1 = 1;
int buttonState2 = 1;     // variable for reading the pushbutton status
int currentButtonState1;
int currentButtonState2;
int ledState1; 
int ledState2;
int startBtn = 1;                 //the START winbutton on the screen 

void setup() {
  Serial.begin(9600);
  genie.Begin(Serial);
  genie.AttachEventHandler(myGenieEventHandler); 
  pinMode (ledOutput1, OUTPUT);
  pinMode (ledOutput2, OUTPUT) ;
  pinMode (buttonInput1, INPUT);
  pinMode (buttonInput2, INPUT);
  genie.WriteStr(0,  GENIE_VERSION);
}

void loop() {
  genie.DoEvents(); 
  buttons();
}

void myGenieEventHandler(){
  genieFrame Event;   
  char charbuf[50];     //to convert into char for showing state of winbutton in string0
  genie.DequeueEvent(&Event);  
  if (genie.EventIs(&Event, GENIE_REPORT_EVENT, GENIE_OBJ_WINBUTTON, 0) || genie.EventIs(&Event, GENIE_REPORT_OBJ, GENIE_OBJ_WINBUTTON, 0))
  {                                                                                                                                      
    startBtn = genie.GetEventData(&Event);   //Receive the event data from the zone1
    sprintf(charbuf, "%d", startBtn);       //convert zone1 int value to char value to show in string0
    genie.WriteStr(0, charbuf);  //write char to string0 --> Strings0 will display the status of zone1

    if(startBtn == 1) {   // check the status of the START
      if(ledState1 == 0)
        ledState1 = 1;

      if(ledState2 == 0)
        ledState2 = 1;
    }
    digitalWrite(11, ledState1);   
    digitalWrite(12, ledState2);   
    startBtn = 0;
    timer();
    buttons();
    delay(10);
  }
}

void timer() {           //Start Timer
  genie.WriteStr(3, "Leddigits at\nvarious states");
  for(int i =0;i<10000;i++){  
    genie.WriteObject(GENIE_OBJ_LED_DIGITS, 0x00,i);
    genie.WriteObject(GENIE_OBJ_LED_DIGITS, 0x01,i);
  }
}

void buttons(){
  currentButtonState1 = digitalRead(5);
  currentButtonState2 = digitalRead(6); 
  if (currentButtonState1 == 0)   
    ledState1 = 0;            
  else if (currentButtonState2 == 0)     
    ledState2 = 0;   
}
[\code]

Hi,
Try this.

I tried modifying my code and this is how it looks like now.
I hope this gives a better understanding.
The start winbutton on the ULCD-43PT-AR touchscreen will activate the timer on the screen as well as turning on both of the hardware LEDs at the same time.
Next,I need the hardware pushbuttons to switch off individual LEDs one at a time.
However,the LEDs are not responding to the pushbuttons,they remained on.
In need of a solution,I would really appreciate anyone's help!

I suggest you read your post before posting it, and use spaces after full stop and new lines at times.

Your explanations are very hard to read all in one paragraph.

Sorry to criticise but to make your case readable you need to read it back to yourself.
It might be a good exercise to run your posts past your project co-ord to help with your posts, thats what your teachers are there for.

And thanks :slight_smile: for using code tags, you must have read the sticky at the top of the forum list.

Tom.... :slight_smile:

void buttons(){
  currentButtonState1 = digitalRead(5);
  currentButtonState2 = digitalRead(6); 
  if (currentButtonState1 == 0) 
    ledState1 = 0;          
  else if (currentButtonState2 == 0)    
    ledState2 = 0;  
}

What if they are both 0? The else part will not be executed and LedState2 will not be set to 0
Probably reduces to:

void buttons(){
    ledState1 = digitalRead(5);      
    ledState2 = digitalRead(6);  
}

Hardly worth a function.

I've tried reducing the void buttons() codings but the pushbuttons are still unable to turn off the LEDs.

#include <genieArduino.h>

Genie genie;
int buttonInput1 = 6;
int buttonInput2 = 5; // the number of the pushbutton input pin
int ledOutput1 =  11; 
int ledOutput2 =  12; // the number of the LED output pin
int buttonState1 = 1;
int buttonState2 = 1; // variable for reading the pushbutton status
int ledState1 = 0; 
int ledState2 = 0; // variable for reading the LED status
int startBtn = 0;  //varible for reading START winbutton on the screen 

void setup() {
  Serial.begin(9600);
  genie.Begin(Serial);
  genie.AttachEventHandler(myGenieEventHandler); 
  pinMode (ledOutput1, OUTPUT);
  pinMode (ledOutput2, OUTPUT) ;
  pinMode (buttonInput1, INPUT);
  pinMode (buttonInput2, INPUT);
  genie.WriteStr(0,  GENIE_VERSION);
}

void loop() {
  genie.DoEvents(); 
  buttons();
}

void myGenieEventHandler(){
  genieFrame Event;   
  char charbuf[50]; //to convert into char for showing state of winbutton in string0
  genie.DequeueEvent(&Event);  
  if (genie.EventIs(&Event, GENIE_REPORT_EVENT, GENIE_OBJ_WINBUTTON, 0) || genie.EventIs(&Event, GENIE_REPORT_OBJ, GENIE_OBJ_WINBUTTON, 0))
  {                                                                                                                                      
    startBtn = genie.GetEventData(&Event);   //Receive the event data from the zone1
    sprintf(charbuf, "%d", startBtn);  //convert zone1 int value to char value to show in string0
    genie.WriteStr(0, charbuf);  //write char to string0 --> Strings0 will display the status of zone1

    if(startBtn == 1) {   // check the status of the START
      if(ledState1 == 0)
        ledState1 = 1;

      if(ledState2 == 0)
        ledState2 = 1;
    }
    digitalWrite(11, ledState1);   
    digitalWrite(12, ledState2);   
    startBtn = 0;
    timer();
    buttons();
    delay(10);
  }
}

void timer() { //Start Timer
  genie.WriteStr(3, "Leddigits at\nvarious states");
  for(int i =0;i<10000;i++){  
    genie.WriteObject(GENIE_OBJ_LED_DIGITS, 0x00,i);
    genie.WriteObject(GENIE_OBJ_LED_DIGITS, 0x01,i);
  }
}

void buttons(){    
    ledState1 = digitalRead(5);            
    ledState2 = digitalRead(6);   
}  
[\code]
  if (genie.EventIs(&Event, GENIE_REPORT_EVENT, GENIE_OBJ_WINBUTTON, 0) || genie.EventIs(&Event, GENIE_REPORT_OBJ, GENIE_OBJ_WINBUTTON, 0))
  {                                                                                                                                      
    startBtn = genie.GetEventData(&Event);   //Receive the event data from the zone1
    sprintf(charbuf, "%d", startBtn);  //convert zone1 int value to char value to show in string0
    genie.WriteStr(0, charbuf);  //write char to string0 --> Strings0 will display the status of zone1

//TRY MOVING YOUR CLOSING CURLY BRACKET TO HERE
}
if(startBtn == 1) {   // check the status of the START
      if(ledState1 == 0)
        ledState1 = 1;

      if(ledState2 == 0)
        ledState2 = 1;
......... ETC.