eliteio / Arduino_New_Ping

New Ping Library from http://playground.arduino.cc/Code/NewPing
12 stars 5 forks source link

Rounding #2

Closed collinthornton closed 5 years ago

collinthornton commented 5 years ago

Awesome library!

However I did find one bug, The ping_cm() function:

unsigned long NewPing::ping_cm(unsigned int max_cm_distance) {
    unsigned long echoTime = NewPing::ping(max_cm_distance); // Calls the ping method and returns with the ping echo distance in uS.
#if ROUNDING_ENABLED == false
    return (echoTime / US_ROUNDTRIP_CM);              // Call the ping method and returns the distance in centimeters (no rounding).
#else
    return NewPingConvert(echoTime, US_ROUNDTRIP_CM); // Convert uS to centimeters.
#endif
}

has a bug preventing it from ever outputting nonrounded values. For one, its signature declares the return value as an integer datatype. Both variables need to be explicitly cast to a float as well. I ended up adjusting the function to something like this:

float NewPing::ping_cm(unsigned int max_cm_distance) {
    unsigned long echoTime = NewPing::ping(max_cm_distance); // Calls the ping method and returns with the ping echo distance in uS.
#if ROUNDING_ENABLED == false
    return (float)((float)echoTime / (float)US_ROUNDTRIP_CM);              // Call the ping method and returns the distance in centimeters (no rounding).
#else
    return NewPingConvert(echoTime, US_ROUNDTRIP_CM); // Convert uS to centimeters.
#endif
}

That said, my "fix" isn't very efficient as is was thrown together in about thirty seconds. It'd be great if there was a second set of functions specifically for unrounded data.

Either way you go about it, you need to at least get rid of the "ROUNDING_ENABLED == false" checks. They're checking a parameter that is unimplementable with the current set of functions.

ScruffR commented 5 years ago

This is the implementation of the original Arduino library with the only addition of SparkIntervalTimer.

The rounding vs. not-rounding does not mean that you will get a more accurate float when not rounding, but rather that non-rounding is typically less accurate due to the way integer divisions are done. An integer division will always give you the truncated/floor value, so the counteraction is to add 0.5 and then truncate - that's what NewPingConvert() does. Due to the way how distance is measured and the variance in air pressure and temperature there is little sense in having fractions after all.

collinthornton commented 5 years ago

So, to clarify:


Setting ROUNDING_ENABLED to false does not increase the precision of the value beyond that of an integer. Instead, it merely instructs the library to perform unadjusted integer division, which can lead to values above 3.5 cm being truncated to 3 cm.

On the other hand, setting ROUNDING_ENABLED to true takes slightly more time due to a few extra operations, but ensures that values such as 3.5 cm are increased by .5 cm so that the ensuing integer division truncates the value to 4 cm, not 3 cm.


If this is the case, I apologize for bringing up the issue. I understand the issues in precision with ultrasonic range finding with factors such as air pressure and temperature, but is there any way to increase precision to +/- 10 mm reliably?

ScruffR commented 5 years ago

is there any way to increase precision to +/- 10 mm reliably?

I'd say that's a question beyond the scope of this library but rather something to search the web for. But especially the reliably part migh pose some problem as you'd need to feed the external factors (temperature, humidity, wind, ringing, echo, ...) into the calculation. However if you have a "precision" of +/-10 mm in mind, isn't that what the library already provides? e.g. 278mm should give you 28cm which fits well into range interval 278 +/-10 = [ 288 ; 268 ]