| ||||
| Home Products Download Events Support | ||||
Variable over written
Next Thread | Thread List | Previous Thread Start a Thread | Settings
| Details | Message |
|---|---|
Read-Only Author John Garrelts Posted 4-Oct-2005 09:50 GMT Toolset C51 |  Variable over writtenJohn Garrelts Dear all,I am completely stumped. I have a piece of code in which (normally) the process will not enter in to. It concerns a uart routine where it enters into the routine if a packet size has been detected. For some reason it enters into the routine even if there is no comm data. The variable(PacketSize) is previously initialized to "0". When the target device is powered up, I should have no output on the comm port. I do not recieve normal string data but garbage. Due to it sending anything at all I conclude that the variable PacketSize must be over written at some point. I have read on previous postings that it could have something to do with the pointers. I just can't figure out what. By the way, the simulator runs perfectly, the target device (AT89C51ED2) in a live enviroment gives the problem. Any advise will be welcome. Sorry if I'm not very clear in explaining the problem, it just that it's not that clear to me either.
void main(void)
{
InitVars();
com_baudrate ();
while(1)
{
if (PacketSize > 0)
{
if (DecodeStringData())
{
MainConveyorMotor = 1;
SendComm("Hello", 0);
}
PacketSize = 0;
}
}
}
unsigned char DecodeStringData(void)
{
unsigned char i;
unsigned short shReadCheckSum;
unsigned char chMode;
unsigned char chPos;
unsigned short shChecksum;
chMode = 0;
chPos = 0;
SBUF = PacketSize + '0';
shChecksum = calculateChecksum(&ReceivedPacket[1], PacketSize - 6);
shReadCheckSum = 0;
for (i = 1; i < (PacketSize - 1);i++)
{
if (chMode == 0)
{
// get command
if ((ReceivedPacket[i] == ',') ||
(ReceivedPacket[i] == ';'))
{
Command[chPos] = 0;
chPos = 0;
if (ReceivedPacket[i] == ',')
etc....etc....
void SendComm(unsigned char * pStringbuffer,unsigned char Stringlength) reentrant
{
idata unsigned char Ctr;
if (Stringlength != 0)
{
for(Ctr=0; Ctr<Stringlength; Ctr++)
{
tbuf[t_in++] = pStringbuffer[Ctr];
}
}
else
{
for(Ctr=0; Ctr < 256; Ctr++)
{
if (pStringbuffer[Ctr] == 0)
{
break;
}
tbuf[t_in++] = pStringbuffer[Ctr];
}
}
if( t_in != t_out)
{
TI = 1;
}
}
Thanks in advance for any advise. Regards John Garrelts |
Read-Only Author Hans-Bernhard Broeker Posted 4-Oct-2005 11:55 GMT Toolset C51 | RE: Variable over written Hans-Bernhard Broeker The variable(PacketSize) is previously initialized to "0".The symptoms you describe strongly indicate that this is not, in fact, the case. Since you neglected to show how this supposed initialization is actually done, it's impossible to be any more specific than this. By the way, the simulator runs perfectly, the target device (AT89C51ED2) in a live enviroment gives the problem. This further supports my diagnosis. The simulator tends to initialize variables to zero even if your code doesn't. |
Read-Only Author John Garrelts Posted 4-Oct-2005 12:07 GMT Toolset C51 | RE: Variable over written John Garrelts Thanks for your response. I here with add the part where the initiation is done.
void InitVars(void)
{
EA = 0;
ES = 0;
TR0 = 0;
TR1 = 0;
TR2 = 0;
MainConveyorMotor = 0;
check = 0;
PacketSize = 0;
SensorDelay = 1000;
TransportDistance = 0x2E;
}
This is called from the main function.To be complete I also add the variable and function definitions which are in a separate "c" file. unsigned char *Stringbuffer; extern unsigned char DecodeStringData(void); extern unsigned short calculateChecksum(unsigned char *pBuffer,int iSize); extern void SendComm(unsigned char *Stringbuffer,unsigned char Stringlength); extern void EncodePacket(unsigned char * pBuffer); extern void InitVars(void); extern void com_baudrate(void); extern void CommandAction(void); unsigned char xdata PusherOutput1 _at_ 0x2000; unsigned char xdata PusherDirection1 _at_ 0x4000; unsigned char xdata PusherPosInput1 _at_ 0x8000; unsigned char xdata DipSwitchSetting _at_ 0xC000; unsigned char xdata DistanceSwSetting _at_ 0xA000; // hex switches //------------- Pointers -------------------- extern unsigned char *Stringbuffer; //------------- Bits ----------------------- //----------- Chars ------------------------- idata unsigned char PacketSize; idata unsigned char Stringlength; //------------ Integers --------------------- //unsigned int count; // incremented from ticktimer for general timing idata unsigned int SensorDelay; idata unsigned int TransportDistance; //------------ Arrays ----------------------- xdata unsigned char TempString[128]; xdata unsigned char ReceivedPacket[128]; xdata unsigned char TransmitPacket[128]; xdata unsigned char Data_1[24]; xdata unsigned char Data_2[24]; xdata unsigned char Command[8]; Hopefully this will help define the problem. Thanks for the help Regards John Garrelts |
Read-Only Author erik malund Posted 4-Oct-2005 13:53 GMT Toolset C51 | RE: Variable over written erik malund MainConveyorMotor = 0;check = 0; PacketSize = 0; this seems to indicate that you have done something "funny" with startup.a51, have you? AT89C51ED2 another possibility is that you do not set the SFR that indicates "use internal RAM" in the very beginning of startup.a51. If you are in the C is C mode and set it in the beginning of main() you will lose all initialization of internal RAM. I know this should not affect idata, but the initialization code does strange things when it talks to "open" RAM (justifiably so). Erik |
Read-Only Author Stefan Duncanson Posted 4-Oct-2005 15:46 GMT Toolset C51 | RE: Variable over written Stefan Duncanson "AT89C51ED2another possibility is that you do not set the SFR that indicates "use internal RAM" in the very beginning of startup.a51. If you are in the C is C mode and set it in the beginning of main() you will lose all initialization of internal RAM." This device defaults to using internal RAM from addresses 0-0x2ff, but can be configured for internal RAM up to 0x6ff. |
Read-Only Author Stefan Duncanson Posted 4-Oct-2005 16:01 GMT Toolset C51 | RE: Variable over written Stefan Duncanson Your code is quite strange. The Decode() function writes to SBUF, which seems odd, and it doesn't wait for TI, which may cause garbled serial output.Your SendComm() function, or at least the part of it you have shown, doesn't seem to send anything, but sets TI. Have you got a mixture of interrupt driven and polled serial comms going on here? You don't show us how PacketSize normally gets modified. Does this happen in an ISR? "idata unsigned char Ctr; for(Ctr=0; Ctr < 256; Ctr++)" This is an infinite loop. The SendComm function is declared reentrant. I've never used a reentrant function, but doesn't it mean locals are stored on a stack in XDATA? How does using an idata local fit in with that? Note that the simulator does not behave the same as real code when dealing with the UART. It is quite possible to see good stuff in the serial window and see garbage on the target for the same code. If your complete program isn't too big I think you ought to post the whole thing so we can see what's going on. Please detail any changes you have made to startup.a51, also which memory model you are using. |
Read-Only Author John Garrelts Posted 4-Oct-2005 17:40 GMT Toolset C51 | RE: Variable over written John Garrelts I'll post the full code so that things are clear. Starting with the reentrant thingy, I just did that so to test if it would help it wasn't origionally there.
static void com_isr (void) interrupt 4
{
static idata unsigned char CopyCounter;
check = 1;
if (RI != 0)
{
RI = 0;
rbuf [r_in] = SBUF;
if(rbuf[r_in] == STX)
{
rbuf[0] = STX;
r_in = 1;
}
else if (rbuf[r_in] == ETX)
{
r_in &= 0x7f;
for (CopyCounter = 0; CopyCounter <= r_in; CopyCounter++)
{
ReceivedPacket[CopyCounter] = rbuf[CopyCounter];
}
PacketSize = CopyCounter;
r_in = 0;
}
else
{
r_in++;
}
}
if (TI != 0)
{
TI = 0;
if (t_in != t_out)
{
SBUF = tbuf [t_out++];
}
}
}
//---------------------------------------------------
//---------------------------------------------------
void com_baudrate(void) // using internal BRG
{
EA = 0;
t_in = 0;
t_out = 0;
t_disabled = 1;
BRL = 100; // This value sets the baudrate
BDRCON = 0x1F;
SM0 = 0;
SM1 = 1;
SM2 = 0;
REN = 1;
PCON |= 0x80;
CKCON0 = 0x0B;
PacketSize = 0;
ES = 1;
EA = 1;
}
void SendComm(unsigned char * pStringbuffer,unsigned char Stringlength)
{
idata unsigned char Ctr;
if (Stringlength != 0)
{
for(Ctr=0; Ctr<Stringlength; Ctr++)
{
tbuf[t_in++] = pStringbuffer[Ctr];
}
}
else
{
for(Ctr=0; Ctr < 256; Ctr++)
{
if (pStringbuffer[Ctr] == 0)
{
break;
}
tbuf[t_in++] = pStringbuffer[Ctr];
}
}
if( t_in != t_out)
{
TI = 1;
}
}
unsigned char DecodeStringData(void)
{
unsigned char i;
unsigned short shReadCheckSum;
unsigned char chMode;
unsigned char chPos;
unsigned short shChecksum;
chMode = 0;
chPos = 0;
shChecksum = calculateChecksum(&ReceivedPacket[1], PacketSize - 6);
shReadCheckSum = 0;
for (i = 1; i < (PacketSize - 1);i++)
{
if (chMode == 0)
{
// get command
if ((ReceivedPacket[i] == ',') ||
(ReceivedPacket[i] == ';'))
{
Command[chPos] = 0;
chPos = 0;
if (ReceivedPacket[i] == ',')
{
chMode = 1;
}
else
{
chMode = 3;
}
}
else if (chPos < 31)
{
Command[chPos++] = ReceivedPacket[i];
}
}
else if (chMode == 1)
{
// get data 1
if ((ReceivedPacket[i] == ',') ||
(ReceivedPacket[i] == ';'))
{
Data_1[chPos] = 0;
chPos = 0;
if (ReceivedPacket[i] == ',')
{
chMode = 2;
}
else
{
chMode = 3;
}
}
else if (chPos < 31)
{
Data_1[chPos++] = ReceivedPacket[i];
}
}
else if (chMode == 2)
{
// get data 2
if (ReceivedPacket[i] == ';')
{
Data_2[chPos] = 0;
chPos = 0;
chMode = 3;
}
else if (chPos < 31)
{
Data_2[chPos++] = ReceivedPacket[i];
}
}
else if (chMode == 3)
{
// get checksum
shReadCheckSum <<= 4;
if ((ReceivedPacket[i] >= '0') && (ReceivedPacket[i] <= '9'))
{
shReadCheckSum |= ReceivedPacket[i] - '0';
}
else if ((ReceivedPacket[i] >= 'A') && (ReceivedPacket[i] <= 'F'))
{
shReadCheckSum |= ReceivedPacket[i] - 'A' + 10;
}
else if ((ReceivedPacket[i] >= 'a') && (ReceivedPacket[i] <= 'f'))
{
shReadCheckSum |= ReceivedPacket[i] - 'a' + 10;
}
}
}
// check if we have collected all data
if (i < (PacketSize - 1))
{
// error in data collection
return 0;
}
// check checksum
if (shReadCheckSum != shChecksum)
{
EncodePacket("STS,CKS:ERR");
// error in checksum, do something
return 0;
}
return 1;
}
void EncodePacket(unsigned char * pBuffer)
{
unsigned char Counter;
unsigned char Counter2;
unsigned short Checksum;
TransmitPacket[0] = STX;
for(Counter = 0; Counter<120; Counter++)
{
if(pBuffer[Counter] == 0)
{
break;
}
TransmitPacket[Counter+1] = pBuffer[Counter];
}
Counter++;
TransmitPacket[Counter++] = ';';
Checksum = calculateChecksum(&TransmitPacket[1],Counter - 1);
for(Counter2 = 0; Counter2 < 4; Counter2++)
{
if (((Checksum >> 12) & 0x000f) > 9)
{
TransmitPacket[Counter++] = ((Checksum >> 12) & 0x000f) + 'A' - 10;
}
else
{
TransmitPacket[Counter++] = ((Checksum >> 12) & 0x000f) + '0';
}
Checksum <<= 4;
}
TransmitPacket[Counter++] = ETX;
SendComm(TransmitPacket, Counter);
}
unsigned short calculateChecksum
(
unsigned char *pBuffer,
unsigned char BSize
)
{
unsigned char BByteCounter;
unsigned char BBitCounter;
unsigned short WValue;
unsigned short WTemp;
WValue = 0;
for (BByteCounter = 0; BByteCounter < BSize; BByteCounter++)
{
WTemp = ((unsigned char *) pBuffer)[BByteCounter];
for (BBitCounter = 0; BBitCounter < 8; BBitCounter++)
{
if ((WTemp ^ WValue) & 0x0001)
{
WValue = (WValue >> 1) ^ 0x8408;
}
else
{
WValue >>= 1;
}
WTemp >>= 1;
}
}
return WValue;
}
I think this will do otherwise it will an extremely long posting. Regarding the ram: I have use on-chip xram (0x0-0x06FF) and use on-chip rom (0x0-0xFFFF)checked under options and in the startup.A51, I have the following entered: XDATASTART EQU 0H ; the absolute start-address of XDATA memory XDATALEN EQU 6FFHWhich I have used in several projects with the same processor. Unless I'm completely overlooking something :( Regards John |
Read-Only Author John Garrelts Posted 4-Oct-2005 17:50 GMT Toolset C51 | RE: Variable over written John Garrelts Your code is quite strange. The Decode() function writes to SBUF, which seems odd, and it doesn't wait for TI, which may cause garbled serial output. I forgot to mention. The SBUF thingy was only in there for testing purposes. I wasn't meant to be there. I the above posting it's corrected (Sorry). But anyway... Still the same problem. Meaning PacketSize is being overwritten somewhere. Regards John Garrelts |
Read-Only Author Jay Daniel Posted 4-Oct-2005 18:20 GMT Toolset C51 | RE: Variable over written Jay Daniel John,There is something problematic in your code. Does this function get called spuriously ALL the time, or just sometimes? The reason I ask is this: You say that you receive "garbage" on startup. From this, you conclude that your ISR would never intentionally set the PacketSize variable to something. But look -- all that it takes for PacketSize to be set to non-zero is for that string of "garbage" to include an ASCII ETX at some point. Even assuming the data is truly random, you've got a 1/255 chance on every single character you spuriously receive. Further, I don't see any buffer length checks in your ISR. That is, every time you get a character, you're storing the character in the "rbuf" buffer at position "r_in," but you never check to make sure that r_in is not greater than the size of your array. So... if you have a 25-position array and you receive 100 characters of garbage at startup, your routine is going to happily stomp over whatever happens to be above the buffer in memory. Perhaps try something like this: #define MAX_PACKET_SIZE 25 unsigned char rbuf[MAX_PACKET_SIZE]; and then in your ISR
if (RI) {
RI = 0;
if (r_in < MAX_PACKET_SIZE) {
rbuf[r_in] = SBUF;
r_in++;
} else {
//Do something here to reset the state
//because you've overflowed
}
}
|
Read-Only Author mark Hickman Posted 5-Oct-2005 01:18 GMT Toolset C51 | RE: Variable over written mark Hickman Hi John,A minor thing is XDATLEN should be 0700H to cover 0x0..0x6FF, but more importantly have you initialised within AUXR the XRAM size bits XRS2, XRS1 and XRS0 to '100' to enable 1792 byte of internal XRAM within STARTUP.A51 in the 'IF XDATALEN <> 0' conditional assembly code before the MOV DPTR,#XDATASTART line? Hope this helps, Mark. |
Read-Only Author Drew Davis Posted 5-Oct-2005 02:30 GMT Toolset C51 | RE: Variable over written Drew Davis PacketSize (and any other global variable shared between main and an interrupt handler) should be declared volatile. |
Read-Only Author John Garrelts Posted 5-Oct-2005 21:45 GMT Toolset C51 | RE: Variable over written John Garrelts Thanks for all the suggestions. I didn't have access to the live system today. I have already implemented the changes and will let you know how it goes.Regards John |
Read-Only Author erik malund Posted 5-Oct-2005 22:04 GMT Toolset C51 | RE: Variable over written erik malund if, indeed, (I believe it is so) that the ISR get pushes and pops when no 'using' is stated, you STILL have the problem, just now it is something else that get overwritten.Erik |
Read-Only Author John Garrelts Posted 6-Oct-2005 14:46 GMT Toolset C51 | RE: Variable over written John Garrelts Thanks for all the responses, The problem is solved. Drew was right, it was the volatile that needed to be added to the variables. Of course I did check all the other stuff, like adding the "using" (which I incedently had in the code before trying to find what was wrong).Anyways thanks everyone for the backup. Regards John Garrelts |
Read-Only Author Stefan Duncanson Posted 7-Oct-2005 09:24 GMT Toolset C51 | RE: Variable over written Stefan Duncanson "Drew was right, it was the volatile that needed to be added to the variables."Be aware that if any of these variables are larger than char then you'll need to do more than declare them volatile to ensure reliable operation. |
Read-Only Author C D Posted 7-Oct-2005 15:26 GMT Toolset C51 | RE: Variable over written C D Variables larger that char? So int and float is not protected with volatile, or are you talking about structures, arrays, etc... |
Read-Only Author Walter Conley Posted 7-Oct-2005 15:33 GMT Toolset C51 | RE: Variable over written Walter Conley Do a search on atomic and check out this tread.http://www.keil.com/forum/docs/thread2166.asp |
Read-Only Author Andrew Neil Posted 7-Oct-2005 22:52 GMT Toolset C51 | RE: Variable over written Andrew Neil "Variables larger that char? So int and float is not protected with volatile..."The 'volatile' keyword just prevents the compiler from optimising-away apparently redundant accesses - it does nothing to protect against the problems of interrupts, etc, interrupting non-atomic operations! |
Next Thread | Thread List | Previous Thread Start a Thread | Settings
| Contact Site Map Press Privacy | Copyright © 2012 ARM Ltd and ARM Germany GmbH. All rights reserved. |
