Multiple (High and Low Priority) Interrupt Issues...

Coding and general discussion relating to the compiler

Moderators: David Barker, Jerry Messina

Post Reply
User avatar
SteveB
Posts: 23
Joined: Fri Oct 06, 2006 1:40 am
Location: Del Rio, TX

Multiple (High and Low Priority) Interrupt Issues...

Post by SteveB » Tue Oct 10, 2006 1:49 am

Hello,
I've been tinkering a little with the compiler and have run into some issues with interrupts. I've got an LAB-X1 from MELabs, setup with a bit of additional harware items, all of which runs without a hitch using code from another compiler. :wink: Starting from the standard "Hello World" type of program, I've worked up to what I've listed below. It will probably look a little familiar, since most of it was based on Help file examples.

Here are the issues:
  1. Both interrupts run as expected when only 1 of them is enabled (by commenting out one or the other "activate" subroutine calls). But, when both are enabled, a change on PortB (by turning the encoder) causes the Pic to hang. I took a quick look at the .lst file to see if I had the priority numbers correct, and saw that address 8h and 18h have the bra's to the correct subs, but didn't look into it any deeper.
  2. Using SAVE(anything meaningful)...RESTORE causes irratic operation when used in either of the interrupt routines. Most noticable is garbage on the LCD in the position just to the right of either of the numeric outputs.

Code: Select all

 // Device Setup
DEVICE = 18f4620
CLOCK = 40
PUBLIC CONFIG
   OSC = HSPLL

// LCD
    #option LCD_DATA = PORTD.4
    #option LCD_RS = PORTD.0
    #option LCD_EN = PORTD.1

// Base Module Includes
    INCLUDE "LCD.bas" 
    INCLUDE "utils.bas"
    INCLUDE "USART.bas"
    INCLUDE "Convert.bas"
        

// Alias
DIM LCDOut AS LCD.Write
DIM HSEROut AS USART.Write
 
// Constants
CONST
    TransDelayMulti = 10,
    UpdateMS = 10,
    BASE_BAR = 0,                  // ASCII value of 0 bar (blank)
    FULL_BAR = 3,                  // ASCII value of ||| bar
    BAR_WIDTH = 5,                // Max width in characters of bar
    MAX_BAR_COUNT = BAR_WIDTH * FULL_BAR // Max bar counts 


// initialise bit patterns...
// programmable characters are available that use codes $00 to $07. 
// Create the bit patterns that make up the bars in the LCD's CGRAM.
// The vertical bars are made up of 8 identical bit patterns  
CONST CGRAM(8*(FULL_BAR+1)) AS BYTE =   ($00,$00,$00,$00,$00,$00,$00,$00,  // base bar
                                        $10,$10,$10,$10,$10,$10,$10,$00,  // 8 x %10000 = |
                                        $14,$14,$14,$14,$14,$14,$14,$00,  // 8 x %10100 = | |
                                        $15,$15,$15,$15,$15,$15,$15,$00)  // 8 x %10101 = | | |
              
'--------------------------------------------------------------------------------
'    Subroutines
'--------------------------------------------------------------------------------

' ---=== output byte pRepValue times ===---
NOINLINE SUB Rep(pValue, pRepValue AS BYTE)
   DIM Index AS BYTE
   Index = 0
   WHILE Index < pRepValue
      LCDOut(pValue)
      INC(Index)
   WEND
END SUB
 
' ---=== display the bar ===---
NOINLINE SUB Bargraph(pLine, pBarValue AS BYTE)
 
   DIM NumberOfBars AS BYTE
   DIM Balance AS BYTE
' ---=== Dim BalanceChar As Byte ===---
 
   NumberOfBars = pBarValue / FULL_BAR
   Balance = pBarValue MOD FULL_BAR
   
   MoveCursor(pLine,1)
   Rep(FULL_BAR,NumberOfBars)  
   LCDOut(Balance)
   Rep(BASE_BAR,BAR_WIDTH - (NumberOfBars + Min(Balance,1)))
END SUB

' ---=== Timer Interrupt Setup ===---
CONST TimerReloadValue = 10
CONST TimerValue = 15536 'Aprrox every 10 msec
DIM Timer0 AS TMR0L.AsWord
DIM Timer0IF AS INTCON.2
DIM Timer0IE AS INTCON.5
DIM Timer0On AS T0CON.7
DIM TimerCounter AS BYTE
DIM ElapsedCounter AS LONGWORD

