Arduino microcontroller based event handler program, send sensor readings over serial bus

Question

I am working on a project with an Arduino microcontroller and a Raspberry Pi. The code will have to do the following:

• If the variance calculated of the ultrasonic sensor detections exceeds 1000, a event will be sent over the serial bus starting with the prefix /E/

• If a request is received on the serial bus asking for the temperature, the Arduino shall measure the resistance of a NTC resistor. By adding another resistor to the NTC resistor, we can create a voltage divider. Once the output of the voltage divider is known, we can go back and calculate the resistance of the sensor. Once the resistance is known we can use the Steinhart–Hart equation to calculate the temperature. When the temperature is known it can be sent back to the Raspberry Pi in a serial message starting with the prefix /R/.

The code works fine, however the code is not optimized. My belief is the code can be made faster and more elegant. Since I am not very familiar with C++ I am looking for tips on how.

Any help would be appreciated!

#define TRIG_PIN 10 #define ECHO_PIN 9 #define THRESHOLD 1000.0f #define COOLDOWN 1000 #define R2 72000.0f #define Vin 3.3f /* * These are the coefficients of Steinhart–Hart required to calculate * the temperature based on the resistance of my NTC resistor. */ #define A 0.00056510530716328503247902759198950661811977624893f #define B 0.00024125080426957092754637612674883939689607359469f #define C 0.00000000066126633960212339995476015836904301603560f /* * In the setup function we initialize the serial bus and prepare the pins * because we want to communicate with the ultrasonic sensor. */ void setup() { Serial.begin(9600); pinMode(TRIG_PIN, OUTPUT); pinMode(ECHO_PIN, INPUT); } /* * This function is used to calculate the variance of six samples from * the ultrasonic sensor. */ float calculate_variance_distance() { int i; float distances[6]; float avrg, var, sum = 0, sum1 = 0; /* * Obtain six samples and wait 25 ms between each sample to avoid * fetching the same value over and over again. */ for (i = 0; i<6; i++) { distances[i] = measure_distance(3); sum = sum + distances[i]; delay(25); } /* * To calculate the variance we need to: * * Work out the Mean (the simple average of the numbers) * Then for each number: subtract the Mean and square the result (the squared difference). * Then work out the average of those squared differences. */ avrg = sum / 6.0f; for (i = 0; i<6; i++) { sum1 = sum1 + pow((distances[i] - avrg), 2); } var = sum1 / 6.0f; /* * We need the variance to tell how spread out the samples are. */ return var; } /* * In order to calculate the median we need a function to sort an array. */ void sort_array(float array[], int s) { int i, j, k; /* * We read multiple values and sort them to get the median. The sorted * values are storted in the array sorted. */ float n, sorted[s]; for (i = 0; i<s; i++) { n = array[i]; if (i == 0 || n<sorted[0]) { j = 0; // Insert at first position. } else { for (j = 1; j<i; j++) { if (sorted[j - 1] <= n && sorted[j] >= n) { // Now j is insert position break; } } } for (k = i; k>j; k--) { // Move all values higher than current reading up one position. sorted[k] = sorted[k - 1]; } sorted[j] = n; // Insert current reading. } for (i = 0; i<s; i++) { array[i] = sorted[i]; } } /* We need a function to calculate the distance according to the ultrasonic sensor. */ float measure_distance(int recursion) { long duration; float distance; digitalWrite(TRIG_PIN, LOW); // Set trigger pin low. delayMicroseconds(2); // Let signal settle. digitalWrite(TRIG_PIN, HIGH); // Set trigger pin high. delayMicroseconds(10); // Delay in high state. digitalWrite(TRIG_PIN, LOW); // Ping has now been sent. duration = pulseIn(ECHO_PIN, HIGH); // Duration is presented in microseconds. /* * It takes half the time for sound to travel to the object and back. We * know that it takes approximently 29 µs to travel one centimeter. */ distance = ((float)(duration) / 2.0) / 29.1; /* The accuracy of my ultrasonic sensor is 4 metres. If it exceeds 4 metres it is most * likely a false reading. So we try again until we get a readinge below that or recursion is zero. */ if (distance > 400) { if (recursion > 0) return measure_distance(--recursion); else return 400; } else { return distance; } } /* * We use Steinhart–Hart equation to calculate the temperature. */ float measure_temperature() { int readValue = analogRead(0); // Fetch the analog read value in the range of 0-1023 float Vout = 3.3f - ((float)(readValue) / 1023.0f*Vin); // Calculate the voltage. /* * In our voltage divider we have another resistor where the resistance is known, * so we can go back and calculate the resistance of the sensor. */ float R1 = ((R2*Vin) / Vout) - R2; float T = 1.0f / (A + B*log(R1) + C*pow(log(R1), 3)) - 273.15; // Use the Steinhart–Hart equation to calculate the temperature. return T; } /* * We fetch five samples and calculate the median to eleminate noise from sensor readings. */ float measure_median_temperature() { int i; float temperatures[5]; /* * Obtain five samples using the function above. */ for (i = 0; i<5; i++) { temperatures[i] = measure_temperature(); delay(50); } /* * Sort the array with the function above. */ sort_array(temperatures, 5); return temperatures[2]; } /* * We use the loop function to check if there are any requests and also to check * if the variance of the sensor readings exceeds THRESHOLD. */ void loop() { /* * We have a cooldown value to avoid unneccesary repeats. */ static unsigned int cooldown = 0; if (Serial.available() > 0) { String str = Serial.readStringUntil('\0'); if (str == "temperature") { float temperture = measure_median_temperature(); Serial.print("/R/"); Serial.print(round(temperture * 10)); return; } } else if (abs(millis() - cooldown) >= COOLDOWN && calculate_variance_distance() >= THRESHOLD) { Serial.print("/E/"); Serial.print("motion"); cooldown = millis(); } } 

