EmbeddedRelated.com
Forums

Interrupter not working

Started by pseudo_ral March 6, 2012
Hi All,

Would appreciate some help on the code below which is supposed to interrupt a processor every PERIOD seconds.

The code works with anything up to a PERIOD of 400 but any increase on that does not function (no interrupt is produced). I suspect it is a register size limit or something. Increasing the number of bytes available to Two_Sec_Count makes no difference.

My code knowledge is extremely limited, any assistance appreciated
Regards,
Ralph.
#define PERIOD 800 // time in seconds - must be an even number
#define MULTIPLE (PERIOD/2)

Two_Sec_Count DS 1
Timer_Overflow
PUSH R12
MOV.B &Two_Sec_Count,R12
INC R12
CMP #MULTIPLE,R12
JNE Not_flag
BIS #TIMER_OVERFLOW,&System_Flags
CLR R12
Not_flag
MOV.B R12,&Two_Sec_Count
POP R12
BIC #0xF0,0(SP)
RETI

Beginning Microcontrollers with the MSP430

wHY ARE YOU DOING THIS IN AN INTERRUPT? TOTALLY THE WRONG WAY TO GO
ABOUT THINGS . yOUR ENTIRE PROCESS IS STALLED UNTIL you complete this.
Secondly why are you using a loop when you have some excellent timer
facilities at you rdisposal? Thirdly you don't show how you have the
timer configured, it could well be, for example that you have set the
time into RUN TO mode and it can't count higher than 400. Fourth, why
use an absolute value to clear the stored status register? The code
becomes far clearer if you name this bit pattern , for example:-

bic #LPM4,0(SP)

It appears that the intent is for this code to interrupt on timer
overflow, then hang in the ISR until some delay has past before waking
up. WHY? why not simply extend the sleep period, or why not trigger a
second timer to force the delay if you really need a second period of time?

Hanging in the interrupt has lots of drawbacks, firstly you are drawing
active current while doing nothing but loop, secondly your CPu is doing
nothing constructive so why wake it up in the first place, thirdly it is
generally bad programming practice to do more than the barest minimum
needed to process an event in an ISR. Typically an ISR might increment a
count, set a flag or push a task onto a queue then exit. this ensures
minimal time in active mode, minimum loss of processor availability and
lowest risk of missing other interrupt events.

Cheers

Al

There are many ways to do this, every one I can think of is better!

On 6/03/2012 11:56 PM, pseudo_ral wrote:
> Hi All,
>
> Would appreciate some help on the code below which is supposed to interrupt a processor every PERIOD seconds.
>
> The code works with anything up to a PERIOD of 400 but any increase on that does not function (no interrupt is produced). I suspect it is a register size limit or something. Increasing the number of bytes available to Two_Sec_Count makes no difference.
>
> My code knowledge is extremely limited, any assistance appreciated
> Regards,
> Ralph.
> #define PERIOD 800 // time in seconds - must be an even number
> #define MULTIPLE (PERIOD/2)
>
> Two_Sec_Count DS 1
> Timer_Overflow
> PUSH R12
> MOV.B&Two_Sec_Count,R12
> INC R12
> CMP #MULTIPLE,R12
> JNE Not_flag
> BIS #TIMER_OVERFLOW,&System_Flags
> CLR R12
> Not_flag
> MOV.B R12,&Two_Sec_Count
> POP R12
> BIC #0xF0,0(SP)
> RETI
>
> It appears that the intent is for this code to interrupt on timer
overflow,
> then hang in the ISR until some delay has past before waking up. WHY? why
not
> simply extend the sleep period, or why not trigger a second timer to force
the
> delay if you really need a second period of time?

There's no loop in the code, it seems to be straight line with a conditional
kink. However, the guy is using byte storage and comparing to 400. Yeah.

--
Paul Curtis, Rowley Associates Ltd http://www.rowley.co.uk
SolderCore running Defender... http://www.vimeo.com/25709426

