Timer interrupt not working

Go To Last Post
28 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello everyone,
I've written a small program that is suppose toggle a LED on and off every second. My oscillator is 8MHz. My LED turns on and just stays on. Why isn't it turning on and off? My development board is setup so that zero turns the LED on and one turns the LED off.


.def tmp=r16

.cseg
rjmp reset

.org 0x00c ;Timer interrupt vector
rjmp turnon

reset:
ldi tmp, low(ramend)
out spl, tmp
ldi tmp, high(ramend)
out sph, tmp

ldi tmp, 0b10000000 ;Globally enable interrupts
out sreg, tmp 

sbi ddra, 0 ; Set pin0 on PORTA as output
sbi porta, 0 ; turn LED off

ldi tmp, 0b00001100; CTC mode with OCR1A as TOP
out tccr1b, tmp    ; prescaler of 256

clr tmp
ldi tmp, 1<<ocie1a ;output compare a match enable 
out timsk, tmp

ldi tmp, high(31250)
out ocr1ah, tmp
ldi tmp, low(31250)
out ocr1al, tmp

main:
rjmp main

turnon:
cbi porta, 0 ; Turn LED on
reti

Last Edited: Fri. Jan 3, 2014 - 08:18 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Where in your code do you turn the led off based on the timer interrupt?

Note it does help if you put some comments in your code. How do we know what mode the timer is, how fast etc. you just have lots of magic numbers.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ldi tmp, 0b10000000 ;Globally enable interrupts
out sreg, tmp

The global int enable should be the very LAST thing you do just before main and it only needs the SEI instruction.

John Samperi

Ampertronics Pty. Ltd.

https://www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In "turnon:" do not turn Led on, instead toggle it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I did the exact same thing in C and it works fine. What I wrote above is a "translation" in assembly.

#include 
#include 

int main(void)
{
	sei();
	
	DDRA |= 1<<PINA0;

	TCCR1B |= 1<<CS12 | 1<<WGM12;
	TIMSK |= 1<<OCIE1A;
	OCR1A = 31250;
	
    while(1)
    {
    }
}

