proper use of while( ) loop?

This is an addendum to an earlier post, but it’s a fresh topic because it’s more general, I believe. I’m doing my best to get a handle on looping and conditionals and I’m hopeful that this bit of code will represent the correct objective.

// some code at beginning of loop() containing ping.fire() and allocation to variable toShift

  while(toShift < 36 && toShift > 0)  //  toShift is value from ultrasonic sensor, trigger camera 1 under 36"
  {  
    digitalWrite(cam_1, HIGH);        //  This block triggers auto-focus
    delay(camDelay);                  //
    digitalWrite(cam_1, LOW);         //  This block triggers shutter
    delay(camDelay);                  //  camDelay is about 1/4 second, slow enough for auto focus, fast enough for good lighting conditions
    cam2Flag = 1;                     //  cam2Flag set to trip 2nd cam when loop done
    ping.fire();                      //  read distance again to exit while loop when appropriate.
    int toShift = ping.inches();      //
  }
  while(toShift > 36 && cam2Flag == 1) //  after distance is exceeded and after cam1 is tripped
  {
    for (int i = 0; i < 4; i++)       //  fire camera 4 times if distance remains open and if camera 1 has tripped
    {
      digitalWrite(cam_2, HIGH);      //  same as above
      delay(camDelay);                //
      digitalWrite(cam_2, LOW);       //
      delay(camDelay);                //
      ping.fire();                    //  check distance to stop cam2 triggers if distance is less than 36"
      int toShift = ping.inches();    //
    }
    cam2Flag = 0;                     // reset flag for cam2, don't fire if camera 1 isn't tripped first
  }

The project is two cameras, a Ping))) ultrasonic sensor and a digital display. The digital display is working properly and was the bulk of the code, so it’s omitted here.

When an object comes into range of the sensor, but greater than 36" away, nothing happens. I’m hoping that the cam2Flag properly represents that condition.
When an object comes within 36" of the sensor, camera one trips every half second until it moves away, at which point, the original condition is met, but the flag for camera 2 is set, causing the second while( ) loop to execute.

If my understanding is correct, the inclusion of the ping.fire() code is what provides an exit from the first while( ) loop and also from the second while( ) loop.

The second while( ) loop is to cause camera two to trigger four times and return control to the main loop. If the distance returned by the sensor is less than 36", control returns immediately to the main loop, regardless of the 4x counter. It appears that if such a circumstance arises, the cam2 flag does not get reset. Considering that the reason to exit the second loop is based on the distance line being crossed, the cam2 flag would become set during the first loop anyway.

Is this how the code appears to function as written?

If my understanding is correct, the inclusion of the ping.fire() code is what provides an exit from the first while( )

Your understanding is incorrect- the assignment of "toShift" is what provides the exit condition

while(toShift < 36 && toShift > 0) // toShift is value from ultrasonic sensor, trigger camera 1 under 36"
{
int toShift = ping.inches(); //
}
The toShift variable in the while statement is not the same variable that is valued in the while loop. Why are you declaring a new variable in the while loop?

Oops, missed the scope issue.

AWOL:

If my understanding is correct, the inclusion of the ping.fire() code is what provides an exit from the first while( )

Your understanding is incorrect- the assignment of "toShift" is what provides the exit condition

That's true, and I realize I was not correct in my wording. The ping.fire() is to put a current figure from the sensor in the variable.

PaulS:

while(toShift < 36 && toShift > 0)  //  toShift is value from ultrasonic sensor, trigger camera 1 under 36"


    int toShift = ping.inches();      //
  }



The toShift variable in the while statement is not the same variable that is valued in the while loop. Why are you declaring a new variable in the while loop?

That was done out of ignorance, a lack of understanding of syntax, scope and everything else that goes with that.

If I remove the int declaration, does it become correct?

If I remove the int declaration, does it become correct?

Hmmm. Let's see. Fire up the IDE, delete 4 characters, compile, link, upload, and test OR post a question on the forum and wait for an answer. Which would be faster?

It appears, from the snippet you posted that removing the int keyword from in front of toShift inside the while loop will address the problem that you described. However, there is only one way to be sure.

I’ve removed the declaration from the code. It does verify, which means, in my alleged mind, that it will upload to the device. I’ve not yet installed the assembly as it’s going to be four individual components stretched over nine feet, using the Uno breadboard and a couple of solder perfboards. I’m hoping to reduce the set-up and break-down to a minimum, to prevent connection fatigue or breakage.

Having had that specific failure pinpointed isn’t what I expected, but it’s very much what I needed to see. I hope my future coding doesn’t result in the same simplistic mistake.

I didn’t realize that I had described a problem in my post, but that’s again due to a lack of perception, I suppose. Since no one has suggested that the logic presented is in error, I’m encouraged to drag out the duct tape and start attaching things.

It does verify, which means, in my alleged mind, that it will upload to the device.

Unless there is a problem with the hardware, that is a valid assumption.

I hope my future coding doesn't result in the same simplistic mistake.

One way to know. Post all of your code. We're usually happy to review it for you. It is a very common problem, though, so don't feel bad.

