Just another program for my bot: 4 Ping sensors with Buzzer alarm (updated)

Hello,
Just put together this program which uses 3 ping sensors (mounted on my robot). For the time being(or may leave it in the code), I added a buzzer to emit sound when a certain minimum distance was reached for each sensor.
The program works but was wondering if I can improve it in any way or make it more efficient.

// 3 Ping sensors with buzzer alarm 
// Ping sensors mounted on robot: 2 Ping sensors mounted on front and 1 Ping sensor mounted on rear
// Have buzzer emit sound when a minimum threshold distance in inches is reached for each Ping sensor
// 6May12


int durationPingLeft;                 // Stores duration of pulse on Left Sensor
int durationPingRight;                // Stores duration of pulse on Right Sensor
int durationPingRear;                 // Stores duration of pulse on Rear Sensor
int distancePingLeft;                 // Stores distance Left Sensor
int distancePingRight;                // Stores distance Right Sensor
int distancePingRear;                 // Stores distance Rear Sensor
const int LeftPingSens = 2;           // Left Ping sensor connected to digital pin D2
const int RightPingSens = 4;          // Right Ping sensor connected to digital pin D4
const int RearPingSens = 7;           // Rear Ping sensor connected to digital pin D7
const int buzzOut = 8;                // Buzzer connected to digital pin D8               


void setup() {
  Serial.begin(9600);
  pinMode(buzzOut, OUTPUT); 
}

void loop()
{
  
// Left Ping sensor:  
  pinMode(LeftPingSens, OUTPUT);	                 // Set pin to OUTPUT Left Sensor
  digitalWrite(LeftPingSens, LOW);	                 // Ensure pin is low Left Sensor
  delayMicroseconds(2);
  digitalWrite(LeftPingSens, HIGH);                      // Start ranging Left Sensor
  delayMicroseconds(10);                               
  digitalWrite(LeftPingSens, LOW);                       // End ranging Left Sensor
  pinMode(LeftPingSens, INPUT);	                         // Set pin to INPUT Left Sensor
  int durationPingLeft = pulseIn(LeftPingSens, HIGH);    // Read echo pulse Left Sensor
  distancePingLeft = durationPingLeft / 74 / 2;          // Convert to inches Left Sensor

// Right Ping sensor:  
  pinMode(RightPingSens, OUTPUT);                        // Set pin to OUTPUT Right Sensor
  digitalWrite(RightPingSens, LOW);                      // Ensure pin is low Right Sensor
  delayMicroseconds(2);
  digitalWrite(RightPingSens, HIGH);                     // Start ranging Right Sensor
  delayMicroseconds(10);                               
  digitalWrite(RightPingSens, LOW);                      // End ranging Right Sensor
  pinMode(RightPingSens, INPUT);	                 // Set pin to INPUT Rear Sensor
  int durationPingRight = pulseIn(RightPingSens, HIGH);  // Read echo pulse Right Sensor
  distancePingRight = durationPingRight / 74 / 2;        // Convert to inches Right Sensor

// Rear Ping sensor:   
  pinMode(RearPingSens, OUTPUT);	                 // Set pin to OUTPUT Rear Sensor
  digitalWrite(RearPingSens, LOW);	                 // Ensure pin is low Rear Sensor
  delayMicroseconds(2);
  digitalWrite(RearPingSens, HIGH);                      // Start ranging Rear Sensor
  delayMicroseconds(10);                               
  digitalWrite(RearPingSens, LOW);                       // End ranging Rear Sensor
  pinMode(RearPingSens, INPUT);	                         // Set pin to INPUT Rear Sensor
  int durationPingRear = pulseIn(RearPingSens, HIGH);    // Read echo pulse Rear Sensor
  distancePingRear = durationPingRear / 74 / 2;          // Convert to inches Rear Sensor
  
  digitalWrite(buzzOut, LOW);    

  if(distancePingLeft < 24) digitalWrite(buzzOut, HIGH);   // sets a min. threshold value of 24" to sound alarm for Left Ping sensor
  if(distancePingRight < 24) digitalWrite(buzzOut, HIGH);  // sets a min. threshold value of 24" to sound alarm for Right Ping sensor
  if(distancePingRear < 18) digitalWrite(buzzOut, HIGH);   // sets a min. threshold value of 18" to sound alarm for Rear Ping sensor



  Serial.print(F("Left Sensor  "));
  Serial.println(distancePingLeft);
  Serial.print(F("Right Sensor  "));
  Serial.println(distancePingRight);
  Serial.print(F("Rear Sensor  "));
  Serial.println(distancePingRear);                   
  Serial.println("");

{  
delay(800);            
  }

}

