help needed in my program to light up leds

Hello, i am newbie to arduino so please bear with me as my questions might seem noob like :stuck_out_tongue:
I have written a LED lighting program that checks 6 input switches arranged in an array and lights up 6 LED’s on an output array. If there is no input from the switches then it starts running the LED’s in a predefined automated pattern. But whenever it finds a high from a switch it breaks the automated pattern and lights up the relative output LED.
In my program i have set pin 2 to pin 7 as digital read that reads the switches.
The pin 8 to pin 13 are the output pins that has the output LED’s.
In the automation the LED’s first blink from left to right and then right to left one by one while the 13th output pin stays on.
All this is happening in a loop.
I have run my program on 123dcircuits arduino emulator before putting it to my arduino so that i dont fry it by programming mistake but it is straightly going to the automated part without reading the input switch :frowning: also in the emulation the automated part is running just once and then stopping without being in a loop.

My program:

int thisPin = 2 ;
int cPin=13;
int oPin=8;
int orPin=12;
int oiPin;
void setup()
{
pinMode(thisPin, INPUT);
pinMode(oiPin, OUTPUT);
pinMode(cPin, OUTPUT);
pinMode(oPin, OUTPUT);
}
void loop()
{
for (thisPin; thisPin<8; thisPin++)
{
if(digitalRead(thisPin)==HIGH)
{
oiPin=thisPin;
digitalWrite(oiPin+6, HIGH);
}
else
if(digitalRead(thisPin)==LOW)
{
digitalWrite(cPin, HIGH);
for (oPin ; oPin < 13; oPin++)
{
pinMode(oPin, OUTPUT);
digitalWrite(oPin, HIGH);
delay(3000);
digitalWrite(oPin, LOW);
delay(1000);
}
digitalWrite(cPin, LOW);
delay(1000);
digitalWrite(cPin, HIGH);
for (orPin ; orPin > 7; orPin‚Äď)
{
digitalWrite(orPin, HIGH);
delay(3000);
digitalWrite(orPin, LOW);
delay(1000);
}
digitalWrite(cPin, LOW);
delay(1000);
}
}
}

P.S.- I just have basic C++ knowledge and have never done this before. The program i have written is whatever i thought should work with my requirements and therefore may be wrong.
If anyone has any other method to make this work i am all open to suggestions. :slight_smile:

for (oPin ; oPin < 13; oPin++)

Oops.

Don’t cross-post, it wastes time.

Duplicate deleted

I am a noob also but I believe you are defining your arrays incorrectly
It should be something like this.

Const byte thisPin [] ={2,3,4,5,6,7};

Void setup()
{
for (thisPin [0]; ThisPin <=6;thisPin++)
 {
   pinMode  (thisPin, INPUT)
 }
}

This just to initialize pins 2-7 as inputs

It should be something like this

No, it really shouldn’t.
You‚Äôll notice you haven‚Äôt declared an array, and ‚Äúconst‚ÄĚ is not capitalised.

for (thisPin [0]; ThisPin <=6;thisPin++)

Nope.

If it’s a const, how can it change for a variable?

const byte thisPin;

or

byte pinsArray[] = {2,3,4,5,6,7,8,};  // pinsArray[0] = pin 2, ... pinsArray[6] = pin 8

setup:
for (thisPin = 0; thisPin <=6; thisPin++)
{
pinMode  (pinsArray[thisPin], INPUT);
}

I knew I should of my mouth shut till I looked it up to be sure.
But it should be Const. Because pins don’t change right?

But it should be Const

No, it should be const.

Lol, yes of course

"should be" but will work just fine as byte also.

First of all thanks to everyone taking their time to help me. I really appreciate it. I am also sorry i couldn’t respond sooner as i was out for some work.
I went through the posts and it seems i am doing something wrong in declaring the input pins? Should i declare the input pins part within voidsetup() like this?
Quote-
"
byte pinsArray = {2,3,4,5,6,7,8,};

setup:
for (thisPin = 0; thisPin <=6; thisPin++)
{
pinMode (pinsArray[thisPin], INPUT);
}
"