Make sure that you fixed both while loops.

I didn't realize that I had described a problem in my post, but that's again due to a lack of perception, I suppose.

Now that I actually read all of your first post, I see that you didn't. The scope issue jumped out at me, before I got that far, so I assumed that you were asking why the while loop didn't terminate correctly. I guess I shouldn't have.

I’m glad that the scope jumped out, even without a clearly defined problem. I did catch the second while int declaration and corrected that as well.
Most of the code is stuff pulled from the forums here, and attributed in the comments. I had tried to use the “stock” shiftOut function with more difficulty than I cared for, so I dropped back to that which worked for SpikedCola and used his custom version. Since it works, and displays the Ping))) readings properly, I left it as is.

#include "Ping.h"
Ping ping = Ping(9,0,0); // ping sensor on pin 9
int cam_1 = 10; // camera one on pin ten
int cam_2 = 11; // camera two on pin eleven
int camDelay = 250;
boolean cam2Flag = 0;
int dataPin = 5;        
int latchPin = 6;
int clockPin = 7;
int j = 1;
int toShift = 0;         
char m[2];               
byte data;
byte dataArray[10];

void setup() {
  pinMode(cam_1, OUTPUT);
  pinMode(cam_2, OUTPUT);
  pinMode(latchPin, OUTPUT);
  dataArray[0] = 0x3F; // 0   bytes representing each LED digit
  dataArray[1] = 0x06; // 1
  dataArray[2] = 0x5B; // 2
  dataArray[3] = 0x4F; // 3
  dataArray[4] = 0x66; // 4
  dataArray[5] = 0x6D; // 5
  dataArray[6] = 0x7C; // 6
  dataArray[7] = 0x07; // 7
  dataArray[8] = 0x7F; // 8
  dataArray[9] = 0x67; // 9

  // debug - Serial.begin(9600);

  // clear both displays with 0's
  // top
  digitalWrite(latchPin, 0);
  shiftOut(dataPin, clockPin, dataArray[0]);
  shiftOut(dataPin, clockPin, dataArray[0]);
  digitalWrite(latchPin, 1);
}

void loop()
{
  ping.fire();
  int toShift = ping.inches();
  // debug - Serial.println(toShift);


  // the following code is by EmilyJane of arduino.cc forums
  int temp = toShift;
  int k = 10;  
  for (int i = 1; i >= 0; i--)
  {
    m[i] = temp / k;
    temp = temp - (m[i] * k);
    k = k / 10;
  }
  // end EmilyJane code

  // send bytes to shift register - if digits are in reverse order, reverse 0,1 in shiftOut order
  digitalWrite(latchPin, 0);
  shiftOut(dataPin, clockPin, dataArray[m[0]]);
  shiftOut(dataPin, clockPin, dataArray[m[1]]);
  digitalWrite(latchPin, 1);

  //  camera firing code 
  while(toShift < 36 && toShift > 0)  //  toShift is value from ultrasonic sensor, trigger camera 1 under 36"
  {  
    digitalWrite(cam_1, HIGH);        //  This block triggers auto-focus
    delay(camDelay);                  //
    digitalWrite(cam_1, LOW);         //  This block triggers shutter
    delay(camDelay);                  //  camDelay is about 1/4 second, slow enough for auto focus, fast enough for good lighting conditions
    cam2Flag = 1;                     //  cam2Flag set to trip 2nd cam when loop done
    ping.fire();                      //  read distance again to exit while loop when appropriate.
    toShift = ping.inches();          //
  }
  while(toShift > 36 && cam2Flag == 1) //  after distance is exceeded and after cam1 is tripped
  {
    for (int i = 0; i < 4; i++)       //  fire camera 4 times if distance remains open and if camera 1 has tripped
    {
      digitalWrite(cam_2, HIGH);      //  same as above
      delay(camDelay);                //
      digitalWrite(cam_2, LOW);       //
      delay(camDelay);                //
      ping.fire();                    //  check distance to stop cam2 triggers if distance is less than 36"
      toShift = ping.inches();        //
    }
    cam2Flag = 0;                     // reset flag for cam2, don't fire if camera 1 isn't tripped first
  }
}

// the heart of the program by SpikedCola on forum
void shiftOut(int mydataPin, int myClockPin, byte mydataOut)
{
  // This shifts 8 bits out MSB first,
  //on the rising edge of the clock,
  //clock idles low

  //internal function setup
  int i=0;
  int pinState;
  pinMode(myClockPin, OUTPUT);
  pinMode(mydataPin, OUTPUT);

  //clear everything out just in case to
  //prepare shift register for bit shifting
  digitalWrite(mydataPin, 0);
  digitalWrite(myClockPin, 0);

  //for each bit in the byte mydataOut?
  //NOTICE THAT WE ARE COUNTING DOWN in our for loop
  //This means that %00000001 or "1" will go through such
  //that it will be pin Q0 that lights.
  for (i=7; i>=0; i--)  {
    digitalWrite(myClockPin, 0);

    //if the value passed to mydataOut and a bitmask result
    // true then... so if we are at i=6 and our value is
    // %11010100 it would the code compares it to %01000000
    // and proceeds to set pinState to 1.
    if ( mydataOut & (1<<i) ) {
      pinState= 1;
    }
    else {
      pinState= 0;
    }

    //Sets the pin to HIGH or LOW depending on pinState
    digitalWrite(mydataPin, pinState);
    //register shifts bits on upstroke of clock pin
    digitalWrite(myClockPin, 1);
    //zero the data pin after shift to prevent bleed through
    digitalWrite(mydataPin, 0);
  }

  //stop shifting
  digitalWrite(myClockPin, 0);

}