was wondering if I can improve it in any way

Yes, you can. You have a bunch of global variables that are only used in loop(). Those should be local in scope, instead.

Many of them are not actually used at all, since you have local variables of the same name.

You have 3 nearly identical blocks of code. One copy should be in a function that takes a pin number as an argument and returns a distance value. In loop(), then, you should just make 3 calls to that function.

or make it more efficient.

For you? For the compiler? For the Arduino?

PaulS:

was wondering if I can improve it in any way

Yes, you can. You have a bunch of global variables that are only used in loop(). Those should be local in scope, instead.

Many of them are not actually used at all, since you have local variables of the same name.

You have 3 nearly identical blocks of code. One copy should be in a function that takes a pin number as an argument and returns a distance value. In loop(), then, you should just make 3 calls to that function.

Thanks Paul, will go back and try what you said and post back.

or make it more efficient.

For you? For the compiler? For the Arduino?

I believe all 3 go hand in hand :slight_smile: For me in the long run & better software makes for better use of hardware ...i guess? hehe

I believe all 3 go hand in hand

Not entirely. The compiler is pretty darn good at optimizing code, to make the Arduino's job easier. What it can't do anything about, though, is redundant code, as you have. Redundant code may be quick to create, but it is not easy to maintain.

The compiler runs once; the compiled code may run once, a dozen times, or millions of times. Writing code that makes the compilers job easy ought not really be a concern. You need to focus, for now, on making code that is understandable and maintainable. Limiting scope and creating atomic functions (As Major Charles Emerson Winchester was fond of saying "I do one thing at a time, and I do that very well"). Here, you have an opportunity to learn to do both.

PaulS:

I believe all 3 go hand in hand

Not entirely. The compiler is pretty darn good at optimizing code, to make the Arduino's job easier. What it can't do anything about, though, is redundant code, as you have. Redundant code may be quick to create, but it is not easy to maintain.

The compiler runs once; the compiled code may run once, a dozen times, or millions of times. Writing code that makes the compilers job easy ought not really be a concern. You need to focus, for now, on making code that is understandable and maintainable. Limiting scope and creating atomic functions (As Major Charles Emerson Winchester was fond of saying "I do one thing at a time, and I do that very well"). Here, you have an opportunity to learn to do both.

When you say 'not easy to maintain', you mean not easily optimized, changed, upgraded, etc...?

Just looked up the definition of scope (computer science), and I feel mind numbingly dumb atm lol :slight_smile: I will have to read up some more on it. Seems to be a lot about variables..

Also, when you say atomic functions, this is when you can complete a function in one clock cycle, without any interruptions or unnecessary rendundancies?

When you say 'not easy to maintain', you mean not easily optimized, changed, upgraded, etc...?

And more. If you change to a different type of sensor, you have a lot of code to change now. Put one of the similar blocks of code in a function, and you have 1/3 as much code to maintain, optimize, change, upgrade, etc. (comment, document...).

Seems to be a lot about variables..

It is all about variables, and how much of the code can see (and modify, inadvertently or deliberately) the variable. A global variable can be seen by all the code in a file. A local variable can be seen by much less code. Limiting the scope of a variable is usually a good thing.

Also, when you say atomic functions, this is when you can complete a function in one clock cycle, without any interruptions or unnecessary rendundancies?

There are multiple meanings to "atomic". A function will not complete in one clock cycle. It takes more than one clock cycle to push the return code onto the stack, branch to the proper place, and pop the value off the stack. In the case of functions, atomic means that it does one thing. A function like digitalRead() does one thing - it returns the value of a digital pin. Now, it may take a lot of steps to convert the pin number to a port, to actually get the value, etc. But, that is encapsulated in the function, and you do not need to understand how it does it's magic to perform the one task that is set for it.

On the other hand, a function like buzzIfDistanceIsLessThan24Inches(int sensorPin1, int sensorPin2, int sensorPin3, int buzzerPin), is not an atomic function. It does not do one thing. It does several things. You should strive to develop functions like analogRead(), digitalWrite(), etc. that do one thing, and do that one thing well. Such functions are easy to name.

You will be tempted, sooner or later, to develop functions that are hard to name, because they do more than one thing. Resist that temptation, and make two (or more) functions.

Thanks Paul,
I'll try out some smaller programs and get more familiar with using variables.