'#option ISR_SHADOW = false

interrupt OnTimer(2)
    Timer0On = 0 
    Timer0 = TimerValue
    DEC(TimerCounter)
    IF TimerCounter = 0 THEN
        TimerCounter = TimerReloadValue
    ENDIF
    INC(ElapsedCounter)
    Timer0On = 1 
    Timer0IF = 0
END interrupt

SUB ActivateTimer()
    TimerCounter = TimerReloadValue
    Timer0 = TimerValue
    T0CON = %00001000        
    Timer0IF = 0   
    Timer0IE = 1  
    Timer0On = 1 
    ENABLE(OnTimer)
END SUB


' ---=== Rotary Encoder Interrupt Setup ===---

CONST   Rot_Count_Max = 64
CONST   Rot_Count_Min = 0
DIM     Old_Bits AS BYTE
DIM     Rot_Count AS WORD
DIM     RBIF AS INTCON.0
DIM     RBIE AS INTCON.3

interrupt OnChgB(1)
    DIM New_Bits AS BYTE
    RBIE = 0      ' (RBIE) Disable Interrupts
'    save(0)
    New_Bits = PORTB AND (%00110000)
    IF New_Bits = Old_Bits GOTO No_Change_Rot1
    IF (New_Bits.4 XOR Old_Bits.5) = 1 THEN
        Rot_Count = Rot_Count + 1
        IF Rot_Count = Rot_Count_Max+1 THEN 
            Rot_Count = Rot_Count_Max
        ENDIF
    ELSE
        Rot_Count = Rot_Count - 1
        IF (Rot_Count = Rot_Count_Min - 1) OR (Rot_Count = $FFFF) THEN 
            Rot_Count = Rot_Count_Min
        ENDIF
    ENDIF
    Old_Bits = New_Bits
No_Change_Rot1:
    RBIF = 0      ' (RBIF) Clear Flag
'    restore
    RBIE = 1      ' (RBIE) Enable Interrupts
END interrupt

SUB ActivateOnChgB()
    Rot_Count = (Rot_Count_Min + Rot_Count_Max)>>1
    INPUT (PORTB.4)
    INPUT (PORTB.5)
    Old_Bits = PORTB AND %00110000
    '---=== Initialize Interrupt ===---
    RBIF = 0      ' (RBIF) Clear Flag
    RBIE = 1      ' (RBIE) Enable Interrupts
    ENABLE(OnChgB)
END SUB


'--------------------------------------------------------------------------------
'    Main Program
'--------------------------------------------------------------------------------

// Variables
DIM Index AS BYTE
' ---=== Add Custom Characters ===--
LCDOut(CGRAM)

' ---=== Setup Serial ===---
SetBaudrate(br115200)
HSEROut(10,13)
   


ElapsedCounter = 0
ActivateTimer

ActivateOnChgB

' ---=== Main Loop ===---
WHILE true
    FOR Index = 0 TO MAX_BAR_COUNT
        Bargraph(1,Index)
        Bargraph(2,MAX_BAR_COUNT - Index)
        DELAYMS(UpdateMS)
        LCD.WriteAt(1,8,DecToStr(ElapsedCounter,10," "))
        LCD.WriteAt(2,8,DecToStr(Rot_Count,10," "))
    NEXT
    HSEROut("Up  ")
    DELAYMS(UpdateMS*TransDelayMulti)


    FOR Index = MAX_BAR_COUNT TO 0 STEP -1
        Bargraph(1,Index)
        Bargraph(2,MAX_BAR_COUNT - Index)
        DELAYMS(UpdateMS)
        LCD.WriteAt(1,8,DecToStr(ElapsedCounter,10," "))
        LCD.WriteAt(2,8,DecToStr(Rot_Count,10," "))
    NEXT
    HSEROut("Down",10,13)
    DELAYMS(UpdateMS*TransDelayMulti)
WEND
Keep in mind, this code is just a quick and dirty method of getting comfortable with this compiler. I've tinker with a couple of the context saving option, but nothing seemed to make a difference.

Looking forward to hearing some ideas, especially if I've overlooked something which is causing the problems.
Steve

User avatar
Darrel Taylor
Posts: 29
Joined: Wed Oct 04, 2006 4:44 pm
Location: California