A curiosity in this code from SpikedCola…the latchPin is set to OUTPUT in setup(), but dataPin and clockPin are not, yet the display works as it should. Shouldn’t the lack of setting pinMode to OUTPUT on those two prevent it from working? Also I noted there’s a j=1 declaration that has no reference within the program, as best as I can tell. I’ll comment it out to see if all continues to work properly.

dataArray[0] = 0x3F; // 0   bytes representing each LED digit
  dataArray[1] = 0x06; // 1
  dataArray[2] = 0x5B; // 2
  dataArray[3] = 0x4F; // 3
  dataArray[4] = 0x66; // 4
  dataArray[5] = 0x6D; // 5
  dataArray[6] = 0x7C; // 6
  dataArray[7] = 0x07; // 7
  dataArray[8] = 0x7F; // 8
  dataArray[9] = 0x67; // 9
byte dataArray [10] = {0x3F, 0x06, 0x5B...etc};

Just yesterday, I read some instructional material that had an array loaded in that manner. I had considered to change it, but having the visual presentation that was created by SpikedCola is convenient. Is there a memory difference or processing difference between the two?

  while(toShift < 36 && toShift > 0)  //  toShift is value from ultrasonic sensor, trigger camera 1 under 36"

This just grates on me for some reason. In normal English, you would say “while the distance is greater than 0 and less than 36…”, not “while the distance is less than 36 and greater than 0…”, so the code should be expressed the same way.

  while(toShift > 0 && toShift < 36)  //  toShift is value from ultrasonic sensor, trigger camera 1 under 36"

Other than the issue AWOL points out (which I no longer need to), the code is well organized, nicely commented, and quite easy to read and follow.

but having the visual presentation that was created by SpikedCola is convenient

Sorry, I have no idea what that means.

byte dataArray[10] = {0x3F,
                                  0x06,
                                  0x5B,
                                  0x4F,
                                  0x66,
...etc

For the 7-segment LED modules being used, SpikedCola created the array in the code. Each byte listed there corresponds to the appropriate digit being displayed on the LED. Those two digits come from the ping.fire converted to inches, then broken out by EmilyJane's code to be sent to the shift registers.

if the digit to be displayed is a 4, the array index is 4 and the byte that gets sent will turn on the appropriate segments to display a 4 on the module.

Is there a memory difference or processing difference between the two?

No. Either one works. You'll find, though, that the array declaration and initialization in one step is more natural, the more you use it.

PaulS:

Is there a memory difference or processing difference between the two?

No. Either one works. You'll find, though, that the array declaration and initialization in one step is more natural, the more you use it.

The declaration and initialization use less program memory. On a microprocessor that is helpful. Example:

const byte dataPin = 2;
const byte clockPin = 3;

byte dataArray[10];

void setup() {
  dataArray[0] = 0x3F; // 0   bytes representing each LED digit
  dataArray[1] = 0x06; // 1
  dataArray[2] = 0x5B; // 2
  dataArray[3] = 0x4F; // 3
  dataArray[4] = 0x66; // 4
  dataArray[5] = 0x6D; // 5
  dataArray[6] = 0x7C; // 6
  dataArray[7] = 0x07; // 7
  dataArray[8] = 0x7F; // 8
  dataArray[9] = 0x67; // 9

  shiftOut(dataPin, clockPin, dataArray[0], MSBFIRST);
  shiftOut(dataPin, clockPin, dataArray[0], MSBFIRST);
}

void loop() {}

Compiles to:

Binary sketch size: 942 bytes (of a 32,256 byte maximum)

Pre-initializing like this:

const byte dataPin = 2;
const byte clockPin = 3;

byte dataArray[10] = { 0x3F, 0x06, 0x5B, 0x4F, 0x66, 0x6D, 0x7C, 0x07, 0x7F, 0x67 };

void setup() {

  shiftOut(dataPin, clockPin, dataArray[0], MSBFIRST);
  shiftOut(dataPin, clockPin, dataArray[0], MSBFIRST);
}

void loop() { }

Compiles to:

Binary sketch size: 894 bytes (of a 32,256 byte maximum)

So you save 48 bytes.

The reason is that in the first case, procedural code has to be generated to move the data into the array. And the larger the array, the more code. In the second case the array is pre-populated by the compiler.

Making the changes suggested for int versus byte and adding the constant declaration, I reduced the size of the program by more than 200 bytes. Certainly this program isn't going to be lacking space, but I'd rather develop the right habits early.