Button Interrupt Failing [PIC][XC8]

[Rerouter]

this is the module i made up for my own AVR based project, at the time i needed the external interrupts to never be delayed, so made a latch out of 2 resistors and 2 transistors, and the thing works reliably,

pretty much close your contact to ground, it turns on the pnp which holds on the npn latching the input until the internal 20K liftup on the pin is cycled resetting the latch, this way i could run a flag check even in rather large code loops, and even implement press and hold functionality by counting the time between the first attempt and last attempt to reset it,

using sot23's you can get the design down to 10mm^2 per latch,

[Rufus]

I am not quite sure how you can use a output pin which changes the digits in the multiplexed display to read the button state unless you are rapidly changing it between input and output as you update each digit?

You have the digit common outputs which you are already driving one at a time. You connect switches between these outputs and one input. When you see the input driven it must be the switch connecting to the output you are currently driving that is closed.

Closing multiple switches will short the outputs together. If that is an issue you can use series diodes to block the interaction.

If your outputs don't need to be there all the time (LEDs for example don't need to be driven 100% of the time) you can flip them to inputs and read stuff.

Attached is an old (nearly 20 years) design of mine for a hand held calculator type device. A 4x4 matrix keypad connects to JP1.

[blewisjr]

Ah that is really cool thanks Rufus.  This would easily allow me to use the 20 pin controller you learn something new all the time.  Never thought of being able to multitask a pin that way.

[C]

blewisjr:

The Clock
I think you said that you were doing an AVR based clock an switched to PIC so you had more pins.
So have you asked your self if C works on an AVR and C works on a PIC why are you rewriting the C code?
Even if you have stopped all work on the AVR based clock, Thinking of this could help you write better C code for the current clock.
Your basic clock code needs to run once a second, so in C it could be the same. You just need to get the inputs from the AVR & PIC to look the same to the basic code. Then do same with the outputs.
Now if you think about it, the more code you can put in the basic clock section, the less programing you have to do to have two clocks one on PIC and one on AVR.

So on the input side you have the differences in reading the switches and how to get to the once a second running of basic clock. On the output side the display and other outputs.

So as you write the basic clock, keep asking your self, is there a better way to do this part so it works on more clock hardware, Is there a better way to do this part so you do not need to make changes to the special C code for each clock.

Display:
I think you are using a multiplexed display on both clocks. What changes could you make to put more code in the basic clock section so that it is not needed to be done in the two different display sections. What changes could you make so that in the display part you  have more C code in common between the clocks? 

Looking at your code I see that you have made a choice that instead of having separate digit to 7 segment decoder chip(a) to have the mpu do it.
Lets look at the costs.
4 pins for a digit output vs 7 or 8 pins for a segment output.
digit to segment decoder chip vs Software
Note: by using a segment output can now display any segment output you want so you have more options.
I see a lot on the good side for a cost of 3-4 pins.
So Looking at your display code why are you using digit data an not segment data?
Digit data does not change fast so it would be less work for the mpu to display segment data then create segment data from digit data on each scan. Think you said alarm, so if you want to switch to display alarm time or something else you have to change display code section again for each new display data. If display section just displayed an array of segment data, you could change segment data almost any where and display section would display it with no changes.  If you wanted to change from a 4 digit clock to a 6 digit clock, you would just need to change the array size and how much of the array is displayed counter. So in basic clock section create segment data for the display section to use for display and The display section of code gets simpler. 

If you look at croberts hardware, there are 5 digits not 4. The colon for the display is treated as if it were a digit. You could also add a fake digit where you just treated 7-8 leds like they were a digit, This would just cost one pin instead of 7-8 pins. With display code displaying segment data, you would only need to change how many digits the display code was to display. You could build the segment data for the colon or the fake digit leds anywhere. 

mux Switch Input:
If you look at my other post, I was not trying to get fancy, just using a normal input pin like you are using for one switch. In simple terms, some would call this a wired or gate. When a switch is pressed it connects that digit output pin to your input pin. When not pressed the pull-up or pull-down resistor sets the level the input pin sees. So the input pin is going up and down and to find out which switch caused the change you need to know which digit output pin is currently active. The function of your cap will need to be done in software.

