Project RGB with Switches [closed]

Question

I am working on project in which I need to display different colors on RGB led. I am using pwm to drive different colors on LED. My Pic is PIC24FJ64GA004 with which I am working on now. Basic concept of this project is to use switch to controls colors.

Colors on RGB led will be according to days and month in year. For that I am using 7-segment led with switch to count days and month.

Problem is that I have only one switch on my board. I have designed hardware and bits for it has been tested as well. So hardware works fine.

At the moment my problem is pic code.

I am stuck in switch use now. I have one switch on board. I need to display how many times switch pressed on 7 segment. Problem is that I am new to Pic code and confuse as well. First switch status will be checked. If switch pressed for two seconds it will go days mode. otherwise it will stick to months mode.It will display month or days according to that.I have pasted my code here please give me some positive advice.

I would like to know is this coding is going in right way or not.

void switch_function(void)
{
    if (PORTAbits.RA4==1)               // is SW1 pressed?
    {                            
        IEC0bits.T1IE       = 1;        // Enable Output Compare interrupts
           T1CONbits.TON        = 1;        // Start Timer1 with assumed settings   
        if (modecounter==0)                 // Checking status of month  
        {
            if (PORTAbits.RA4==1) 
            {    
                counter++;
                if (counter== 12)
                {   
                    counter = 0;
                } 
            }
        } 
        else if(modecounter ==1)            // Checking status of days
        {
            if (PORTAbits.RA4==1)
            { 
                counter++;
                if (counter== 32)
                {  
                    counter = 0 ;
                }
            }
        }
        else
        {
        }     
    }
    else
    {
        counter =0;
    }
    return ;
}

  //***** Timer1 interrupt *****//

void __attribute__((interrupt, auto_psv)) _T1Interrupt(void)
{ 
    timer_counter++;

    if (timer_counter== 2)
    {
        if (PORTAbits.RA4==1) 
        {  // is SW1 still has pressed status after 2sec delay?
            modecounter = 1;         // switch to days 
        }
        else
        {
            modecounter = 0;  // switch to months
        }          
    }
    else
    {
    }
    IFS0bits.T1IF=0; /* clear timer1 interrupt flag */      
    return;
} 

//***** Timer2 interrupt *****//

void __attribute__ ((__interrupt__, no_auto_psv)) _T2Interrupt(void)
{
    IFS0bits.T2IF = 0; /* clear timer2 interrupt flag */    
    return;
}

Answer 1

A few comments:

1. Why check this twice PORTAbits.RA4==1 in the same if-block?

2. Following can be rewritten:

   counter++;
   if (counter== 12)
   {   
       counter = 0;
   } 
   

   to:

   if(++counter == 12)
   {
       counter = 0;
   }
   

   or:

   counter = (++counter == 12) ? 0 : counter;
   

3. Same here

   if (PORTAbits.RA4==1) 
   {  // is SW1 still has pressed status after 2sec delay?
       modecounter = 1;         // switch to days 
   }
   else
   {
       modecounter = 0;  // switch to months
   }
   

   to:

   modecounter = (PORTAbits.RA4==1) ? 1 : 0;