Post by Darrel Taylor » Tue Oct 10, 2006 6:51 am

Hi Steve,

Glad to see you around here. :)

Bear in mind that I'm still trying to figure all this stuff out too, so this is just my guess at this point.

I don't see anything that set's the priority for the RB Port Change Interrupt. I've looked through the ASM file too.

Maybe just a ...

Dim RBIP As INTCON2.0

and

RBIP = 0
Best regards,
DT

User avatar
David Barker
Swordfish Developer
Posts: 1214
Joined: Tue Oct 03, 2006 7:01 pm
Location: Saltburn by the Sea, UK
Contact:

Post by David Barker » Tue Oct 10, 2006 9:08 am

I've just taken a quick look at your program and there are a few things that you need to look at. As Darrel has already mentioned, you need to ensure that the IP of the interrupt matches the priority level of the ISR.

You should also not clear and set any 'enable' flags from within your ISR. From the datesheet...
When an interrupt is responded to, the Global Interrupt Enable bit is cleared to disable further interrupts. If the IPEN bit is cleared, this is the GIE bit. If interrupt priority levels are used, this will be either the GIEH or GIEL bit.
Therefore, you should remove all references to 'RBIE' and 'Timer0On' from your ISRs, as the ISR is blocked anyway. Obviously, you still need to clear each flag (RBIF and Timer0IF) - so leave them as is.

There is a little bit of code on the wiki site for handling PORTB interrupts which might be useful...

http://www.sfcompiler.co.uk/wiki/pmwiki ... SROnChange

The module breaks down to

Code: Select all

interrupt OnChange(PriorityLevel)
   // end mismatch condition and clear interrupt flag...
   WREG = PORTB   
   FOnChangeIF = false  
   ...
end interrupt
and to initialise

Code: Select all

Initialize()
   FPullupsDisabled = false          // use weak pullups
   FIPHigh = false                       // set priority (low)
   FOnChangeEnable = true        // enable RB interrupts
   enable(OnChange)                 // enable the ISR
end sub
Hope this helps...

User avatar
SteveB
Posts: 23
Joined: Fri Oct 06, 2006 1:40 am
Location: Del Rio, TX

Follow-up

Post by SteveB » Tue Oct 10, 2006 5:38 pm

The RBIP and TMR0IP bits were the culprit. I don't know why I made the assumption that these would be handled by the compiler. Once it was mentioned, I realized there is really no reason the complier would know which IP bits to set/clear. It does appear to handle the IPEN, GIEH, and GIEL bits though. I also added the WREG = PORTB just to be sure the port change was cleared. The RBIE set/clear were just thrown in there late as an attempt to sort out the problem; I just hadn't removed them before posting the code.

Now, any words about the context saving issue. Once I got both ISRs working, I tried adding SAVE(0)...RESTORE. This generated the same erratic operation, particularly garbage on the LCD, but also resetting the value of the numbers. I know that it is overkill for my ISRs, but I would like to get a better feel for how this works. It seems to me to be one the key features of Swordfish.

Thank,
Steve

User avatar
David Barker
Swordfish Developer
Posts: 1214
Joined: Tue Oct 03, 2006 7:01 pm
Location: Saltburn by the Sea, UK
Contact:

Post by David Barker » Tue Oct 10, 2006 7:30 pm

Now, any words about the context saving issue. Once I got both ISRs working, I tried adding SAVE(0)...RESTORE. This generated the same erratic operation, particularly garbage on the LCD, but also resetting the value of the numbers. I know that it is overkill for my ISRs, but I would like to get a better feel for how this works. It seems to me to be one the key features of Swordfish.
The 'Save(0)' command will context save all of the swordfish system variables. That is, variables used for certain mathematical operations, delays etc. To save a block of RAM, swordfish will use FSR0 and FSR1, as this is the quickest way to move blocks of data around on a PIC18. However, string operations will also use FSR0 and occasionally FSR1. To correctly save the system registers, you should use

Code: Select all

Save(FSR0,FSR1,0)
..
Restore
this will context save FSR0L, FSR0H, FSR1L and FSR1H, then context save the system block. This will prevent FSR0 and FSR1 from being damaged outside the ISR.

I've just taken a look at the manual and online help, as I thought I had documented this. However it is clear that it has not. My apologies for the omission. I will try and make the changes in time for the compiler release.