Just thinking about variables gave me an idea to try later on down the road.
Having a variable min. threshold for the Ping sensor where, for example, if the motors are running at full speed foward have the Ping min. threshold maybe somewhere around 8 feet. As the speed decreases so does the Ping min. threshold. Then when moving very slowly, the min. threshold could be at maybe 1-2 feet.
For now I just have it written down in my notepad to remember later :slight_smile:

t

Been messing around a bit more with the (4X Ping) code. Unfortunately something isn't cooperating :confused:
Atm, getting (0in, 0in, 0in, 0in) in serial monitor...and the scrolling/updating is a bit slow.

// 4 Ping sensor code for left, right, rear & head ping sensors mounted on robot.
// Using Arduino Mega 2560 & IDE 1.0.1
//
// 10June12

#define LT_PING 20
#define RT_PING 21
#define RR_PING 22
#define HD_PING 23
long in1 = 0;
long in2 = 0;
long in3 = 0;
long in4 = 0;
//const int buzzPin = 4;

void setup()
{
  Serial.begin(9600);
  //pinMode(buzzPin, OUTPUT);
}

long ping(int pin)
{
  pinMode(pin, OUTPUT);
  digitalWrite(pin, LOW);
  delayMicroseconds(2);
  digitalWrite(pin, HIGH);
  delayMicroseconds(5);
  digitalWrite(pin, LOW);
  pinMode(pin, INPUT);
  
long duration = pulseIn(pin, HIGH);
  
  return (microsecondsToInches(duration));
}

long microsecondsToInches(long microseconds)
{
  return microseconds / 74 / 2;
}

void loop()
{
  in1 = ping(LT_PING);
  delay(5);
  in2 = ping(RT_PING);
  delay(5);
  in3 = ping(RR_PING);
  delay(5);
  in4 = ping(HD_PING);
  delay(5);
  
  Serial.print(in1);
  Serial.print("in, ");
  Serial.print(in2);
  Serial.print("in, ");
  Serial.print(in3);
  Serial.print("in, ");
  Serial.print(in4);
  Serial.print("in, ");
  Serial.println();
    
}

// Use buzzer, main motors to react to different Ping threshold distances

  //digitalWrite(buzzPin, LOW);    
  //if(in1 < 24) digitalWrite(buzzPin, HIGH);    // sets a min. threshold value of 24" to sound alarm for Left Ping sensor
  //if(in2 < 24) digitalWrite(buzzPin, HIGH);    // sets a min. threshold value of 24" to sound alarm for Right Ping sensor
  //if(in3 < 18) digitalWrite(buzzPin, HIGH);    // sets a min. threshold value of 18" to sound alarm for Rear Ping sensor
  //if(in4 < 60) digitalWrite(buzzPin, HIGH);    // sets a min. threshold value of 60" to sound alarm for Head Ping sensor

thomas

You need to concentrate on getting results for a single sensor.
Debug prints may help.
Then, when that is debugged, it should be relatively easy to expand for all your sensors.

(Your "in1", "in2" etc variables don't need to have global scope, and would probably be better expressed as an array)

Thanks very much for the reply.
Yeah, my first attempt with just one worked as well as my '3 ping sensor' code at the beginning of this post. I just wanted to make it more efficient. I was writing out 3 almost identical blocks of code, when I could/should write just one block and then make 3 calls to that single block from the main loop.
So I should go back to the single Ping sensor code and debug?

Ok, for an array, would this be something I would start to look at?

int pingInts[5];
//or
int pingInts[] = {in1, in2, in3, in4};
//or
int pingInts[5] = {in1, in2, in3, in4};

//then to retrieve a value from the array
x = pingInts[3];

Uncompiled, untested:

#define N_PINGS 4
const byte pingPins [N_PINGS] = {20, 21, 22, 23};
const int minRanges [N_PINGS] = {24, 24, 18, 60}; // for later.

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  int range [N_PINGS];
  for (int i = 0; i < N_PINGS; ++i) {
    range [i] = ping(pingPins [i]);
    delay(5);
  }  
  
  for (int i = 0; i < N_PINGS; ++i) {
    Serial.print(range [i]);
    Serial.print(" ");
  }  
  Serial.println("inches");
}
  
long ping(int pin)
{
  pinMode(pin, OUTPUT);
  digitalWrite(pin, LOW);
  delayMicroseconds(2);
  digitalWrite(pin, HIGH);
  delayMicroseconds(5);
  digitalWrite(pin, LOW);
  pinMode(pin, INPUT);
  
  return (microsecondsToInches(pulseIn(pin, HIGH)));
}