You currently have an input with a resistor pull up. First step is to look at your digit outputs. One of your digit outputs will be at a different state. If that state is high then the input pin of your mpu needs a pull down resistor connected. If that state us low then the pull up resistor you have will work. You will need to remove the cap as it's function will need to be done in software. You just connect a switch & diode in series between your current input pin and one of the digit output pins. Each switch to a different digit output pin. To keep things simple just put a diode in series with each switch, With out diode you could have problems when two switches were pressed at the same time or push-pull digit outputs.

Now it takes time for the signal to go out a digit output pin and through the wires, switch, ____ so the best place to read the input pin is just before you change to the next digit on output. Unless you are short of ram, it easer in software to just pretend that you have a switch connected to each digit output pin even if you do not.   

for software debounce look for idea in previous post

NOTE: A lot of mpu's can do a test for number = 0 with a lot less code then a test for number = ____  the same is true if the mpu is testing for less that or greater than.

Long post and A lot of things so in summary:

Display code just displays an array of segment data.
Just before switching to a new digit it reads input pin for switches and stores the result in an array of switch data.

read the input pin something like this.
If input pin is 0 then
  if  abs (switch_array_data.)< bounce_count then
    switch array data. = switch array data -1
If input pin is 1 then
  if  abs (switch_array_data.)< bounce_count then
    switch array data. = switch array data +1
Note: the above gives you a byte size integer use in your main code. It will be a negative number for one switch state and positive number for other switch state.

The above gives you two arrays for main clock code to use.
Segment array containing what to display.
Input switch array for current status of switches.

If you think I goofed or have questions, Ask

C   


 
 

[blewisjr]

I debounced the switches with smoothing caps on the current version of the clock.  The main reason I was thinking of finding a way to convert the project back over to the AVR's I have is because I have come across a few issues with the XC8 compiler.  I have to run it in free mode because I am poor and can't buy it.  This is causing some occasional hiccups in the alarm clock.  I think I tracked down the issue to some extraneous branching code that the compiler is generating which is causing about a 100ms fluctuation.  So for instance when the clock changes and the alarm triggers there is about 100ms delay before you hear the buzzer start to beep.  This delay/fluctuation is not always present it actually varies quite a bit which is why I think it is coming from the awful code that was generated.  It is so awful I almost cried looking at the disassembly.

Nothing serious but can easily be solved with a C compiler that will actually optimize the assembler output.  Heck it can be easily solved by just coding the clock in assembler but I have been told numerous times by numerous people I should be using C.  So it kind of puts me in a rock and a hard place with the PIC controllers no matter how much I love the chips themselves I will never have a reasonable C compiler to use.

It really is no big deal the clock works great I would just like it to be even better because now that it is done I can iterate on it and learn more from it before I move onto my next project.

[Rufus]

This delay/fluctuation is not always present it actually varies quite a bit which is why I think it is coming from the awful code that was generated.

I doubt the compiler is an issue. You never mentioned if you found and understood the bug I said you had a couple of pages ago. Perhaps you have written a worse version of the same bug.

[blewisjr]

Was that the bug about the one pin being input but not implemented?  If so that is long fixed because that button is there and implemented now.  The large 20 - 100 ms execution gap has to do with the compiler.  If I compile with the Pro free trial it goes away.  There may be a way to eliminate it with heavy code refactoring possibly but I am not so sure.  Just for the sake of it here is the current final code maybe you can spot where it is coming from.

I apologize in advance if the code is hard to follow I tried to keep it organized but it kind of came to fruition iteratively.

Code: [Select]

#include <xc.h>

// PIC16F1782 Configuration Fuse Settings

// CONFIG1
#pragma config FOSC = INTOSC    // Oscillator Selection (INTOSC oscillator: I/O function on CLKIN pin)
#pragma config WDTE = OFF       // Watchdog Timer Enable (WDT disabled)
#pragma config PWRTE = OFF      // Power-up Timer Enable (PWRT disabled)
#pragma config MCLRE = OFF      // MCLR Pin Function Select (MCLR/VPP pin function is digital input)
#pragma config CP = OFF         // Flash Program Memory Code Protection (Program memory code protection is disabled)
#pragma config CPD = OFF        // Data Memory Code Protection (Data memory code protection is disabled)
#pragma config BOREN = ON       // Brown-out Reset Enable (Brown-out Reset enabled)
#pragma config CLKOUTEN = OFF   // Clock Out Enable (CLKOUT function is disabled. I/O or oscillator function on the CLKOUT pin)
#pragma config IESO = OFF       // Internal/External Switchover (Internal/External Switchover mode is disabled)
#pragma config FCMEN = OFF      // Fail-Safe Clock Monitor Enable (Fail-Safe Clock Monitor is disabled)

