Problem with my 2nd PIC program...

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
Hi All! Hope y'all doing great...

Here's what I'm working on:

"I'm trying to drive two multiplexed 7 segment displays and make them display all the alphanumeric characters 0-9 and a-z except where no suitable 7 segment display can be created (as for X) in which case the display will indicate an '-', IE only segment G lit.

The value displayed will be changed by pressing a pushbutton with every press causing the sequence to move forward one character.
The left hand display merely indicating the previous value that was displayed on the right hand display. This will then present the effect of a right to left scrolling display. "


Here's what I've done so far:

- I've already built my circuit on breadboard using: PIC16F690, 2 x 7 segment displays (LSD5116-11, common cathod), 2 x npn transistors (CTBC547B), 1 pushbutton, 7 x 330 ohms resistors for 7 segment display, 1 x 10k ohm for pushbutton, 2 x 1k ohms for transistors. PLEASE see my pin configuration in the attachement!!!

- my code attached has DEFINATELY got problems...and I need your help on that...

- I tried working the multiplexing and switch debounce in the timer0 isr...but kind of messy...

- my BIG trouble is the TABLE...I tried doing some kind of state but no! Maybe not doing it well...

MY first question: what should be the code structure??? I mean what should the interrupt contain? What should the main program do?

Ok I think I can stop here for now

All your comments/ guidance/helps will be greatly appreciated!!!

Thanks All
 

Attachments

MMcLaren

Joined Feb 14, 2010
861
Very nice Eric. You seem to have a very good grasp of the concepts, Sir.

I'm not sure the interrupt context save and restore is correct, and, I would use the "common" area of RAM between 0x70 and 0x7F for the context variables since you can't guarantee which bank you're in when you get an interrupt.

Here's an example (minus "config" and "init" sections) of how I might do it. I'm not trying to imply that this way is better. It's just different and maybe you'll see something you like.

Back to my homework...

Have fun. Cheerful regards, Mike

Rich (BB code):
;******************************************************************
;  variables                                                      *
;******************************************************************
        cblock    0x20
index                           ; array index, 0..9
left_dsp                        ; left display segment data
right_dsp                       ; right display segment data
swctr                           ; isr interval counter
        endc    
;
;  use "shared" memory common to all banks for isr context vars
;
        cblock    0x70
w_isr                           ; isr context save for W
s_isr                           ; isr context save for STATUS
        endc

;******************************************************************
;  reset vector                                                   *
;******************************************************************

        org     0x000           ;
v_reset
        goto    init            ;

;******************************************************************
;  interrupt vector                                               *
;******************************************************************

        org     0x004           ;
        radix   dec
v_int
        movwf   w_isr           ; save WREG                       |B?
        swapf   STATUS,W        ;                                 |B?
        movwf   s_isr           ; save STATUS reg                 |B?
        clrf    STATUS          ; force bank 0, IRP 0             |B0
        bcf     INTCON,T0IF     ; clear timer 0 interrupt flag    |B0
;
;  toggle/refresh mux'd displays (488 Hz refresh rate)
;
        clrf    PORTC           ; blank the display               |B0
        movlw   b'01100000'     ; mask for digit select pins      |B0
        xorwf   PORTB,F         ; toggle digit select pins        |B0
        movf    left_dsp,W      ; get left segment data           |B0
        btfss   en_left         ; if left display, skip, else     |B0
        movf    right_dsp,W     ; get right segment data          |B0
        movwf   PORTC           ; display new digit               |B0
;
;  sample/debounce switches at 16-msec "debounce" intervals
;
        decfsz  swctr,F         ; 16-msec interval?               |B0
        goto    v_xit           ; no, branch (exit), else         |B0
        movlw   16              ;                                 |B0
        movwf   swctr           ; reset swctr for 16 msecs        |B0
