0
\$\begingroup\$

I am trying to write my first program in assembler for PIC16F886. The program is supposed to do the following:

  • Read ADC value on channel AN0
  • If the value is smaller than 128 - turn LED off
  • Otherwise - turn LED on

The schematic is rather simple: enter image description here

And here is my assembly code.

    list        p=16f886        ; list directive to define processor
    #include    <p16f886.inc>   ; processor specific variable definitions

    __CONFIG    _CONFIG1, _LVP_OFF & _FCMEN_ON & _IESO_OFF & _BOR_OFF & _CPD_OFF & _CP_OFF & _MCLRE_ON & _PWRTE_ON & _WDT_OFF & _INTRC_OSC_NOCLKOUT
    __CONFIG    _CONFIG2, _WRT_OFF & _BOR21V

    org 00                      ;start at 0x00

VALUE EQU 0x80                  ;value to compare adc results

    cblock  0x20
        RESULTLO                ;ADC 8-bit result
    endc

port_config
    movlw b'11111110'           ;set RB0 on PORTB to output
    movwf TRISB

start
    bsf PORTB, 0                ;turn LED on

adc_config
    BANKSEL ADCON1
        movlw 0x00                  ;left justify              
    MOVWF ADCON1                ;Vdd and Vss as Vref
    BANKSEL TRISA               ;
    BSF TRISA,0                 ;Set RA0 to input
    BANKSEL ANSEL               ;
    BSF ANSEL,0                 ;Set RA0 to analog

adc_sample
    BANKSEL ADCON0;
    MOVLW b'11000001'           ;ADC Frc clock,
    MOVWF ADCON0                ;AN0, On
    BSF ADCON0,GO               ;Start conversion
    BTFSC ADCON0,GO             ;Is conversion done?
    GOTO $-1                    ;No, test again

    BANKSEL ADRESH              
    MOVF ADRESH,W               ;Read lower 8 bits
    MOVWF RESULTLO              ;Store in GPR space

    ; if RESULT <= VALUE
    movf RESULTLO,w
    addlw 255 - VALUE
    skpnc
    goto led_off

    ; if RESULT > VALUE
    movf VALUE,w
    addlw 255 - RESULTLO + 1
    skpnc
    goto led_on

    ;loop
    goto adc_sample

led_on
    bsf PORTB, 0
    goto adc_sample
led_off
    bcf PORTB, 0
    goto adc_sample

    end

I expect the led to turn on/off when I change the resistance of the pot but instead the led constantly stays on.

Update: I suspect it has something to do with the way I compare RESULT and VALUE since I get the following warning during compilation: Argument out of range. Least significant bits used.

Update: As I've mention in comments, the schematic is taken directly from Proteus. On the real device the controller is powered from 5V source, there should be a limiting resistor in series with a led (unless I drive the mcu from a smaller voltage) and nMCLR is tied to +5V through a 5k resistor.

\$\endgroup\$
  • \$\begingroup\$ You'll want to add a resistor in series with that LED. Without one, it's likely overdriving both the LED and the PIC's output driver. It's possible that the PIC driver is already damaged, and is now "always on". You could test that by simply toggling the pin on and off (with a delay in between) and verifying that the LED does, in fact, turn off when it's supposed to. If the output driver is damaged, you can probably just move the LED to a different pin and change the code appropriately. \$\endgroup\$ – bitsmack May 20 '14 at 6:35
  • \$\begingroup\$ Right now I'm only testing the circuit in Proteus, so I didn't add a resistor. However, pic datasheet says: "High current source/sink for direct LED drive". I suppose it means I can safely drive leds without a resistor? \$\endgroup\$ – Ashton H. May 20 '14 at 8:38
  • \$\begingroup\$ No, don't drive a standard LED without a resistor or some current limiting device. No good will come of it if you do. BTW what is RV1 connected to? \$\endgroup\$ – Andy aka May 20 '14 at 8:47
  • \$\begingroup\$ It forms a voltage divider between +5V bus and ground. \$\endgroup\$ – Ashton H. May 20 '14 at 8:50
  • \$\begingroup\$ A "standard" LED can be driven at 20mA for maximum brightness. This varies; check the datasheet for the one you're using. The PIC's "direct LED drive" statement means that it can provide that much current. It used to be that a microcontroller had to drive a transistor, which then drove the LED... Anyway, to choose the resistance: R = (Vcc - Vf) / I, where Vcc is your supply voltage, Vf is the forward voltage of the LED (see the datasheet), and I is the desired current (keep it under 20mA). e.g.: (5V-2V)/0.02A = 150-Ohm. \$\endgroup\$ – bitsmack May 20 '14 at 8:52
3
\$\begingroup\$

You are setting the ADFM bit in ADCON1, which, as your comments indicate, right justifies the result. This results in the bits being stored as follows:

    (ADRESH)  7 6 5 4 3 2 1 0     (ADRESL)  7 6 5 4 3 2 1 0
   10-bit ADC reading:    9 8               7 6 5 4 3 2 1 0

You then read in the low 8-bits of the result, ADRESL. You never read in the high byte, ADRESH, containing the top two bits of the result. Thus you are throwing them away, which are the most signifcant (literally). When your pot reaches the mid-point, the top bit of the result changs, but you never see it.

Instead, you want to left justify the result, by setting ADFM to 0, and read in just the top 8-bits (ADRESH). Your test for 128 is now valid. There is no harm in throwing away the lower 2 bits in ADRESL.

                (ADRESH)  7 6 5 4 3 2 1 0    (ADRESL)  7 6 5 4 3 2 1 0
   10-bit ADC reading:    9 8 7 6 5 4 3 2              1 0
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thanks, I haven't noticed that - updated my code. Unfortunately, it still doesn't work \$\endgroup\$ – Ashton H. May 20 '14 at 8:33
1
\$\begingroup\$