// CONFIG2
#pragma config WRT = OFF        // Flash Memory Self-Write Protection (Write protection off)
#pragma config VCAPEN = OFF     // Voltage Regulator Capacitor Enable bit (Vcap functionality is disabled on RA6.)
#pragma config PLLEN = OFF      // PLL Enable (4x PLL disabled)
#pragma config STVREN = ON      // Stack Overflow/Underflow Reset Enable (Stack Overflow or Underflow will cause a Reset)
#pragma config BORV = LO        // Brown-out Reset Voltage Selection (Brown-out Reset Voltage (Vbor), low trip point selected.)
#pragma config LPBOR = OFF      // Low Power Brown-Out Reset Enable Bit (Low power brown-out is disabled)
#pragma config LVP = OFF        // Low-Voltage Programming Enable (High-voltage on MCLR/VPP must be used for programming)

// Global Variables

// display digit lookup table
// each segment pin needs to be turned on to make a digit
// the circuit has the A-G pins hooked up to a I/O port where A-G = 0-6 respectively
// the format for the bits is 0bGFEDCBA
volatile unsigned char digit_tbl[10] = {
    0b00111111,     // digit 0
    0b00000110,     // digit 1
    0b01011011,     // digit 2
    0b01001111,     // digit 3
    0b01100110,     // digit 4
    0b01101101,     // digit 5
    0b01111101,     // digit 6
    0b00000111,     // digit 7
    0b01111111,     // digit 8
    0b01101111      // digit 9
};

unsigned char interrupt_cnt = 0;       // number of times interrupt is called (250 = 1 second)
unsigned char seconds = 0;

// when clock is started it defaults to 12:00
unsigned char hours = 12;
unsigned char minutes = 0;
unsigned char alarm_hours = 12;
unsigned char alarm_minutes = 30;
bit alarm_active = 0;
bit alarm_set_enabled = 0;
bit alarm_enabled = 0;
bit snooze_active = 0;
unsigned char alarm_hours_shad = 12;
unsigned char alarm_minutes_shad = 0;

// extracts the first digit from an integer ie. the 1 in the number 12
unsigned char extract_dig1(const unsigned char* value)
{
    return *value / 10;
}

// extracts the second digit from an integer ie. the 2 in the number 12
unsigned char extract_dig2(const unsigned char* value)
{
    return *value % 10;
}

// updates the display digits
void update_display(const unsigned char* h_val, const unsigned char* m_val)
{
    if (LATB0) {
        // Display 0 was updated last update digit 1
        LATB0 = 0;                             // turn off display 0
        LATB1 = 1;                             // turn on display 1
        LATC = digit_tbl[extract_dig2(h_val)]; // load the segments with the proper value
        return;
    } else if (LATB1) {
        // Display 1 was updated last update display 2
        LATB1 = 0;                             // turn off display 1
        LATB2 = 1;                             // turn on display 2
        LATC = digit_tbl[extract_dig1(m_val)]; // load the segments with the proper value
        return;
    } else if (LATB2) {
        // Display 2 was updated last update display 3
        LATB2 = 0;                             // turn off display 2
        LATB3 = 1;                             // turn on display 3
        LATC = digit_tbl[extract_dig2(m_val)]; // load the segments with the value 0
        return;
    } else {
        if (*h_val > 9) {
            // Either display 3 was updated last or no display is enabled (first interrupt cycle only)
            LATB3 = 0;                             // turn off display 3
                                                   // on first run this is a wasted instruction but saves program space
            LATB0 = 1;                             // turn on display 0
            LATC = digit_tbl[extract_dig1(h_val)]; // load the segments with the proper value
            return;
        } else {
            // we do not need to use display 0 so update display 1 instead
            LATB3 = 0;                             // turn off display 3
            LATB1 = 1;                             // turn on display 1
            LATC = digit_tbl[extract_dig2(h_val)]; // load the segments with the proper value
            return;
        }
    }
}

