View previous topic :: View next topic |
Author |
Message |
albus
Joined: 13 Apr 2013 Posts: 8
|
MUL88 MUL1616 Interrupts disabled |
Posted: Sun Apr 21, 2013 4:36 am |
|
|
I'm writing a program where timer interrupts are vital. The code is long but the timer routine goes like this:
Code: | #int_timer2
void timer2_isr()
{
globalcounter++;
chronocall();
}
|
Code: | void chronocall()
{
int i=0; // Dummy variable for loop
int j=0; // Dummy variable for loop
for(i=0; i<MAX_USERS; i++) // For all users
{
if(List[i].valid==0) // If the user is deleted
{
continue; // break out of for loop
}
for(j=0; j<INSTANCE_SIZE; j++) // For all daily records
{
if(List[i].instance_list[j].enable==1) // If pressed start
{
chronometer(&List[i].instance_list[j], &List[i]); // call chronometer function
List[i].counter++; // increase 40 milliseconds
break; // If one of them is working, that means the others don't. Break out of loop, don't check for others
}
}
}
} |
I've commented out some lines and found that
Code: | if(List[i].valid==0) |
is causing the problem. I get MUL88 error when I try it with PIC16F877 and MUL1616 with PIC18F4620.
So I've searched the forum for similar problems with MUL and DIV and the solution had something to do with #ORG and creating another routine for these internal functions. I didn't quite understand them. Can someone please explain to me how #ORG command works and what I should do? |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9245 Location: Greensville,Ontario
|
|
Posted: Sun Apr 21, 2013 5:51 am |
|
|
MUL88,MUL1616 are 'math' functions and apparently being called inside your chronocall() function.
You do NOT want to do ANY 'math' inside an ISR!
ISRs must be short,fast code.No printf(), no math,pointers,etc. It'd be interesting to see the listing to see how much code you really have there.
ISRs should only set 'flags',set simple variables,etc.Allow the 'main' code to read those flags and act accordingly.
Odds are real good you do not need to use #ORG.It's purpose is to place code at a specific memory location.
If you press F11 while your project is open, you can easliy access the CCS Help manual.Then just scroll over and open the section about #ORG.
hth
jay |
|
|
albus
Joined: 13 Apr 2013 Posts: 8
|
|
Posted: Sun Apr 21, 2013 6:24 am |
|
|
Of course ISRs should be short codes, but the problem is I do have a menu system inside main(). There are a lot of submenus and branches and so on. Even if I just set a bit and check it, I should place this chronocall function every 10 lines, just to make sure I don't miss an interrupt inside a submenu.
Do you think that would be a good solution?
I do know that MUL88 is executed because of that if condition. Is there a way for me to get around that problem? Because I need this chrono_call function to be executed inside the ISR. |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9245 Location: Greensville,Ontario
|
|
Posted: Sun Apr 21, 2013 6:55 am |
|
|
Without knowing/seeing the overall program I can only make general comments.
1) menus and submenus usually mean 'someone pressing buttons' which is a very,very slow process compared to ISRs being triggered.
2)all 'menu' systems I've done have timeouts.If the operator doesn't choose within say 2 seconds, the program goes back up 'one level'.So a 3 level menu system would take 6 seconds to go back to 'main'.
3)As you're aware ISRs need to be short compact code, so I think you need to rethink your overall program construction.
If you give us more information as to what is supposed to happen we can probably figure out a reasonable solution.
hth
jay |
|
|
albus
Joined: 13 Apr 2013 Posts: 8
|
|
Posted: Sun Apr 21, 2013 7:14 am |
|
|
The overall program is 1200 lines. It's not going to be useful for anybody to copy the code here.
My main aim is, can also maybe understood from the function name, to make a chronometer, multiple chronometers for multiple users actually. Every second, a variable, namely second will be incremented, and then minutes and so on. The ISR in my case gets called to do that.
1) Yeah someone presses a button and goes to a submenu. The users can access their records and etc. So even if the user is not on the main function, ISR should always be running.
2) It doesn't matter if there is timeout functionality or someone presses a button to go back. The main thing is that for that period the ISR should be running. I guess that's the problem here how can the microcontroller know 2 seconds is passed, when ISR is not working? If I can make ISR work at those submenus, there is already no problem.
3) It's just a little too late for me to change the overall program construction at this point. I can't make radical changes only some modifications.
Code: | void chronometer(time *timer, user *tester)
{
if((*timer).enable==1 && (*tester).counter >= 125) // Eğer kronometre aktif haldeyse
{
(*tester).counter=0;
(*timer).sec++;
if((*timer).sec>=60)
{
(*timer).sec=0;
(*timer).min++;
}
if((*timer).min>=60)
{
(*timer).min=0;
(*timer).hour++;
}
if((*timer).hour>=24)
{
(*timer).hour=0;
}
}
} |
That is the function that gets called inside the chronocall(), but it doesn't even matter, because even with that line commented out, the interrupts get disabled. They are caused by that if condition. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19545
|
|
Posted: Sun Apr 21, 2013 9:18 am |
|
|
The compiler uses the basic multiply functions to calculate locations in tables. Now these functions are quick (only typically 8 instructions), so having the interrupts disabled round them, doesn't cost a lot, but it will happen if you have things like arrays with indexes accessed by variables, inside an ISR. These in themselves are quite slow code....
Could you alter the approach here?. Consider (for instance), a switch table, with fixed indexes to the table, which is called for each user, and breaks out, when one is found. Since the table entries are then fixed for each entry in the switch, the search would be vastly faster, and wouldn't have to use the mult function either.
Best Wishes |
|
|
albus
Joined: 13 Apr 2013 Posts: 8
|
|
Posted: Sun Apr 21, 2013 10:41 am |
|
|
@Ttelmah thank you for the suggestion.
I didn't understand what you mean exactly by switch tables, do you mean a switch statement where I check the users and break out of loop when one is found? There will be multiple users in my case, so it would be bad if I broke out of switch statement, only after one user is found. I should look for all users if they exist and then I should look if their chronometer bits are enabled. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19545
|
|
Posted: Sun Apr 21, 2013 11:02 am |
|
|
I suggested breaking out, on the basis of the comment in your routine:
Quote: |
// If one of them is working, that means the others don't. Break out of loop, don't check for others
|
Best Wishes |
|
|
albus
Joined: 13 Apr 2013 Posts: 8
|
|
Posted: Sun Apr 21, 2013 12:57 pm |
|
|
No that's something else. Every user has more than one chronometer. If one of them is working, the others are not. It's not about the users |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19545
|
|
Posted: Sun Apr 21, 2013 1:01 pm |
|
|
Fair enough. The same applies then to the chronometers.
It is often the case, that though 'hardwired' code is larger than using loops, it is also much faster. Since speed should be one of the prime criteria in an ISR, it is worth separating rather than using loops.
Best Wishes |
|
|
albus
Joined: 13 Apr 2013 Posts: 8
|
|
Posted: Sun Apr 21, 2013 1:04 pm |
|
|
Ok, I'll try that for the chronometers but the question still stands |
|
|
Mike Walne
Joined: 19 Feb 2004 Posts: 1785 Location: Boston Spa UK
|
|
Posted: Sun Apr 21, 2013 2:49 pm |
|
|
Quote: | 3) It's just a little too late for me to change the overall program construction at this point. I can't make radical changes only some modifications. |
Sometimes you have to bite the bullet!
As I see it you've got two processes going on.
1) A slow one, human key-strokes.
2) A fast one. keeping several clocks updated/synchronised.
An ISR should be able to look after the clocks & key-presses.
Main() simply polls the user inputs, and deals with each on a time shared basis.
Other than that you're not giving us much to work on.
If you've presented yourself with a difficult system, it often works out easier to have a radical re-think, and use something that's simpler and easier to manipulate.
Just my two-pennies worth, as you're hiding behind your code size.
Mike |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9245 Location: Greensville,Ontario
|
|
Posted: Sun Apr 21, 2013 3:02 pm |
|
|
In re-reading this thread...
If this...
" Every second, a variable, namely second will be incremented"
..is what the timer ISR is for
then one heckuva lot of code can be run before the next time the ISR is hit!!
Any PIC from the lowly 16C84 with a 4MHx xtal can do almost a million instructions(1,000s of lines of code) before the next interrupt.
It sounds like you're doing a 'multiple person stopwatch'.If so that's less complicated than the remote energy control system I did over a decade ago,capable of 512 'panels' using the then 'new' 16F877.
My instinct says it's your program 'design' not actual code that's causing your problem.
hth
jay |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19545
|
|
Posted: Mon Apr 22, 2013 1:18 am |
|
|
albus wrote: | Ok, I'll try that for the chronometers but the question still stands |
No it doesn't.....
The point is that if you switch to using fixed indexes in the table in the ISR, then the compiler no longer has to do the maths at run time to work out where things are. This can be done at compile time instead. The code gets larger, but executes faster, and no longer has to use mul.
Best Wishes |
|
|
|