| View previous topic :: View next topic |
| Author |
Message |
gfbankston
Joined: 21 Jul 2014
Posts: 35
|
| global variables |
 Posted: Thu Sep 11, 2014 1:15 pm |
 |
|
Hello,
I just encountered something strange with a variable defined outside of
main{}
Every compiler I have ever used made all storage and variables outside
of main GLOBAL.
I just caught this compiler failing to increment a fifo pointer:
| Code: | unsigned char rx_fifo[32],*rx_ptr;
main{
rx_ptr = &rx_fifo[0];
for(;;){
check_rx();
}
}
void check_rx(void)
{
if(rx_available())
{
ch = rxchar_get();
*rx_ptr++ = ch;
}
} |
When running the debugger, rx_ptr stays at rx_fifo[0] each time I enter
the check_rx routine, and does get incremented to the next position while
I am in the check_rx routine.
Question: Do I have to declare all these variables outside main as static?
TIA
GlenB |
|
 |
gfbankston
Joined: 21 Jul 2014
Posts: 35
|
| compiler |
| Posted: Thu Sep 11, 2014 1:16 pm |
|
|
| Sorry, latest version of the compiler for a 18F45K50 chip |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Thu Sep 11, 2014 1:32 pm |
|
|
You didn't post a test program so I made one, and it works just fine.
This was tested in MPLAB (not MPLAB-X) simulator with vs. 5.026.
I don't have 5.027 installed on this PC.
This shows incrementing pointer values:
| Quote: |
0005 initial ptr value
0006
0007
0008
0009
000a
000b
000c
000d
000e
000f
0010
0011
0012
0013
0014
0015
0016
0017
0018
0019
001a
001b
001c
001d
001e
001f
0020
0021
0022
0023
.
.
.
|
Test program:
| Code: |
#include <18F45K50.h>
#fuses INTRC_IO,NOWDT,PUT,BROWNOUT,NOLVP
#use delay(clock=4M)
#use rs232(baud=9600, UART1, ERRORS)
void check_rx(void);
int8 rxchar_get(void)
{
return('A');
}
int8 rx_available(void)
{
return(TRUE);
}
unsigned char rx_fifo[32],*rx_ptr;
//==========================================
void main(void)
{
rx_ptr = &rx_fifo[0];
printf("%lx initial ptr value \r", rx_ptr);
for(;;){
check_rx();
printf("%lx \r", rx_ptr);
}
}
void check_rx(void)
{
int8 ch;
if(rx_available())
{
ch = rxchar_get();
*rx_ptr++ = ch;
}
}
|
|
|
|
ronaldoklais
Joined: 18 Dec 2012
Posts: 13
|
|
| Posted: Thu Sep 11, 2014 5:41 pm |
|
|
Try this:
ch = rxchar_get();
*rx_ptr = ch;
rx_ptr++; |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
| Re: global variables |
| Posted: Fri Sep 12, 2014 1:48 am |
|
|
| gfbankston wrote: |
| Code: |
unsigned char rx_fifo[32],*rx_ptr;
*rx_ptr++ = ch;
|
|
Just a couple of style points:
Its advised that such declarations are on separate lines:
| Code: |
unsigned char rx_fifo[32];
unsigned char *rx_ptr;
|
I can see that with buffers its tempting to declare both on one line to keep them together. The problem is that its not clear what is and what is not a pointer when both are declared in the same statement.
Consider this:
| Code: |
unsigned char *a, b;
unsigned char c, *d;
|
Is b a pointer? No it isn't, but it looks as though it should be. Its clearer that c is not a pointer - and this is the form you are using - but its still confusable. To fix this, simply split the declaration. It also makes assigning an initial value clearer.
| Code: |
*rx_ptr++ = ch;
// Actually means:
*(rx_ptr++) = ch;
// and not as some might expect:
(*rx_ptr)++ = ch;
// but
++*rx_ptr = ch;
// means
++(*rx_ptr) = ch;
// Or use ronaldoklais's suggestion of separating the two operations.
|
Here the point is that its better to clarify what's meant by adding the brackets than relying on the reader knowing C's at times rather non-intuitive rules of precedence. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19529
|
|
| Posted: Fri Sep 12, 2014 2:52 am |
|
|
There is one thought that has not been made.
The original poster does not show a complete program.
As such there are other possibilities. For instance, the watchdog could be triggering, in which case every time the check_rx routine was called, the pointer could have been set back to the first address in the array, as the chip restarts....
This sort of thing is why a complete small compilable program, like PCM_programmer posted, is essential. |
|
|
gfbankston
Joined: 21 Jul 2014
Posts: 35
|
| global variables |
| Posted: Fri Sep 12, 2014 6:24 am |
|
|
Well taken, the point about the watchdog...but, I assume that when the watchdog fires, it resets the cpu. I would have seen repeated sign-on messages on the LCD.
Also, the code is over 4000 lines, so just tried to illustrate what the problem was. The code worked perfectly when compiled on the XC8 compiler. All my issues have cropped up when using the CCS compiler. I am hoping this is because I "do not understand the rules" of this compiler.
This project is (an attempt) to move a working project using Atmel AVR processor to a PIC45K50(25K50), primarily for cost savings and the fact that CCS usb code really works...
Here is some other code that worked fine under XC8 that did NOT work in CCS:
typedef unsigned int16 uint;
typedef unsigned int8 uchar;
uint read16_eeprom(uchar addr)
{
uint val,val1;
init_eeprom(addr);
MCU_RD =1;
val = MCU_EEDATA;
addr++;
MCU_EEADR = addr;
MCU_RD =1;
#if XC8_COMPILER
val += (MCU_EEDATA << 8);
#else //CCS compiler
val1 = (MCU_EEDATA << 8) ; //upper 8 bits
val += val1;
#endif
return(val);
}
I am wondering how many places in my code that need changes???
GlenB K4KV |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Fri Sep 12, 2014 7:11 am |
|
|
After having discussed how important it is to post a complete program you still choose to post fragments...
Also, please use the 'code' buttons to preserve formatting.
If you don't want to be precise when asking your question, how can you expect us to spend our free time on your problem?
Now the fragment is useless without the declaration of MCU_EEDATA.
I don't have your compiler version and didn't want to waste time in creating my own test program, but I can just imagine the compiler doesn't know that MCU_EEDATA is a register and the optimizer is trying to be smart and re-uses the data from the previous access.
Is it wrong? Perhaps. But my guess is that you have forgotten to mark the register as 'volatile'.
As an extra not on coding style: | Code: | typedef unsigned int16 uint;
typedef unsigned int8 uchar; |
Why????
the name int16 contains so much more information than uint. I have seen many different variations on the theme, like Uint16, Uint16_t, int16_t, U16, etc. But at least try to keep the size in the type definition, then other programmers reading your code don't have to find your type redefinition to know the byte size. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Fri Sep 12, 2014 8:10 am |
|
|
Also, CCS has helper routines to do all that stuff, such as r/w of 16-bit
values to eeprom. See this file:
| Quote: | | c:\program files\picc\drivers\internal_eeprom.c |
Get rid of the low level code and write CCS programs they way they
are supposed to be written - at a higher level than XC8, etc. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