User avatar
SteveB
Posts: 23
Joined: Fri Oct 06, 2006 1:40 am
Location: Del Rio, TX

Post by SteveB » Tue Oct 10, 2006 8:55 pm

I had noticed the use of FSR0 and FSR1 when I added 'Save(0)..Restore'. I just wasn't able to make the connection with the strings and the LCD corruption though. I also noticed the use of FSR0 and FSR1 if just context saving a single function (like the example given in the documentation).
To save a block of RAM, swordfish will use FSR0 and FSR1, as this is the quickest way to move blocks of data around on a PIC18. However, string operations will also use FSR0 and occasionally FSR1.
Seems to me that I would want to save these any time I used 'Save()...Restore' , since there is no way to know for sure (short of opening the asm files) if a) the main program is using these registers, or b) the 'Save()...Restore' is going to use them as well.
My apologies for the omission. I will try and make the changes in time for the compiler release.
No Worries! And I appreciate your quick response. However, I think it would be a nice feature to make context saving of the FSR0 and FSR1 automatic anytime the 'Save()...Restore' uses these registers. :)



Great info!
Thanks,
Steve
Last edited by SteveB on Fri Dec 22, 2006 5:32 am, edited 2 times in total.

User avatar
SteveB
Posts: 23
Joined: Fri Oct 06, 2006 1:40 am
Location: Del Rio, TX

Post by SteveB » Tue Oct 10, 2006 9:01 pm

David,
Just noticed this in the last paragrapgh in the context saving section of the pdf manual (p41):
FSR0 and FSR1 are used when context saving the system registers, so these are
saved first.
Seems I didn't read carefully enough, and just jumped into the examples. :oops:

Steve

TimB
Posts: 262
Joined: Wed Oct 04, 2006 7:25 am
Location: London UK

Post by TimB » Tue Oct 10, 2006 9:21 pm

I am putting together a bit for the wiki on interrupts but before I post it I thought I would point out something.

As soon as you start down the road of adding FSR0 and FSR1 automatically to the 'Save()...Restore' list you will end up having to save every thing.

For example if you have a = a * b you end up using the hardware maths registers which then should be saved. Its a slippery slope. The decision to fix the system registers and not just generate them only when they were used was done to make context saving easier. This means though that you have to save all 26 every time you enter the interrupt routine, add all the others in on top and the time lag would add up to unacceptable levels.

Until some writes something like BISVS for the compiler then I have to recommend people think hard about the consequences of interrupt usage and perhaps even get there hands dirty looking at he asm.

User avatar
David Barker
Swordfish Developer
Posts: 1214
Joined: Tue Oct 03, 2006 7:01 pm
Location: Saltburn by the Sea, UK
Contact:

Post by David Barker » Wed Oct 11, 2006 3:42 pm

I've been thinking this one over and agree with Tim that 'saving everything' would be an extremely bad road to follow.

However, I also have to agree with Steve that FSR0 and FSR1 should be saved automatically when performing a Save(0), as clearly these registers will be changed as part of the system register dump. Apart from being a potential support nightmare, clearly any call to 'Save' should be made safe.

I have therefore made changes to Save(), which will automatically save FSR0 and FSR1 when requesting a system register context save. That is,

Code: Select all

Save(0)
will context save FSR0, FSR1 and the Swordfish system registers. Just so everyone knows, Save(0) will also automatically save PRODL and PRODH, as there are a number of routines which touch these registers also. This is not a recent addition, Save(0) has always worked like that.

Interrupts can be a messy business at the best of times. The mantra with any ISR is keep it mean and lean! For bullet proof programs, it is always worth taking a look at the generated ASM to see what context saving may be needed.

To download the updated version (1.2.0.0)...

http://www.sfcompiler.co.uk/swordfish/d ... index.html

User avatar
SteveB
Posts: 23
Joined: Fri Oct 06, 2006 1:40 am
Location: Del Rio, TX

Post by SteveB » Wed Oct 11, 2006 8:56 pm

Dave and Tim,
I think we are looking at two sides of the same coin, both involving the complexities encountered with interrupts.

Heads: As you both have poinited out, ISRs, in general, sould be lean enough not to require an inordinate amount of context saving. Otherwise, the time spent in the ISR may start to become a problem for the rest of the program.