ISR(TIMER1_COMPA_vect)
{
	PORTA ^= 1<<PINA0;
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Visovian wrote:
In "turnon:" do not turn Led on, instead toggle it.

How do I toggle it in assembly?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Again the sei(); should be pretty much the LAST thing you do AFTER everything has been initialised

Quote:
How do I toggle it in assembly?
check if the bit is zero, if so set it, if one then clear it. If you have a reasonably new type of chip you can also write a 1 to the PINA bit to toggle it.

John Samperi

Ampertronics Pty. Ltd.

https://www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Personally, I would just write it in C and be done with it!

If you want to know how to do something in ASM, write the C first and inspect the generated code. You will pick up some good tips. e.g.

   PORTA ^= 1<<PINA0;   // uses XOR 
   PINA = 1<<PINA0;     // uses a single OUT

I am guessing that you have a Mega16/32 which do not support the PINA toggle.
A Mega164/324 supports PINA toggle but calls TIMSK TIMSK1.

Have you ever wondered about

    result = (x << 4);

See how clever the C compiler is.

David.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Depending on the chip used this may or may not work, it's old code before I started to use macros.

;Flash running LED
FLIPLED:
	lds		temp,Led_port
	ldi		temp1,1<<Led
	eor		temp,temp1			;toggle led
	sts		Led_port,temp
	ret

John Samperi

Ampertronics Pty. Ltd.

https://www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am using an ATMega32. I really want to do it in assembly. This is my idea of toggling as js described it. The LED doesn't flash every second but is just a bit dimmer than normal.

turnon:
sbis porta, 0
sbi porta, 0
sbic porta, 0
cbi porta, 0
rjmp turnon
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
;toggle PORTA.0
in r16,PORTA
ldi r17,(1<<PINA0)
eor r16,r17
out PORTA,r16
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

js wrote:
Depending on the chip used this may or may not work, it's old code before I started to use macros.
;Flash running LED
FLIPLED:
	lds		temp,Led_port
	ldi		temp1,1<<Led
	eor		temp,temp1			;toggle led
	sts		Led_port,temp
	ret

I just tried it, doesn't work unfortunately. I'm not sure if I adapted it properly for my code. I can't say I understand it. LDS Load direct from SRAM? STS Store direct to SRAM? What is in SRAM?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Visovian wrote:

;toggle PORTA.0
in r16,PORTA
ldi r17,(1<<PINA0)
eor r16,r17
out PORTA,r16

Works perfectly! Thank you.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm trying to get all the LEDs on PORTA to turn on and stay on one by every second and then start over. I've almost done it but the last LED doesn't turn on. I don't want to start over by jumping to RESET. Before I started using interrupts I did something like this:

ser tmp
main:
out porta,tmp
lsl tmp
rcall delay
brcs main ; branch if carry set
ldi tmp, 0xff
rjmp main 

This turns on leds 0 to 6 and then and then starts over. I want it to turn on LEDs 0 to 7 and start over.

.def tmp=r16
.def tmp1=r17

.cseg
rjmp reset

.org 0x00c
rjmp turnon

reset:
ser tmp1
ldi tmp, 0b10000000
out sreg, tmp 

ldi tmp, low(ramend)
out spl, tmp
ldi tmp, high(ramend)
out sph, tmp

ldi tmp, 0xff
out ddra, tmp
out porta, tmp

ldi tmp, 0b00001100
out tccr1b, tmp

ldi tmp, 0b00010000
out timsk, tmp

ldi tmp, high(31250)
out ocr1ah, tmp
ldi tmp, low(31250)
out ocr1al, tmp

main:
rjmp main

turnon:
out porta, tmp1
lsl tmp1
sbrs tmp1, 7 ; skip if bit 7 is set 
ser tmp1
reti
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You want to test the carry bit rather than bit 7. With the lsl instruction, bit 7 falls into carry.
Still no useful comments - get into the habit of writing comments. When you look at the code 6 months later you'll be able to understand it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
You want to test the carry bit rather than bit 7. With the lsl instruction, bit 7 falls into carry.

How do I test it? I don't want to branch using BRCS. I tried using SBRS SREG, 7 and SBIS SREG, 7 and I get an error for both.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why would you not want to use brcs/brcc? Why did sbrs and sbis not work?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
Why would you not want to use brcs/brcc? Why did sbrs and sbis not work?

None of these work.

;using brcs with label inside of subroutine
turnon:
out porta, tmp1
lsl tmp1
brcs here
here: 
ser tmp1
reti

;using brcs with label outside of subroutine
turnon:
out porta, tmp1
lsl tmp1
brcs here
reti

here: 
ser tmp1

;using sbrs
turnon:
out porta, tmp1
lsl tmp1
sbrs sreg, 7 ;compiler error invalid register
ser tmp1
reti

;using sbis
turnon:
out porta, tmp1
lsl tmp1
sbis sreg, 7 ;compiler error operand 1 out of range:0x3f
ser tmp1
reti

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
;using brcs with label inside of subroutine
turnon:
out porta, tmp1
lsl tmp1
brcs here
here: 
ser tmp1
reti

The brcs instruction branches if the condition (carry set) is true. If the condition isn't true (carry clear) then it doens't branch, it just continues on to the next instruction. So what do you think will happen with your above code?

;using brcs with label outside of subroutine
turnon:
out porta, tmp1
lsl tmp1
brcs here
reti

here: 
ser tmp1

So when the carry is set you, how will you ever reach the reti?

;using sbrs
turnon:
out porta, tmp1
lsl tmp1
sbrs sreg, 7 ;compiler error invalid register
ser tmp1
reti

The sbrs instruction takes one of the general purpose registers (r0 -> r31) as an argument. You have tried to pass one of the I/O registers (SREG).

;using sbis
turnon:
out porta, tmp1
lsl tmp1
sbis sreg, 7 ;compiler error operand 1 out of range:0x3f
ser tmp1
reti

The sbis instruction can only test the first 32 I/O registers. SREG is the 64th.

You seem to be confusing the various 'skip-if-bit-in-X-is-Y' instructions with the 'branch-if-condition-is-true' instructions. The former skip one instruction. The latter branch directly to any (in range) instruction/label.

If you insist on tackling the world one assembly instruction at a time, for heaven's sake keep a copy of the AVR Instruction Set Manual open on your desktop. All of the answers to all of your questions are in there.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I did it. My logic wasn't that far off. I just copied SREG to another register and then checked if the bit which corresponded to CARRY in SREG was set in that register.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

binaryavr wrote:
I did it. My logic wasn't that far off. I just copied SREG to another register and then checked if the bit which corresponded to CARRY in SREG was set in that register.
But you haven't answered: what's wrong with brcs?

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
binaryavr wrote:
I did it. My logic wasn't that far off. I just copied SREG to another register and then checked if the bit which corresponded to CARRY in SREG was set in that register.
But you haven't answered: what's wrong with brcs?

The problem is that

ser tmp1

will be executed either way.

I'm not really sure. Is it maybe that using a branch in a interrupt subroutine is incorrect? I always think of a branch instruction always branching out of the current "loop" that it is in.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

An isr is just another thread of execution. You can branch, call and jump all you like. As Joey suggests, read the instruction set manual carefully and understand what each instruction does. Atmel gave you brcs/brcc for a good reason as the carry is an important flag. Judicious use will solve your problem. You would be best to run your code in the simulator to see step by step what is happening.


turnon: 
out porta, tmp1 
lsl tmp1
brcs to1
ser tmp1 
to1:
reti 

Almost too simple!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, now that you showed me it does look really simple. This is the only way I could think of:

turnon:
out porta, tmp1
lsl tmp1
in tmp, sreg
sbrs tmp, 0 
ser tmp1
reti 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sounds like you may now understand what you were missing, but:

binaryavr wrote:
joeymorin wrote:
But you haven't answered: what's wrong with brcs?
The problem is that
ser tmp1

will be executed either way.

... you are correct, based on the way you were using the branch instruction.

It seemed to me you were thinking of the two types of instructions as the same. They are not. They are similar in that they test for some condition and alter program flow based on the results of that test, but each tests for different types of conditions, and can alter program flow in different ways.

'Skip' type instructions can examine general purpose registers r0-r31 and I/O registers 0x00-0x1F. 'Branch' type instructions can examine one or more flags in SREG.

'Skip' type instruction alters program flow by skipping one (the next) instruction. A 'branch' type instruction alters program flow by transferring execution to a different, arbitrary instruction. That instruction can be anywhere within the range of the branch instruction. For the AVR architecture, that's any instruction word within the range of -64 to +63 words from the current program counter.

Indeed, the conditional branch instructions are specifically designed for efficiently testing conditions after arithmetical (add, subtract, etc.) and logical (shift, and, or, etc.) operations. Note that with your approach, you need an 'in' (1 cycle, 1 word), and an 'sbrs' (1-3 cycles, 1 word), and if the 'false' execution path needs to be more than the 1 single skipped instruction, you would have needed an 'rjmp/jmp' (2-3 cycles, 1-2 words). All of this can be accomplished with a single 'brcs' (1-2 cycles, 1 word).

Right tool for the right job.

JJ

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
LDS Load direct from SRAM? STS Store direct to SRAM?
And/Or ports located in SRAM area like PORTG where the LED is located for the board where that code came from.

Replace LDS with IN and STS with OUT (as the port you are using is in I/O area) and the code will work.

If you want to learn more about ASM you can also start using AVR001 which are macros to help with code like the above in selecting the correct instruction automatically, in which case you end up using LOAD and STORE instead.

John Samperi

Ampertronics Pty. Ltd.

https://www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How could I reproduce this pattern in the ISR?

 ldi tmp, 0b11111111 
 out porta, tmp
 ldi tmp, 0b11100111 
 out porta, tmp
 ldi tmp, 0b11011011 
 out porta, tmp
 ldi tmp, 0b10111101 
 out porta, tmp
 ldi tmp, 0b01111110 
 out porta, tmp
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A question can be unclear if too brief.
Maybe you hired a keyboard and have to pay every stroke?

Say you want to change the pattern every second in the ISR.

1. You have to count interrupts. Use a register for counter.

2. In the ISR inrement the counter.
If counter=0 display pattern0
If counter=1 display pattern1
If counetr=2 display pattern2 etc.

3. If all patterns done, clear the counter.

Example for 2 patterns

.def overflow_counter = r22

isr:
   push r1                  // save sreg
   in   r1, sreg

   inc  overflow_counter

   cpi  overflow_counter,2  // if counter >= 2 
   brlo isr1                // then counter=0
   clr  overflow_counter    

isr1:
   cpi  overflow_counter,0  // if counter=0
   breq pattern0            // goto pattern0

   cpi  overflow_counter,1  // if counter=1
   breq pattern1            // goto pattern1

pattern0:
   ldi  tmp, 0b11111111
   out  porta, tmp
   rjmp end

pattern1:
   ldi  tmp, 0b11100111
   out  porta, tmp

end:
   out  sreg, r1            // restore sreg
   pop  r1

reti

You can simplify the code when you create a table of patterns and read from it using pointers X, Y, Z.