From a034951be271cc2dd35505ccf450ff02e1d51a89 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 15 Apr 2021 19:09:59 +0200 Subject: [PATCH 1/2] proposal : let the compiler optimize ? While compiling for feather32u4, current code throws a ``` libraries/arduino-lmic/src/lmic/lmic.c: In function 'LMICcore_adjustForDrift': libraries/arduino-lmic/src/lmic/lmic.c:1480:81: warning: integer overflow in expression [-Woverflow] u2_t const maxError = LMIC_kMaxClockError_ppm * MAX_CLOCK_ERROR / (1000 * 1000); ``` This change makes the warning disappear but I haven't really tested whether it's valid. Maybe that instead ? ```cpp u2_t const maxError = ((LMIC_kMaxClockError_ppm * MAX_CLOCK_ERROR) / 1000) / 1000; ``` --- src/lmic/lmic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lmic/lmic.c b/src/lmic/lmic.c index 957d6222..5dccac86 100644 --- a/src/lmic/lmic.c +++ b/src/lmic/lmic.c @@ -1476,7 +1476,7 @@ ostime_t LMICcore_adjustForDrift (ostime_t delay, ostime_t hsym, rxsyms_t rxsyms // a compile-time configuration. (In other words, assume that millis() // clock is accurate to 0.1%.) You should never use clockerror to // compensate for system-late problems. - u2_t const maxError = LMIC_kMaxClockError_ppm * MAX_CLOCK_ERROR / (1000 * 1000); + u2_t const maxError = LMIC_kMaxClockError_ppm * MAX_CLOCK_ERROR / 1000 / 1000; if (! LMIC_ENABLE_arbitrary_clock_error && clockerr > maxError) { clockerr = maxError; From 3cd190029193d06916ea9170f14ef5cc4fad2b8e Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 15 Apr 2021 23:45:23 +0200 Subject: [PATCH 2/2] add comment per review --- src/lmic/lmic.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lmic/lmic.c b/src/lmic/lmic.c index 5dccac86..6a6f288c 100644 --- a/src/lmic/lmic.c +++ b/src/lmic/lmic.c @@ -1476,6 +1476,11 @@ ostime_t LMICcore_adjustForDrift (ostime_t delay, ostime_t hsym, rxsyms_t rxsyms // a compile-time configuration. (In other words, assume that millis() // clock is accurate to 0.1%.) You should never use clockerror to // compensate for system-late problems. + // note about compiler: The initializer for maxError is coded for + // maximum portability. On 16-bit systems, some compilers complain + // if we write x / (1000 * 1000). x / 1000 / 1000 uses constants, + // is generally acceptable so it can be optimized in compiler's own + // way. u2_t const maxError = LMIC_kMaxClockError_ppm * MAX_CLOCK_ERROR / 1000 / 1000; if (! LMIC_ENABLE_arbitrary_clock_error && clockerr > maxError) {