void interrupt interrupt_service(void)
{
    // Check for Timer0 overflow interrupt
    if (TMR0IE && TMR0IF) {
        TMR0IF = 0;         // clear interrupt flag

        interrupt_cnt++;    // update the interrupt called count

        // Update our time if necessary
        if (interrupt_cnt == 250) {
            interrupt_cnt = 0;
            // One second has passed
            seconds++;
            if (seconds == 60) {
                seconds = 0;
                minutes++;
                if (minutes == 60) {
                    minutes = 0;
                    hours++;
                    if (hours == 13) {
                        hours = 1;
                    }
                }
            }

            // is our alarm enabled
            if (alarm_enabled) {
                // is it time to trigger alarm
                if (hours == alarm_hours && minutes == alarm_minutes)
                    alarm_active = 1; // trigger alarm
            } else {
                alarm_active = 0; // un trigger alarm
            }
           
            // Toggle alarm if triggered
            if (alarm_active)
                LATA0 ^= 1;
        }

        // The overall display is a multiplexed 4 display 7 segment unit
        // displays corrispond to pins on PORTB in this circuit.
        // Display 0 is on LATB0 - Display 3 is on LATB3
        // In the circuit these displays are switched through NPN transistors

        LATC = 0;           // Clear all lit segments to prevent ghosting.

        // Update the appropriate display for this interrupt cycle

        if (alarm_set_enabled) {
            // we need to display the alarm so we can set the time
            update_display(&alarm_hours, &alarm_minutes);
            return;
        } else {
            update_display(&hours, &minutes);
            return;
        }
    } else if (IOCIE && IOCIF) {
        if (IOCAF1) {
            IOCAF = 0;     // clear interrupt flag
            // need to update to appropriate hours
            if (alarm_set_enabled) {
                alarm_hours++;
                if (alarm_hours == 13)
                    alarm_hours = 1;
                return;
            } else {
                hours++;
                if (hours == 13)
                    hours = 1;
                return;
            }
        } else if (IOCAF2) {
            IOCAF = 0;     // clear interrupt flag
            // need to update appropriate minutes
            if (alarm_set_enabled) {
                alarm_minutes++;
                if (alarm_minutes == 60)
                    alarm_minutes = 0;
                return;
            } else {
                minutes++;
                if (minutes == 60)
                    minutes = 0;
                return;
            }
        } else if (IOCAF3) {
            IOCAF = 0;             // clear interrupt flag
            alarm_set_enabled ^= 1; // enable alarm set mode
            return;
        } else if (IOCAF4) {
            IOCAF = 0;
            LATA0 = 0;                              // prevent buzzer always on bug
            // snooze function.
            if (alarm_active) {
                alarm_active = 0;                   // turn off alarm
               
                if (!snooze_active) {
                    alarm_hours_shad = alarm_hours;     // save current alarm hours state
                    alarm_minutes_shad = alarm_minutes; // save current alarm minutes state
                    snooze_active = 1;
                }

                alarm_minutes = minutes + 5;                 // increment minutes by 5

                if (alarm_minutes >= 60) {
                    alarm_minutes = 0;
                    alarm_hours++;
                    if (alarm_hours == 13)
                        alarm_hours = 1;
                }
                return;
            }
        } else if (IOCAF5) {
            IOCAF = 0;              // clear interrupt flag
            alarm_enabled ^= 1;     // toggle alarm_enabled state
            LATB4 ^= 1;
            LATA0 = 0;

            if (!alarm_enabled) {
                // return alarm state
                alarm_hours = alarm_hours_shad;
                alarm_minutes = alarm_minutes_shad;
                snooze_active = 0;
            }
            return;
        } else {
            IOCAF = 0;
            return;
        }
    } else {
        return;
    }
}

