1. Welcome to our site! Electro Tech is an online community (with over 170,000 members) who enjoy talking about and building electronic circuits, projects and gadgets. To participate you need to register. Registration is free. Click here to register now.
    Dismiss Notice

Unexplainable Math Error?

Discussion in 'Arduino' started by MrAl, Nov 12, 2015.

  1. MrAl

    MrAl Well-Known Member Most Helpful Member

    Joined:
    Sep 7, 2008
    Messages:
    11,049
    Likes:
    961
    Location:
    NJ
    Hello there,

    I ran into this little problem using the Arduino IDE but not sure if it is only with that IDE or with AVR in general. It happens to be the Uno this time.

    The code is for creating two simple clock timers. They count the seconds, minutes, hours, and days. The days might be limited for the data type used, but that is not a concern here. The main concern here is how the math is being evaluated for a particular calculation.

    The code in question is:
    unsigned long offset=0;

    and then later the offending statements:
    offset=1*86400; //set 1 day (in units of seconds) (the '1')
    offset=offset+13*3600; //set 13 hours (in seconds)
    offset=offset+53*60; //set 53 minutes (in seconds)
    offset=offset*1000;//set clock 1 offset, converting to milliseconds

    The above code, surprisingly, does not work. It sets the clock to some seeming random time but im sure it is not random, the compiler must not be interpreting the code correctly.
    I broke the calculation down into separate lines as shown in order to track down the offending statement and also to avoid using too much stack space just in case it was using too much stack space (doubtful but tried anyway).
    The offending line was found to be line 2, the calculation of the hours into units of seconds.
    What works is:
    offset=offset+1*3600;
    offset=offset+2*3600;
    offset=offset+3*3600;
    all the way to:
    offest=offset+9*3600;

    but when i get to:
    offset=offset+10*3600;

    and above like:
    offset=offset+13*3600; //the max for this would be 23 hours, here is shown 13 hours.

    it totally screws up the time, including the days and minutes and seconds (totally incorrect when for every other one shown it works good).

    For some reason as soon as i go to "10" hours, the compiler cant interpret this statement correctly.

    What else works but seems a little unnecessary is:
    offset=offset+10UL*3600UL; //UL stands for "type unsigned long"

    So in other words, without the UL it wont set the time correctly, but with the UL's it works fine.
    The other statements work fine without the UL.

    Any ideas?
     
  2. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    9,310
    Likes:
    914
    Location:
    Rochdale UK
    Yep!! The compiler treats constants as signed int's... Therefore if you take 9*3600 ( multiply first ) we get 32,400

    Remember a signed int is.. +32767 ~ -32768

    Then 10* 3600 will be -3233 Not what you want!!!

    The safest way is to just put L on the end of then all!!

    EDIT**** Its the "10" that needs to be the long, not so much the 3600!!
     
  3. MrAl

    MrAl Well-Known Member Most Helpful Member

    Joined:
    Sep 7, 2008
    Messages:
    11,049
    Likes:
    961
    Location:
    NJ
    Hi Ian,

    Yeah that makes a lot of sense. What i expected was that the compiler would default to the largest size type seeing that the return type was the largest, which in this case would have been the unsigned long (4 bytes) and therefore do all the math in type unsigned long. Now i see that it might only default when a variable type is already in that particular part of the math operation. For example:

    unsigned long c=10*3600;

    might not work because neither of the OPERANDS are unsigned long, while:

    unsigned long a=10;
    unsigned long c=a*3600;

    should work or:

    unsigned long a=3600;
    unsigned long c=10*a;

    would also work i think, because again one of the operands (not the return type) is an UL type.
    So it doesnt look at the return type, just the arguments in order to determine the type it will default to.

    I guess it also does not have a problem with 1*86400 because it must see the 96400 as being larger than int already.

    Live and learn or live and burn :)

    Thanks.
     
  4. dave

    Dave New Member

    Joined:
    Jan 12, 1997
    Messages:
    -
    Likes:
    0


     
  5. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    9,310
    Likes:
    914
    Location:
    Rochdale UK

    These are the bugs that take the fun from programming.... Some compilers do use longs for constants...

    There is a guy ( you must have seen the posts ) that is learning to program a TFT LCD.. The two constants XMAX and YMAX were multiplied together.... He was right they should have made 76k but alas only 11k.... It's a good job I have since seen and rectified these bugs.. In the manual it isn't clear where the constants are defined...
     
  6. MrAl

    MrAl Well-Known Member Most Helpful Member

    Joined:
    Sep 7, 2008
    Messages:
    11,049
    Likes:
    961
    Location:
    NJ
    Hi again Ian,

    Well i might have to take the blame for this one though, because i think i remember reading a long time ago when learning C and C++ that the operands determine the data type, but yes, i also did not realize that the default type was type int not type long. And that is because i must have missed the writeup in the Help section of the Arduino documentation which reads and i quote:
    [
    Integer Constants
    Integer constants are numbers used directly in a sketch, like 123. By default, these numbers are treated as int's but you can change this with the U and L modifiers (see below).
    ]

    Not knowing that can really screw up a program. I was seeing all kinds of nutty stuff coming up on the display, like "49 days" when it was supposed to be zero :)

    What else happened is i was more used to programming in C and C++ for the PC computer, which as you say treats them as type long.
     
  7. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    9,310
    Likes:
    914
    Location:
    Rochdale UK
    Programmers are trying to push the wording.... An Int is so ambiguous!! It really means integer value ( whole number ) The reason it was pushed into the world as a 16 bit whole number was that Intel and Zilog made register pairs so you could access 16 bit addresses... 64k used to be the building blocks... When expanded memory was introduced to the AT series PC it was through a 64k window...

    On a 8 bit machine the integer is 8 bit, on a 16 bit.. yada yada yada.... Words, Long words, Double long words are easier to remember....
     
  8. nsaspook

    nsaspook Well-Known Member

    Joined:
    Mar 24, 2010
    Messages:
    1,141
    Likes:
    219
    Location:
    Fairview, Or
    This problem is why most embedded programmers use the standard INT types with const variables if possible instead of constants.
    Code (text):

    #ifdef INTTYPES
    #include <stdint.h>
    #else
    #define INTTYPES
    /*unsigned types*/
    typedef unsigned char uint8_t;
    typedef unsigned int uint16_t;
    typedef unsigned long uint32_t;
    typedef unsigned long long uint64_t;
    /*signed types*/
    typedef signed char int8_t;
    typedef signed int int16_t;
    typedef signed long int32_t;
    typedef signed long long int64_t;
    #endif
     
     
  9. MrAl

    MrAl Well-Known Member Most Helpful Member

    Joined:
    Sep 7, 2008
    Messages:
    11,049
    Likes:
    961
    Location:
    NJ
    Hi again,

    Well that's another mystery, because the Atmel 328P chip is 8 bit, but the Arduino lib wants to make "int" 16 bits :)

    I'll just have to remember to be more specific from now on. I'll have to go over some of my other code now too to make sure there are no screw ups like this in them either. At least i know now.

    In a slightly related issue, i also found that if we want to make custom types we have to include the typedef's in the .h file, and it can not be in the .ino file or it wont compile. This might be standard with C and C++ but i cant remember now. But i also read that some people were putting their typedef structs in the .ino file and getting some weird results.
    When i programmed in C and C++ for Windows, i always made a point to make an alias for "unsigned long" as "ulong", as this would save me a lot of retyping of "unsigned integer" or "unsigned long".
     
  10. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    9,310
    Likes:
    914
    Location:
    Rochdale UK
    I may have confused the issue with this saying.
    An integer is just a whole number... End of..
     
  11. nsaspook

    nsaspook Well-Known Member

    Joined:
    Mar 24, 2010
    Messages:
    1,141
    Likes:
    219
    Location:
    Fairview, Or
    http://embeddedgurus.com/stack-over...t-c-tips-1-choosing-the-correct-integer-size/
    http://embeddedgurus.com/stack-overflow/2009/08/a-tutorial-on-signed-and-unsigned-integers/
     
  12. misterT

    misterT Well-Known Member Most Helpful Member

    Joined:
    Apr 19, 2010
    Messages:
    2,697
    Likes:
    368
    Location:
    Finland
    I ran quick test:

    printf("\n%d", sizeof(1000));
    printf("\n%d", sizeof(200));
    printf("\n%d", sizeof(40000));
    printf("\n%d", sizeof(4000*4000));

    This prints out:
    2
    2
    4
    2

    .. So the 40 000 is treated as Long, but 4000*4000 is two int variables multiplied and truncated.
    Compiled with avr-gcc (same compiler that Arduino IDE uses) -std=gnu99 flag on.

    avr-gcc and avr-libc documentation could be a better source than Arduino IDE documentation for this kind of problems.
    https://gcc.gnu.org/wiki/avr-gcc
    http://www.nongnu.org/avr-libc/user-manual/modules.html

    You can force the int to be 8-bits if you want, but I would not recommend doing that:
    "With -mint8 int is only 8 bits wide which does not comply to the C standard. Notice that -mint8 is not a multilib option and neither supported by AVR-Libc (except stdint.h) nor by newlib"

    I always use standard integer types, then you know exactly what kind of variable you get:
    #include <stdint.h>

    uint8_t variableUnsigned;
    int8_t anotherVariable;
    int16_t variable16bits;
    int32_t largeVariable;
    int64_t hugeVariable;
    http://www.nongnu.org/avr-libc/user-manual/group__avr__stdint.html
     
    Last edited: Nov 16, 2015
  13. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    9,310
    Likes:
    914
    Location:
    Rochdale UK
    Same result in XC8...
     
  14. MrAl

    MrAl Well-Known Member Most Helpful Member

    Joined:
    Sep 7, 2008
    Messages:
    11,049
    Likes:
    961
    Location:
    NJ
    Hi,

    Yes very good idea Mr T.

    What i remember reading a long time ago was that the operation size is based on the largest data type size in the calculation, so in the case of 4000*4000 each of the two types is 16 bits, so the operation must provide a result that is 16 bits.
    If you would like to try next:
    sizeof(40000*4000);
    this should result in a "4" because 40000 does not fit into type int which is 16 bits (which would have resulted in a "2" of course), and 40000 must be type long (4 bytes) so it should provide a result that is as long as the largest type which of course is type long, 4 bytes.
    If you would like to try this we can verify that is how the compiler works.

    The definition above does not include the return type, which is a little strange, but you could test that too if you like...
    long A;
    A=4000*4000;
    printf("\n%d", sizeof(A)); //see what this prints
    printf("\n%d", A); //see what this prints

    There the return type is long but the two arguments are both just int. I suspect it might truncate to int anyway, but we'd have to see the return value this time i think because the 'sizeof' operator will probably still return a '4'.

    BTW what environment are you doing the test in?
     
  15. KeepItSimpleStupid

    KeepItSimpleStupid Well-Known Member Most Helpful Member

    Joined:
    Oct 30, 2010
    Messages:
    9,971
    Likes:
    1,099
    Would this "offset=offset+13.*3600"; //set 13 hours (in seconds)" or something similar work?

    The little addition of the decimal point. Used to do stuff like a = 1.*5000*5000 long ago in a different era.
     
  16. misterT

    misterT Well-Known Member Most Helpful Member

    Joined:
    Apr 19, 2010
    Messages:
    2,697
    Likes:
    368
    Location:
    Finland
    long A;
    A=4000*4000;
    printf("\n%d", sizeof(4000*4000));
    printf("\n%d", sizeof(A));
    printf("\n%ld", A);

    This outputs:
    2
    4
    9216

    9216 in binary is:
    00000000 00000000 00100100 00000000
    and 16000000 in binary:
    00000000 11110100 00100100 00000000

    Adding the 'L' -modifier fixes the bug A=4000L*4000;
    Changing the other variable to float also works A=4000.0*4000;

    I am using AtmelStudio.
     
    Last edited: Nov 17, 2015
  17. misterT

    misterT Well-Known Member Most Helpful Member

    Joined:
    Apr 19, 2010
    Messages:
    2,697
    Likes:
    368
    Location:
    Finland
    I think that would work, because adding the decimal point makes the constant 32bit floating point instead of 16bit integer. And then the 3600 would have to be promoted to float also. Not a good solution, because the use of floating point is not needed in this case. But it should work.
    I can try this tomorrow at work.
     
    Last edited: Nov 16, 2015
  18. MrAl

    MrAl Well-Known Member Most Helpful Member

    Joined:
    Sep 7, 2008
    Messages:
    11,049
    Likes:
    961
    Location:
    NJ
    Hi,

    I neglected to mention that i tried that mostly because i didnt think anyone would want to do it that way.
    Yes, that does work, but as Mr T points out it will not be as good as simply keeping everything integer because it will have to promote everything to floating point before it can do the math, then afterwards demote the result back to type long (or UL).
    Thanks for mentioning that though.
     

Share This Page