;
;   wreg  __---___---___---____   sample active hi switches
;  swold  ___---___---___---___   switch state latch
;   wreg  __-__-__-__-__-__-___   changes, press or release
;  swnew  __-_____-_____-______   filter out 'release' bits
;    
        movf    PORTB,W         ; sample active hi switches       |B0
        andlw   b'10000000'     ; on the RB7 pin                  |B0
        xorwf   swold,W         ; changes, press or release       |B0
        xorwf   swold,F         ; update switch state latch       |B0
        andwf   swold,W         ; filter out 'release' bits       |B0
        iorwf   swnew,F         ; save new press flags for main   |B0
v_xit
        swapf   s_isr,W         ;                                 |B0
        movwf   STATUS          ; restore STATUS                  |B?
        swapf   w_isr,F         ;                                 |B?
        swapf   w_isr,W         ; restore W                       |B?
        retfie                  ;                                 |B?

;******************************************************************
;  main init                                                      *
;******************************************************************

;******************************************************************
;  main loop                                                      *
;******************************************************************
;
;  while(1)
;  { if(swnew.7)                // if "new press"
;    { swnew.7 = 0;             // clear "new press" flag
;      left_dsp = right_dsp;    // 
;      if(index++ == 9)         // increment index
;        index = 0;             // rollover from 9 to 0
;      right_dsp = segdata[index];
;    }                          //
;  }
;
        radix   dec
loop
        btfss   swnew,7         ; new press? yes, skip, else      |B0
        goto    loop            ; loop                            |B0
        bcf     swnew,7         ; reset "new press" flag and      |B0
;
;  process "new press"
;
        movf    right_dsp,W     ;                                 |B0
        movwf   left_dsp        ; left_dsp = right_dsp            |B0
        movf    index,W         ; index, 0..9                     |B0
        xorlw   9               ; upper limit (W=0)?              |B0
        skpz                    ; yes, skip, else                 |B0
        incf    index,W         ; increment index                 |B0
        movwf   index           ; save it                         |B0
        call    segdata         ; get segment data                |B0
        movwf   right_dsp       ; right_dsp = segdata[index]      |B0
        goto    loop            ;                                 |B0

;******************************************************************
;  subroutines                                                    *
;******************************************************************

segdata
        addwf    PCL,F           ;                                |B0
        retlw    b'00111111'     ; "0"   -|-|F|E|D|C|B|A          |B0
        retlw    b'00000110'     ; "1"   -|-|-|-|-|C|B|-          |B0
        retlw    b'01011011'     ; "2"   -|G|-|E|D|-|B|A          |B0
        retlw    b'01001111'     ; "3"   -|G|-|-|D|C|B|A          |B0
        retlw    b'01100110'     ; "4"   -|G|F|-|-|C|B|-          |B0
        retlw    b'01101101'     ; "5"   -|G|F|-|D|C|-|A          |B0
        retlw    b'01111101'     ; "6"   -|G|F|E|D|C|-|A          |B0
        retlw    b'00000111'     ; "7"   -|-|-|-|-|C|B|A          |B0
        retlw    b'01111111'     ; "8"   -|G|F|E|D|C|B|A          |B0
        retlw    b'01101111'     ; "9"   -|G|F|-|D|C|B|A          |B0

;******************************************************************
        end
 
Last edited:

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
@Mike: Thx so much...lemme analyse your suggestion and i'll get back to you BUT you SO RIGHT about the context saving...ima fix that....
 

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
@Mike: i had a second look at your piece of code...and i MUST say that i am impressed at the way you simplified the problem!!! very good!

i like your multiplexing routine...i was like: "Oh wow...so simple"...lol
i also like your pushbutton debouncing routine...but i'll ask a question about it later...

BUT swnew, swold, and wreg (are they variable registers??) need to be declared, right? coz you did NOT...and you talked about wreg but i dont see where you used it...

i'm editing my code according to yours...and i"ll try running it....

Thanks again
 

Markd77

Joined Sep 7, 2009
2,806
BUT swnew, swold, and wreg (are they variable registers??) need to be declared, right? coz you did NOT...and you talked about wreg but i dont see where you used it...
wreg is just another way of saying W, so don't worry about that.
swnew, swold and anything else that make the compiler complain need to be defined in the cblock.
I noticed en_left, which I think needs declaring.
Try:
#define en_left PORTB, 6
or
#define en_left PORTB, 5
This could be put just after the cblock section.
 
