My First C program.

[brucehoult]

C seems to be very nitpicky about  spaces. I tried adding spaces in the macro and it refused to compile. Is there a rule about where spaces can and cannot go?

That's absolutely incorrect.  Almost all spaces in a C program can be removed with no ill effect, and more whitespace than the minimum can never hurt at all.

The place where you *need* a space is between two tokens that would look like one token without a space e.g. "int i".

That's C. That's not the pre-processor, which has its own rules. And you can screw things up by not being careful with pre-processor macros.

Note that a macro definition such as ...

Code: [Select]

#define MASKIN(reg,mask,x) (reg = (mask & x)|(~mask & reg))

... is very dangerous. It can do quite unexpected things if reg, mask, or x are not simple variables names or constants.

For example, what if you call it as:

Code: [Select]

#define PIN(n) (1<<n)

MASKIN(sGPIO, PIN(1) | PIN(4), PIN(4));

... to set pin 4 and clear pin 1.

It's not going to do what you want AT ALL because after the macros are expanded you're going to have...

Code: [Select]

(sGPIO = ((1<<1)|(1<<4) & (1<<4))|(~(1<<1)|(1<<4) & sGPIO))

... which is going to be parsed as ...

Code: [Select]

(sGPIO = ((1<<1) |  ((1<<4) & (1<<4)))   |   (~(1<<1)  |  ((1<<4) & sGPIO)))

(sGPIO = (2 |  ((16 & 16)))   |   (~2  |  (16 & sGPIO)))

(sGPIO = (2 | 16)   |   (~2  |  (16 & sGPIO)))

sGPIO = 2 | 16  |  ~2  |  (16 & sGPIO)

sGPIO = (2 | ~2)  |  (16  |  (16 & sGPIO))

sGPIO = -1  |  16

sGPIO = -1

ALWAYS PUT PARENS AROUND MACRO ARGUMENTS IN THE EXPANSION

Code: [Select]

#define MASKIN(reg,mask,x) ((reg) = ((mask) & (x))|(~(mask) & (reg)))

Now it will do AND #-3; OR #16 as intended.  (or the AND might be with #-19, depending on the optimiser .. same thing)

 

[Zero999]

I've sorted it thank you to everyone who posted here.

If the watchdog timer isn't reset. GP2 and GP5 pulse at 20% duty cycle at 1Hz and the other ports change to high impedance inputs, with the puil-ups disabled.

Code: [Select]

/*
 * File:   main.c
 *
 * Created on 14 April 2022, 18:47
 */

//  Turns on an LED for 10 seconds after being triggered by a pushbutton.
//  GP0 = Pushbutton switch 1
//  GP1 = LED1
//  GP2 = Error
//  GP3 = Pushbutton switch 2
//  GP4 = LED2
//  GP5 = Error
//
// PIC12F509 Configuration Bit Settings

#pragma config OSC = IntRC      // Oscillator Selection bits (internal RC oscillator)
#pragma config WDT = ON         // Watchdog Timer Enable bit (WDT disabled)
#pragma config CP = OFF         // Code Protection bit (Code protection off)
#pragma config MCLRE = OFF      // GP3/MCLR Pin Function Select bit (GP3/MCLR pin function is digital input, MCLR internally tied to VDD)

// #pragma config statements should precede project file includes.
// Use project enums instead of #define for ON and OFF.
#define _XTAL_FREQ 4000000      // oscillator frequency for _delay()

#define sGPIObits (*(GPIObits_t * volatile)&sGPIO)              // shadow port structure
#define MASKIN(reg,mask,x) ((reg = (mask & x)|(~mask & reg)))   // mask macro to select which bits to keep. Not thread safe but smaller.
//#define MASKIN(reg,mask,x) (reg^=(reg^(x))&(mask))            // mask macro. Slightly larger binary code, but thread safe.

// pin definitions
#define trigger1    GPIObits.GP0
#define trigger1_s  sGPIObits.GP0   // shadow trigger 1
#define LED1        sGPIObits.GP1
#define trigger2    GPIObits.GP3
#define trigger2_s  sGPIObits.GP3   // shadow trigger 2
#define LED2        sGPIObits.GP4
#define TRIS_set    0b101101        // configure GP1 & GP4 as outputs
#define TRIS_error  0b011011        // if WDT GP2 & GP5 are error outputs, which pulse 200ms at 1Hz
#define timeout1 (int)(10/0.008)    // 10/8ms
#define timeout2 (int)(10/0.008)

#include <xc.h> 
#include <stdint.h>

volatile unsigned char sGPIO = 0;   // shadow register
unsigned short counter1 = 0;        // timer counter 1
unsigned short counter2 = 0;        // timer counter 2
     
void main(void) {  
    if (!__timeout)             // If the watchdog timer healthy flag is not set
    {
        OPTION = 0b11010010;    // configure Timer0:
                                //-1------ disable weak pull-ups on GP0, 1 & 3
                                //--0----- timer mode (T0CS = 0)
                                //----0--- pre scaler assigned to Timer0 (PSA = 0)
                                //-----010 pre scaler = 8 (pre scaler = 10)
                                // -> increment every 8us  
        TRIS = TRIS_error;

        for (;;)                // pulse error outputs
        {
            if (TMR0 >= 125)    // decrement counter every 125 * 8us = 1ms
            {
                CLRWDT();                   // clear watchdog timer
                TMR0 = 0;                   // reset timer
                if (counter1) (counter1)--;
                else counter1 = 1000;                   // 1000ms period
                if (counter1 < 200) GPIO = 0b111111;    // 200ms on
                else GPIO = 0;
            }
        }        
    }
    
    OPTION = 0b10010100;        // configure Timer0:
                                //-0------ enable weak pull-ups on GP0, 1 & 3
                                //--0----- timer mode (T0CS = 0)
                                //----0--- pre scaler assigned to Timer0 (PSA = 0)
                                //-----100 pre scaler = 32 (pre scaler = 100)
                                // -> increment every 32us  
    TRIS = TRIS_set;            // configure TRIS
    GPIO = 0;                   // set all outputs low
    TMR0 = 0;                   // reset timer
              
    for (;;)
    {
        if (TMR0 >= 250)        // timer exceeds 250 * 32us = 8ms
        {
            CLRWDT();           // clear watchdog timer
            TMR0 = 0;           // reset timer

            if (counter1)       // decrement counter if not zero
                counter1--;
            else
            {
                LED1 = 0;
                if (trigger1_s && !trigger1) // trigger changed from high to low & counter is zero
                {
                    counter1 = timeout1;
                    LED1 = 1;
                }
            }                       
            if (counter2)       // decrement counter if not zero
                counter2--;
            else
            {
                LED2 = 0;
                if (trigger2_s && !trigger2) // trigger changed from high to low & counter is zero
                {
                    counter2 = timeout2;
                    LED2 = 1;
                }
            }
            GPIO = sGPIO;
            MASKIN (sGPIO,TRIS_set,GPIO);
        }
    }
return;
}