jasonacox / TM1637TinyDisplay

Arduino library to display numbers and text on a 4 and 6 digit 7-segment TM1637 display modules.
GNU Lesser General Public License v3.0
71 stars 19 forks source link

showNumber() function uses abs() instead of labs() for long variables #32

Closed KelevraSlevin7 closed 1 year ago

KelevraSlevin7 commented 1 year ago

When trying to display a number bigger than 32.767 it overflows because the used abs() function returns an int and not long. When abs() is changed to labs() the function is working as intended.

Or I did something wrong handling the function and didnt realize it...

jasonacox commented 1 year ago

Great find @KelevraSlevin7 ! I was confused as to why you were seeing this but I wasn't. I use an Uno for testing. I remember testing for overflow on the Uno and didn't see a problem. It turns out that Arduino uses this macro for their device core:

#define abs(x) ((x)>0?(x):-(x))

Which works with any type. Of course, that isn't standard for C and there is no macro for ESP or other MCUs. To test, I added #undef abs to the top of TM1637TinyDisplay6.cpp which causes it to default to the C library abs(). I saw the error with the int overflow occuring.

In any case, you are right. We need to switch to labs() or we could add the override macro to TM1637TinyDisplay6.cpp.

jasonacox commented 1 year ago

I see you have submitted PR #33

jasonacox commented 1 year ago

We need to apply the same change to the 4-digit display (TM1637TinyDisplay.cpp). For the most part, the 4-digit display which can only render up to 9,999 would not see this and would overflow (show ----) above the digit capacity. However, if you send display.showNumber(66000) you will get 464 in the display (the overflow above the max unsigned int 65,536).

jasonacox commented 1 year ago

Actually, as I test this, the labs() function call adds about 16 bytes to the progmem, likely due to the C library function call overhead. The use of abs() in the display calculations are simple and aren't stacking so a macro would work fine. I'm going to override the labs() function with a macro at the top of the cpp files and see if everything still works.

#define labs(x) ((x)>0?(x):-(x))

Note: I'm not putting the macro in the header files so it won't override the users calls to labs().

KelevraSlevin7 commented 1 year ago

Ah yeah didnt think about the added overhead of the labs() function. Thanks for the fast response and change of the code.