Last edited:

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
@Markd77: thx for your comment...
i did declare en_left an en_right in my first post....

i just edited my code according to Mike BUT when running it Nothing happen at all....i'll post it so you can see how i edited it...and help me out.....

when i first ran my code, although i new there was mistakes, only the left display was blinking and right one not responding to anything...

AND the BIG problem is i HAVEN'T LEARNED at to simulate code yet...but i checked my circuit and everything is on point!!

i'll post in a few minutes
 

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
i edited as i said...but now i would like you guyz to check my initialisation!

Also i need to know if en_left and en_right need to be initialized!!???

i tried running the attached code and sometimes it display 11 (eleven) or nothing and not responsive to pushbutton at all......

Attached is the code!

Still thinking how to solve it but any help would be appreciated!
 

Attachments

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
well i'll still have to edit the table as i also have to diplay the letter...but that woul not be big deal if it runs for the digits....
 

MMcLaren

Joined Feb 14, 2010
861
Hi Eric,

I just have a few minutes between classes.

I apologize for not including an "init" section. Seems you're missing an instruction to init the digit select pins; PORTB = b'01000000' or b'00100000'.

The "swnew" and "swold" variables need to be cleared too.

There may be other errors too but I've gotta run. Sorry!!!

Cheerful regards, Mike
 

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
Thanks Mike for confirming...i made those changes but still no improvement...
Can you simulate it? sorry about it...but i dont know yet how to simulate...
 

MMcLaren

Joined Feb 14, 2010
861
Looks like you may have had some problems in your "init" section.

Here's a complete example that simulates fine. You're using an active high switch with pull-down, right? Let me know how it goes?

Cheerful regards, Mike
 

Attachments

Last edited:

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
I'm using active low I think coz when I press the button it pull the voltage to ZERO!!!
I have a pullup resistor connected to +5V...

I'll have a look at ur code in a moment!

Thx so much!!
 

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
@MMclaren: Oh!!!!!! it working so well....
i have added the letters as well and edited "xorlw 9 ; upper limit?" to "xorlw 35 ; upper limit? and it all perfect!

i still have to fix the fact that its the left display that should display the previous of the right BUT the opposite happening but it can be easily fixed!!

thx again

im learning so much and quickly......
 

MMcLaren

Joined Feb 14, 2010
861
Ok, if "active low" configuration then you should make the following change (from 1st example to 2nd example);
Rich (BB code):
        movf    PORTB,W         ; sample active hi switches       |B0
Rich (BB code):
        comf    PORTB,W         ; sample active lo switches       |B0
The difference is that with the first instruction and an "active lo" switch you're getting a "new press" flag when you release the switch.

i still have to fix the fact that its the left display that should display the previous of the right BUT the opposite happening but it can be easily fixed!!
Try changing the "btfss PORTB,6" instruction to "btfss PORTB,5"...

Regards, Mike

<added>

The section below includes a correction (sorry!);

Rich (BB code):
;
;  setup TMR0 for 1024 us overflows (8 MHz clock, 4 us 'ticks')
;
        movlw   b'10000010'     ; 10000010                        |B1
                                ; 1------- /RABPU, pull-ups off
                                ; -0------ INTEDG, INT edge n/a
                                ; --0----- T0CS, TMR0 int source
                                ; ---0---- T0SE, source edge n/a
                                ; -----010 PS<2:0>, prescale 1:8
        movwf   OPTION_REG      ; for 1024 us TMR0 overflows      |B1
 
Last edited:

Thread Starter

Eric007

Joined Aug 5, 2011
1,158
Ok mike! I changed movf to comf...but was working anyway...

And in the interrup routine, multiplexing part, I swapped right_dsp and left_dsp...and it working as I want now...

One little problem is that it starts displaying '1' instead of '0'...
I fixed it by adding retlw b'00000000' on top of retlw b'00111111' and it works but I don't like this way of fixing...

