PIC12F683 question

Discussion in 'Embedded Systems and Microcontrollers' started by bug13, Nov 18, 2012.

  1. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    Hi guys,

    Say if I want to turn on a couple of leds, the following codes work fine:

    Code ( (Unknown Language)):
    1. GPIO |= (1<<LED1)+(1<<LED2);
    or

    Code ( (Unknown Language)):
    1.     GPIO |= (1<<LED2);
    2.     GPIO |= (1<<LED1);
    but if I use the following codes, only the LED2 lights up, why?
    Code ( (Unknown Language)):
    1.     GPIO |= (1<<LED1);
    2.     GPIO |= (1<<LED2);
    I am using MPLAB X with XC8 complier, PIC12F683 under win7.
    LED1 and LED2 are defined at the beginning of the code.

    PIC12F683 datasheet
     
    Last edited: Nov 18, 2012
  2. spinnaker

    AAC Fanatic!

    Oct 29, 2009
    4,887
    1,012
    What have you done to debug this problem?

    Have you run the debugger an looked at the values in the register?

    Have you measured the output of the pins with a scope, logic probe, voltmeter, locic analyzer etc?
     
  3. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,388
    1,605
    OK, so it works the first two ways, but not the third. This rules out things like TRISIO or ANSEL being set wrong.

    Does it work just fine in the simulator, but not in the real hardware? That would indicate you're bumping up against the read-modify-write (R-M-W or just RMW) problem. As statements such as "GPIO |= (1<<LED2);" can and will compile down to a single assembly instruction even C is quite vulnerable to this.

    If you can place another instruction like a series of nop()'s between the two instructions and that makes it work then you have a RMW issue.

    Google the term, there is lots of info concerning it out there.
     
  4. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38

    I have measured the voltage with a DMM and scope, but I have no debug header for this chip as it requires one.
     
  5. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    Thanks ErnieM,

    following your advice, I found the solution here, its a RMW problem:

    http://www.mikroe.com/download/eng/documents/compilers/mikroc/pro/pic/help/rmw.htm

    PS:
    I have tried the following codes:
    Code ( (Unknown Language)):
    1.     GPIO |= (1<<LED1);
    2.     __delay_ms(5000);
    3.     GPIO |= (1<<LED2);
    the interesting thing is, the LED1 turns on first, then LED2 turns on but at the same time, the LED1 turns off as well...:confused: the end result, only LED2 turn on.


     
    Last edited: Nov 18, 2012
  6. Markd77

    Senior Member

    Sep 7, 2009
    2,803
    594
    Usually the best thing to do is to use a variable like GPIO_temp, do all your work on that and then copy that to GPIO when required. That way there will never be any problems. It's only a few extra instructions.
     
    bug13 likes this.
  7. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    Not necessarily. For example, you know you can set an analog pin to be an output, right? In this case, if I recall correctly, you can set the pin to output a '0' or a '1' but the pin will always read as a '0'. And so, if the OP has LED1 and LED2 connected as active high outputs with LED2 on the GP5 pin, then it's possible he/she has forgotten to turn off the analog functions.
     
    Last edited: Nov 18, 2012
  8. takao21203

    Distinguished Member

    Apr 28, 2012
    3,577
    463
    What's the LED resistors value? Have you tried 2.2K or 4.7K? Or are you using a very low resistance value?
     
  9. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    so after reading your replay, I added these codes to turn off the analog functions, still get the same result.

    here are my codes to turn off the ADC

    Code ( (Unknown Language)):
    1.     /*
    2.     ANS<3:0>: Analog Select bits
    3.     Analog select between analog or digital function on pins AN<3:0>, respectively.
    4.     1 = Analog input. Pin is assigned as analog input(1).
    5.     0 = Digital I/O. Pin is assigned to port or special function.
    6.      */
    7.     ANSELbits.ANS=0;
    8.     /*
    9.      ADON: ADC Enable bit
    10.     1 = ADC is enabled
    11.     0 = ADC is disabled and consumes no operating current
    12.      */
    13.     ADCON0bits.ADON=0;
     
  10. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    I am using some cheap RED led with 1.8v voltage drop, about 10mA normal operation current.

    the resistor value was 330R, then I replaced one of them to a 10K, the result is still the same.

    Then remove the led and the resistor all together, and I probe the pin connected to the pin directly, still get the same result.
     
  11. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    That's not how you turn off the analog pin functions. Please look at example 4.1 in section 4 of the datasheet...

    If you're not going to post your program, can you at least tell us how you're defining LED1 and LED2?

    <added>

    Oops! Just found the "ANS" structure member in the 12F683 header... My apologies... What you're doing should disable the ADC analog functions...
     
    Last edited: Nov 19, 2012
  12. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,388
    1,605
    That is interesting, and it is similar to what the RMW problem looks like. However, after a full 5 second delay it has to be something else. Somehow when the port value is read the LED1 pin is read as a zero, and is thus reset when LED2 is set.

    Your code to turn off the analog pin functions is very good, you must have read section 4 of the datasheet.

    Could you post your code, at least a portion to demonstrate this problem? What are LED1 and LED2?

    If I had the code I could run it in the MPLAB simulator and perhaps see something. This is an interesting problem.
     
  13. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    my code is not intended to do anything, just teaching myself PIC and messing around. I have comment out all the stuff that are irrelevant(intended to isolate the faulty code), still get to the same result, but feel free to check my commented out code as well.

    here is all my code:

    PS:
    PIC12F683, XC8(v1.10) complier, MPLAB X(v1.41), Win7 64bit, PICKit3(Firmware Suite Version.....01.28.07)


    Code ( (Unknown Language)):
    1. /*
    2.  * File:   main.c
    3.  * Author: JMS
    4.  *
    5.  * Created on 16 November 2012, 10:23 PM
    6.  */
    7. //#pragma config CPD = OFF, BOREN = OFF, IESO = OFF,
    8. //#pragma config FOSC = INTOSCIO, FCMEN = OFF, MCLRE = OFF,
    9. //#pragma config WDTE = OFF, CP = OFF, PWRTE = OFF
    10. #pragma config FOSC = INTOSCIO, WDTE = OFF
    11.  
    12. #define _XTAL_FREQ 4000000
    13.  
    14. #define LED1 1 //LED1 connects to GP1
    15. #define LED2 5 //LED2 connects to GP5
    16.  
    17. //#define PINA_DDR TRISIObits.TRISIO1
    18. //#define PINB_DDR TRISIObits.TRISIO5
    19. //#define PINC_DDR TRISIObits.TRISIO4
    20. //#define PIND_DDR TRISIObits.TRISIO2
    21.  
    22. //#define PINA GPIObits.GP1
    23. //#define PINB GPIObits.GP5
    24. //#define PINC GPIObits.GP4
    25. //#define PIND GPIObits.GP2
    26.  
    27. //#define DelayMS 100
    28.  
    29. //#include <stdint.h>
    30. #include <xc.h>
    31.  
    32. void InitialIO(void);
    33. //void Leds(void);
    34.  
    35.  
    36. int main(void) {
    37.     InitialIO();
    38.  
    39.  
    40.     GPIO |= (1<<LED1);
    41.     __delay_ms(2000);
    42.     GPIO |= (1<<LED2);
    43.     while(1)
    44.     {
    45.         //Leds();
    46.     }
    47.  
    48. }
    49.  
    50. /*
    51. void Leds(void)
    52. {
    53.         PINA=1;
    54.         __delay_ms(DelayMS);
    55.         PINA=0;
    56.         __delay_ms(DelayMS);
    57.         PINB=1;
    58.         __delay_ms(DelayMS);
    59.         PINB=0;
    60.         __delay_ms(DelayMS);
    61.         PINC=1;
    62.         __delay_ms(DelayMS);
    63.         PINC=0;
    64.         __delay_ms(DelayMS);
    65.         PIND=1;
    66.         __delay_ms(DelayMS);
    67.         PIND=0;
    68.         __delay_ms(DelayMS);
    69. }
    70. */
    71.  
    72. void InitialIO(void)
    73. {
    74.     /*
    75.     ANS<3:0>: Analog Select bits
    76.     Analog select between analog or digital function on pins AN<3:0>, respectively.
    77.     1 = Analog input. Pin is assigned as analog input(1).
    78.     0 = Digital I/O. Pin is assigned to port or special function.
    79.      */
    80.     ANSELbits.ANS=0;
    81.     /*
    82.      ADON: ADC Enable bit
    83.     1 = ADC is enabled
    84.     0 = ADC is disabled and consumes no operating current
    85.      */
    86.     ADCON0bits.ADON=0;
    87.  
    88.  
    89.     //PINA_DDR = 0;
    90.     //PINB_DDR = 0;
    91.     //PINC_DDR = 0;
    92.     //PIND_DDR = 0;
    93.  
    94.  
    95.  
    96.     TRISIObits.TRISIO1=0;  //set GP1 output
    97.     TRISIObits.TRISIO5=0;  //set GP5 output
    98.  
    99.     GPIObits.GP1 = 0;  //turn LED1 off
    100.     GPIObits.GP5 = 0;  //turn LED5 off
    101.  
    102.     //PINA=0;
    103.     //PINB=0;
    104.     //PINC=0;
    105.     //PINC=0;
    106. }
     
    Last edited: Nov 18, 2012
  14. MMcLaren

    Well-Known Member

    Feb 14, 2010
    759
    116
    You have to disable the analog comparator functions too (example 4.1 in the datasheet).

    Good luck on your project.

    Happy Holidays. Mike
     
  15. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,388
    1,605
    MMcLaren got it. The comparators are by default turned on at power up, and also need to be disabled using:

    CMCON0bits.CM = 0b111;

    (See FIGURE 8-4: COMPARATOR I/O OPERATING MODES)

    When I added this to your code the simulator (I ran your code plus some mods) was able to correctly set all possible I/O lines high, and also set LED1 and then LED2.

    When the comparator is active the I/O pins associated with then are read as zero. They may be set correctly, but read incorrectly. This is why setting the LED2 and then the LED1 bit worked.

    It is interesting that the simulator does not show this correctly: it shows the bits not being able to be set, which is different from the hardware. I've noted a few simulator discrepancies such as this, but they are limited to cases where the settings are incorrect anyway. Properly set config settings will simulate correctly, improper setting simulate differently then the actual hardware will act, but never in a way that hides the error.

    I just noticed your line "GPIO |= (1<<LED1)+(1<<LED2);" also worked, as it compiles to a simple write statement so no read is required. However, you have mixed logical statements (1<<LED1) with arithmetic statement (the +) which is generally something to be avoided as a style issue, but may in some cases (such as signed variables) may lead to undesired results.

    In case you have not found it, the simulator is built inside MPLAB, and can be accessed by selecting MPLAB SIM in the Debugger | Select Tool menu item. It is a very useful program!
     
  16. takao21203

    Distinguished Member

    Apr 28, 2012
    3,577
    463
    This depends on the compiler.

    Some will produce the arithmetic value, because the + operator is arithmetic. If for instance && would be used, then it would interprete the values as boolean. But not neccessarily. You'd have to look up the ANSI specification what to do in such a case.

    (1<<value) is not neccessarily a logical value. I am not even sure if it logical at all. Aren't shift operations arithmetic by default?
     
  17. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    Thanks for helping me Mike, you have a good holiday too :)

     
  18. bug13

    Thread Starter Well-Known Member

    Feb 13, 2012
    1,208
    38
    Thank you so much for helping me on this and taking your time to explain it to me in details.

    It looks like I will need to look at the datasheet more thoroughly, as a bad habit of mine, I usually jump the section that I will be using and don't pay too much attention to other parts of the datasheet.

    And thanks for point out the DEBUG SIM in mplab, this is an area I will need to spend some more time on.

    as for this, I was intended to use something like this:
    Code ( (Unknown Language)):
    1. GPIO |= (1<<LED1)|(1<<LED2);
    I am not sure why I typed "+" instead.
     
    Last edited: Nov 19, 2012
  19. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,388
    1,605
    Well you are most welcome. I actually quite enjoy "puzzle problems" such as this, especially when the problem is as well defined as your.

    Reading the data sheets actually became easier in the last decade. In the past some of the more subtly interactions were not flagged as well as they are now. Yes, you got caught this time but next time you either will not get caught or wiggle right out of the trap.

    Using the | or a + to combine bits is mostly a style issue, but if you accidentally include the same bit twice you'll be very happy you or'ed bits instead of added them.

    Good luck!
     
  20. THE_RB

    AAC Fanatic!

    Feb 11, 2008
    5,435
    1,305
    As a rule on PIC 10/12/16F you should NEVER write to a single bit on an output port.

    It is good form to always use a shadow register, set or clr the bits in that shadow register and then write it once to the PORT as a whole register.

    Not only does that fix RMW faults like the one you had, it also allows some benefits like ensuring all the PORT pins change at the same instant and in one single instruction. That can be useful in making manual PWM outputs and LED multiplexing etc.
     
    bug13 likes this.
Loading...