Fix Map function - IntegerDivideByZero #8938

[TD-er]

Fixes: #8938

Also increase accuracy of mapping and fixing map issues with large values. Still only using integer operations.

Test function to see absolute error compared to floating point map function using double precision floating point values.

# include <iostream>
# include <string>

long test_map(long x, long in_min, long in_max, long out_min, long out_max) {
    const double out_length = out_max - out_min;
    const double in_length  = in_max - in_min;

    if (in_length == 0) { return in_min; }

    if (out_length == 0) { return out_min; }

    const double delta = x - in_min;
    const double value = (delta * out_length + (in_length / 2)) / in_length + out_min;
//    return std::round(value);
    return value;
}

long check(long x, long in_min, long in_max, long out_min, long out_max) {
    const long floatvalue = test_map(x, in_min, in_max, out_min, out_max);
    const long map_value = map(x, in_min, in_max, out_min, out_max);
    return floatvalue - map_value;
}

int main()
{
    long input = 0;

    for (size_t i = 0; i < 15; ++i) {
        input = (i / 3) * 2500 - 1 + i % 3;
        const long values[] = {
            check(input, 0,        1000,   255,      0),
            check(input, 0,        1000,   0,        255),
            check(input, 10000,    0,      100,      10100),
            check(input, 10000,    0,      10100,    100),
            check(input, 10000,    0,      0,        1024),
            check(input, 10000,    0,      1024,     0),
            check(input, 1000,     0,      0,        10240),
            check(input, 1000,     0,      10240,    0),
            check(input, 0,        1000,   0,        10240),
            check(input, 0,        1000,   10240,    0),
            check(input, 0,        10000,  10240,    0),
            check(input, 10234567, -12345, 10234567, 100),
            check(input, 10000,    0,      10234567, 0),
            check(input, 50,       1,      10234567, 0)
        };
        std::cout << input << ":";
        constexpr size_t nrvalues = sizeof(values) / sizeof(values[0]);

        for (size_t i = 0; i < nrvalues; ++i) {
            std::cout << "\t" << values[i];
        }
        std::cout << "\n";
    }
}

Output:

-1:	0	-1	0	0	0	-1	0	2	2	0	0	-1	2	2
0:	0	0	0	0	0	0	0	0	0	0	0	-1	0	1
1:	-1	0	0	0	-1	0	1	0	0	1	1	-1	0	0
2499:	1	-1	0	0	0	0	1	1	1	1	0	0	0	0
2500:	1	1	0	0	0	0	1	0	0	1	0	0	1	0
2501:	0	0	0	0	0	0	2	0	0	2	1	0	0	1
4999:	1	-1	0	0	0	0	1	1	1	1	0	0	0	0
5000:	1	0	0	0	0	0	1	0	0	1	0	0	1	1
5001:	0	0	0	0	0	0	2	0	0	2	1	0	1	0
7499:	1	-1	0	0	0	0	1	1	1	1	0	0	1	1
7500:	1	1	0	0	0	0	1	0	0	1	0	0	0	0
7501:	0	0	0	0	0	0	2	0	0	2	1	0	1	0
9999:	1	-1	0	0	0	-1	1	1	1	1	0	0	1	0
10000:	1	0	0	0	0	0	1	0	0	1	0	0	0	0
10001:	0	0	0	0	-1	0	2	0	0	2	2	0	0	1

[d-a-v]

Why is in_min returned when input range is 0? Why not out_min or (out_min + out_max)/2 ?
User always expects to get a value of the same order as out_min..out_max.

The arduino reference is

long map(long x, long in_min, long in_max, long out_min, long out_max) {
  return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
}

The issue is the division by 0, why not just handling this case ?
How would this compare with your test_map() (with fixed return when input range is 0)

[TD-er]

In the latest commit this was alread addressed, as I also noticed this odd choice of mine :)

    if (in_length == 0 || out_length == 0) { 
        return out_min; 
    }

I guess returing the average of the output range also makes sense, but might be harder to detect?

The issue is the division by 0, why not just handling this case ?

Because there were more issues with map, as it was experiencing integer overflow issues quite easily.
You don't need to get really high values to experience those overflow issues.
Only with delta and dividend (renamed to out_range in my code as I constantly got confused) approaching 31 bits in size would already result in overflow issues.

How would this compare with your test_map() (with fixed return when input range is 0)

Just tested it and it will just return what has been given as out_min value.
Thus before the 'swap' of values.
map(X, N, N, out_min, out_max) will return out_min.