Thank you for your prompt responses and whilst I appreciate your comments our products based on this code have been working fine for seven years, draw the lowest current possible, deliberately sit in a 'non-productive loop' and multiple timers are already in use elsewhere in the code.

However as I have already pointed out I struggle to understand code (though this code was originally produced by a person who was a highly valued contributor to this group).

My question is why does it work up to a PERIOD set to 400 (seconds) but no higher?

I have included (below) some of the interrupt vector and start bit interrupts;

Regards
Ralph

COMMON INTVEC
DS 6
DS 2
DS 8
DW Timer_A1_Interrupt
DW Timer_A0_Interrupt

RSEG DATA16_Z

Two_Sec_Count DS 1

RSEG CODE

TIMER_OVERFLOW EQU 1

Timer_A1_Interrupt
ADD.W &TAIV,PC
RETI
JMP Timer_Module_1
JMP Timer_Module_2
JMP Timer_Module_3
JMP Timer_Module_4
JMP Timer_Overflow
JMP Timer_Reserved_1
JMP Timer_Reserved_2

Timer_Module_2
BIS #SEARCH_TIME_EXPIRED,&System_Flags
;;_Finished
BIC #0xF0,0(SP) RETI

Timer_Module_3
Timer_Module_4


> Thank you for your prompt responses and whilst I appreciate your comments
our
> products based on this code have been working fine for seven years, draw
the
> lowest current possible, deliberately sit in a 'non-productive loop' and
> multiple timers are already in use elsewhere in the code.
>
> However as I have already pointed out I struggle to understand code
(though
> this code was originally produced by a person who was a highly valued
> contributor to this group).

You have a FET, yes? That means you have a debugger. Is the art of
debugging lost? How about tracing through the code and looking at what it
does? I've already given you a hint.

--
Paul Curtis, Rowley Associates Ltd http://www.rowley.co.uk
SolderCore running Defender... http://www.vimeo.com/25709426

On 03/06/2012 08:54 AM, pseudo_ral wrote:
> My question is why does it work up to a PERIOD set to 400 (seconds) but no higher?
>

The value stored at Two_Sec_Count is always in the range of 0 to 255
(0x00 to 0xff) as is usual for bytes. If you want to represent larger
numbers you must use more bits.

--
David W. Schultz
http://home.earthlink.net/~david.schultz
"Who? What? Where? When? Aahhhg!" - Duck Dodgers
Thank you David, a concise, clear response.

I had thought just extending Two-Sec_Count to 2 bytes (which I thought was Two_Sec_Count DS 2) or more would go some way to solving the limitation but it made no difference.

I would love to be able to monitor R12 and others but IAR doesn't seem to allow me to do that (attempts to add it to Watch are unsuccessful).

I have made contact with the original developer who may be able to assist later today (I am no developer as you can probably tell).

Thank you again.

Regards
Ralph

> I would love to be able to monitor R12 and others but IAR doesn't seem to
> allow me to do that (attempts to add it to Watch are unsuccessful).

You look at it in the Registers window...
>
> I have made contact with the original developer who may be able to assist
> later today (I am no developer as you can probably tell).
>
> Thank you again.
>
> Regards
> Ralph
>
>
Thanks Paul

R12 is clearly not going to be large enough to store the count I require (800 secs) so I will wait for assistance later today.

Thank you again.
Ralph

> R12 is clearly not going to be large enough to store the count I require
(800
> secs) so I will wait for assistance later today.

R12 is an internal 16-bit register and it *is* big enough. The problem is
you are using the '.B' suffix, to store a BYTE, which can only be between 0
and 255. Hence, MOV.B x, R12 can never make R12 greater than 255. You will
need to use a .W extension and change the data reservation appropriately.

--
Paul Curtis, Rowley Associates Ltd http://www.rowley.co.uk
SolderCore running Defender... http://www.vimeo.com/25709426