Answer 1

I see a number of things which may help you improve your code. First, I think it's important for you to know that the language used by Arduino is only based on C++, but isn't C++.

Simplify the mathematics

If we look at the equations used to derive the temperature in measure_temperature, the mathematics can be simplified. Using a little algebra, we can derive the fact that $$ R_1 = \frac{x R_2}{1023 - x}$$ with \$x\$ being the readValue. Further, we really need the log of that, rather that the resistance value, so the measure_temperature function can be written like this:

float measure_temperature() { int readValue = analogRead(0); // Fetch the analog read value in the range of 0-1023 /* * In our voltage divider we have another resistor where the resistance is known, * so we can go back and calculate the resistance of the sensor. */ float logR1 = log((R2 * readValue)/(1023 - readValue)); return 1.0f / (A + B*logR1 + C*pow(logR1, 3)) - 273.15; // Use the Steinhart–Hart equation to calculate the temperature. } 

Understand the limits of floating point mathematics

On the Arduino, both float and double floating point types are represented as 32-bit quantities. That means that the terribly long values used for the coefficients A, B and C will mostly be unused in the actual calculations. Better would be to express them as floating point using no more than 7 digits of precision.

Consider using a look-up table

There are 1024 possible input values for the analog input and a float is 32 bits (4 bytes) so a 4K table could be used and would be much faster than all of that calculation. Precompute the results for all possible values and create a table to use within the source code. I'm sure you could write a simple program to do so. With many embedded processors, including that in the Arduino, there is no hardware support for floating point arithmetic, so floating point mathematics is typically much slower than integer calculations.

Spelling is important

The name of the variable in loop should be temperature rather than temperture. Having typos in your code leads to difficult-to-spot bugs and other maintenance difficulties. Being careful to spell things correctly will pay dividends in more robust, easier to maintain code.

Prefer to bail out early

In the measure_distance routine, if the distance is more than 400 cm, it is assumed to be faulty and the routine tries again. That corresponds to a reading of 23280 microseconds (29.1 microseconds times 400 cm time 2). However, the pulseIn function will wait up to 1 second. Since that's all just wasted time, it would instead make sense to use the form of pulseIn that has a timeout and then check for a value of 0 as indication that the value is faulty. See the pulseIn() reference page for details.

Prefer iteration to recursion

Recursion, as in the use in the measure_distance routine, is better expressed as an iteration instead. That is, something like this psuedocode:

for (tries = 3; tries; --tries) { if (attemptedMeasurement is good) { return attemptedMeasurement; } } 

The reason is that recursion uses stack space to set up a stack frame containing local variables for the function and the return location, while the iteration form only uses space for the tries counter. It's also a little bit more clear to the reader of the code.

Use a better sort algorithm

There are much better, more efficient (in terms of speed and space) sorting algorithms available that are better than you're using in your code. Even the much maligned bubble sort would likely outperform the algorithm currently implemented in sort_array.

Answer 2

1. You are defining the loop variables separately, I don't think you need to, you could do for (int i = 0; i < 6; i++)

2. In C++ post increment is slower for non-simple objects, so its good practice to pre-increment whenever you can. for (int i = 0; i < 6; ++i)

3. There are a lot of literal values in the code. Looking a calculate_variance_distance() the number of samples, 6, should be a static const (local to the function).

4. You are assigning ints to floats as initial values.
   float avrg, var, sum = 0, sum1 = 0;

5. As a point of style I would always declare each variable of a separate line.

6. Division is an expensive operation, so it might be preferable to multiply instead where you can.

7. You are using a diddy little Arduino to do complex maths operations before passing a nicely formatted string to a comparatively supercharged Pi. Would it be better to shift all the maths onto the PI and just read the data on the Arduino and pass larger strings of data. You'd need to speed test it, but it might work out better, especially if you have one of the quad core PIs.