|
|
View previous topic :: View next topic |
Author |
Message |
Guest
|
#defined calculating returns wrong value |
Posted: Sun Apr 03, 2005 9:34 am |
|
|
another stupid question...
i'm defining some constans and making some calculations...
with this code DeviceNo always 0.
DeviceNo = RecordSaveNo / (int16) EEPROM_RECORD_COUNT;
but this one DeviceNo has correct result.
DeviceNo = RecordSaveNo;
DeviceNo /= (int16) EEPROM_RECORD_COUNT;
here is the test code i used and results from terminal screen.
Code: |
#include <18F452.h>
#device adc=10
#use delay(clock=4000000)
#fuses XT, NOWDT,NOPROTECT, NOLVP, NOPROTECT, PUT
#use rs232(baud=19200, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
//headers
#Define EEPROM_SIZE 32768
#Define RECORD_SIZE 64
#Define EEPROM_RECORD_COUNT EEPROM_SIZE/ RECORD_SIZE
void GetDeviceAddress(int16 RecordSaveNo)
{
int16 DeviceNo;
//wrong result
DeviceNo = RecordSaveNo / (int16) EEPROM_RECORD_COUNT;
printf("\n\rDevNo=%lu RecNo=%lu EEPROM_REC_CNT=%lu", DeviceNo,RecordSaveNo,EEPROM_RECORD_COUNT);
//correct result
DeviceNo = RecordSaveNo;
DeviceNo /= (int16) EEPROM_RECORD_COUNT;
printf("\n\rDevNo=%lu RecNo=%lu EEPROM_REC_CNT=%lu", DeviceNo,RecordSaveNo,EEPROM_RECORD_COUNT);
}
|
if i call GetDeviceAddress with parameter 3750
Code: |
void main
{
GetDeviceAddress(3750);
}
|
it returns
DeviceNo=0 RecordSaveNo=3750 EEPROM_RECORD_COUNT=512
DeviceNo=7 RecordSaveNo=3750 EEPROM_RECORD_COUNT=512
why this happens? |
|
|
Guest
|
|
Posted: Sun Apr 03, 2005 9:39 am |
|
|
i forgot to say:
if i remove (int16) still same
DeviceNo = RecordSaveNo / (int16) EEPROM_RECORD_COUNT;
DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT; //WRONG
but i replace EEPROM_RECORD_COUNT with 512 i get correct results.
DeviceNo = RecordSaveNo / (int16) EEPROM_RECORD_COUNT;
DeviceNo = RecordSaveNo / 512; //CORRECT
this is why title is '#defined calculating returns wrong value' |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Sun Apr 03, 2005 1:22 pm |
|
|
I didn't try to analyze your code, but the very first step is to
put parenthesis around your defined values. That way, you
are certain of the order of evaluation.
Do it like this:
Code: | #Define EEPROM_RECORD_COUNT (EEPROM_SIZE/ RECORD_SIZE) |
|
|
|
Guest
|
|
Posted: Sun Apr 03, 2005 3:10 pm |
|
|
as seen in the post above:
printf("\n\rDevNo=%lu
RecNo=%lu
EEPROM_REC_CNT=%lu",
DeviceNo,
RecordSaveNo,
EEPROM_RECORD_COUNT);
creates these lines in the console.
DeviceNo=0 RecordSaveNo=3750 EEPROM_RECORD_COUNT=512
DeviceNo=7 RecordSaveNo=3750 EEPROM_RECORD_COUNT=512
these means EEPROM_RECORD_COUNT=512 and no problem with it.
it works with constants but not work with defined values. i'm using working code.
Nothing much to anaylze... most of them printf...
Code: |
//defines
#Define EEPROM_SIZE 32768
#Define RECORD_SIZE 64
//EEPROM_RECORD_COUNT=512
#Define EEPROM_RECORD_COUNT EEPROM_SIZE/ RECORD_SIZE
|
Code: |
//wrong result - DeviceNo always =0
DeviceNo = RecordSaveNo / (int16) EEPROM_RECORD_COUNT;
//wrong result - DeviceNo always =0
DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT;
|
Code: |
//correct result - DeviceNo has value as expected
DeviceNo = RecordSaveNo;
DeviceNo /= (int16) EEPROM_RECORD_COUNT;
|
Code: |
//correct result - DeviceNo has value as expected
DeviceNo = RecordSaveNo / 512;
|
|
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Sun Apr 03, 2005 5:48 pm |
|
|
Now I'm at a computer where I can do some testing, and I did so,
and it works with the parens added. I tested it with PCM vs. 3.222.
The output of the program is:
0
1
2
3
....
61
62
63
Code: | #include <16F877.H>
#fuses XT, NOWDT, NOPROTECT, BROWNOUT, PUT, NOLVP
#use delay(clock = 4000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
#Define EEPROM_SIZE 32768
#Define RECORD_SIZE 64
#Define EEPROM_RECORD_COUNT (EEPROM_SIZE/ RECORD_SIZE)
//============================
main()
{
int16 RecordSaveNo;
int16 DeviceNo;
for(RecordSaveNo = 0; RecordSaveNo < 32768; RecordSaveNo+= 512)
{
DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT;
printf("%lu\n\r", DeviceNo);
}
while(1);
} |
|
|
|
Ttelmah Guest
|
|
Posted: Mon Apr 04, 2005 2:21 am |
|
|
Anonymous wrote: | as seen in the post above:
Code: |
//defines
#Define EEPROM_SIZE 32768
#Define RECORD_SIZE 64
//EEPROM_RECORD_COUNT=512
#Define EEPROM_RECORD_COUNT EEPROM_SIZE/ RECORD_SIZE
|
Code: |
//wrong result - DeviceNo always =0
DeviceNo = RecordSaveNo / (int16) EEPROM_RECORD_COUNT;
//wrong result - DeviceNo always =0
DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT;
|
Code: |
//correct result - DeviceNo has value as expected
DeviceNo = RecordSaveNo;
DeviceNo /= (int16) EEPROM_RECORD_COUNT;
|
Code: |
//correct result - DeviceNo has value as expected
DeviceNo = RecordSaveNo / 512;
|
|
In fact the code works exactly as expected in each case...
This is a 'classic' macro problem with C. In the line 'DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT;
Think for a moment about what this expands to:
DeviceNo=RecordSaveNo/32768/64;
Now C evaluates division left to right, and you are running in int16, so you get the RecordSaveNo, divided by 32768, which unless RecordSaveNo, is greater than 32768, will allways result in '0'. Then this is divided by 64, to again give zero.
This is not a problem with CCS, but is covered in some detail in most C books, where it is pointed out, that you _must_ always think through the evaluation order when using macros, and for safety, when deaing with expressions like this, it is worth always bracketting, to avoid this problem (this forces the macros division to be executed first, to give the result expected...
You will see in code posted in the pastm that several of the posters do this with regularity, to avoid exactly this problem.
Best Wishes |
|
|
Guest
|
|
Posted: Mon Apr 04, 2005 3:06 am |
|
|
ahhh. after your post i understand what is my error. #defines are text replaces.
1) #Define EEPROM_RECORD_COUNT EEPROM_SIZE/ RECORD_SIZE //=512
2) DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT;
when 1 and 2 merged:
DeviceNo = RecordSaveNo / EEPROM_RECORD_COUNT EEPROM_SIZE/ RECORD_SIZE;
not
DeviceNo = RecordSaveNo / 512
i always accept it 'DeviceNo = RecordSaveNo / 512'. it's wrong.
as i said my first post, i made very stupid error. and i don't understand PCM programmer's first post because i always accept DeviceNo equals to 512.
i never think this caused by compiler because / 512 works well.i anayzed asm code, and the code generated by compiler is very optimized asm code.
i must read more C - ANSI C book.
i'm sorry. but now i understand.
thank you very much. you are great people. |
|
|
Ttelmah Guest
|
|
Posted: Mon Apr 04, 2005 3:15 am |
|
|
A very good point. The fact that a #define, is a macro expansion, and not a 'numeric' defintion, is not intrinsically 'obvious', and leads to exactly your problem.
Glad I managed to 'trigger' the light on this. :-)
Best Wishes |
|
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|