void main(void)
{
   /*
    * OSCCON Register
    * bit 0 = Software PLL Enable
    * bit 6:3 = Internal oscillator frequency
    * bit 2 = Unimplemented
    * bit 1:0 = system clock select
    */
    OSCCON = 0b01101000;                // 4 MHZ oscillator frequency clk selected via FOSC fuse
    ANSELA = 0;                         // ensure PORTA analog inputs are disabled
    ANSELB = 0;                         // ensure PORTB analog inputs are disabled
    TRISA = 0b00111110;                 // set RA0 as output RA1 - RA5 as input
    TRISB = 0;                          // set PORTB data direction to output
    TRISC = 0;                          // set PORTC data direction to output
    PORTA = 0;
    LATB = 0;
    LATC = 0;
    TMR0 = 0;                           // clear the Timer0 counter and prescaler

    // Enable interrupt on change falling edge for PIN RA1 - RA5
    IOCAN = 0b00111110;

    /*
     * INTCON
     * bit 7 = Global Interrupt Enable
     * bit 6 = Periferal Interrupt Enable
     * bit 5 = Timer0 Overflow Interrupt Enable
     * bit 4 = INT External Interrupt Enable
     * bit 3 = Interrupt-On-Change Interrupt Enable
     * bit 2 = Timer0 Overflow Interrupt Flag
     * bit 1 = INT External Interrupt Flag
     * bit 0 = Interrupt-On-Change Interrupt Flag
     */
    INTCON = 0b10101000;        // enable global interrupts, enable Timer0 interrupt and pin change interrupts

    /*
     * OPTION_REG Register
     * bit 7 = Weak Pull-Up Enable
     * bit 6 = Interrupt Edge Select
     * bit 5 = Timer0 Clock Source Select
     * bit 4 = Timer0 Source Edge Select
     * bit 3 = Prescale Assignment
     * bit 2:0 = Prescale Rate
     */
    OPTION_REG = 0b11010011;            // bit 7:6 is not used by Timer0
                                        // set timer to use internal instruction clock (FOSC/4)
                                        // bit 4 is not used and prescale Timer0 by 1:16
                                        // this will give us a overflow interrupt of aprox. 4ms
    while (1);
}

[Rufus]

Was that the bug about the one pin being input but not implemented?

The bug is avoided by moving the alarm time check out of the main loop. I looked at the program and don't see anything wrong. I did note it won't keep time because the interrupt runs at 4.096ms not 4, but, the internal oscillator isn't accurate enough to keep time anyway.

I attach the posted program compiled in Pro mode, doubt it will function differently.

[blewisjr]

Yea it does not keep time perfectly accurately due to using the internal oscillator there is a small variance from that which is unavoidable without using a external oscillator but I probably can compensate in software for it but not a major concern of mine atm.  What I am probably seeing is the internal oscillator error range and not actually the branching now that I think about it.

[croberts]

You can get the 4mS interrupt interval by adding 5 to the current value in tim0 and then loading that value into tim0 at the beginning of the interrupt service routine. This will effectively cause tim0 to rollover after 250 counts instead of 255 counts. The addition is necessary to compensate for the counts already accumulated before the interrupt is serviced. You can compensate somewhat for the oscillator inaccuracy by adding or subtracting a fixed number of seconds every hour depending on the time running slow or fast.

[blewisjr]

You can get the 4mS interrupt interval by adding 5 to the current value in tim0 and then loading that value into tim0 at the beginning of the interrupt service routine. This will effectively cause tim0 to rollover after 250 counts instead of 255 counts. The addition is necessary to compensate for the counts already accumulated before the interrupt is serviced. You can compensate somewhat for the oscillator inaccuracy by adding or subtracting a fixed number of seconds every hour depending on the time running slow or fast.

Yea this is what I was thinking about handling it in software.  I am probably going to move onto another project now because I really don't want to go through all the effort of designing a PCB/housing for it.  I think it would be rather tough to put it on a perfboard cleanly.  I got what I wanted out of the project.  I completed the project for one and for two I learned so much from it.  Which is something that would not have happened if I did not start it in the first place.  I think it was a good first project for me.

[C]

Learning is great, you might want to ask your self these.
  Could you learn some more now that would make the next project easer?

Could you make some changes that lower the amount of computing work the mpu is doing?
Could you make some changes that would let you use parts of this code for other projects?
Is there simple code changes that would make the code less breakable and easer to change?

C

[Mr Smiley]

5+3 cos TMR0 is stopped for 3 cycles when it is added to

 

[C]

    if (LATB0) {
        // Display 0 was updated last update digit 1
        LATB0 = 0;                             // turn off display 0
        LATB1 = 1;                             // turn on display 1
        LATC = digit_tbl[extract_dig2(h_val)]; // load the segments with the proper value
        return;

Not much time involved here but for a short time you do have the current digit displaying the previous digits segments.

    if (LATB0) {
        // Display 0 was updated last update digit 1
        LATB0 = 0;                             // turn off display 0
        LATC = digit_tbl[extract_dig2(h_val)]; // load the segments with the proper value
         LATB1 = 1;                             // turn on display 1
       return;

I think would be better.

C