make it simpler

Projects Programming
1

Hello everyone first of all i want to say that i'm a beginner it've been working with arduino's for about a month now so for my first project i've build a light sensor using a HC-SR04 and 4 LED's when an object get closer to the sensor the LED's turn on i've tested the code and everything works but i think the code is a bit long and i think it can be shorter using functions maybe ?
so here's the code and please tell me if i can make it shorter thanks (and is this the right way to upload coads ? )

int trig = 8;
int echo = 7;
int LED1 = 3;
int LED2 = 5;
int LED3 = 6;
int LED4 = 11;
long duration;
int distance;
int distance1;
int distance2;
int distance3;
int distance4;

void setup() {

pinMode(echo,INPUT);
pinMode(trig,OUTPUT);
pinMode(LED1,OUTPUT);
pinMode(LED2,OUTPUT);
pinMode(LED3,OUTPUT);
pinMode(LED4,OUTPUT);

}

void loop() {

digitalWrite(trig,LOW);
delayMicroseconds(2);
digitalWrite(trig,HIGH);
delayMicroseconds(10);
digitalWrite(trig,LOW);

duration=pulseIn(echo,HIGH);
distance=duration*0.034/2;

if ( distance <= 40 ) {

distance1=map(distance,50,40,0,255);
distance1=constrain(distance1,0,255);
analogWrite(LED1,distance1);
} else {

analogWrite(LED1,LOW);

}

if (distance <= 30 ) {

distance2=map(distance,40,30,0,255);
distance2=constrain(distance2,0,255);
analogWrite(LED2,distance2);
} else {

analogWrite(LED2,LOW);

}

if (distance <= 20 ) {

distance3=map(distance,20,10,0,255);
distance3=constrain(distance3,0,255);
analogWrite(LED3,distance3);
} else {

analogWrite(LED3,LOW);

}

if (distance <= 10 ) {
distance4=map(distance,10,2,0,255);
distance4=constrain(distance4,0,255);
analogWrite(LED4,distance4);
} else {

analogWrite(LED4,LOW);

}

}

2

Hi,

Is there a reason that you use :distance4=map(distance,10,2,0,255);instead of distance4=map(distance,10,0,0,255);The intensity of the LED4 will not be proportional to the other LEDS.

Otherwise:

if (distance <= 20 ) {

distance3=map(distance,20,10,0,255);
distance3=constrain(distance3,0,255);  ///<<<This is useless since you mapped the value
analogWrite(LED3,distance3);
} else {

analogWrite(LED3,LOW);   
  
}

if (distance <=20) {
  analogWrite(LED3, map(distance,20,10,0,255));
}
else {
analogWrite(LED3,LOW);
}

If you are willing to lose the mapping with 10, 0, we could simplify if a lot more.

Jacques

3

Use CTRL T to format the sketch.
Please use code tags.
Use the </> icon in the posting menu.

[code] Paste sketch here. [/code]
4

Start small then get larger.
Consider using:
else if

5

How about "case" ?

6

Anytime you see almost exactly the same code repeated 4 times, it is time to consider making it a function.

You appear to be scaling the numbers down, and then up again. With integers, that will result in a loss of accuracy.

7

What changes in those statements?

map(distance, 40, 30, 0, 255)
map(distance, 30, 20, 0, 255)
map(distance, 20, 10, 0, 255)
map(distance, 10,  2, 0, 255)

A function takes arguments for things that changes, ant takes care of other things that do not.

byte aFunction(byte first, byte second) {
  return map(first, second, 0, 255);
  //What more could be done here? 
}

And if we had:

void aFunction(byte aLED, byte first, byte second) {
  byte val = map(first, second, 0, 255);
  //Now, what more could be done here? 
}

Jacques

8

There's no need for any of those ifs, because you are using constrain.

Consider this:

distance3=map(distance,20,10,0,255);
distance3=constrain(distance3,0,255);

If the distance is > 20, map will map it to a negative number. Constrain will clip it to 0.
If the distance is < 10, map will map it to a number >= 255. Constrain will clip it to 255.

An analogWrite of 0 is the same as a digitalWrite of LOW. So this fragment will do all the things that your whole sketch currently does:

analogWrite(LED1,constrain(map(distance,50,40,0,255),0,255));   
analogWrite(LED2,constrain(map(distance,40,30,0,255),0,255));   
analogWrite(LED3,constrain(map(distance,20,10,0,255),0,255));   
analogWrite(LED4,constrain(map(distance,10,2,0,255),0,255));

Oh by the way, you have a gap between 20 and 30. Any distance value in that range will put LED1 and 2 fully on and 3 and 4 fully off.

9

I fail to understand how either constrain() and/or map() works.

I thought that using map(value, fromLow, fromHigh, toLow, toHigh) would at the same time constrain the value between toLow and toHigh...

Jacques

EDIT: Just made some tests. Now I understand.

10

thank you so much guys for your help i've fixed some mistakes is the code but now i have a new problem

i've made a new topic hope you guys can help thanks

https://forum.arduino.cc/index.php?topic=499307.0

11

jbellavance:
I fail to understand how either constrain() and/or map() works.

I thought that using map(value, fromLow, fromHigh, toLow, toHigh) would at the same time constrain the value between toLow and toHigh...

From the documentation

Re-maps a number from one range to another. That is, a value of fromLow would get mapped to toLow, a value of fromHigh to toHigh, values in-between to values in-between, etc.

Does not constrain values to within the range, because out-of-range values are sometimes intended and useful. The constrain() function may be used either before or after this function, if limits to the ranges are desired.