I have managed to fix my code eventually. It had the following problems:

  1. I have forgot to add a small delay for ADC to initialize properly
  2. PORTB was not initialized properly
  3. Comparison routine did no work as I expected
    list        p=16f886    ; list directive to define processor
    #include    <p16f886.inc>   ; processor specific variable definitions

    __CONFIG    _CONFIG1, _LVP_OFF & _FCMEN_ON & _IESO_OFF & _BOR_OFF & _CPD_OFF & _CP_OFF & _MCLRE_ON & _PWRTE_ON & _WDT_OFF & _INTRC_OSC_NOCLKOUT
    __CONFIG    _CONFIG2, _WRT_OFF & _BOR21V

    org 00                      ;start at 0x00

VALUE EQU 0x80                  

    cblock  0x20
        RESULTHI                ;ADC 8-bit result
    endc

port_config
    bsf    STATUS,RP0           ;select Registers at Bank 1
    movlw b'11111111'           ;set PORTA to input
    movwf TRISA
    movlw b'11111110'           ;set RB0 on PORTB to output
    movwf TRISB
    bcf    STATUS,RP0           ;select Registers at Bank 0

start
    bsf PORTB, 0                ;turn LED on

adc_config
    BANKSEL ADCON1              
    movlw 0x00                  ;left justify
    MOVWF ADCON1                ;Vdd and Vss as Vref
    BANKSEL TRISA               ;
    BSF TRISA,0                 ;Set RA0 to input
    BANKSEL ANSEL               ;
    BSF ANSEL,0                 ;Set RA0 to analog
    BANKSEL ADCON0;
    MOVLW b'11000001'           ;ADC Frc clock,
    MOVWF ADCON0                ;AN0, On
    call delay                  ;Allow some time to settle
    CLRF ADRESH

adc_sample
    BSF ADCON0,GO               ;Start conversion
    BTFSC ADCON0, GO            ;Is conversion done?
    GOTO $-1                    ;No, test again
    call delay                  ;Allow some time to settle
    BANKSEL ADRESH
    MOVF ADRESH,W               ;Read high 8 bits
    MOVWF RESULTHI              ;Store in GPR space

compare_result
    clrc
    movlw VALUE
    subwf RESULTHI,w
    btfsc STATUS, Z             ;if (RESULTHI <= VALUE)
    goto led_off                ;turn led off
    btfsc STATUS, C
    goto led_on                 ;otherwise: turn led on
    goto led_off

    goto adc_sample             ;loop forever

led_on
    bsf PORTB, 0
    goto adc_sample
led_off
    bcf PORTB, 0
    goto adc_sample

delay
COUNT1 equ 0xFF
loopl0
    decfsz COUNT1,1
    goto loopl0
    return

    end
|improve this answer|||||
\$\endgroup\$
  • 1
    \$\begingroup\$ Per my answer, when you modified your code, the comment for the instruction MOVF ADRESH,W should be ;Read high 8 bits. You are reading the high 8 bits of the 10 bit ADC result and throwing away the bottom two. Code is right, just the comment is wrong. \$\endgroup\$ – tcrosley May 20 '14 at 22:19
  • \$\begingroup\$ You are absolutely right - fixed! \$\endgroup\$ – Ashton H. May 20 '14 at 22:31
0
\$\begingroup\$

There are some serious problems with your cicuit:

  1. You forgot to power the PIC. Actually, the power and ground pins aren't even shown. Obviously something is wrong here.

  2. There is nothing to limit the current thru the LED. Green LEDs typically drop about 2.1 V. Trying to drive it from higher voltage will either cause excessive current thru the LED or excessive current out the port pin. Depending on what exactly you eventually connect for power, that may also cause the whole power supply to collapse, possibly resetting the PIC.

    Common T1-3/4 LEDs are rated for 20 mA, but will be plenty bright enough in a office environment at half that. Let's say you eventually power the PIC from a stiff 5 V supply, then you want a resistor that drops 5V - 2.1V = 2.9V at 10 mA. (2.9 V)/(10 mA) = 290 Ω, so the common value of 300 Ω should give you a clearly visible result without pushing either the LED or the PIC to its limits.

  3. There is no bypass cap, even if there magically were power connections somewhere.

Until these things are fixed, it is pointless to think about the firmware.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ It's obviously powered, since he said the LED is always on. I agree with your other points. \$\endgroup\$ – tcrosley May 20 '14 at 20:17
  • 2
    \$\begingroup\$ @tcrosley: Then there are important parts of the circuit the OP is not showing us. That is just as bad. We don't know, for example, what voltage the PIC is actually powered at, how stiff that supply is, and whether it is properly bypassed. It is not up to the OP to decide what is relevant information. Leaving out the power supply connections is just plain irresponsible and misleading. \$\endgroup\$ – Olin Lathrop May 20 '14 at 20:43
  • \$\begingroup\$ @OlinLathrop as I've mentioned, I'm only testing the circuit in Proteus simulator. The schematic was just to show where the pot and led are connected. Applying the power to the uC is quite obvious and is not worth mentioning, especially if my question was about code. \$\endgroup\$ – Ashton H. May 20 '14 at 22:01
  • \$\begingroup\$ @AshtonHearts, you'd be surprised at how difficult it can be to assess a questioners level of expertise. When you answer enough such questions, you realize nothing is obvious, and assuming a questioner knows more than what's posted is a losing proposition that leads to frustration. \$\endgroup\$ – Scott Seidman May 21 '14 at 3:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.