So im trying to make something where i light up leds to show on wich side of the wanted value on my pot meter i am. First i had 1 pushbutton and 2 pot meters. I wanted to be at 300-500 when i pushed to button, and if its higher 1 led indicating its higher went on and if its lower another led indicating lower. And with 4 leds(2 for each pot) and 1 pushbutton it worked perfect.
But now i want 2 extra buttons and 2 more ranges where it has to be in. But if i do this like i did in the code here the power in the leds is going all over the place. So what did i do wrong? Or is it all wrong and do i need to start over again.
const int led = 0; // for low potentiometer 1
const int led2 = 1; // for high potentiometer 1
const int led3 = 2; // for low potentiometer 2
const int led4 = 3; // for high potentiometer 2
const int button = 12; // for lowest setting
const int button2 = 11; // for middle setting
const int button3 = 10; // for high setting
const int sensorpin = 0; //potentiometer 1
const int sensorpin2 = 1; //potentiometer 2
int sensorValue = 0;
int sensor2Value = 0;
int buttonState = 0;
int button2State = 0;
int button3State = 0;
// the setup routine runs once when you press reset:
void setup() {
pinMode(led, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
pinMode(led4, OUTPUT);
pinMode(button, INPUT_PULLUP);
pinMode(button2, INPUT_PULLUP);
pinMode(button3, INPUT_PULLUP);
}
// the loop routine runs over and over again forever:
void loop() {
int sensorValue = analogRead(sensorpin);
int sensor2Value = analogRead(sensorpin2);
int buttonState = digitalRead(button);
int button2State = digitalRead(button2);
int button3State = digitalRead(button3);
if (buttonState == LOW) {
if (sensorValue > 10) digitalWrite(led, HIGH);
}
if (buttonState == LOW) {
if (sensor2Value > 10) digitalWrite(led3, HIGH);
}
if (button2State == LOW) {
if (sensorValue < 300) {
digitalWrite(led2, HIGH);
}
if (sensorValue < 500) {
if (sensorValue > 300)
digitalWrite(led2, LOW);
digitalWrite(led, LOW);
}
else digitalWrite(led, HIGH);
}
else {
digitalWrite (led2, LOW);
digitalWrite (led, LOW);
}
if (button2State == LOW) {
if (sensor2Value < 300) {
digitalWrite(led4, HIGH);
}
if (sensor2Value < 500) {
if (sensor2Value > 300)
digitalWrite(led4, LOW);
digitalWrite(led3, LOW);
}
else digitalWrite(led3, HIGH);
}
else {
digitalWrite (led4, LOW);
digitalWrite (led3, LOW);
}
if (button3State == LOW) {
if (sensorValue < 500) {
digitalWrite(led2, HIGH);
}
if (sensorValue < 800) {
if (sensorValue > 500)
digitalWrite(led2, LOW);
digitalWrite(led, LOW);
}
else digitalWrite(led, HIGH);
}
else {
digitalWrite (led2, LOW);
digitalWrite (led, LOW);
}
if (button3State == LOW) {
if (sensor2Value < 500) {
digitalWrite(led4, HIGH);
}
if (sensor2Value < 800) {
if (sensor2Value > 500)
digitalWrite(led4, LOW);
digitalWrite(led3, LOW);
}
else digitalWrite(led3, HIGH);
}
else {
digitalWrite (led4, LOW);
digitalWrite (led3, LOW);
}
}
i had that first, somehow must have remove the A1 part after retrying it again and again. But that dosn't make any difference to what happens, so i dont think it matters?
that is a check if the sensorvalue was below 500 it will check if its above 300 and then do those 2 things, and it worked like that so it seems to me i dont need to add braces then? The problem only starts when i add 2 buttons, with just 1 button its fine. But if i add a second button it starts going crazy.
it will check if its above 300 and then do those 2 things
Wrong. If the value is 300 or above, it will do the first one. It will ALWAYS do the second one.
and it worked like that
For some definition of work, maybe. Not for MY definition of work.
so it seems to me i dont need to add braces then?
Wrong.
The problem only starts when i add 2 buttons, with just 1 button its fine. But if i add a second button it starts going crazy.
I'll tell you what's crazy. Using meaningless names like button, button2, and button3 is crazy. Surely there is some reason why you attached SWITCHes to the Arduino (leave the buttons for shirts). Name the pins to reflect what the switch is supposed to do. Name the value to match.
Then, you have a hope in hell of referring to the correct name in the code, without having to scroll back to the top of the code to find out what sensor2value (or any other meaningless name) means.
well ill tell you what sensor2value means. It means the value of sensor 2, big surprise isnt it? And why is it called sensor 2? Because it is sensor nr2 in my project, not sensor 1, not sensor 3, sensor 2. An why is button 2 called button 2? Because it has a big 2 on it in my project. Why would i call it something different then? Seems perfectly clear to me. Or should i give it some name that has nothing to do with my project? Like fluxcapacitor159384?
I dont see why you have a problem with what i called it? Those names make perfect sense, if you have 2 sensors that are both the same but in a different place what would you call them then? And what would you call a button with a 2 on it then? And in my opinion a switch is something that holds its position if you put it there and a button needs to be pushed to keep in the on position. So this is a button because i have to keep it pressed to turn it on.
Those names make perfect sense, if you have 2 sensors that are both the same but in a different place what would you call them then?
Let me ask, instead, WHY you have two? The thoughts that come to mind are that one points up and one points down, or one points left and one points right, or one points forward and one points backward.
Regardless, they are not sensors you grabbed from a box labeled sensors. They are specific KINDS of sensors. Why shouldn't the name reflect that?
Look at ANY appliance in you house that has more than one switch. Are the switches labeled 1, 2, 3? Or are they labeled something more like Play, Record, Stop, Rewind?
this is the keypad i am using, so they are number 1-4, but im only using 3 atm so the last 1 is just out of use (for now). So its seems pretty normale for me to number them like that then right?
And for my test i have the sensors in a row so for me its more clear that they are numberd like this so i know wich 1 of the 2 should be it. As soon as it works and im going to put it to use ill rename them to the position they are gonna be in. I could have named them left sensor and right sensor too, but if you dont see it you still wont have an idea from that either.
Only thing i could(and should) have done different was the leds. They could be renamed so that its more clear on wich 1 they are. I could rename them 1up and 1down and 2up and 2down.
// Pin 13 has an LED connected on most Arduino boards.
// give it a name:
const int downled1 = 0;
const int upled1 = 1;
const int downled2 = 2;
const int upled2 = 3;
const int button1 = 12;
const int button2 = 11;
const int button3 = 10;
const int sensorpin = 0;
const int sensorpin2 = 1;
int sensorValue = 0;
int sensor2Value = 0;
int buttonState = 0;
int button2State = 0;
int button3State = 0;
// the setup routine runs once when you press reset:
void setup() {
pinMode(downled1, OUTPUT);
pinMode(upled1, OUTPUT);
pinMode(downled2, OUTPUT);
pinMode(upled2, OUTPUT);
pinMode(button1, INPUT_PULLUP);
pinMode(button2, INPUT_PULLUP);
pinMode(button3, INPUT_PULLUP);
}
// the loop routine runs over and over again forever:
void loop() {
int sensorValue = analogRead(sensorpin);
int sensor2Value = analogRead(sensorpin2);
int button1State = digitalRead(button1);
int button2State = digitalRead(button2);
int button3State = digitalRead(button3);
if (button1State == LOW) {
if (sensorValue > 10) digitalWrite(downled1, HIGH);
}
if (button1State == LOW) {
if (sensor2Value > 10) digitalWrite(downled2, HIGH);
}
if (button2State == LOW) {
if (sensorValue < 300) {
digitalWrite(upled1, HIGH);
}
if (sensorValue < 500) {
if (sensorValue > 300) {
digitalWrite(upled1, LOW);
digitalWrite(downled1, LOW);
}
}
else digitalWrite(downled1, HIGH);
}
if (button2State == LOW) {
if (sensor2Value < 300) {
digitalWrite(upled2, HIGH);
}
if (sensor2Value < 500) {
if (sensor2Value > 300) {
digitalWrite(upled2, LOW);
digitalWrite(downled2, LOW);
}
}
else digitalWrite(downled2, HIGH);
}
if (button3State == LOW) {
if (sensorValue < 500) {
digitalWrite(upled1, HIGH);
}
if (sensorValue < 800) {
if (sensorValue > 500) {
digitalWrite(upled1, LOW);
digitalWrite(downled1, LOW);
}
}
else digitalWrite(downled1, HIGH);
}
if (button3State == LOW) {
if (sensor2Value < 500) {
digitalWrite(upled2, HIGH);
}
if (sensor2Value < 800) {
if (sensor2Value > 500) {
digitalWrite(upled2, LOW);
digitalWrite(downled2, LOW);
}
}
else digitalWrite(downled2, HIGH);
}
if (button3State && button2State && button1State == HIGH)
{ digitalWrite(upled1, LOW);
digitalWrite(downled1, LOW);
digitalWrite (upled2, LOW);
digitalWrite (downled2, LOW);
}
}
The problem was that it was checking if a buttonstate was high and then switching of the leds. But it should check if all 3 buttons where off like it do's now in the end. Stupid mistake, but i figured it out. Thank you for the help anyway.
Good that it works. That is the first objective reached, now improve it.
Why have you got a whole load of int variables when the values they hold will never exceed 255 and so could fit into a byte ?
It doesn't matter to the program but things like this
int sensorValue = analogRead(sensorpin);
int sensor2Value = analogRead(sensorpin2);
look silly to me. It looks like you started with one sensor pin then added a second but could not be bothered to change the name of the first set of variables, which is sloppy. There are also redundant variables, buttonState, which looks like it is left over from adding a second button, and you have 2 different variables named sensorValue, sensor2Value, button1State, button2State and button3State which could cause confusion if you add to the code again.
You have some repeated code such as
if (sensorValue < 500)
{
if (sensorValue > 300)
{
digitalWrite(upled1, LOW);
digitalWrite(downled1, LOW);
}
}
and
if (sensor2Value < 500)
{
if (sensor2Value > 300)
{
digitalWrite(upled2, LOW);
digitalWrite(downled2, LOW);
}
}
Could you write a function to replace the duplicate code and/or use arrays to remove the duplicate code ?
That is not totally the same. The sensor value is the same, but different sensor and leds. And this was the easiest wat die me as im pretty new to all this.
I can see why you did it that way and you are right, they are not completely the same, but you can easily deal with that using a function that takes paramaters. The function could look like this
You will find this type of programming very helpful when writing more complicated programs. It is even useful where code is not duplicated because it breaks a program into more manageable chunks which can be tested separately and using meaningful names for functions makes the code more readable.
And why would that be better? I dont see the point really, but im the new guy so its better to ask. But to me it looks like adding more code thats not really adding anything?