long microsecondsToInches(long microseconds)
{
  return microseconds / 74 / 2;
}

Reducing the limit in the "for" loops will allow you to concentrate on just one sensor.

Wow :slight_smile: Thanks for taking the time to write up some code for me.
It's frustrating as hell when you have some ideas but hit the programming 'brick wall' (in my case hehe).
Like, for example have the motor speed directly proportional to the ping sensor(s) threshold. So for at full motor speed, the threshold is set at maybe 8feet. When the speed of the motor decreases, so does the ping sensor(s) min. threshold...

I'll try and go through each line of your code and figure out what each line does. I'll try and understand the best I can...I may have a few questions in the next few hours.

thanks again AWOL :slight_smile:

t

When you create an 'object' you are basically creating an array?

Is 'i' a (local?)variable for a stored calculation? Can this be used again in the program for other calculations from a totally different source (i.e. another sensor)?

Here:

#define N_PINGS 4

You are defining/letting the program know there are 4 Ping inputs?

Here you assign the Ping pins in an array? :

const byte pingPins [N_PINGS] = {20, 21, 22, 23};

Here you are adding the minimum threshold values to the ping sensors in an array:

const int minRanges [N_PINGS] = {24, 24, 18, 60}; // for later.

In the main loop, you are cycling through each Ping (via it's pin number):

int range [N_PINGS];
  for (int i = 0; i < N_PINGS; ++i) {
    range [i] = ping(pingPins [i]);
    delay(5);
  }

Setting variable i back to 0(zero) again, and cycling back through each pin? :

for (int i = 0; i < N_PINGS; ++i) {

Just let me know if I'm starting to get too annoying...with all the questions, etc..

t

When you create an 'object' you are basically creating an array?

An object and an array are two different concepts. 'Objects' are an objected oriented programming concept (ie C++), which Arduino does support. That's beyond the scope of what code has been provided in this thread though. An array is a continuous sequence of variables of the same type that can be iteratively or independently accessed via the array index operator: [].

Is 'i' a (local?)variable for a stored calculation? Can this be used again in the program for other calculations from a totally different source (i.e. another sensor)?

This gets back into variable scope. Yes, i is a local variable, but it's important to understand HOW local it is. In the following case:

for (int i = 0; i < N_PINGS; ++i) {
    range [i] = ping(pingPins [i]);
    delay(5);
  }

i is only local to the for block. Once the for block finishes, that instance of the variable i no longer exists. Which is why in subsequent for loops, he's redeclaring the variable i (the purpose of the 'int' text in the for loop). The following has some not so obviously different behavior:

  int i;
  for (i = 0; i < N_PINGS; ++i) {
    range [i] = ping(pingPins [i]);
    delay(5);
  }

In this case, because i was declared prior to the for loop, and not inside it, it remains in scope outside the for loop, and will remain in scope until the block of code it was declared in is no longer in scope. What constitutes a 'block' of code? Anything within a a set of {} is a block of code. The body of the for loop is a block of code. The body of a function is a block of code. Variable scope extends into child blocks, but not back out to parent blocks, ie:

void loop()
{
  //This starts the loop function block of code
  int x = 25;

  while(x > 0)
  {
    //This starts the while block of code, which can gets visibility to it's parent's scope, so can see x
    int y;
    if(x > 10)
    {
      //This is yet another block.
      y = 5;
    }
    else
    {
      //And another block.
      y = 1;
    }
    x -= y;
  }
  //Now that you've left the while block, y is no longer in scope.  x remains in scope because you are still in the loop block  
  
}
//Now that you've left the loop block, x is no longer in scope either

#define N_PINGS 4 defines a macro. A very simple one. Everywhere in the code that the symbol N_PINGS exists will get replaced with the value 4 prior to actual compilation of the code. This gets into the code maintenance of things, making it potentially easy to change the number of sensors on your device and minimize the amount of changes in the code necessary to support it.

A common coding philosophy is that you should never have any hard coded values in your source code (commonly referred to as magic numbers). Those magic numbers should always be in either a variable, or a define. When dealing with thousands, tens of thousands, or hundreds of thousands of lines of code, it will rarely be obvious what those magic numbers actually mean. I've seen some programmers take it to the extreme and remove even hardcoded zeroes from their code, like:

int baseValue = 0;  //could also be #define'd (or const'd)
int numSensors = 5;  //etc.
for(int i = baseValue; i < numSensors; i++)