Problem with my 2nd PIC program...

Discussion in 'Embedded Systems and Microcontrollers' started by Eric007, Sep 11, 2011.

  1. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    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
     
  2. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    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

    Code ( (Unknown Language)):
    1.  
    2. ;******************************************************************
    3. ;  variables                                                      *
    4. ;******************************************************************
    5.         cblock    0x20
    6. index                           ; array index, 0..9
    7. left_dsp                        ; left display segment data
    8. right_dsp                       ; right display segment data
    9. swctr                           ; isr interval counter
    10.         endc    
    11. ;
    12. ;  use "shared" memory common to all banks for isr context vars
    13. ;
    14.         cblock    0x70
    15. w_isr                           ; isr context save for W
    16. s_isr                           ; isr context save for STATUS
    17.         endc
    18.  
    19. ;******************************************************************
    20. ;  reset vector                                                   *
    21. ;******************************************************************
    22.  
    23.         org     0x000           ;
    24. v_reset
    25.         goto    init            ;
    26.  
    27. ;******************************************************************
    28. ;  interrupt vector                                               *
    29. ;******************************************************************
    30.  
    31.         org     0x004           ;
    32.         radix   dec
    33. v_int
    34.         movwf   w_isr           ; save WREG                       |B?
    35.         swapf   STATUS,W        ;                                 |B?
    36.         movwf   s_isr           ; save STATUS reg                 |B?
    37.         clrf    STATUS          ; force bank 0, IRP 0             |B0
    38.         bcf     INTCON,T0IF     ; clear timer 0 interrupt flag    |B0
    39. ;
    40. ;  toggle/refresh mux'd displays (488 Hz refresh rate)
    41. ;
    42.         clrf    PORTC           ; blank the display               |B0
    43.         movlw   b'01100000'     ; mask for digit select pins      |B0
    44.         xorwf   PORTB,F         ; toggle digit select pins        |B0
    45.         movf    left_dsp,W      ; get left segment data           |B0
    46.         btfss   en_left         ; if left display, skip, else     |B0
    47.         movf    right_dsp,W     ; get right segment data          |B0
    48.         movwf   PORTC           ; display new digit               |B0
    49. ;
    50. ;  sample/debounce switches at 16-msec "debounce" intervals
    51. ;
    52.         decfsz  swctr,F         ; 16-msec interval?               |B0
    53.         goto    v_xit           ; no, branch (exit), else         |B0
    54.         movlw   16              ;                                 |B0
    55.         movwf   swctr           ; reset swctr for 16 msecs        |B0
    56. ;
    57. ;   wreg  __---___---___---____   sample active hi switches
    58. ;  swold  ___---___---___---___   switch state latch
    59. ;   wreg  __-__-__-__-__-__-___   changes, press or release
    60. ;  swnew  __-_____-_____-______   filter out 'release' bits
    61. ;    
    62.         movf    PORTB,W         ; sample active hi switches       |B0
    63.         andlw   b'10000000'     ; on the RB7 pin                  |B0
    64.         xorwf   swold,W         ; changes, press or release       |B0
    65.         xorwf   swold,F         ; update switch state latch       |B0
    66.         andwf   swold,W         ; filter out 'release' bits       |B0
    67.         iorwf   swnew,F         ; save new press flags for main   |B0
    68. v_xit
    69.         swapf   s_isr,W         ;                                 |B0
    70.         movwf   STATUS          ; restore STATUS                  |B?
    71.         swapf   w_isr,F         ;                                 |B?
    72.         swapf   w_isr,W         ; restore W                       |B?
    73.         retfie                  ;                                 |B?
    74.  
    75. ;******************************************************************
    76. ;  main init                                                      *
    77. ;******************************************************************
    78.  
    79. ;******************************************************************
    80. ;  main loop                                                      *
    81. ;******************************************************************
    82. ;
    83. ;  while(1)
    84. ;  { if(swnew.7)                // if "new press"
    85. ;    { swnew.7 = 0;             // clear "new press" flag
    86. ;      left_dsp = right_dsp;    //
    87. ;      if(index++ == 9)         // increment index
    88. ;        index = 0;             // rollover from 9 to 0
    89. ;      right_dsp = segdata[index];
    90. ;    }                          //
    91. ;  }
    92. ;
    93.         radix   dec
    94. loop
    95.         btfss   swnew,7         ; new press? yes, skip, else      |B0
    96.         goto    loop            ; loop                            |B0
    97.         bcf     swnew,7         ; reset "new press" flag and      |B0
    98. ;
    99. ;  process "new press"
    100. ;
    101.         movf    right_dsp,W     ;                                 |B0
    102.         movwf   left_dsp        ; left_dsp = right_dsp            |B0
    103.         movf    index,W         ; index, 0..9                     |B0
    104.         xorlw   9               ; upper limit (W=0)?              |B0
    105.         skpz                    ; yes, skip, else                 |B0
    106.         incf    index,W         ; increment index                 |B0
    107.         movwf   index           ; save it                         |B0
    108.         call    segdata         ; get segment data                |B0
    109.         movwf   right_dsp       ; right_dsp = segdata[index]      |B0
    110.         goto    loop            ;                                 |B0
    111.  
    112. ;******************************************************************
    113. ;  subroutines                                                    *
    114. ;******************************************************************
    115.  
    116. segdata
    117.         addwf    PCL,F           ;                                |B0
    118.         retlw    b'00111111'     ; "0"   -|-|F|E|D|C|B|A          |B0
    119.         retlw    b'00000110'     ; "1"   -|-|-|-|-|C|B|-          |B0
    120.         retlw    b'01011011'     ; "2"   -|G|-|E|D|-|B|A          |B0
    121.         retlw    b'01001111'     ; "3"   -|G|-|-|D|C|B|A          |B0
    122.         retlw    b'01100110'     ; "4"   -|G|F|-|-|C|B|-          |B0
    123.         retlw    b'01101101'     ; "5"   -|G|F|-|D|C|-|A          |B0
    124.         retlw    b'01111101'     ; "6"   -|G|F|E|D|C|-|A          |B0
    125.         retlw    b'00000111'     ; "7"   -|-|-|-|-|C|B|A          |B0
    126.         retlw    b'01111111'     ; "8"   -|G|F|E|D|C|B|A          |B0
    127.         retlw    b'01101111'     ; "9"   -|G|F|-|D|C|B|A          |B0
    128.  
    129. ;******************************************************************
    130.         end
    131.  
     
    Last edited: Sep 12, 2011
    stahta01 likes this.
  3. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    @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....
     
  4. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    @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
     
  5. Markd77

    Senior Member

    Sep 7, 2009
    2,803
    594
    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: Sep 12, 2011
  6. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    @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
     
  7. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    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!
     
  8. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    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....
     
  9. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    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
     
  10. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    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...
     
  11. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    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
     
    Last edited: Sep 12, 2011
  12. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    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!!
     
  13. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    @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......
     
  14. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    Ok, if "active low" configuration then you should make the following change (from 1st example to 2nd example);
    Code ( (Unknown Language)):
    1.         movf    PORTB,W         ; sample active hi switches       |B0
    2.  
    Code ( (Unknown Language)):
    1.         comf    PORTB,W         ; sample active lo switches       |B0
    2.  
    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.

    Try changing the "btfss PORTB,6" instruction to "btfss PORTB,5"...

    Regards, Mike

    <added>

    The section below includes a correction (sorry!);

    Code ( (Unknown Language)):
    1. ;
    2. ;  setup TMR0 for 1024 us overflows (8 MHz clock, 4 us 'ticks')
    3. ;
    4.         movlw   b'10000010'     ; 10000010                        |B1
    5.                                 ; 1------- /RABPU, pull-ups off
    6.                                 ; -0------ INTEDG, INT edge n/a
    7.                                 ; --0----- T0CS, TMR0 int source
    8.                                 ; ---0---- T0SE, source edge n/a
    9.                                 ; -----010 PS<2:0>, prescale 1:8
    10.         movwf   OPTION_REG      ; for 1024 us TMR0 overflows      |B1
    11.  
     
    Last edited: Sep 14, 2011
  15. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    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
     
  16. MMcLaren

    Well-Known Member

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

    Code ( (Unknown Language)):
    1.  
    2.         movlw   -1              ;                            |B0
    3.         movwf   index           ;                            |B0
    4.  
    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.

    Code ( (Unknown Language)):
    1.  
    2.         movf    index,W         ; 0..35                      |B0
    3.         call    segdata         ; get 'index' segment data   |B0
    4.         movwf   right_dsp       ; display initial "index"    |B0
    5.         clrf    left_dsp        ; leave left display blank   |B0
    6. ;
    7. ;  or perhaps simply
    8. ;
    9.         movlw   b'00111111'     ; pattern for "0"            |B0
    10.         movwf   right_dsp       ; display initial "index"    |B0
    11.         clrf    left_dsp        ; leave left display blank   |B0
    12.  
    13.  
     
    Last edited: Sep 12, 2011
  17. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    Eric,

    Have you thought about adding a switch, one for "up" and one for "down"?
     
  18. Eric007

    Thread Starter Senior Member

    Aug 5, 2011
    1,044
    33
    It worked perfectly with -1!!! Thanks for confirming...
     
  19. joeyd999

    AAC Fanatic!

    Jun 6, 2011
    2,693
    2,763
    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.
     
    Eric007 likes this.
  20. joeyd999

    AAC Fanatic!

    Jun 6, 2011
    2,693
    2,763
    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.
     
Loading...