So was wondering if it possible to store a 'NEGATIVE' value in a register!? Lol
If so I can initialise index with '-1' so that when the main program first increment it index becomes '0'. And 0 will be the first digit to be displayed...

What do u think Mike
 

MMcLaren

Joined Feb 14, 2010
861
Yes, setting index to -1 will do what you want.

Rich (BB code):
        movlw   -1              ;                            |B0
        movwf   index           ;                            |B0
If you want the display to light up right off the bat then initialize 'right_dsp' to the segment pattern for the initial index value.

Rich (BB code):
        movf    index,W         ; 0..35                      |B0
        call    segdata         ; get 'index' segment data   |B0
        movwf   right_dsp       ; display initial "index"    |B0
        clrf    left_dsp        ; leave left display blank   |B0
;
;  or perhaps simply
;
        movlw   b'00111111'     ; pattern for "0"            |B0
        movwf   right_dsp       ; display initial "index"    |B0
        clrf    left_dsp        ; leave left display blank   |B0
 
Last edited:

joeyd999

Joined Jun 6, 2011
5,234
Eric had asked me in a PM for comments, so here goes:

1. If you were to use the gettim and getkey routines that I originally showed you in the other thread, you could eliminate the need to count out 16 ms in your interrupt routine to check the switch. Just check to see when the TC16ms bit is set. In the future, this could save you at least 1 register for every (simultaneous) time delay you require.

2. Here's another one of my "nevers": Don't ever do something in an interrupt that can be done in normal code space just as effectively. I like to keep my interrupts as short as humanly possible. Interrupt, do something really fast, and get out. In the case of TMR0, I usually only set the timer bits. If I am using an A/D, I'll set the 'GO' bit (to avoid jitter in the A/D sampling period. BTW, the multiplexer will have already been set in the previous A/D interrupt). Then I use the timer bits to initiate actions in the main program. 16ms is an eternity and a few tens or hundreds of microseconds of jitter (the time it takes to get back to your keypad routine in the main loop) will not be noticed.

The display is a special case. If the MCU is responsible for multiplexing the display, and there is lots of other stuff going on, updating the display within an interrupt is usually the right approach. In the case of your (short) program, you could use the TC1ms flag in the main loop to trigger display updates. I've done it both ways.

3. In every program I write (except the one I did for you), I always include a routine called 'CLRMEM' which sets all (non-special) file registers to 0 upon power on. This way I only need to initialize values that are not zero.

4. Get used to placing a NOP at address 0x00, before your goto init. You'll need this if you ever decide to use an ICD.

5. Understand that registers used by interrupts may change in value during execution of your main program. As your programs get larger, your logic may get confused if registers such as 'SWNEW' change (because an interrupt occurred). There is no problem with this particular program, I just want you to be aware now that you're delving into interrupts. If you go back to my original code, you'll see that I capture (static) copies of the (volatile) interrupt registers each pass of the main loop. I usually name "volatiles" (those that can change within in interrupt) differently than I name "statics" (those that do not change within interrupts). I usually preface it with an '_', i.e. _timer. This is a reminder to me that I cannot trust the value to be the same from one reading of it to the next.

6. It is tempting, with your small programs, to bunch everything into one loop. Hopefully, you programs will get much larger. Learn to modularize functionality into smaller routines that will be easier to maintain and debug. You've got a stack. Use it.

Overall, good work! Be proud of yourself for the *very* fast progress you have made.
 

joeyd999

Joined Jun 6, 2011
5,234
One more thing:

At some point, you are going to be using more than one interrupt source. Get in the habit of testing the interrupt flag bits to determine which action to take. Even in your small program, with one interrupt, this is helpful. I've occasionally initialized my interrupt setup registers incorrectly, and got interrupts from the wrong source. If I didn't test the interrupt flags, it would not be apparent that I was triggering off the wrong signal (the timing would probably be messed up, but then I'd have to waste time figuring if I set up the signalling peripheral incorrectly).

This also helps with extensibility, as adding another interrupt source is a simple as adding a btf and goto/bra.
 
Top