From: Raphael N. <rn...@we...> - 2009-05-10 19:03:29
|
Dear Néstor, > I'm running into trouble (once more) when executing my app. I attach > the code so you can see. Your code is too complex to effectively debug it without the hardware, so I can just contribute some ideas: You modify totaltime[] both within Intr() and outside it without protecting it (e.g., by disabling interrupts while updating it); this may lead to inconsistently setup data but should not cause the problem at hand. > The problem arises from my interrupt, which is calling some functions > outside of it. It almost works, I explain myself: Calling functions from within the interrupt handler is broken, because this would overwrite the STK00 .. STK0F (pseudo stack in sharebank) without saving the interrupted state. If you disable interrupts globally after entering the interrupt handler (to avoid nested interrupts), you can save the pseudo stack manually like #define disable_interrupts(x) do { x = (INTCON & 0xC0); INTCON &= 0x3F; } while (0) #define enable_interrupts(x) do { INTCON |= x; } while (0) void Intr(void) __interrupt(0) { unsigned char u; unsigned char int_state; char int_stack[16]; disable_interrupts(int_state); for (u = 0; u < 16; u++) { int_stack[u] = *(__data char *)(0x70 + u); } /* handler code */ out: for (u = 0; u < 16; u++) { *(__data char *)(0x70 + u) = int_stack[u]; } enable_interrupts(int_state); } (the above code is untested and target device dependent). You will also have to replace all return statements in the handler code with goto out; to restore the interrupted stack. This is a compiler bug that should be addressed once I find the time ... likely not very soon, sorry. You should also consider replacing multiplication by 2, 4, or 8 with shift operations, as the compiler does not optimize these cases (yet). If you use *4, the compiler will call a support routine to perform the multiplication --- which will corrupt the interrupted function's stack. Furthermore, you have static variable definitions (not declarations!) in your temete4.h: addressH, addressL, menu_option, selected_item, treatment, power, spot_time, and sound will all be instantiated in every .c (and .o) file that includes temete4.h (i.e., temete[134].c). This wastes memory and leads to unexpected behaviour if you access the variable in more than one source file: these variables are not shared among the .c files, each one gets its own copy -- and the linker won't tell you, because the linker symbols are made file-local due to the use of static. You really should declare all shared functions and variables in a .h file (using the extern keyword) and implement/define them in the according .c file. The current state with repeated declarations in the .c files leads to errors that the compiler cannot even warn about: As an example, count_down is defined as volatile BYTE count_down[2] in temete1.c but declared as non-volatile BYTE count_down[2] in temete3.c. Luckily for you, temete3.c does not access the variable at all ... You often use the volatile keyword on function arguments. Though this is possible, it is not often useful as the arguments are passed by value and not by reference: the callee gets its own copy of the values passed to him at the time of the call. Passing pointers to volatile data requires the volatile keyword (as in volatile char *ptr), marking the arguments themselves volatile (as in volatile char c or char * volatile ptr) only leads to complicated/slow code being generated. To aid in debugging, reduce the interrupt routine to as little as possible while still showing bogus behaviour to rule out race conditions on shared variables. > This application shows a menu with some options on screen, so you can > select between them with an encoder. This encoder is read by the > temete2.c:testEncoder() function, which enters a loop and uses global > variable BYTE encoderStatus and local BYTE key[2] to put the old and > both old and new encoder values, respectively. These values are taken > from some input gates (specifically, from RC6, RD3, RD2). These input > gates are only used for this purpose, and connected to the output of > the encoder. And they are only called from this function, nowhere > else. I have checked it throughfully. Some gates from PORTD are called > from other places, but not Gates 2 nor 3. And TRISD is only used once, > and switched to '0x0c' (So RD2, RD3 are inputs, the others are all > outputs). > > Well, for the other side, we have a countdown, which draws some > numbers on LCD screen via software I2C. To achieve this, I have > written an interrupt, called temete1c:Intr(), which calls some > functions to draw these values (temete4.c:PaintChar(), for example). > Then, PaintChar calls other functions, until the low-level ones are > reached. > > The problem is that I have observed that this interrupt actually > modifies RD2's value, always to the opposite. So, while the > testEncoder loop is running, key[] (and maybe the encoderStatus) > variable is magically changed, like if the encoder was pressed. So, a > menu option gets selected each time the clock is updated, and then, it > gets unselected again. I'm speaking without the user actually pressing > the encoder. I have observed no other strange behaviour, only this. It > does not change RD3's value, indeed, which is very very curious. Does RD2 change in fact or does only key[] change? Can you print the values of key[] and PORTD to find out? Does this happen even with an empty interrupt handler? Does it suffice to call PaintChar from within the handler to corrupt your data? > Am I incorrectly calling these functions from my interrupt? Have I > made some other mistake? I think it's not possible to put all the code > directly inside the interrupt, it's just too big, too repetitive. It's > better put in a function. But it seems not to work 100%. As indicated above, the call stack STK* is garbled when function are called from within the interrupt handler. If you protect the stack with the code given above, this should be no problem. Good luck, Raphael |