But what should i declare the input pin type- const or byte? I kept it int so that i can transfer the values within the program.
Example-
"
{
if(digitalRead(thisPin)==HIGH)
{
oiPin=thisPin;
digitalWrite(oiPin+6, HIGH);
}
"
as you can see i am transferring the thisPin value to oiPin which is int type.

Also if anyone would be kind enough to correct my program or has an alternate I will be very gateful. Thank you all.

Please use [ code ] tags.

Since you've created an array, you can work with your pins as if they were numbered 0-6, so you should ALWAYS use that array to work with the pins. oiPin+6 is worse than meaningless. You won't remember what 6 represents in a month from now when you're trying to make just a small update to the code.

if(digitalRead(pinsArray[thisPin])==HIGH)      
{
oiPin=thisPin;
digitalWrite(pinsArray[oiPin], HIGH);
}

But somehow, I don't think this is what you intended. Perhaps you need two arrays: one for input and one for output.

^ Ok i will use the tags from now on, I am new to the forum and getting used to it. I also thought the oiPin+6 is to blame for the problem, but at the time I thought it might work to do what i wanted from the program.

For clarification let me explain you the purpose of the program:

I have got an interactive lighting idea for a project. Think the inputs will be from six IR proximity sensors so if some one waves his hand over it will light up an output LED located in the output pin position (input pin +6). So to sum it up i have the input IR sensors at pin {2,3,4,5,6,7} and output LEDs at pin{8,9,10,11,12,13}.

For a working example if someone waves his hand on the IR sensor located at pin 3 of arduino it will take that input and then it will light up the LED located at pin 9 of arduino making that as an output pin.

But if there is no one there to interact with the lighting then it will run random lights in a pattern until some one waves his hand over the IR prox. sensor. I also want a delay of at least 3 secs between the changes as because the lights are inside glass spheres with a photoluminescence coating and 3 secs gives it a good time to recharge. Fade effect between the LED lights are not required as the photoluminescence generates a better dimming effect.

Are there any alternate and better way to program the arduino for the task? If my program seems inefficient for the task suggest an alternate, i know my program has got lot of issues.

Edit: Code tags removed. They are for code not your comments.

^Please omit the post above the entire thing got written within the code tag (didn't know the code within the brackets will do that!!) I am new to the forum and getting used to it.

I also thought the oiPin+6 is to blame for the problem, but at the time I thought it might work to do what i wanted from the program.

For clarification let me explain you the purpose of the program:

I have got an interactive lighting idea for a project. Think the inputs will be from six IR proximity sensors so if some one waves his hand over it will light up an output LED located in the output pin position (input pin +6). So to sum it up i have the input IR sensors at pin {2,3,4,5,6,7} and output LEDs at pin{8,9,10,11,12,13}.

For a working example if someone waves his hand on the IR sensor located at pin 3 of arduino it will take that input and then it will light up the LED located at pin 9 of arduino making that as an output pin.

But if there is no one there to interact with the lighting then it will run random lights in a pattern until some one waves his hand over the IR prox. sensor. I also want a delay of at least 3 secs between the changes as because the lights are inside glass spheres with a photoluminescence coating and 3 secs gives it a good time to recharge. Fade effect between the LED lights are not required as the photoluminescence generates a better dimming effect.

Are there any alternate and better way to program the arduino for the task? If my program seems inefficient for the task suggest an alternate, i know my program has got lot of issues.

You can edit your own posts too.

There's different kinds of inefficiency. In small microcontrollers like the Arduino, you are often trying to cram the code into a small amount of memory, with a limited amount of processing power, so you try to make the code itself more efficient. But that makes it very difficult to write the code and absolutely impossible to understand that code in a few months from now when you need to make a small change. So the goal of all programming since the 1960s is to make it more efficient for the programmer.

If it takes your program 16,000,000 clock cycles of "effort" to turn an LED on then that's actually pretty good if your goal is to have a one-second delay. Don't worry about making the code "efficient." Make it understandable and maintainable.

I forsee your code will use lots of arrays. Input pins and output pins have already been identified as arrays. You will need arrays of TimeLEDWentOn[], TimeLEDWentOff[] and AdjacentLEDToIgnite[] and possibly more.

Thank you for the info :slight_smile: yes the memory is a big problem, moreover my chip is an atmega 8 so even less memory. For the moment i am feeling too lost, I am doing this solo, the only programming knowledge i have is of java and c++ which I did in school long ago and all my friends who do these kind of programming are busy with their job. The program i have written is a result of lot of google search and existing examples. Can you please assist me in the first part of the program i,e. the part which reads the input pins and transfers it to output, maybe an existing example? You dont have to correct my program, if you have got an alternate way please suggest one.

OK, so the number of inputs exactly equals the number of outputs? Then the following code sets up two arrays to keep track of the pins. This is useful if you decide that the switch connected to pin 2 should really control the LED on pin 13 to make the wiring easier. Then just change that at the top and the rest of the program doesn’t care.

#define NUM_PINS 7
byte inputs[] = {2,3,4,5,6,7,8};
byte outputs[] = {9,10,11,12,13,A1,A2};  //remember that analog pins can be used as digital outputs

void setup() 
{
  for (byte thisPin = 0; thisPin < NUM_PINS; thisPin++)
  {
    pinMode(inputs[thisPin], INPUT_PULLUP);     //switch connected between each input and ground
    digitalWrite(outputs[thisPin], HIGH); //start with the LEDs off (set it high before making it an output so it doesn't flash on at this point)
    pinMode(outputs[thisPin], OUTPUT); //LED is powered from the 5v rail, the output goes LOW to turn it on
  }
}

void loop
{
  bool AnySwitchPressed;
  AnySwitchPressed = false;
  for (byte thisPin = 0; thisPin < NUM_PINS; thisPin++)
  {
    //active-low switches mean the pin is LOW (false) when the button is pressed
     if(! digitalRead(inputs[thisPin])) AnySwitchPressed = true;
  }
  if(AnySwitchPressed)
  {
    for (byte thisPin = 0; thisPin < NUM_PINS; thisPin++)
    {
      //both inputs and outputs are active-low, so we don't need to invert the logic
      digitalWrite(outputs[thisPin], digitalRead(inputs[thisPin]));
    }
    //maybe reset some of the variables related to the not-pressed state? 
    //...
  } 
  else
  {
    //do whatever you want when nothing is pressed
    //...
  }
}

Thank you Morgan, :slight_smile: I will surely include your code in my program, however the day before yesterday I have also written a new program myself with some reference to an example at:

http://www.tinkerhobby.com/led-patterns-using-dip-switch-and-arduino/

The program was somewhat similar to what i needed. I have therefore modded it according to my need. But upon compiling the program i am getting some errors. I have googled the error and found some solutions however I am not able to remove the error from my program. The errors started coming in when i included the random() command in the program.

CODE:

 // Arduino pins used for the LEDs
  // LED1 13
  // LED2 12
  // LED3 11
  // LED4 10
  // LED5 9
  // LED6 8

  // Arduino pins used for the switches
  // S1 7
  // S2 6
  // S3 5
  // S4 4
  // S5 3
  // S6 2

  // State of each switch (0 or 1)
  int state[6];

  // Random values for LED state and delay
  long onoroff;
  long millisecs;

  // loop counters
  int i, j;

  // delay
  int d = 2500;
  
  // automatic random value generator
  int auto; // * error: declaration does not declare anything*
  
  void setup() 
    {
    // pins for LEDs are outputs
    // LEDs 1-6 on pins 13-8
    for (i = 13; i >= 8; i--) 
    {
      pinMode(i, OUTPUT);
    }
    // pins for switches are inputs
    // switches 1-6 on pins 7-2
    for (i = 7; i >= 2; i--) 
    {
      pinMode(i, INPUT);
    }
  }

  void loop() 
    {
      // this part of the program lights up the revelant LED when it finds digitalread = high from input pins
    for (i = 0, j = 7; i < 6, j >= 2; i++, j--) 
    {
    state[i] = digitalRead(j);
    }
    if (state[0]) 
    {
     // write pin13 high
    {
    digitalWrite(13, HIGH);
    } 
    digitalWrite(13, LOW);
    } 
    else if (state[1]) 
    {
    // write pin12 high
    {
    digitalWrite(12, HIGH);
    delay(300);
    } 
    digitalWrite(12, LOW);
    delay(300);
    } 
    else if (state[2]) 
    {
    // write pin11 high
    {
    digitalWrite(11, HIGH);
    delay(300);
    } 
    digitalWrite(11, LOW);
    } 
    else if (state[3]) 
    {
    // write pin10 high
    {
    digitalWrite(10, HIGH);
    delay(300);
    } 
    digitalWrite(10, LOW);
    } 
    else if (state[4]) 
    {
    // write pin9 high
    {
    digitalWrite(9, HIGH);
    delay(300);
    } 
    digitalWrite(9, LOW);
    } 
    else if (state[5]) 
    {
    // write pin8 high
    {
    digitalWrite(8, HIGH);
    delay(300);
    } 
    digitalWrite(8, LOW);
    } 
    else 
    {
    // this part of the program shuffles between preset LED patterns and automatically runs when digitalread=low from all input pins  
    auto=random(0,5); // *error: expected unqualified-id before '=' token*
    // automated
    if (auto==0) //*error: expected primary-expression before 'auto'* *error: expected `)' before 'auto'*
    {
    // scroll right
    for (i = 13; i >= 8; i--) 
    {
        digitalWrite(i, HIGH);
        delay(d);
        digitalWrite(i, LOW);
    }
    } 
    else if (auto==1) //*error: expected primary-expression before 'auto'* *error: expected `)' before 'auto'*
    {
    // scroll left
    for (i = 8; i <= 13; i++) 
    {
         digitalWrite(i, HIGH);
         delay(d);
         digitalWrite(i, LOW);
    }
    } 
    else if (auto==2) //*error: expected primary-expression before 'auto'* *error: expected `)' before 'auto'*
    {
    // scroll in
    // light up LEDs on pins i and 8+(13-i)
    for (i = 13; i >= 11; i--) 
    {
        digitalWrite(i, HIGH);
        digitalWrite(21 - i, HIGH);
        delay(d);
        digitalWrite(i, LOW);
        digitalWrite(21 - i, LOW);
    }
    } 
    else if (auto==3) //*error: expected primary-expression before 'auto'* *error: expected `)' before 'auto'*
    {
    // scroll out
    // light up LEDs on pins i and 8+(13-i)
    for (i = 11; i <= 13; i++) 
    {
         digitalWrite(i, HIGH);
         digitalWrite(21 - i, HIGH);
         delay(d);
         digitalWrite(i, LOW);
         digitalWrite(21 - i, LOW);
    }
    } 
    else if (auto==4) //*error: expected primary-expression before 'auto'* *error: expected `)' before 'auto'*
    {
    // scroll back and forth
    for (i = 13; i >= 8; i--) 
    {
        digitalWrite(i, HIGH);
        delay(d);
        digitalWrite(i, LOW);
    }
    for (i = 9; i <= 12; i++) 
    {
        digitalWrite(i, HIGH);
        delay(d);
        digitalWrite(i, LOW);
    }
    } 
    else if (auto==5) //*error: expected primary-expression before 'auto'* *error: expected `)' before 'auto'*
    {
    // random
    for (i = 13; i >= 8; i--) 
    {
        randomSeed(analogRead(i - 8));
        onoroff = random(0, 2);
        millisecs = random(1000, 3000);
        digitalWrite(i, onoroff);
        delay(millisecs);
    }
    }
    }}
  



Compiled Error description(also written in the above program in the respective lines):
sketch_may17a:31: error: declaration does not declare anything
sketch_may17a.ino: In function 'void loop()':
sketch_may17a:113: error: expected unqualified-id before '=' token
sketch_may17a:115: error: expected primary-expression before 'auto'
sketch_may17a:115: error: expected `)' before 'auto'
sketch_may17a:125: error: expected primary-expression before 'auto'
sketch_may17a:125: error: expected `)' before 'auto'
sketch_may17a:135: error: expected primary-expression before 'auto'
sketch_may17a:135: error: expected `)' before 'auto'
sketch_may17a:148: error: expected primary-expression before 'auto'
sketch_may17a:148: error: expected `)' before 'auto'
sketch_may17a:161: error: expected primary-expression before 'auto'
sketch_may17a:161: error: expected `)' before 'auto'
sketch_may17a:177: error: expected primary-expression before 'auto'
sketch_may17a:177: error: expected `)' before 'auto'

Give the auto variable another name and try again.

Hey UKHeliBob it worked, i verified the program on 123d arduino simulation and it worked perfectly!!! The only problem i am facing now is the programmer board i have got. It is a Chinese arduino(or Indian) and while uploading the program it is giving an error of "cannot set sck period". While there is no problem uploading the code but the program is running slower on this board (leds blink after a lot of time). I thought the clock crystal was to blame but the features claimed isn't indicating any slow clock:

Features:

ATmega8 microcontroller (Self Programmable) Buit In Power Supply : Input voltage - +7-+12V 14 Digital I/O Pins (6 PWM outputs) 6 Analog Inputs 8k Flash Memory 16Mhz Clock Speed(even the crystal has 16mhz written on it)

should i change the cpu mhz?

Ok I think the delays in the start of the program was making it run slow. I think now it should not cause any problem.