This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Warnings I cannot understand after compilation-Part1

This is going to be a strange request. I must warn that I am new to embedded programming and therefore what is obvious to you may not be to me.
I am trying to implement a real time clock program with the data then being transmitted on CAN. I am getting some warning when I compile my code and cannot understand what I need to do. Any help would be appreciated. I will be posting my code in parts as I cannot exceed a certain character limit.

Parents
  • You are trying to say that a function currentRT() exists in another module. But why, unless you where planning on also call the function?

    Are you really sure that you have the correct time when you get into troubles?

    9:06:08 = 9*3600 + 6*60 + 8 = 32768 = 0x8000.
    9:06:07 = 9*3600 + 6*60 + 7 = 32767 = 0x7fff.

    How big numbers to you think you can store in a signed 16-bit integer? I would kind of guess at -32768 .. +32767.

    If you change to an unsigned integer, you will get 0 .. 65535 which corresponds to 00:00:00 .. 18:12:15.

    So it seems like you have to switch to a long 32-bit value to keep track of your time. There are 86400 seconds in 24 hours.

Reply
  • You are trying to say that a function currentRT() exists in another module. But why, unless you where planning on also call the function?

    Are you really sure that you have the correct time when you get into troubles?

    9:06:08 = 9*3600 + 6*60 + 8 = 32768 = 0x8000.
    9:06:07 = 9*3600 + 6*60 + 7 = 32767 = 0x7fff.

    How big numbers to you think you can store in a signed 16-bit integer? I would kind of guess at -32768 .. +32767.

    If you change to an unsigned integer, you will get 0 .. 65535 which corresponds to 00:00:00 .. 18:12:15.

    So it seems like you have to switch to a long 32-bit value to keep track of your time. There are 86400 seconds in 24 hours.

