A macro gives a wrong value in C language

Question

I am trying to write a subroutine that generates a delay of 1000 ms using the Timer1 of the PIC18F4321 in 16-bit mode in MPLAB X IDE with XC8 compiler. This delay is exploited to toggle a LED. My problem is that I can't get the delay wanted (1000 ms). I tried to debug the program I observed that the value of "count" calculated using the macro is not correct. it gives a value of 0x4000 rather than 0x0F42. I don't know what is wrong with the macro:

#define count (((timeDelay) * 1000) / (timerPeriod) * 256)

// C-program using polled I/O:
#include <P18F4321.h>
#define timeDelay (1000) // 1000 ms
#define Fosc (4) // 4 MHz
#define timerPeriod (1 / ((Fosc) / 4)) // us
#define count (((timeDelay) * 1000) / (timerPeriod) * 256)
#define countInit ((0xFFFF - (count)) + 1)
#define countInitHigh ((countInit & 0xFF00) >> 8)
#define countInitLow (countInit & 0x00FF)

void T0Delay(); // A subroutine that generates a delay of 1000 ms

void main()
{
    OSCCON = 0x60; // 4MHz Internal Oscillator
    TRISC = 0x00; // Port C output
    T0CON = 0x07; // 16-bit, 1:256 prescaler, internal clock
    for(;;) // loop forever
    {
        PORTCbits.RC0 = 0; // turn LED OFF
        T0Delay(); // Wait 10 seconds
        PORTCbits.RC0 = 1; // turn LED ON
        T0Delay(); // Wait 10 seconds
    }
}

void T0Delay(void)
{
    TMR0H = countInitHigh;
    TMR0L = countInitLow;
    INTCONbits.TMR0IF = 0; // clear timer overflow flag
    T0CONbits.TMR0ON = 1; // start Timer0
    while(!INTCONbits.TMR0IF); //polling, wait until timer finishes counting
    T0CONbits.TMR0ON = 0; // Stop Timer0
}

Is `timerPeriod` really supposed to expand to `(1 / (((4)) / 4))`, i.e. 1? — zwol, Apr 29 '18 at 19:28
If you are on a 16-bit int system, `timeDelay * 1000` causes undefined behaviour — M.M, Apr 29 '18 at 22:24
@M.M Yes i'm on a 16-bit system. But how can i resolve this problem??? — Samir Fassak, Apr 30 '18 at 01:17
One way would be to use 32-bit arithmetic (change `1000` to `UINT32_C(1000)` or `1000UL`) — M.M, Apr 30 '18 at 01:27
@M.M I tried your proposition `1000UL` and it worked finally thank you very much. So i can understand that when the numerator overflows more than 16-bit we have to use UL?? — Samir Fassak, Apr 30 '18 at 14:18
Yeah a 16-bit signed multiplication can't exceed 32767 or else it is undefined behaviour — M.M, Apr 30 '18 at 20:50

score 3 · Answer 1 · answered Apr 30 '18 at 14:08

Finally i could resolve the problem thank you very much guys. The issue was that in the macro definition #define count (((timeDelay) * 1000) / ((timerPeriod) * 256)) the result on the numerator overflows more than 16-bit. So the solution is to use 32-bit arithmetic by changing (1000) to (1000UL). The final code that runs correct it as follows:

// C-program using polled I/O:
#include <P18F4321.h>
#define timeDelay (1000UL) // 1000 ms
#define Fosc (4) // 4 MHz
#define timerPeriod (1 / ((Fosc) / 4)) // us  
#define count (((timeDelay) * 1000UL) / ((timerPeriod) * 256))
#define countInit ((0xFFFF - (count)) + 1)
#define countInitHigh ((countInit & 0xFF00) >> 8)
#define countInitLow (countInit & 0x00FF)

void T0Delay(); // A subroutine that generates a delay of 1000 ms

void main()
{
    OSCCON = 0x60; // 4MHz Internal Oscillator
    TRISC = 0x00; // Port C output
    T0CON = 0x07; // 16-bit, 1:256 prescaler, internal clock
    for(;;) // loop forever
    {
        PORTCbits.RC0 = 0; // turn LED OFF
        T0Delay(); // Wait 10 seconds
        PORTCbits.RC0 = 1; // turn LED ON
        T0Delay(); // Wait 10 seconds
    }
}

void T0Delay(void)
{
    TMR0H = countInitHigh;
    TMR0L = countInitLow;
    INTCONbits.TMR0IF = 0; // clear timer overflow flag
    T0CONbits.TMR0ON = 1; // start Timer0
    while(!INTCONbits.TMR0IF); //polling, wait until timer finishes counting
    T0CONbits.TMR0ON = 0; // Stop Timer0
}

score 2 · Answer 2 · answered Apr 29 '18 at 19:30

2

If you add parentheses around (timerPeriod) * 256, you'll get the result you want:

#define count (((timeDelay) * 1000) / ((timerPeriod) * 256) /*<=parentheses here*/ )

Mutliplication and division have the same precedence in C (just like in math) so if you want to divide by (timerPeriod) * 256, you'll need to parenthesize it.

answered Apr 29 '18 at 19:30

Petr Skocik

58,047
6
95
142

I just tried your proposition but still gives a wrong value! – Samir Fassak Apr 29 '18 at 21:26
@SamirFassak The value is 0xf42 after the redefinition. Here's proof: https://ideone.com/zMX9FD – Petr Skocik Apr 29 '18 at 21:30
1

Yes i tried it on the online compiler it gives a correct value (0x0f42) https://ideone.com/OtuCrf. It gives also a correct value on Visual C++ 2008. But when i try the code on the XC8 compiler in MPLAB X IDE i get a wrong value (0x0042)??? – Samir Fassak Apr 29 '18 at 21:55
1

@SamirFassak does `(timeDelay) * 1000` overflow in "16-bit mode"? – Weather Vane Apr 29 '18 at 22:00
@WeatherVane Yes (timeDelay) * 1000 = 1000 * 1000 = 1000000 so it overflows since in 16-bit mode the max is 65335 – Samir Fassak Apr 29 '18 at 23:49
1

@SamirFassak Sorry, didn't realize you could be on a 16bit machine. Well, in 16 bits you can't do the calculation accurately, but perhaps you could simply define the macro as 0xf42 or generate the code with something other than the c preprocessor. – Petr Skocik Apr 30 '18 at 07:41
@PSkocik No problem welcome, yes i'am programming to the PIC18F4321 microcontroller. – Samir Fassak Apr 30 '18 at 14:21

A macro gives a wrong value in C language

2 Answers2