We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.
Hello everyone,
I am currently having an issue with some code I have that seems to vary based on optimization:
In CommBuff ISR:
if (commData == 0x61) /* 0x61 = 'a' */ { if(DAC1DifVoltn==1) { DigPressure1 = DiffPressure1; // Enters this section, problem here } else { DigPressure1 = Pressure1>>3; } COMBUF = DigPressure1>>8; } else if (commData == 0x62) /* 0x62 = 'b' */ { COMBUF = DigPressure1; }
Currently, DAC1DifVoltn is set to 1. When the COMBUF register gets a 0x61, it updates DigPressure1 and puts out the MSB on the COMBUF. When it receives 0x62, it sends out the LSB bits.
The problem I'm having is with the line:
DigPressure1 = DiffPressure1;
DiffPressure1 is set to 0x0FF8. When optimization is set to 4-8, DigPressure1 is set to 0x0097. When it's set to 3, it gets 0x0F97. When it's set to 2-0, it gets the correct value (0x0FF8). Realistically, this has to do with the way my code is written rather than it being an assembly issue but I can't seem to see a better way to handle this.
Does anyone have any explanations as to what I can do? If you need me to post the assembly code, I can do that as well. Any help would be appreciated. Thanks!
There was a reason why I wrote "Some interrupt sources will trig automatically when simulating - for example timer interrupts, ADC conversion done interrupts etc. Some interrupts requires that you have a simulation script to trig the event - such as changing a state on a processor pin, or sending a character to the UART."
You need to play with stimuli manually or using scripts to fake external interrupts.
But for internal devices, the simulator handles them just as normal.
Your code initializes the ADC and tells it to convert. The debugger script can then fake values to the ADC and your ADC ISR will pick up the values the debugger script supplied. The script can generate noise, or ramps or whatever you want.
So you really should simulate as much as possible of the complete system - allows you to test the code long before you get first prototype hardware.
This makes sense. Are you guys saying that by simply calling the ISR directly as I am in my simulation, I am creating potential issues? It seems to me that there clearly is a problem storing DiffPressure1 into DigPressure1 since it is placing a 0x0097 in DigPressure1 rather than the 0x0FF8 in DiffPressure1. I don't believe the simulation is creating an issue here (especially since I am seeing the same kind of thing with it burned directly to a chip).
I do agree that when simulated on an actual circuit, there may be things I would normally miss because of timers but in this particular case, it looks like something specific is happening here. It sounds like it's pointing to source code to me.
And looking back over the previous suggestions (because someone mentioned the disassembly code), Mike hit it right on the head, it was assigning an absolute address. I checked the "don't use absolute addresses" checkbox and got the right value.
Mike, you are a lifesaver! I'll have to burn the code to see if that fixed it but it fixed the simulation issue.
The question I have is is this something that was done by Keil (since changing that checkbox fixed it) or is this something that was done/could have been avoided by me and if so, how? I'd like to be correct my programming practices if this is something I caused by missing an important programming step.
Here is how it works. The 8051 CPU supports multiple register banks. In assembly, you can access registers as R0...R7 in which case you get the current register bank (0...3). Or you can access them by absolute address: say, 0...7, which means you are accessing register bank 0 only.
www.keil.com/.../is51_ov_cpu8051variants.htm
So if the compiler uses absolute register accesses, it assumes that register bank 0 is selected. If such code is executed with a different bank selected, you'll get bugs like the one you experienced, since some parts of the code access currently selected register bank, and others access registers from a different bank.
http://www.keil.com/support/man/docs/c51/c51_le_regbankaccess.htm http://www.keil.com/support/man/docs/c51/c51_le_regbankspec.htm
a) which bank is this ISR using b) does this ISR call functions c) is any such function called from Main as well d) do you have interrupts of differnt priorities
and, finally, post the complete ISR you are running
Erik
The question I have is is this something that was done by Keil (since changing that checkbox fixed it) or is this something that was done/could have been avoided by me and if so, how?
A bit of both. But it's mostly your fault, for not paying enough attention to the documentation.
You called a function (the only one you've actually shown here) from another one that assumed a different register bank (your actual __interrrupt function). That is known and documented to cause incorrect program behaviour. So you shouldn't have done that.
Bah, rookie mistake on my end! I posted the code I thought was relevant but probably masked the real problem: calling a function from within an ISR (as Eric alluded to). I moved the function call code to within the ISR and the problem seems to have disappeared with me, even with the disable direct addressing box unchecked. I really should know better than to do this!
Thanks for the insight and links, everyone! I learned a lot from this (like I need to BE MORE CAREFUL). I appreciate the patience and explanations.
rookie mistake on my end two comments: 1) you alluded in a previous post that you might have atomicity problems, carefully make sure you do not a) 2) avoid calling subroutines from an ISR b)
a) to obtain a variable longer than 8 bits used in an ISR from main
EAsave =EA; EA = 0; local variable = shared variable; EA= EAsave;
b)calling a subroutine from an ISR is wrong for 3 reasons: 1) it makes the ISR slower; 2) if you need to for clarity purposes, your ISr is already too complex to adhere to KISS and 3) not having a subroutine makes the mistake of calling a subroutine from main and ISr impossible.