I am struggling with many conditions in my code. I have used switch case but it is not clean at all. the code has lot of errors. I am not good at coding. if you could suggest something. help it in structuring.
here's the code(this code is wrong. but i want to perform these actions ) how should i do it? help needed
#include <Servo.h>
Servo myservo1;
Servo myservo2;
const int buttonPin1 = 2; // Axthe pin that the pushbutton is attached to
const int buttonPin1a = 4; // Bx
const int buttonPin2 = 5; //Ay
const int buttonPin2a = 6; //By
const int serPin1 = 9; // the pin that the servo is attached to
const int serPin2 = 10;
// Variables will change:
//int buttonPushCounter1 = 0; // counter for the number of button presses
//int buttonPushCounter1a = 0;
//int buttonPushCounter2 = 0;
//int buttonPushCounter2a = 0;
int buttonState1 = 0; // current state of the button
int buttonState2 = 0; // current state of the button
int buttonState1a = 0;
int buttonState2a = 0;
//int lastButtonState1 = 0; // previous state of the button
//int lastButtonState2 = 0;
//int lastButtonState2a = 0;
//int lastButtonState1a = 0;
int val= 180;
int val1= 0;
int temp
int n;
void setup() {
myservo1.attach(9);
myservo2.attach(10);
// initialize the button pin as a input:
pinMode(buttonPin1, INPUT);
pinMode(buttonPin1a, INPUT);
pinMode(buttonPin2, INPUT);
// initialize the LED as an output:
pinMode(serPin1, OUTPUT);
pinMode(serPin2, OUTPUT);
// initialize serial communication:
Serial.begin(9600);
}
void loop() {
// read the pushbutton input pin:
buttonState1 = digitalRead(buttonPin1);
buttonState1a = digitalRead(buttonPin1a);
buttonState2 = digitalRead(buttonPin2);
buttonState2a = digitalRead(buttonPin2a);
delay(500);
}
switch (n) {
val=val1;
temp=val1;
val1=val;
case 0:
if (buttonState1 == HIGH || buttonState1a == HIGH)// A/B-x
myservo1.write(val);
delay(1000);
Serial.println("A/B-x");
break;
case 1:
if (buttonState2 == HIGH || buttonState2a == HIGH)// A/B-y
myservo2.write(val);
delay(1000);
Serial.println("A/B-y");
break;
case 2:
if (buttonState1 == HIGH && buttonState2 == HIGH)// A-A
myservo1.write(val);
delay(1000);
Serial.println("AA");
break;
case 3:
if (buttonState1a == HIGH && buttonState2a == HIGH)//B-B
myservo1.detach();
myservo2.detach();
Serial.println("BB");
break;
case 4:
if (buttonState1 == HIGH && buttonState2a == HIGH)// Ax-By
myservo2.write(val);
delay(1000);
Serial.println("Ax-By");
break;
case 5:
if (buttonState1a == HIGH && buttonState2 == HIGH)
myservo1.write(val);
delay(1000);
myservo2.write(val);
delay(1000);
Serial.println("Bx-Ay");
break;
}
}
One simple way would be to pack the button states into a single byte or int, and use the numeric value as the selector.
What is "n"?
Comments would be good.
Can I also suggest that before posting code, you use the auto-format tool in the IDE (ctrl-T)?
AFAIK this is not allowed (never seen it before), although the compiler accepts it - after cleaning up the { and } in the code a bit
Think these assignments - swap of two vars- should be before the switch()
Where gets n its value???? its allways zero! ( default initializer global var)
int temp is missing a ;
The whole switch statement should be inside loop() I guess ?
Every path of the switch statement has a delay(1000) in it, that means it possibly can be extracted outside (before/after) the switch => less lines of code
Pay attention to the layout and these things would be visible faster,
AFAIK this is not allowed (never seen it before), although the compiler accepts it - after cleaning up the { and } in the code a bit
Think these assignments - swap of two vars- should be before the switch()
I want the position of the servo to remain at the last position after rotation. As in it rotates from 0-180 then next time the code is read it should start from 180 degrees and goes like 180-0. i want this to perform every time rotation happens. I thought the code is read from top to bottom so it would swap the numbers then use those numbers in the following code.
Dived into this piece of code as the compiler didn't complain (maybe warnings could see that in the IDE)
switch (n)
{
val=val1;
temp=val1;
val1=val;
...
It is legal C / C++ as the statements are in a valid block { }
However the compiler makes code that jumps just over it, even if there is no matching case and no default in the switch. See testapp below.
void setup()
{
Serial.begin(115200);
Serial.println("I am Arduino");
}
int n = 0;
void loop()
{
switch(n)
{
Serial.println("!!!!");
case 1: Serial.println(n); break;
Serial.println("****");
case 10: Serial.println(n); n = 0; break;
Serial.println("????");
}
n++;
delay(100);
}
Now lets think how this can be made usefull (except for the obfuscated C contest) .....
It is hiding valid code that is unreachable .... Add labels and GOTO's to jump between cases?
easteregg hiding in a switch?
Definitely no good programming practice ...
Conclusion: Don't do this.
-- update -- With goto's and labels the unreachable code can be made reachable again, but I will not publish this horror code here
this should be performed. But I don't know how to achieve it. I tried using if-else but then again too many loops and its not reading all lines of code.
Take paper and pencil and draw a flowchart of how your application should work. Make a design. Dumping your minds eye directly in code is difficult even for a pro.
And build your application step (then test) by step (and test) by step (and test) ....
The constructions in your code are not too bad but there is too much I don't understand, to start with
What is n? what does it represent, the name is as meaningles as x or q or a
Where is the switch(n) for?
Comments like // Variables will change: makes live not really easier
if (buttonState1 == HIGH || buttonState1a == HIGH) // A/B-x <= obfuscated comment detected
(will you know what that means in 6 months?
if (buttonState1 == HIGH || buttonState1a == HIGH) // ALL MOTORS FORWARD (more meaningfull)
{
myservo1.write(val);
delay(1000);
Serial.println("A/B-x");
}
I cannot tell you if your logic is right, I suppose it isn't (by the look)
Have you made the flowchart?
Think you should just do:
if (...) // first condition
{
}
if (...) // second condition
{
...
}
etc
Maybe just start with testing one condition
#include <Servo.h>
Servo myservo1;
Servo myservo2;
const int buttonPin1 = 2; // the pin that the pushbutton is attached to
const int buttonPin1a = 4;
const int buttonPin2 = 5;
const int buttonPin2a = 6;
const int serPin1 = 9; // the pin that the servo is attached to
const int serPin2 = 10;
int val= 180;
int val1= 0;
int temp;
int buttonState1, buttonState2, buttonState2a, buttonState1a = 0; // current state of the button
void setup()
{
myservo1.attach(9);
myservo2.attach(10);
pinMode(buttonPin1, INPUT);
pinMode(buttonPin1a, INPUT);
pinMode(buttonPin2, INPUT);
pinMode(buttonPin2a, INPUT);
pinMode(serPin1, OUTPUT);
pinMode(serPin2, OUTPUT);
Serial.begin(9600);
}
void loop()
{
buttonState1 = digitalRead(buttonPin1);
buttonState1a = digitalRead(buttonPin1a);
buttonState2 = digitalRead(buttonPin2);
buttonState2a = digitalRead(buttonPin2a);
val1=temp;
temp=val;
val=val1;
if (buttonState1 == HIGH ) // when button 1(x orientation) pressed motor1 rotates
{
myservo1.write(val);
delay(1000);
Serial.println("Ax");
}
}
If this works you add a little and test it until it works.
here is the code, I tested out like you said..
It is working fine till i read single switches HIGH but it is not reading when 2 switches are HIGH because its reading the previous statements and working accordingly. whats the way out?
#include <Servo.h>
Servo myservo1;
Servo myservo2;
const int buttonPin1 = 2; // the pin that the pushbutton is attached to
const int buttonPin1a = 4;
const int buttonPin2 = 7;
const int buttonPin2a = 6;
const int serPin1 = 9; // the pin that the servo is attached to
const int serPin2 = 10;
int val= 180;
int val1= 0;
int temp;
int buttonState1, buttonState2, buttonState2a, buttonState1a = 0; // current state of the button
void setup()
{
myservo1.attach(9);
myservo2.attach(10);
pinMode(buttonPin1, INPUT);
pinMode(buttonPin1a, INPUT);
pinMode(buttonPin2, INPUT);
pinMode(buttonPin2a, INPUT);
pinMode(serPin1, OUTPUT);
pinMode(serPin2, OUTPUT);
Serial.begin(9600);
}
void loop()
{
buttonState1 = digitalRead(buttonPin1);
buttonState1a = digitalRead(buttonPin1a);
buttonState2 = digitalRead(buttonPin2);
buttonState2a = digitalRead(buttonPin2a);
val1=temp;
temp=val;
val=val1;
//if (buttonState1 != lastButtonState1)
if (buttonState1 == HIGH ) // when button 1(x orientation) pressed motor1 rotates
{
myservo1.write(val);
delay(1000);
Serial.println("Ax");
}
else{
if (buttonState1a == HIGH)
{
myservo1.write(val);
delay(1000);
Serial.println("B-x");
}
else{
if (buttonState2 == HIGH ) // A/B-y
{
myservo2.write(val);
delay(1000);
Serial.println("Ay");
}
else{
if (buttonState1 == HIGH && buttonState2 == HIGH)// A-A
{
myservo1.write(val);
delay(1000);
Serial.println("AA");
}
}
}
}
}
your logic is incorrect, you can see it easier with better layout. See below.
There are 4 options.
if you want OPTION 4 to execute 2 lines must be high.
but you test the highness of them seperately in OPTION 1 and in OPTION 2 before option 4 can be tested. This was OPTION 4 can never be reached.
(that is due to all those else staements, I my previous post I removed them
Define one variabele to set the keystate , and use bitmanipulations to add state of all buttons in one keystate .
Note the keystate can have 16 values (0..15)
void loop()
{
bs1= digitalRead(buttonPin1); // LOW = 0 HIGH = 1
bs1a = digitalRead(buttonPin1a);
bs2 = digitalRead(buttonPin2);
bs2a = digitalRead(buttonPin2a);
unsigned int keystate = 0;
if (bs1 == HIGH) bitSet(keystate , 0);
if (bs1a == HIGH) bitSet(keystate , 1);
if (bs2 == HIGH) bitSet(keystate , 2);
if (bs2a == HIGH) bitSet(keystate , 3);
Serial.print("STATE: "); Serial.println(keystate, BIN); // 0-15 = 0 - 1111
switch (keystate )
{
case 1: // bs1 pressed others not
myservo1.write(val);
delay(1000);
Serial.println("Ax");
break;
case 2: // bs1a pressed others not
myservo1.write(val);
delay(1000);
Serial.println("B-x");
break;
case 4: // bs2 pressed others not
myservo2.write(val);
delay(1000);
Serial.println("Ay");
break;
case 5: // bs1 and bs2 pressed others not
myservo1.write(val);
delay(1000);
Serial.println("AA");
break;
case 5: // all keys pressed
Serial.println("you found the easteregg");
break;
case 0: // no keys pressed;
break;
default: // invalid key combination
// error message
Serial.println("Invalid key combination pressed");
break;
}
}
Could you post your new code so we can see what is wrong?
You should probably avoid using goto as you'll end up with spaghetti code that is hard to understand and hard to debug. One option is to make some simple functions and call them from the body of the case statements (i.e. so you can easily run the same code for different cases).
