I have an ATmega328P Arduino and am trying to make the LED blink using assembly without any additional includes. The goal is to have it blink for 1 second on, then 1 second off, however, it blinks incredibly quickly and is barely perceptible. Am I doing something incorrectly?

; DDRB       = 0x04
; PORTB      = 0x05
; TCCROB     = 0x25
; TCNT0      = 0x26
; 0b00000000 = OFF
; 0b00100000 = ON

    ldi r16, 0b00100000     ; Set r16 to LED ON value
    out 0x04, r16           ; Set DDRB to output
    out 0x05, r16           ; Set PORTB to ON
    ldi r16, 0b00000101     ; Set r16 with prescaler 1024 value
    out 0x25, r16           ; Set the TCCROB to 1024
    ldi r20, 0              ; Reset r20
    ldi r21, 1              ; Hold value 1 to increment r20
    in r17, 0x26            ; Read the timer 
    cpi r17, 128            ; Check to see if the timer is halfway done (1/2 second, max 255 value)
    brbc 0, increment       ; If r17 <= 128 (max 255 val before resets to 0), invert current LED state
    rjmp loop
    adc r20, r21            ; Add r21 (1) to r20 (with carry)
    brbs 0, reset           ; If the carry branch is set, reset the register
    rjmp loop               ; Otherwise, run the clock
    ldi r20, 0              ; Reset the delay register
    cpi r16, 0b00000000     ; Check if r16 is set to LED OFF
    brbs 1, switchon        ; If it is, change it to ON
    ldi r16, 0b00000000     ; Otherwise, change it to OFF
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock
    ldi r16, 0b00100000     ; Set r16 to LED ON value
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock
  • It's part of my code. I thought that line should be there, but when it isn't commented, it doesn't blink, it remains solidly lit (as far as I can tell). Apr 15 '16 at 6:01
  • How long is your delay? It might be too small to see the LED blink.
    – Paul
    Apr 15 '16 at 6:48
  • 1
    “incredibly quickly”? It's so slow I can even see it blinking! Apr 15 '16 at 15:10

There is a bug with the logic of your program: once TCNT0 gets larger than 128, it will continue to be larger than 128 for quite some time... during which time you are continuously incrementing your r20 counter. The simplest fix would be to reset TCNT0 right after the increment label, with something like out TCNT0, zero_reg. Then you may want to compare TCNT0 with 61 instead of 128, in order to get closer to the desired blink frequency of 0.5 Hz.

Other than that, there are quite a few problems in the way you write your code. jwpat7 has already pointed out some of them, and here are more:

  • You should not increment a register with adc, but with plain add. Both instructions update the carry bit, but adc also adds the previous value of the carry bit to the target register.
  • Better yet, increment with inc, as it's purpose is more obvious. But then you should test the zero bit, as inc does not update the carry bit.
  • There is little point in resetting r20 once it has overflowed, as it is zero at this point.

There are also quite a few ways the program could me made simpler. The simplest would probably be to configure timer 1 to deliver an interrupt every second, and toggle the LED in the ISR. If you do not want interrupts, you can configure counter 1 in much the same way, only without the “interrupt enable” bit, and check the interrupt flag in a busy loop.

Below is a slightly simplified version of your program. I tried to keep it as close as possible to yours, without using timer 1, nor interrupts, and not even the interrupt flag. The code checks for timer 0 overflowing by comparing its current value with the previous one.

.set PINB,     0x03
.set DDRB,     0x04
.set TCCR0B,   0x25
.set TCNT0,    0x26
.set LED_MASK, 0b00100000
.set PS_1024,  0b00000101

    ldi r16, PS_1024    ; Set r16 with prescaler 1024 value
    out TCCR0B, r16     ; Set the TCCROB to 1024
    ldi r16, LED_MASK   ; Set r16 to the LED bit
    out DDRB, r16       ; Set LED pin to output
    clr r18             ; Clear the saved timer
    ldi r20, 61         ; Initialize our software counter
    in r17, TCNT0       ; Read the timer
    cp r17, r18         ; Compare with previous value
    mov r18, r17        ; Save current value
    brsh check_timer    ; unless the timer has decreased, repeat
    dec r20             ; decrement the software counter
    brne check_timer    ; if not zero, go back to checking the timer
    out PINB, r16       ; toggle the LED
    rjmp loop

If we can use the timer overflow interrupt flag, then the test is simpler, and we do not need to store the previous and current values of the timer in registers:

    sbis TIFR0, TOV0    ; unless the timer has overflowed
    rjmp check_timer    ; check again for overflow
    sbi TIFR0, TOV0     ; otherwise clear the overflow flag
  • Thank you. I am new to assembly and the low-level mindset and this was very informative and helpful. The reason I didn't use more self-documenting instructions (like dec) is because I'm limited to an instruction subset as a challenge for an assignment. Apr 15 '16 at 21:55
  • Would you mind detailing a little bit more on why you use the value 61 exactly? Apr 15 '16 at 22:26
  • @JosueEspinosa: The Uno is clocked at F_CPU = 16 MHz. The timer ticks at F_T0 = F_CPU/1024. It overflows with a frequency F_TOV0 = F_T0/256. You want the toggling of the LED to happen at 1 Hz ≈ F_TOV0/61. Apr 16 '16 at 8:03
  • 1
    Just to clarify what out PINB, r16 does, Atmel datasheets say "Writing a logic one to PINxn toggles the value of PORTxn, independent of the value of DDRxn." Apr 17 '16 at 18:19

It appears that your intent is to set TCCROB up with a 1024x prescale, and each time it counts up to 128, increment r20, and toggle the LED each time r20 overflows to zero.

That sequence ought to take 1024/16M · 128 · 256 seconds, or about 2.1 seconds, a fairly leisurely toggle rate. If instead it's flashing a hundred or more times too fast, probably one of the comparisons is backwards.

Some of the following items might make it easier to debug the problem:

• Instead of brbc 0,..., say brcc ...
• Instead of brbs 0,..., say brcs ...
• Instead of brbs 1,..., say breq ...

That is, use proper mnemonic branch instruction names, rather than referencing SREG bits.

• If my arithmetic was correct and your branch tests were fixed, your rate would be off by a factor of two. To fix this problem, you could replace one of the more problematic tests (testing for carry clear after cpi r17, 128) with cpi r17, 0 followed by a breq, and change the prescale factor to 256 instead of 1024, which gives 256/16M · 256 · 256 seconds, or about 1.05 seconds.

• Use named constants for register names instead of 0x04, 0x05, 0x25, and 0x26.

• Instead of your lengthy toggle sequence use EOR to toggle the value. That is, replace

    cpi r16, 0b00000000     ; Check if r16 is set to LED OFF
    brbs 1, switchon        ; If it is, change it to ON
    ldi r16, 0b00000000     ; Otherwise, change it to OFF
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock
    ldi r16, 0b00100000     ; Set r16 to LED ON value
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock


    eor   r18, r16          ; Toggle LED bit
    out   PORTB, r18        ; Set the LED to the new state
    rjmp  loop              ; Go run the clock

where r16 and r18 are initialized as follows in setup:

    ldi r16, 0b00100000     ; bit mask for LED pin
    mov r18, r16

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.