Tails: If a registry dump for context saving using FSRs is a feature of the compiler (although not advisable or needed in most cases), it should probably also save those same FSRs to limit the grief of users who won't (or can't) get their hands "messy" with the nuts and bolts of interrupts.


Thanks for listening, and even more, taking the time to implement a suggestion.

Steve B

TimB
Posts: 262
Joined: Wed Oct 04, 2006 7:25 am
Location: London UK

Post by TimB » Wed Oct 11, 2006 11:45 pm

I have to say that you bought to light a good issue. When I spoke to Dave about it I agreed with him that considering the fact that the you would have to do a Save (FSR0,FSR1,0) every time you saved the system vars it was silly not to do it automatically. Btw even if you do use Save (FSR0,FSR1,0) or even Save (FSR1,0) SF can work it out and not save either FSR twice.

I have a thing about interrupts and all my programs run on timer ticks or do usart buffering or such like, so I really care how interrupts work in SF. I'm not disappointed.

The only issue I have had is that you have to look at the asm to see if any system registers are corrupted. As soon as I can twist some ones arm to write a parser to do it for us with a plug in I will. I as a matter of cause look at the asm for most of the code I write.

I really do not want to go around speaking for Dave or getting peoples back up trying to sound like I know every thing. SF is new to me and I'm still bit green with procedural coding. But I have his ear and I bend it often to find out the inside info on how it all works. My aim is to save Dave from being tied to the forum (when it gets busier) and answer as much as I can so he can get on with the important stuff.

I hope as others learn and help me out with it.

BTW I know that while Dave cannot just have an open book and implement everybody's whims he is keen to show that he takes all suggestions very seriously. If its a good valid point then he will do something about it.

A very conscious decision was made to have the library's open so people can play and learn from them. This openness I hope will be shared.

If you have a little snippet of code that you would not mind sharing then please just post it on the form and I will transfer it to the wiki if you don't fancy doing it your self.

xor
Posts: 286
Joined: Sun Nov 05, 2006 1:15 pm
Location: NYC
Contact:

Post by xor » Sun Nov 05, 2006 10:30 pm

TimB wrote:A very conscious decision was made to have the library's open so people can play and learn from them. This openness I hope will be shared.

If you have a little snippet of code that you would not mind sharing then please just post it on the form and I will transfer it to the wiki if you don't fancy doing it your self.
Open libraries will help immensely and spur the growth and acceptance of this program. As an active user of mB I know there has been a reluctance by mE to open up the source code of the libraries. I believe that access to the library source code would have accelerated the growth and expansion of the libraries, along with the voluntary assistance of users to find fixes for problems, besides useful enhancements.

Maybe in trying out your new compiler I will work on converting some of my mB modules which I have listed in my CircuitED Forums (apologies for the self-promotion...you don't have to join) to Swordfish and post them here. It's worth a try to see how it works.

TimB
Posts: 262
Joined: Wed Oct 04, 2006 7:25 am
Location: London UK

Post by TimB » Mon Nov 06, 2006 12:37 am

Hi Xor

I have long been a fan of your work with MB nice to see you here.

xor
Posts: 286
Joined: Sun Nov 05, 2006 1:15 pm
Location: NYC
Contact:

Post by xor » Mon Nov 06, 2006 12:53 am

Probably not as big a fan as I am of your optimized coding with PDS. :)

TimB
Posts: 262
Joined: Wed Oct 04, 2006 7:25 am
Location: London UK

Post by TimB » Mon Nov 06, 2006 8:12 am

Well I've looked at the underlying code in SF and if you code it flat your getting close to Proton. There are some areas where stability takes precedence over size.

An example is Word / Byte in Proton there is a routine for that combination in SF the byte will be raised to a Word then a Word division is called. This means there are less permutations that could go wrong and this improves reliability.

Also currently all the FP trig routine are hand coded in basic, this means you do not get the full optermisation that is possible if it were coded entirely in asm.

On the subject of FP the routines are based on some that are used in CCS C and actually compiled smaller than there implementation by a 3rd.

Of cause the whole point of SF is the modularity and I can do stuff in SF that I would not bother to in Proton so all my new code these days is in SF.

Really SF has a great future and I'm putting all I can into it. As soon as I get 5mins I will be adding library code to do stuff like PulseIn PulseOut etc

Post Reply