Arduino code and PING))) ultrasonic rangefinder

Question

Context

I've started digging into Arduino and different type of sensors which is fun, but I'm worried that that my coding is too explicit. I've lots of experience for coding corporate programs, though it is not in C/C++. I don't consider myself a beginner, but I think something is wrong.

---

Blocks

I'd place my code in this section. It is using PING))) sensor to detect how far an object is based on the travel time of an emitted ultra-sonic signal. This sketch is taken from the TinkerCad website which can be found here: tinkercad.com.

/*
  Ping))) Sensor

  This sketch reads a PING))) ultrasonic rangefinder and returns the distance to the closest object in range. To do this, it sends a pulse to the sensor to initiate a reading, then listens for a pulse to return.  The length of the returning pulse is proportional to the distance of the object from the sensor.

  The circuit:
   * +V connection of the PING))) attached to +5V
   * GND connection attached to ground
   * SIG connection attached to digital pin 7

  http://www.arduino.cc/en/Tutorial/Ping

  This example code is in the public domain.
*/

const int speedOfSoundInAirInMetersPerSecond = 343;
const float speedOfSoundInAirInCentimeterPerMicrosecond = 
  speedOfSoundInAirInMetersPerSecond / 10000.0;
const float centimeterToInchRatio = 2.54;

int inches = 0;
int cm = 0;

long readTravelTimeInMicroseconds(int triggerPin, int echoPin)
{
  pinMode(triggerPin, OUTPUT);  // Clear the trigger
  digitalWrite(triggerPin, LOW);
  delayMicroseconds(2);

  digitalWrite(triggerPin, HIGH);
  delayMicroseconds(10);

  digitalWrite(triggerPin, LOW);
  pinMode(echoPin, INPUT);
  
  // Reads the echo pin, and returns the sound wave travel time in microseconds
  return pulseIn(echoPin, HIGH);
}

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

void loop()
{
  // I had to divided that speed by 2 because
  // I'm only interested in the time which took
  // signal to reach an object. I don't need to know
  // how much distance we covered by the signal to the object and back.
  cm = (speedOfSoundInAirInCentimeterPerMicrosecond / 2) 
    * readTravelTimeInMicroseconds(7, 7);

  inches = (cm / centimeterToInchRatio);
  Serial.print(inches);
  Serial.print("in, ");
  Serial.print(cm);
  Serial.println("cm");
  delay(100);
}

---

My thoughts

• Am I not being too explicit? I've learnt over the years that it is always better to beexplicit. But, am I not going overboard here?

• Are there styles and conventions, styles I should follow when working with Arduino or electronics of this type? My question concerns with the technical side of the code.

Answer 1

• VeryLongIdentifiersInCamelCase do not make your code explicit. They just add noise.

• The definition

  const float speedOfSoundInAirInCentimeterPerMicrosecond = 
    speedOfSoundInAirInMetersPerSecond / 10000.0;
  

  is very hard to read. 10000 stands out as weird. After all, there are 1000000 microseconds in a second. It took me quite a time to realize that two orders of magnitude are hidden in meters-to-centimeters conversion.

• By default, pulseIn timeouts in 1 second. In reality it means that there is no obstacle within 170 or so meters (aka 500 feet). I seriously doubt your hardware is capable detecting an echo at such distances. It feels safe to shorten the timeout for the sake of better responsiveness.

• The production quality code must deal with the ambient noise, which will result in false positives. At least, test that the signal you detected is indeed an echo - that is, it is about as long as the ping. You should also special-case inPulse returning 0. Currently, you are returning 0 distance, whist in fact it means the distance is practically infinite.

• Unless I am seeing things, either the code, or documentation is wrong. According to the spec

  if value is HIGH, pulseIn() waits for the pin to go from LOW to HIGH, starts timing, then waits for the pin to go LOW and stops timing. Returns the length of the pulse in microseconds

  which means it returns the duration of the echo, not the turnaround time you are after. Unfortunately, I don't have the hardware to play with.

  Does your code give reasonable results?

Answer 2

Am I not being to(o) explicit?

Names are not too explicit, but verbose.

Consider using standard physical units with metric units as the default: "meter, second, Volts, Amps, ...".

Example, for speed, use all speed data in meters/second (unless otherwise name/commented). Then the names can be shorter.

// const int speedOfSoundInAirInMetersPerSecond = 343;
const int speedOfSoundInAir = 343 /* m/s */

Rather than ratios, use "per" or "p". Use standard SI abbreviations^*1. For non-SI units, use the word like "inch". Use float constants for float objects - append an f.

// const float centimeterToInchRatio = 2.54;
const float cm_per_inch = 2.54f;

As able, object names should primarily reflect what they are and then maybe, and secondarily, units. Notice how easy it is to read and see the units balance.

// cm = (speedOfSoundInAirInCentimeterPerMicrosecond / 2) *  readTravelTimeInMicroseconds(7, 7);
int distance_cm = (speedOfSoundInAir_cm_per_us * readTravelTime_us(7, 7))/2;

// inches = (cm / centimeterToInchRatio);
int distance_inch = distance_cm * inch_per_cm;

Note that OP's speedOfSoundInAirInCentimeterPerMicrosecond / 2 effectively lost the last bit of the speed.

---

When working with small computing machines and processing time is important, consider the impact of FP math vs. an all int solution. Code may gain performance, yet watch out for overflow.

const float inch_per_cm = 2.54f; /* inch / cm */
int distance_inch = distance_cm * inch_per_cm;

// vs all int solution
const int inch_per_cm_N = 254; /* inch / cm */
const int inch_per_cm_D = 100;
// Round by adding half denominator (assuming pos values)
int distance_inch = (distance_cm*inch_per_cm_N + inch_per_cm_D/2)/inch_per_cm_D;

---

^*1 Using u for μ.