What am I doing wrong with this code for PIC16F54?

Question

I'm new here and I hope I did all right and posted in the right section x)

I started programming PIC microcontrollers and I got stuck sometimes. I have a problem with this code, that is a binary counter that turns on LEDs when bits are on 1 and opens a switch at every count (RB7). When it starts, it goes until add1() and then it stops, as the while condition is not followed (it goes until click() of add1() statements and then doesn't go ahead repeating the while loop).

This is the code in C

#pragma config OSC = HS
#pragma config WDT = OFF
#define _XTAL_FREQ 4000000               // Fosc  frequency for _delay()  library
#include <xc.h>

bit a = 0;                 //variabili per il conteggio binario (10 bit)
bit b = 0;
bit c = 0;
bit d = 0;
bit e = 0;
bit f = 0;
bit g = 0;
bit h = 0;
bit i = 0;
bit j = 0;



void click() {

    if (a==1) {PORTAbits.RA2 = 1;}
    if (a==0) {PORTAbits.RA2 = 0;}

    if (b==1) {PORTAbits.RA3 = 1;}
    if (b==0) {PORTAbits.RA3 = 0;}

    if (c==1) {PORTBbits.RB1 = 1;}
    if (c==0) {PORTBbits.RB1 = 0;}

    if (d==1) {PORTBbits.RB2 = 1;}
    if (d==0) {PORTBbits.RB2 = 0;}

    if (e==1) {PORTBbits.RB3 = 1;}
    if (e==0) {PORTBbits.RB3 = 0;}

    if (f==1) {PORTAbits.RA1 = 1;}
    if (f==0) {PORTAbits.RA1 = 0;}

    if (g==1) {PORTAbits.RA0 = 1;}
    if (g==0) {PORTAbits.RA0 = 0;}

    if (h==1) {PORTBbits.RB6 = 1;}
    if (h==0) {PORTBbits.RB6 = 0;}

    if (i==1) {PORTBbits.RB5 = 1;}
    if (i==0) {PORTBbits.RB5 = 0;}

    if (j==1) {PORTBbits.RB4 = 1;}
    if (j==0) {PORTBbits.RB4 = 0;}

    PORTBbits.RB7 = 1;

    __delay_ms(500);

    PORTBbits.RB7 = 0;

    __delay_ms(5);

}


void add9() {
    if (a==0) {
        a=1; b=0; c=0; d=0; e=0; f =0; g=0; h=0; i=0; j=0; 
        click();
    }
    if (a==1,b==1,c==1,d==1,e==1,f==1,g==1,h==1,i==1,j==1) {
        //interrompe il contatore se sono tutti 1 e li azzera
        a=0;b=0;c=0;d=0;e=0;f=0;g=0;h=0;i=0;j=0; click();
    }
}


void add8() {
    if (b==0) {
        b=1; c=0; d=0; e=0; f =0; g=0; h=0; i=0; j=0; click();
    }
    else { 
        add9();
    }
}

void add7() {
    if (c==0) {
        c=1; d=0; e=0; f =0; g=0; h=0; i=0; j=0; click();
    }
    else { 
        add8();
    }
}

void add6() {
    if (d==0) {
        d=1; e=0; f =0; g=0; h=0; i=0; j=0; click();
        }
    else { 
        add7();
    }
}

void add5() {
    if (e==0) {
        e=1; f =0; g=0; h=0; i=0; j=0; click();
    }
    else { 
        add6();
    }
}

void add4() {
    if (f==0) {
        f =1; g=0; h=0; i=0; j=0; click();
    }
    else { 
        add5();
    }
}

void add3() {
    if (g==0) {
        g=1; h=0; i=0; j=0; click();
    }
    else { 
        add4();
    }
}

void add2() {
    if (h==0) {
        h=1; i=0; j=0; click();
    }
    else { 
        add3();
    }
}

void add1() { 
    if (i==0) {
        i=1; j=0; click(); 
    }
    else { 
        add2();        
    }
}    

void add() {               
    if (j==0) { 
        j=1; click();
    }
    else { 
        add1();
    }        
}                                                                

void main(void) {

    TRISA=0x00;
    TRISB=0x00;        

    while(1) {            
       add();           
    }       

    return;
}

What am I missing? Sorry for some comments are in italian but I'm from Italy x)

Answer 1

What you are doing wrong with this code for PIC16F54 is not knowing how the architecture of the Microchip baseline controllers is implemented in the silicon.

The coding approach you are using looks like some kind of python or Java script.

This is no way to create code for a severely resource restricted controller like the PIC16F54.

READ THE PIC16F54 DATA SHEET !!! UNDERSTAND THE ARCHITECTURE !!!

At your present level of experience you will not understand this explanation:

The reason your code fails is your nested function calls exceed the depth of the call stack.

Answer 2

The expression

a==1,b==1,c==1,d==1,e==1,f==1,g==1,h==1,i==1,j==1

does all the comparisons, but due to how the comma operator work only returns the result of the last one, which in this case is j == 1. All the other results are thrown away and ignored.

So your condition is practically

if (j == 1)

If you want to chain multiple conditions you need to use the logical operators AND && or OR ||. As in

if (a==1 && b==1 && c==1 && d==1 && e==1 && f==1 && g==1 && h==1 && i==1 && j==1)

Now the condition will only be true if all the parts are true.

This should be in just about all books, tutorials or classes. If you missed it then please go back to them to learn more.

---

On another note, when you do

if (a==1) {PORTAbits.RA2 = 1;}
if (a==0) {PORTAbits.RA2 = 0;}

that's really equal to

PORTAbits.RA2 = a;