Tips to improve my code.

Using Arduino Programming Questions
#1

Hi everybody,

I am working on a little project of mine. With the use of 4 sensors i control the speed of a fan.
If the first sensor see's something the fan will run on 25%. If the first en second senor see something the fan will run 50%. And so on.
To indicate the speed of the fan i use 4 leds. 1 led burning means 25%, 2 leds 50%.

Now the code i wrote works fine. Only i have the idea that it's written to long-winded.
I hope you guys can give me some advice.

//Robin Hengeveld , 2012 , Fan speed control with LED indication
const int L1 = 2, L2 = 3 , L3 = 4, L4 = 5; //LED pin declarations
const int S1 = 9, S2 = 10, S3 = 11, S4 = 12; // Sensor pin declarations
const int fan = 8; //Fan pin declaration

int sen1 , sen2 ,sen3, sen4; //Sensor variables
int fanspeed;

void setup(){
  Serial.begin(9600);
  //Set Led declararations as outputs
  pinMode(L1, OUTPUT);
  pinMode(L2, OUTPUT);
  pinMode(L3, OUTPUT);
  pinMode(L4, OUTPUT);
  //Set sensor declararations as inputs
  pinMode(S1,INPUT);
  pinMode(S2,INPUT);
  pinMode(S3,INPUT);
  pinMode(S4,INPUT);
  //Set Fan declaration as output
  pinMode(fan,OUTPUT);
}

void loop(){
  //Read all Sensors
  sen1= digitalRead(S1);
  sen2= digitalRead(S2);
  sen3= digitalRead(S3);
  sen4= digitalRead(S4);
  

  
if (sen1 == HIGH){    
if (sen1 == HIGH){ 
    fanspeed =255/4;
    digitalWrite (L1, HIGH);
    analogWrite (fan,fanspeed);
}

if (sen1 == HIGH && sen2 == HIGH){ 
    fanspeed =255/2;
    digitalWrite (L1, HIGH);
    digitalWrite (L2, HIGH);
    analogWrite (fan,fanspeed);
}
else
{
    digitalWrite (L2, LOW);
    digitalWrite (L3, LOW);
    digitalWrite (L4, LOW);
  }

if (sen1 == HIGH && sen2 == HIGH && sen3 == HIGH){ 
    fanspeed =191;
    digitalWrite (L1, HIGH);
    digitalWrite (L2, HIGH);
    digitalWrite (L3, HIGH);
    analogWrite (fan,fanspeed);
}
else
{
    digitalWrite (L3, LOW);
    digitalWrite (L4, LOW);
  }


if (sen1 == HIGH && sen2 == HIGH && sen3 == HIGH && sen4 == HIGH){ 
    fanspeed =255;
    digitalWrite (L1, HIGH);
    digitalWrite (L2, HIGH);
    digitalWrite (L3, HIGH);
    digitalWrite (L4, HIGH);
    analogWrite (fan,fanspeed);
}
else
{
    digitalWrite (L4, LOW);
  }

}
else{
  fanspeed =0;
  analogWrite(fan,fanspeed);
  digitalWrite (L1, LOW);
    digitalWrite (L2, LOW);
    digitalWrite (L3, LOW);
    digitalWrite (L4, LOW);
}}

A second question : i have a project coming up with a lot of in en output's. is there a faster way to declare the pins then:

pinMode(9,OUTPUT);

Robin

#2

You need to use arrays to reduce the amount of code you have.
This tells you what you have to do:-
http://www.thebox.myzen.co.uk/Tutorial/Arrays.html

#3

You may try that:

//LED pin declarations
#define L1 2
#define L2 3
#define L3 4
#define L4 5

// Sensor pin declarations
#define S1 9
#define S2 10
#define S3 11
#define S4 12

//Fan pin declaration
#define FAN 8

void setup(){
  //Set Led declararations as outputs
  pinMode(L1, OUTPUT);
  pinMode(L2, OUTPUT);
  pinMode(L3, OUTPUT);
  pinMode(L4, OUTPUT);
  //Set sensor declararations as inputs
  pinMode(S1,INPUT);
  pinMode(S2,INPUT);
  pinMode(S3,INPUT);
  pinMode(S4,INPUT);
  //Set Fan declaration as output
  pinMode(FAN,OUTPUT);
}

void loop(){
  int speed, light;

  speed = 0;
  light = 0;

  if (digitalRead(S1) == HIGH) { 
    speed += 64;
    light = light << 1 | 1;
    if (digitalRead(S2) == HIGH) {
      speed += 64;
      light = light << 1 | 1;
      if (digitalRead(S3) == HIGH) {
        speed += 64;
        light = light << 1 | 1;
        if (digitalRead(S4) == HIGH) {
          speed += 64;
          light = light << 1 | 1;
        }
      }
    }
  }

  if (speed > 255)
    speed = 255;

  digitalWrite (L1, light & 1);
  digitalWrite (L2, light & 2);
  digitalWrite (L3, light & 4);
  digitalWrite (L4, light & 8);
  analogWrite (FAN, speed);
}

I didn’t test it however. Feel free to ask if something is unclear.

#4

Another couple of tips:

  • use proper indentation
  • separate your code in two parts: first read the 4 inputs, calculate fanspeed and decide how many led to fire, then issue the analogWrite() for the fan and light up the leds, possibly via a function; this way you can avoid repeating analogWrite and digitalWrite all around the code...

HTH

edit: my comment was written before I read ababak's one...

#5

Every { and } should be a separate line and the matching pairs should be indented by the same amount; all code between them should be indented an extra level.

#6

Using CTRL-T will do the formatting for you and help to match up { }s.

#7

Create a class then create an array of your class and iterate through he array, you code becomes methods of the class. It will be a lot more readable and scalable.

#8