Children
  • Just a footnote: Old MS-DOS (FAT directory entries) only stored even seconds for the file time stamp, just to get the clock value to fit in a 16-bit unsigned integer.

  • Just a footnote: Old MS-DOS (FAT directory entries) only stored even seconds for the file time stamp, just to get the clock value to fit in a 16-bit unsigned integer.

    It makes you think: MS do get some things right :-)

  • Thanks. Will try it out. I am sure that was my mistake.

  • Ok.So here we go again. The RTC now resets when the the clock is 18:12:15 (=65,535) which is telling me that the problem could be that the size I am assigning to store the register values is smaller than I need. I checked and I am storing the register values in 'unsigned long' which should hold more than 65,535. Here's the code:

    #include "REG164.h"
    #include "RTC_C16x.h"
    #include "MATH.H"
    struct temptime
    {
            int daysa;
            int daysb;
            int hour;
            int minute;
            int seconds;
        int day;
            int month;
            int year1;
            int year2;
    }time;
    unsigned long RTCvalue;
    int current_day = 6;
    int current_month = 10 ;
    int current_year = 2008;
    union _32bit
    {
            unsigned long rtcreg[1];
            unsigned long a;
    } RTC;
    
    
    void RTC_CAN(void)
    {
    //check for month and then either reset or increment day
            switch (current_month){
                                            case 1:
                                            case 3:
                                            case 5:
                                            case 7:
                                            case 9:
                                            case 11:
                                                            if(current_day == 32)
                                                                    {
                                                                            current_day=1;
                                                                            current_month = current_month+1;
    
                                                                    }
    
                                            case 4:
                                            case 6:
                                            case 8:
                                            case 10:
                                            case 12:
                                                            if(current_day == 31)
                                                                    {
                                                                            current_day = 1;
                                                                            current_month = current_month+1;
                                                                            if(current_month==13)
                                                                                    {
                                                                                            current_month=1;
                                                                                            current_year=current_year+1;
                                                                                     }
                                                                    }
                                            case 2:
                                                            if(current_day == 29)
                                                                    {
                                                                            current_day = 1;
                                                                            current_month = current_month+1;
                                                                    }
    
                                      }
    
    
    //mapping what needs to be transmitted on CAN(BCD value) and it's decimal equivalent.
    
            if(time.hour<=9)
                    {
                            time.hour = time.hour;
                    }
            else if(time.hour>=10 && time.hour<=19)
                    {
                            time.hour = time.hour+6; //hex 10=dec16, hex 11=dec17...hex19=dec25
                    }
            else if(time.hour>=20 && time.hour<=24)
                    {
                            time.hour = time.hour+12; //hex 20=dec32....hex 24=dec36
                    }
    
    
            if(time.minute<=9)
                    {
                            time.minute = time.minute;
                    }
            else if(time.minute>=10 && time.minute<=19)
                    {
                            time.minute = time.minute+6;
                    }
            else if(time.minute>=20 && time.minute<=29)
                    {
                            time.minute = time.minute+12;
                    }
    
            else if(time.minute>=30 && time.minute<=39)
                    {
                            time.minute = time.minute+18;
                    }
            else if(time.minute>=40 && time.minute<=49)
                    {
                            time.minute = time.minute+24;
                    }
            else if(time.minute>=50 && time.minute<=59)
                    {
                            time.minute = time.minute+30;
                    }
    
            if(current_day<=9)
                    {
                            time.day=current_day;
                    }
            else if(current_day >=10 && current_day<=19)
                    {
                            time.day=current_day+6;
                    }
            else if(current_day >=20 && current_day <=29)
                    {
                            time.day=current_day+12;
                    }
            else if(current_day >=30 && current_day <=32)
                    {
                            time.day = current_day+18;
                    }
    
    
            if(current_month<=9)
                    {
                            time.month = current_month;
                    }
            else
                    {
                            time.month = current_month+6;
                    }
    
            if(current_year==2008)
                    {
                            time.year1=32;
                            time.year2=8;
                    }
            else if(current_year==2009)
                    {
                            time.year1=32;
                            time.year2=9;
                    }
            else if(current_year==2010)
                    {
                            time.year1=32;
                            time.year2=16;
                    }
    
    }
    
    
    void RTCinit(void)
    {
    
            RTCL = 0; //clearing lower RTC register
            RTCH = 0; //clearing higher RTC register
            RTCvalue = (18*3600) + (10*60)+30;
            RTC.a = RTCvalue;
            RTCL = RTC.rtcreg[0];
            RTCH = RTC.rtcreg[1];
            T14REL = 0xB3B5;
            timer1s();
    }
    void currentRT(void)
    {
            unsigned int x,y,z;
    
            RTC.rtcreg[0] = RTCL;
            RTC.rtcreg[1] = RTCH;
            RTCvalue = RTC.a;
            x = RTCvalue/86400;
            time.daysa = floor(x);
            RTCvalue = RTCvalue-(time.daysa*86400)
            time.daysb = 0;
            if(time.daysa>time.daysb)
                    {
                            current_day = current_day+1;
                    }
            time.daysb = time.daysa;
            y = RTCvalue/3600;
            time.hour = floor(y);
            RTCvalue = RTCvalue-(time.hour*3600);//extracting the hour information
            z = RTCvalue/60;
            time.minute = floor(z);
            RTC_CAN();
    }
    //end of RTC_CAN
    
    
    

  • 1) The constant 86400 is too large for an integer. You should write 86400ul to specify that the constant is an unsigned long.

    2) You are doing forbidden things with your RTC union. Exactly how do you think that two 32-bit integers (one part of the union - or more specifically: what you think is two long numbers) should fit in the other part of the union?

    To declare an array of two elements (with index 0 and 1), you have to write rtcreg[2].

    But if the RTC registers are 16-bit values, then it is meaningless to try and play with two 32-bit destination entries. Especially since it takes two 16-bit integers to concatenate to a single 32-bit integer - your expected end result.

    3) Why do you use a floating--point function - floor() - with integer variables. What fractions do you think the integer x may contain?

    x = RTCvalue/86400;
    time.daysa = floor(x);
    

  • Let me answer your questions:

    1. Are you saying that the code should look like:

    x = RTCvalue/86400ul;
    


    Interesting
    2. I was just changing the unsigned long reg[1] to just unsigned int as you perhaps wrote. For some reason the timer registers being 32 bits long was stuck in my head. I just re-read the documentation.
    3. If the number of hours becomes>24 the number of seconds stored in the time register will become >86400. Therefore if I divide that value by 86400 and then take the floor() of that value I will end up with 1 indicating 24 hrs has elapsed and therefore I need to increment my day. I will end up with 1,2,3... by taking the floor() which is why I compare the current value to previous. If current>previous I just increment the day value.

  • 1) Yes, you should be explicit with large constants by adding the type suffix.

    2) Yes, you should switch to unsigned or unsigned int (same thing) for the array - but you should also modify the array to have two entries, i.e. reg[2] instead of reg[1].

    3) Let me repeat myself: What fractions do you think the integer x may contain?

    floor() is a function used to remove the fractional part from a floating point number.

    3.0 / 2.0 => 1.5. floor(1.5) => 1.

    But what fractional parts do you think an integer can have?

    3 / 2 => 1

    Since x is unsigned int, there can not be any fractions and hence nothing for floor() to strip away...

  • By the way - incrementing the day is only correct if currentRT() is called at least once / 24 hours. If called more seldom, then you may have stepped more than one day.

    You might do:

    while (time.daysa > time.daysb) {
        time.daysb++;
        current_day++;
    }
    

    or

    if (time.daysa > time.daysb) {
        current_day += (time.daysa - time.daysb);
        time.daysb = time.daysa;
    }
    

  • ooh, thanks for pointing that out. I thought I had changed xy,z to double but obviously I missed that. I think I need coffee. Thanks.

  • Why do you want to use floating point? Don't you think normal integers can manage to figure out if you have stepped past 24 hours - and how many days past? Floating point doesn't make your life easier. It makes it harder, since floating point implies rounding errors.

    If you do call the function at least every 24 hours, then you can skip the division, since a comparison and optionally a subtract will do.

  • Agreed. Well for right now I just want to make sure that I am able to successfully keep track of time and account for a day rollover. After making the necessary changes I currently find that from 18:59:59 there is a sudden jump to 19:44. Here is what I got on the CAN network:
    2944.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.20%
    2944.798350 1 CAN_ID Rx d 6 18 59 06 10 20 08
    2945.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.20%
    2945.798605 1 CAN_ID Rx d 6 18 59 06 10 20 08
    2946.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.19%
    2946.798860 1 CAN_ID Rx d 6 18 59 06 10 20 08
    2947.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.20%
    2947.799115 1 CAN_ID Rx d 6 18 59 06 10 20 08
    2948.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.20%
    2948.799430 1 CAN_ID Rx d 6 19 44 06 10 20 08
    2949.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.20%
    2949.799705 1 CAN_ID Rx d 6 19 44 06 10 20 08
    2950.019787 1 Statistic: D 1 R 0 XD 0 XR 0 E 0 O 0 B 0.20%
    2950.800020 1 CAN_ID Rx d 6 19 44 06 10 20 08

  • So, if you ask a question here directly whenever you get stuck - exactly how will you learn how to find errors in your code? Exactly what do _you_ do right now, to figure out where you go wrong? Are you single-stepping in the debugger? Are you looking at the intermediaries?

    "[...] account for a day rollover."

    But you are not.

    What use do you have of the time.daysb variable, when you always start with the value zero?

    time.daysb = 0;
    if (time.daysa > time.daysb) {
        current_day = current_day + 1;
    }
    time.daysb = time.daysa;
    

    After your counter reaches past 86400 - what do you think will happen on every call to the function?

  • What Per is saying is true. Seems like you are new to programming. It is ok to be a little ambitious in the beginning but be prepared to fight it out. Keep checking your code. Usually you will notice something wrong every 2-3 times. Also see if you can do some debugging when the code is running.

  • Try initializing time.daysb to 0 outside the function. Otherwise it will always be 0 and on a rollover time.days will always be greater. Also try ending the if-else ladder with just an else. Better way to end execution of the ladder.