confussed where to close "Bracket"

Hello guys

Little history: I have a camera system on the house with 8 cameras and DVR. All the cameras are currently stationary mounts. I decided I wanted pan/tilt feature but I didn't want to buy pan/tilt/zoom cameras 1. because of expense and 2. because I knew I could make a tinkering fun project out of it. :slight_smile:
I created a box with spare Arduino, two 100K pots and 9 momentary switches.
I've written simple control for each camera using "while statement" pan/tilt which worked fine.
Momentary switches to select which camera to adjust and two 100K pots to adjust pan tilt angle on each camera. "No need for multi pots"
I also 3D printed pan/tilt mount for each camera with two servos to drive the camera angles.

Then I decided I wanted to add a "pan cruse" feature to one selected camera that I choose. Wanted it to run constant till I select the same camera to adjust which would "break" the loop of cruse leaving that camera position where the loop was broken and ready to adjust again.

So I'm sure some of you might have written this different but this worked for my simple mind.

The question is; I feel pretty confident about the "if statement" to break out of cruse but I'm not sure where to read the switches status to do so and how to close the whole statement with brackets "}".

A little help understanding when inside the statement to read the switches again before the break and how to close each statement inside the statement would be great!

I Attached current whole sketch below. It's only written for two of the camera's so far but the line I'm referring to is this one:

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW)
{
Serial.print (" Button 1 Pressed and Cruse ");
for (i=20; i<150; i++)
{
camera1x.write(i);
delay (100);
for (i=150; i!=20; i--) {
camera1x.write(i);
delay (100);
sw0v = digitalRead(sw0);
sw1v = digitalRead(sw1); // read the input for high or low "looking for (high) to
sw2v = digitalRead(sw2); // read the input for high or low "looking for (high) to
delay (20);
}
if (sw0v == LOW && sw1v == HIGH && sw2v == LOW)

{
break;
}

Thanks for help in advance.

PT_final.ino (5.26 KB)

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW)
    {
      Serial.print (" Button 1 Pressed and Cruse ");
     for (i=20; i<150; i++)
     {
    camera1x.write(i);
    delay (100);
    for (i=150; i!=20; i--) {

Be consistent in placing {. Always on the line with the statement (yuck) or always on a new line (my preference).

Do not mix styles. Indent the code after the {, and it will be obvious where the block should end (and where the } should go).

Thanks Paul S

Agreed!!!
Tried to rush through the lines with out making it more clear.
Cleaning it up and taking another look to figure it out.
Report back in a few.

Okay Paul S
Thank you, yes!!! made more since when I cleaned up the lines. Figured out what Brackets were missing and where to place them.
I'm fixing to test out the loop to see if it breaks but I'm sure you know if it will or not by looking at it :slight_smile:

Now lines read:

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW) {
Serial.print (" Button 1 Pressed and Cruse ");
for (i=20; i<150; i++) {
camera1x.write(i);
delay (100);
}
for (i=150; i!=20; i--) {
camera1x.write(i);
delay (100);
}
sw0v = digitalRead(sw0);
sw1v = digitalRead(sw1);
sw2v = digitalRead(sw2);
delay (20);
if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {
break;
}
}

Ugh. That's uglier than what you started with. Put the { on a new line, so it lines up with the }. Use Tools + Auto Format to correctly indent the code.

Paul S

The original line as I wrote it worked to start the cruse feature, however it ran through the cruse feature once and stopped.
After thinking about it I saw why. The line after the first cruse up and down checked the switches again. It read the switches as LOW, LOW and LOW, so it stopped. "naturally duh"

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW) {
Serial.print (" Button 1 Pressed and Cruse ");
for (i=20; i<150; i++) {
camera1x.write(i);
delay (100);
}
for (i=150; i!=20; i--) {
camera1x.write(i);
delay (100);
}
sw0v = digitalRead(sw0);
sw1v = digitalRead(sw1);
sw2v = digitalRead(sw2);
delay (20);

if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {

break;
}
}

So I moved the switch checks after the if statement a now I'm stuck in a continuous loop of cruse. No switch senerio breaks the loops ?????

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW) {
Serial.print (" Button 1 Pressed and Cruse ");
for (i=20; i<150; i++) {
camera1x.write(i);
delay (100);
}
for (i=150; i!=20; i--) {
camera1x.write(i);
delay (100);
}

if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {
sw0v = digitalRead(sw0);
sw1v = digitalRead(sw1);
sw2v = digitalRead(sw2);
delay (20);

break;
}
}

if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {
Are you reading these switches anywhere outside of this if code?

Listen to PaulS
{ // goes on a separate line
use CTRL T to format your code.

Sorry Paul I wrote before I saw your new post.
Used Auto format.
now I see what you were looking for.
Sketch attached with format corrected.

if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {
Are you reading these switches anywhere outside of this if code?

These switches are for only the lines in this sketch.
They are simply for determining in the code which camera I'm referencing with out the need/use of a key board or additional computer.

PT_final.ino (5.31 KB)

  x = xaxis;                                    // Xaxis value to store in x "pan"
  y = yaxis;                                    // Yaxis value to store in y "tilt"

It does not make sense storing pin numbers in value variables. If you used Pin in the name of variables that contain pin numbers, and Value in the names of variables that contain values read from pins, this kind of mistake wouldn't happen.

Let us assume the 3 conditions in while are met.
If you never update sw0v sw1v or sw2v how will you ever get out of the while?

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW)  
  {
    Serial.print (" Button 1 Pressed and Cruse ");
    for (i=20; i<150; i++)  
    {
      camera1x.write(i);
      delay (100);
    }
    for (i=150; i!=20; i--)
   {
      camera1x.write(i);
      delay (100);
    }

    if (sw0v == LOW && sw1v == HIGH && sw2v == LOW)  
    {
      sw0v = digitalRead(sw0);
      sw1v = digitalRead(sw1);                      // read the input for high or low "looking for (high) to control camera1
      sw2v = digitalRead(sw2);                      // read the input for high or low "looking for (high) to control camera2
      delay (20); 

      break;
    }
  }

So your saying it should have read more like?

A0 = X_val
A1 = Y_val

The condition in the "while" does get met and it does enter into panning back and forth. "test breadboard shows.
What does not happen is when the "if statement" is met; break from the loop.

I know your trying to teach me here, so bare with me.

The way I read what I wrote in my mind is:

When switch 0 & 1 are pressed and switch 2 is not pressed,
enter into panning loop
camera 1 Pan full position 20 to 150 and back again before reading next line.

The "if statement" has the program read the switches high/low
inputs again till a condition matches "switch 0 low switch 1 high and switch 2 low"
If true it breaks and enters pan/tilt adjustment for camera 1.

The part "which is the reason I wrote here to begin with" is understanding whether the closing bracket after the panning loop keeps the next line from being read.
If that all makes since. :slight_smile:

Thanks Larry

Went back and read it through in my mind and solved it.
Works like it's suppose to now.
Do have one other question though........
I have to wait on the pan motion to make a complete cycle before it will read the "break"
Can you think of a may to break the cycle sooner other than having the switches read between one direction to another?

Thanks again.

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW) {
Serial.print (" Button 1 Pressed and Cruse ");
for (i=20; i<150; i++) {
camera1x.write(i);
delay (100);
}
for (i=150; i!=20; i--) {
camera1x.write(i);
delay (100);
}
sw0v = digitalRead(sw0);
sw1v = digitalRead(sw1);
sw2v = digitalRead(sw2);
delay (20);
if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {
break;
}
}

Actually I posted incorrect update:

while(sw0v == HIGH && sw1v == HIGH && sw2v == LOW) {
Serial.print (" Button 1 Pressed and Cruse ");
sw0v = digitalRead(sw0);
sw1v = digitalRead(sw1);
sw2v = digitalRead(sw2);
delay (20);
for (i=10; i<150; i++) {
camera1x.write(i);
delay (50);
}
for (i=150; i!=10; i--) {
camera1x.write(i);
delay (50);
}

if (sw0v == LOW && sw1v == HIGH && sw2v == LOW) {
break;
}
}

Tell us how long this section of code will take?
for (i=20; i<150; i++)
{
camera1x.write(i);
delay (50);
}

for (i=150; i!=10; i--)
{
camera1x.write(i);
delay (50);
}

BTW, Consider making this change:
for (i=150; i!=10; i--)

for (i=150; i >10; i--)

Take a look at this great discussion on how to do several things at once:
http://forum.arduino.cc/index.php?topic=223286.0
Also, read through this:
http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html