You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
So #7027 is the culprit. But code above does not check the value regardless, so our exception is replaced with a run-time error.
Looking at esp32 where PR was also merged, it was eventually reverted back to original implementation because of errors with larger values. With the provided test (0...180 to 1000...2000 and back), error rate is 0. With 0 and 10000 as min and max - error rate becomes 99%, mapping value and mapping value back are off-by-one :/
Suppose in servo case it makes sense, but not in general func context. Or, calc should take a different value (and btw, divide-by-2 effect is not explained even in servo case besides that it is 'better' :/)
Fixed another issue with "inverted shift" (same range, one counting up, one down) and added a test function to show the "error" compared to the interpolation function using double
// Example program
# include<iostream>
# include<string>longmap(long x, long in_min, long in_max, long out_min, long out_max) {
constlong out_length = out_max - out_min;
constlong in_length = in_max - in_min;
if (in_length == 0) { return in_min; }
if (out_length == 0) { return out_min; }
constlong delta = x - in_min;
if (out_length == in_length) {
return out_min + delta;
}
if ((out_length < 0) && (in_length < 0)) {
returnmap(x, in_max, in_min, out_max, out_min);
} elseif (out_length < 0) {
returnmap(in_max - delta, in_min, in_max, out_max, out_min);
} elseif (in_length < 0) {
returnmap(in_max - delta, in_max, in_min, out_min, out_max);
}
// We now know in_min < in_max and out_min < out_max// Make sure x is within range of in_min ... in_maxif ((x < in_min) || (x > in_max)) {
long shift_factor = 0;
if (x < in_min) {
constlong before_min = in_min - x;
shift_factor = -1 - (before_min / in_length);
} elseif (x > in_max) {
constlong passed_max = x - in_max;
shift_factor = 1 + (passed_max / in_length);
}
if (shift_factor != 0) {
constlong in_shift = shift_factor * in_length;
constlong out_shift = shift_factor * out_length;
returnmap(x, in_min + in_shift, in_max + in_shift, out_min + out_shift, out_max + out_shift);
}
}
constlong abs_out_length = abs(out_length);
if (abs_out_length > abs(in_length)) {
constlong intermediate_max = (0x1FFFFFFF - abs(in_length / 2)) / abs_out_length;
if (intermediate_max > (abs_out_length >> 2)) {
constlong intermediate = (delta * intermediate_max * 2 + (in_length / 2)) / in_length - intermediate_max;
returnmap(intermediate, -intermediate_max, intermediate_max, out_min, out_max);
}
// Probably running into integer overflow, so use the slightly less accurate factor approach.constlong factor = out_length / in_length;
return (delta * factor) + out_min;
}
// abs(out_length) < abs(in_length)constunsignedlong overflow_estimator =
(delta > 256 ? (static_cast<unsignedlong>(delta) >> 8) : 1) *
(out_length > 256 ? (static_cast<unsignedlong>(out_length) >> 8) : 1) +
(static_cast<unsignedlong>(in_length / 2) >> 8);
if (overflow_estimator > (1u << 16)) {
// Would result in integer overflow, so use the slightly less accurate factor approach.constlong factor = in_length / out_length;
if (factor != 0) {
return delta / factor + out_min;
}
}
return (delta * out_length + (in_length / 2)) / in_length + out_min;
}
longtest_map(long x, long in_min, long in_max, long out_min, long out_max) {
constdouble out_length = out_max - out_min;
constdouble in_length = in_max - in_min;
if (in_length == 0) { return in_min; }
if (out_length == 0) { return out_min; }
constdouble delta = x - in_min;
constdouble value = (delta * out_length + (in_length / 2)) / in_length + out_min;
// return std::round(value); return value;
}
longcheck(long x, long in_min, long in_max, long out_min, long out_max) {
constlong floatvalue = test_map(x, in_min, in_max, out_min, out_max);
constlong map_value = map(x, in_min, in_max, out_min, out_max);
return floatvalue - map_value;
}
intmain()
{
long input = 0;
for (size_t i = 0; i < 15; ++i) {
input = (i / 3) * 2500 - 1 + i % 3;
constlong 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, 10000, 0, 10240, 0),
check(input, 10000, 0, 10240000, 0),
check(input, 50, 0, 10240000, 0)
};
std::cout << input << ":";
constexprsize_t nrvalues = sizeof(values) / sizeof(values[0]);
for (size_t i = 0; i < nrvalues; ++i) {
std::cout << "\t" << values[i];
}
std::cout << "\n";
}
}
Made it more accurate for large 'delta' and mapping large input range to smaller output range.
Also made it a lot simpler and now with less recursive calls (max 1 recursive call now)
Activity
mcspr commentedon Jun 16, 2023
So #7027 is the culprit. But code above does not check the value regardless, so our exception is replaced with a run-time error.
Looking at esp32 where PR was also merged, it was eventually reverted back to original implementation because of errors with larger values. With the provided test (0...180 to 1000...2000 and back), error rate is 0. With 0 and 10000 as min and max - error rate becomes 99%, mapping value and mapping value back are off-by-one :/
Suppose in servo case it makes sense, but not in general func context. Or, calc should take a different value (and btw, divide-by-2 effect is not explained even in servo case besides that it is 'better' :/)
Arduino/libraries/Servo/src/Servo.cpp
Lines 36 to 38 in 521ae60
TD-er commentedon Jul 16, 2023
Shouldn't a simple fix like this be enough to prevent these crashes?
Increasing resolution and int overflows with large values are much harder to fix.
But my gut feeling tells me there should be 4 separate cases:
This way the first part of the function can be something like this (untested, just notepad memory dump):
TD-er commentedon Jul 16, 2023
OK, just thought of something like this (still untested, just some gut feeling programming)
Not sure how modulo will act on negative values, so another approach could be something like this:
TD-er commentedon Jul 16, 2023
Just did a quick test on cpp.sh with the last suggested option:
And also mapping to a smaller range:
Result:
Edit:
Still work to do:
Not what it should be...
TD-er commentedon Jul 17, 2023
Made some progress....
With output:
@mcspr I think this is working very well and stable, without using a single float.
TD-er commentedon Jul 17, 2023
And some extra checks for int overflow:
Result:
TD-er commentedon Jul 17, 2023
Fixed another issue with "inverted shift" (same range, one counting up, one down) and added a test function to show the "error" compared to the interpolation function using
double
The "error" for each calculation.
I think this is as accurate as can be done using only integer calculations :)
TD-er commentedon Jul 17, 2023
OK, still could be improved :)
Now even more accurate with large numbers
Output:
TD-er commentedon Jul 17, 2023
OK, last update ... (for now) :)
Made it more accurate for large 'delta' and mapping large input range to smaller output range.
Also made it a lot simpler and now with less recursive calls (max 1 recursive call now)
Output:
Fix Map function - IntegerDivideByZero esp8266#8938