program handling of button / switch inputs

[DVX]

Hi, I have a rotary encoder with a push switch which is used via on screen menus to read user input, the code is is in C and runs on a PIC32 with TFT screen. As a non experience coder I have problems with reading the push to make a switch in terms of how to action the user input. For example, once a menu selection is shown on the display via the rotary encoder the user then presses the switch to choose that selection. I read the switch being pressed down, but have decided the jump to a routine to handle the menu change only when the switch is released, otherwise the routine is constantly called while the switch is pressed down. To keep the code simple I have a while loop which waits for the switch to release, not elegant code as besides being blocking it also has to update the display and a few other items while in the loop. Btw the switch is denounced OK and is detected via an interrupt on change, this updates the state of the switch but could also detect a rising or falling edge on the pin reading the switch. Are there any examples or best practices for handling switch states and is it best to set flags when the switch states change and then poll these in the main program loop? 

[rstofer]

If interrupt on change is reliable, you just use the interrupt to set a flag (or two).  You can poll and reset the flag in the main code.

This has the advantage of being non-blocking and you aren't hung up in a while(1) {} loop.

Most arithmetic operations including something as simple as "Flag = 0" are not atomic and the value can change in the interrupt while the main code is still working with the variable.  So, in your main code, disable the interrupts momentarily when you reset the flag.  Depending on the code actually emitted by the compiler, you might get away without doing this for a simple flag.  You will not get away with it if you use any kind of counter.  At some point, the interrupt handler will change the value while you are changing a previous value.

Take "Flag = Flag - 1".  You can see that the first thing is to load the  existing value (one instruction, maybe two), decrement the value (another instruction) and save the value (one instruction, maybe two).  During the sequence, an interrupt can come along and increment the value after the value is fetched and before it is replaced and if this happens, you will miss a count.

[DVX]

I did try implementing polling in the main loop with flags being set in the interrupt, but the response time to the switch changing state and then calling a routine became much slower. Although I did not turn off the interrupt as you suggest when polling.

[rstofer]

Then your superloop is taking too much time.

First, make sure you aren't blocking anywhere.  No while(1) loops.  If a thing isn't ready, skip it and move on!

Next, break your superloop up into pieces.  Execute a different piece each time through the loop.  You can still process the flags before you branch to the proper portion of your superloop.
A 'case' statement works well here  Just make sure to update the case each pass through the loop.
Maybe you process keys on one pass, do the calculations on the next pass and do the display update on the 3rd pass.

[MarkF]

Here's the basic program I did for my Function Generator using a PIC18F2550, AD9834 and PEC11 encoder.
(I deleted 90% of the code to show just the basic processing flow.  Hope it still makes sense.)

Code: [Select]

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

int8_t en0,ev;
uint8_t sw0;
uint8_t lastSW0=1;
uint8_t currA0,lastA0;

uint8_t mCD=0;       // mode: cmd=0, data=1
int8_t menuItem=0;   // item selected


//============================================================
void main(void)
{
   // Setup rotary encoder
   en0=0;
   sw0=0;
   lastA0=PORTCbits.RC1;

   // Setup OLED display
   oledSetup();
   drawDdsDisplay(1);
   
   // Setup DDS
   ddsSetup();
   
   // Setup CCP1 configuration
   CCPR1H=0x10;                  // 500 Hz interrupt (32.768 MHz system clock))
   CCPR1L=0x00;
   CCP1CON=0x0b;                 // Compare mode, trigger special event
   // Setup Timer3 configuration
   TMR3H=0;
   TMR3L=0;
   T3CON=0x65;                   // T3CCP | 1:4 Prescale | TMR3ON bits
   // Enable CCP1 interrupt
   PIR1bits.CCP1IF=0;            // CCP2 Interrupt Flag bit
   PIE1bits.CCP1IE=1;            // CCP2 Interrupt Enable bit
   INTCON=0xc0;                  // GIE and PEIE interrupts


/*** Main Loop ***/
   while (1) {

   // Encoder switch processing
      if (sw0) {
         sw0=0;  // Reset value from interrupt routine
         if (++mCD > 1) mCD=0;
      }

   // Encoder value processing
      ev=en0;
      en0=0;  // Reset value from interrupt routine

      // Command mode (select menu item)
      if (!mCD) {
         if (ev != 0) {  // encoder has been rotated
            menuItem+=ev;
            if (menuItem>18) menuItem=0;
            if (menuItem<0)  menuItem=18;
         }
      }

      // Data mode (change menu item value)
      else {   
         if (ev > 0) {          // Forward (increment value)
            switch (menuItem) {
               // TODO:
            }
         }
         else if (ev < 0) {     // Reverse (decrement value)
            switch (menuItem) {
               // TODO:
            }
         }
      }

/*** Background Processing ***/

      // Update DDS
      if (mCD && ev!=0) {  // Data mode and encoder has been rotated
         // TODO:
      }
      // Update display
      drawDdsDisplay(0);

/*** End of Main Loop ***/
   }
}

//============================================================
void __interrupt() isr(void)
{
   uint8_t pin;

   if (PIE1bits.CCP1IE && PIR1bits.CCP1IF) {

      PIR1bits.CCP1IF = 0;    // clear CCP1 Interrupt Flag

      // Read rotary encoder
      currA0=PORTCbits.RC1;
      if (lastA0==0 && currA0==1) {    // rising edge?
         if (PORTCbits.RC2!=currA0) en0++; else en0--;   // clockwise?
      }
      lastA0=currA0;

      // Switch 0
      pin = PORTCbits.RC0;
      if (lastSW0==0 && pin==1) sw0=1;    // If button release, set sw0
      lastSW0 = pin;
   }
}
//============================================================