|
|
View previous topic :: View next topic |
Author |
Message |
levdev
Joined: 19 May 2010 Posts: 36 Location: UK
|
Function returns wrong value - fixed by declaring unused int |
Posted: Wed Feb 23, 2011 10:20 am |
|
|
I have a stange problem which has taken the best part of 2 days to track down, but I was hoping someone could help me undertand the cause. Part of my program is to test if a coordinate C is inside the boundary of a plane G. I have posted my cut down code below to demonstrate this.
Code: |
//structure to store X and Y values of a coordinate
typedef struct Not_Used1
{
float x;
float y;
} coord;
//structure to store the coordinates of the start and end point of a line
typedef struct Not_Used2
{
coord c1;
coord c2;
} line;
//structure to hold the four lines that make a plane
//line1 is bottom horizontal, line2 top horizontal, line3 left vertical, line4 right vertical
typedef struct Not_Used3
{
line line1;
line line2;
line line3;
line line4;
} plane;
const coord CAL_POINT[5][5] = {
{ {6138.0,6067.0}, {19156.0,5926.0}, {32583.0,5833.0}, {46268.0,6424.0}, {59581.0,6317.0} }
{ {5985.0,19361.0}, {19443.0,19244.0}, {32842.0,19630.0}, {45979.0,19616.0}, {59191.0,19632.0} }
{ {6159.0,32795.0}, {19455.0,32835.0}, {32592.0,32637.0}, {45924.0,32811.0}, {59307.0,32812.0} }
{ {613.06,46279.0}, {19171.0,46086.0}, {32501.0,46061.0}, {46109.0,45846.0}, {59229.0,45927.0} }
{ {6035.0,59309.0}, {19250.0,59571.0}, {32540.0,59532.0}, {45887.0,59342.0}, {59553.0,59361.0} }
};
//Gets the X intersection point when L1 is horizontal (or near horizontal)
coord Get_Intersect_Hor(line L1, line L2)
{
float A2, B2;
coord C;
//find slope (a2) of line 2
A2 = (L2.c1.y - L2.c2.y) / (L2.c1.x - L2.c2.x);
//find offset (b2) of line 2
B2 = L2.c1.y - (A2 * L2.c1.x);
//find intersection - y intersection is always at L1 y position
C.x = (L1.c1.y - B2) / A2;
C.y = L1.c1.y;
return(C);
}
//Gets the Y intersection point when L1 is vertical (or near vertical)
coord Get_Intersect_Vert(line L1, line L2)
{
float A2, B2;
coord C;
A2 = (L2.c1.y - L2.c2.y) / (L2.c1.x - L2.c2.x);
//find offset (b2) of line 2
B2 = L2.c1.y - (A2 * L2.c1.x);
//find intersection - x intersection is always at L1 x position
C.x = L1.c1.x;
C.y = B2 + (A2 * C.x);
return(C);
}
//test to see if point P1 lies within grid G1, return 1 if true
int Test_Plane(plane G, coord P)
{
int a;
line L1, L2;
coord XIL1, XIL2, YIL3, YIL4;
//int32 b,c,d,e,f,h,i,j,k,l;
//get x intersection with line 1 (horizontal base line)
L1.c1 = P;
L1.c2.x = P.x;
L1.c2.y = P.y + 10000.0;
XIL1 = Get_Intersect_Vert(L1, G.line1);
//get x intersection with line 2 (horizontal top line)
XIL2 = Get_Intersect_Vert(L1, G.line2);
//get y intersection with line 3 (vertical left line)
L2.c1 = P;
L2.c2.x = P.x + 10000.0;
L2.c2.y = P.y;
YIL3 = Get_Intersect_Hor(L2, G.line3);
//get y intersection with line 4 (vertical right line)
YIL4 = Get_Intersect_Hor(L2, G.line4);
//Test if point P is inbetween the 4 intersection points
printf("%e %e\n\r",XIL1.x, XIL1.y);
printf("%e %e\n\r",XIL2.x, XIL2.y);
printf("%e %e\n\r",YIL3.x, YIL3.y);
printf("%e %e\n\r",YIL4.x, YIL4.y);
if ( (XIL1.y < P.y) & (XIL2.y > P.y) & (YIL3.x < P.x) & (YIL4.x > P.x) ) return(1);
else return(2);
}
//Returns calibration data in a grid format based on the grid position X and Y
plane Get_Plane(int X, int Y)
{
line L1, L2, L3, L4;
plane G;
L1.c1 = CAL_POINT[Y-1][X-1];
L1.c2 = CAL_POINT[Y-1][X];
L2.c1 = CAL_POINT[Y][X-1];
L2.c2 = CAL_POINT[Y][X];
L3.c1 = CAL_POINT[Y-1][X-1];
L3.c2 = CAL_POINT[Y][X-1];
L4.c1 = CAL_POINT[Y-1][X];
L4.c2 = CAL_POINT[Y][X];
G.line1 = L1;
G.line2 = L2;
G.line3 = L3;
G.line4 = L4;
return(G);
}
void Initialise_uC()
{
setup_oscillator(OSC_64MHZ);
setup_adc(ADC_OFF);
setup_wdt(WDT_OFF);
setup_timer_0(T0_OFF);
setup_timer_1(T1_DISABLED);
setup_timer_2(T2_DISABLED,0,1);
setup_timer_3(T3_DISABLED);
setup_ccp1(CCP_OFF);
setup_comparator(NC_NC_NC_NC);
disable_interrupts(GLOBAL);
}
void main()
{
Initialise_uC();
int a;
plane G;
coord C;
C.x = 23933.0;
C.y = 27779.0;
G = Get_Plane(2, 2);
do
{
a = Test_Plane(G, C);
printf("Res = %u\n\r\n\r",a);
delay_ms(3000);
} while(1);
} |
In this form the functions Get_Intersect_Vert and Get_Intersect_Hor calculate the correct values, but return the wrong values.
By uncommenting the line
Code: |
int32 b,c,d,e,f,h,i,j,k,l;
|
in the function Test_Plane then it returns and prints the correct values. None of these int32 variables are used in the function, but it seems like I need to declare them just to get enough RAM in this function to hold the other values.
The intersection values should all lie in the range of 5000 to 60000, so you can tell if the values are not correct when the come out as zero, or 1xE-37 or 1xE+37.
I found the fix of adding the additional unused int32's by mistake, but I have no idea why this is working, and what the proper fix to this should be. |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Feb 23, 2011 11:13 am |
|
|
Before we have to jump into conclusions, can you please post the missing information required in "Posting code in this Forum", particularly
- Compiler version
- involved PIC type
Experience shows, that's it's often impossible to reproduce some observed bugs without using the same header file, fuses etc. So please post a complete, compilable code that reproduces the problem. |
|
|
levdev
Joined: 19 May 2010 Posts: 36 Location: UK
|
|
Posted: Wed Feb 23, 2011 12:40 pm |
|
|
OK, I've been working on the code, and have reduced the size of the test code, and included the header. I am using V4.118 of the compiler
Code: |
#include <18F26K22.h>
#device adc=10
#FUSES NOWDT //No Watch Dog Timer
#FUSES WDT128 //Watch Dog Timer uses 1:128 Postscale
#FUSES INTRC_IO //Internal RC Osc, no CLKOUT
#FUSES PROTECT //Code protected from reads
#FUSES PUT //Power Up Timer
#FUSES NOBROWNOUT //No brownout reset
#FUSES NOPBADEN //PORTB pins are configured as digital I/O on RESET
#FUSES NOMCLR //Master Clear pin used for I/O
#FUSES NOLVP //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#FUSES NOXINST //Extended set extension and Indexed Addressing mode disabled (Legacy mode)
#FUSES DEBUG //Debug mode for use with ICD
#FUSES IESO //Internal External Switch Over mode enabled
#FUSES STVREN //Stack full/underflow will cause reset
#FUSES NOWRT //Program memory not write protected
#FUSES NOWRTD //Data EEPROM not write protected
#FUSES FCMEN //Fail-safe clock monitor enabled
#use delay(clock=64Mhz)
#use rs232(uart1,baud=38400,bits=8,parity=N,stop=1,errors)
#include <math.h>
#include <stdio.h>
//structure to store X and Y values of a coordinate
typedef struct Not_Used1
{
float x;
float y;
} coord;
//structure to store the coordinates of the start and end point of a line
typedef struct Not_Used2
{
coord c1;
coord c2;
} line;
//structure to hold the four lines that make a plane
//line1 is bottom horizontal, line2 top horizontal, line3 left vertical, line4 right vertical
typedef struct Not_Used3
{
line line1;
line line2;
line line3;
line line4;
} plane;
const coord CAL_POINT[5][5] = {
{ {6138.0,6067.0}, {19156.0,5926.0}, {32583.0,5833.0}, {46268.0,6424.0}, {59581.0,6317.0} }
{ {5985.0,19361.0}, {19443.0,19244.0}, {32842.0,19630.0}, {45979.0,19616.0}, {59191.0,19632.0} }
{ {6159.0,32795.0}, {19455.0,32835.0}, {32592.0,32637.0}, {45924.0,32811.0}, {59307.0,32812.0} }
{ {613.06,46279.0}, {19171.0,46086.0}, {32501.0,46061.0}, {46109.0,45846.0}, {59229.0,45927.0} }
{ {6035.0,59309.0}, {19250.0,59571.0}, {32540.0,59532.0}, {45887.0,59342.0}, {59553.0,59361.0} }
};
//Gets the X intersection point when L1 is horizontal (or near horizontal)
float Get_Intersect_Hor(float Y, line L)
{
float A, B, X;
//find slope (A) of line L
A = (L.c1.y - L.c2.y) / (L.c1.x - L.c2.x);
//find offset (B) of line L
B = L.c1.y - (A * L.c1.x);
//find intersection - y intersection is always at L1 y position
X = (Y - B) / A;
return(X);
}
//Gets the Y intersection point when L1 is vertical (or near vertical)
float Get_Intersect_Vert(float X, line L)
{
float A, B, Y;
//find slope (A) of line L
A = (L.c1.y - L.c2.y) / (L.c1.x - L.c2.x);
//find offset (B) of line L
B = L.c1.y - (A * L.c1.x);
//find intersection - x intersection is always at L1 x position
Y = B + (A * X);
return(Y);
}
//test to see if point P lies within grid G, return 1 if true
int Test_Plane(plane G, coord P)
{
//I is intersection points. Only need 4, but need 10 extras to allocate RAM
//to this function. Looks like a bug with the compiler.
float I[4];
//get x intersection with line 1 (horizontal base line)
I[0] = Get_Intersect_Vert(P.x, G.line1);
printf("%.0g\n\r",I[0]);
//get x intersection with line 2 (horizontal top line)
I[1] = Get_Intersect_Vert(P.x, G.line2);
printf("%.0g\n\r",I[1]);
//get y intersection with line 3 (vertical left line)
I[2] = Get_Intersect_Hor(P.y, G.line3);
printf("%.0g\n\r",I[2]);
//get y intersection with line 4 (vertical right line)
I[3] = Get_Intersect_Hor(P.y, G.line4);
printf("%.0g\n\r",I[3]);
//Test if point P is inbetween the 4 intersection points
if ( (I[0] < P.y) & (I[1] > P.y) & (I[2] < P.x) & (I[3] > P.x) ) return(1);
else return(2);
}
//Returns calibration data in a grid format based on the grid position X and Y
plane Get_Plane(int X, int Y)
{
line L1, L2, L3, L4;
plane G;
L1.c1 = CAL_POINT[Y-1][X-1];
L1.c2 = CAL_POINT[Y-1][X];
L2.c1 = CAL_POINT[Y][X-1];
L2.c2 = CAL_POINT[Y][X];
L3.c1 = CAL_POINT[Y-1][X-1];
L3.c2 = CAL_POINT[Y][X-1];
L4.c1 = CAL_POINT[Y-1][X];
L4.c2 = CAL_POINT[Y][X];
G.line1 = L1;
G.line2 = L2;
G.line3 = L3;
G.line4 = L4;
return(G);
}
void Initialise_uC()
{
setup_oscillator(OSC_64MHZ);
setup_adc(ADC_OFF);
setup_wdt(WDT_OFF);
setup_timer_0(T0_OFF);
setup_timer_1(T1_DISABLED);
setup_timer_2(T2_DISABLED,0,1);
setup_timer_3(T3_DISABLED);
setup_ccp1(CCP_OFF);
setup_comparator(NC_NC_NC_NC);
disable_interrupts(GLOBAL);
}
void main()
{
Initialise_uC();
int a;
plane G;
coord C;
C.x = 23933.0;
C.y = 27779.0;
G = Get_Plane(2, 2);
do
{
a = Test_Plane(G, C);
printf("Res = %u\n\r\n\r",a);
delay_ms(3000);
} while(1);
}
|
If you change the code
to
then it will give you proper output which should be
Code: |
19373
32768
19451
32685
Res = 1
|
if these numbers are not the same, or Res=2 then the function is not working |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Feb 23, 2011 2:28 pm |
|
|
You can see the reason for failure from the symbol file
Get_Intersect_xxx is called from Test_Plane, it's interface variable area is conflicting with the local variable p:
084-093 Get_Intersect_Hor.l
084-093 Get_Intersect_Vert.l
...
090-097 Test_Plane.p
By enlarging array I, the interface variable area is moved.
I also checked, that the erroneous behaviour hasn't changed between PCH V4.112 and V4.119. |
|
|
levdev
Joined: 19 May 2010 Posts: 36 Location: UK
|
|
Posted: Wed Feb 23, 2011 2:43 pm |
|
|
Thanks for the explanation FvM. Is this a known issue? Should I report it to CCS? Will they do anything about it if I do? Is there a better or different way to code this to avoid it happening? I've just found the same problem in another set of functions, although at least I know what it is now so it wont take me another 2 days to work it out.
I'm relatively new to CCS C, and I've read a lot of posts on this forum and there does seem to be a lot of criticism of bugs in the software. In one post (http://www.ccsinfo.com/forum/viewtopic.php?t=22210&highlight=printf+float+zero) it was suggested that CCS C gives good value in terms of price/performance. Personally I would rather pay 10 times more and have no bugs. I find that errors in my program I can fix quite quickly, but bugs like these sap so much time, especially as a newbie to CCS C where I am not familiar with all the quirks. |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Feb 23, 2011 3:26 pm |
|
|
Quote: | Should I report it to CCS? | Absolutely.
Quote: | Will they do anything about it if I do? |
I hope so.
Quote: | there a better or different way to code this to avoid it happening? | In my opinion, it's a very general issue, that most likely can't be reliably avoided by a workaround. The I[14] trick works only by chance, and you can't be sure, if a similar problem (not keeping the integrity of local storage) will happen in another place.
Personally, I'm struggling a lot with PCD (16 Bit PIC compiler) bugs. I had a better impression of the 8-Bit compilers yet, but I don't use it with data structures of similar complexity for local variables or function parameters. I wonder, if avoiding local storage and large parameter interface blocks by using pointers and global variables can help to avoid the issue?
Generally, the problem is brought up by the way, the compiler (must) handle local variables and function parameters. Because PIC18 has only a small stack, it effectively can't be used for data objects. So the compiler is allocating data memory as intermediate storage, considering the function call hierarchy. Apparently, the allocation algorithm fails here. There may be a special reason in the structure of your code, but I'm not yet aware of it. Perhaps other users have an idea. |
|
|
levdev
Joined: 19 May 2010 Posts: 36 Location: UK
|
|
Posted: Thu Feb 24, 2011 6:03 am |
|
|
Thanks againg for your insight. I will email CCS with the details, although if it's anything like the last two emails I sent them about bugs I wont expect a reply.
This seems to me to be a very serious bug. I've written software in many languages on many compilers and interpreters and I've never seen one that couldn't figure out the memory allocation for each function and variable so that they don't overlap.
I will try some experiments to see if declaring more global variables, and passing less from function to function will help. This seems like a backward step though, it's always been my understanding that global variables should be avoided if possible, and with proper planning and structure none should be needed. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19537
|
|
Posted: Thu Feb 24, 2011 6:24 am |
|
|
It is perhaps important to understand, that CCS, _deliberately_ overlaps memory usage (very different from compilers on bigger chips), 're-using' areas in functions that are not static, when you are outside the function. The problem here seems to be that this re-use in this case is taking place while the value is still in use in an external function. A bug, but I suspect it is very limited, only occurring for some reason because of the nature of your variables.
You can force re-use to not take place, by declaring the variable used as the return value as 'static', which should bodge round the problem for now.
It is a bug, but understandable, given the need to re-use memory, because of the limited space on the PIC.
Best Wishes |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9241 Location: Greensville,Ontario
|
|
Posted: Thu Feb 24, 2011 6:50 am |
|
|
It's a case of doing more with less. The poor PIC and the compiler are asked to do things never thought of 25 years ago !
When asking the chip to do 'mainframe' math(floats,trancendentals,etc.) with limited resources(RAM,no fp subprocessor,etc.) it is a challenge.
Seems everyone wants the itty bitty PIC to work and act like a desktop PC but fail to understand that you'll have to compromise somewhere.
As the OP said, there are other compilers out there but higher priced(and NOT bug free).Given the huge range of PICs that CCS has cut compilers for,they've done a darn good job,especially for the price they charge.
To all who berate them, I suggest you cut your own compiler for just ONE type of PIC in one series and see how much time and effort it takes.Thousands of manhours....been there, done that...that's why I appreciate what CCS has done ! |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Thu Feb 24, 2011 6:51 am |
|
|
Quote: | A bug, but I suspect it is very limited, only occurring for some reason because of the nature of your variables. |
Hopefully it is. The bug however reveals a flaw in the memory allocation algorithm, and it's difficult to decide about the likelihood of meeting it again. The fact, that it apparently hasn't been observed yet by other users can be hold against it.
My guess is however, that nobody is stressing the allocation algorithm with structures of similar complexity, and that it will probably fail in other cases as well, if you just try. The case reminds me to my experience with PCD, where I can be rather sure to discover new bugs each time the code exceeds a certain complexity. |
|
|
sseidman
Joined: 14 Mar 2005 Posts: 159
|
|
Posted: Thu Feb 24, 2011 7:36 am |
|
|
temtronic wrote: |
To all who berate them, I suggest you cut your own compiler for just ONE type of PIC in one series and see how much time and effort it takes.Thousands of manhours....been there, done that...that's why I appreciate what CCS has done ! |
A great point! Also applies to customer service and chasing down bugs. A company the size of ccs that doesn't prioritize the use of their resources is a company that won't be around for very long. |
|
|
levdev
Joined: 19 May 2010 Posts: 36 Location: UK
|
|
Posted: Thu Feb 24, 2011 8:18 am |
|
|
Thanks to all for your input. I will try Ttelmah's suggestion and see if I've got enough memory available and if it solves the problem.
I don't buy your point though temtronic. Modern day microcontrollers are expected to do this kind of thing. Microchip are competing with Atmel and ADI and TI and all kinds of others to produce more powerful feature rich chips that take less power and cost less exactly so that engineers can take advantage of these features.
The job of the compiler manufacturer is to provide a tool that makes programming these ever more powerful chips easier, whilst still producing optimised, efficient and stable code. I don't think for a minute that I could do a better job than CCS, nor do I think I could design a microcontroller better than Microchip, or a better car than BMW, but that's not my job. The products I do design are expected to work, and if they don't then my customers have got the right to be unhappy about it. I can't just tell them 'well you got what you paid for'. It isn't a good enough philosphy for a business to operate on. I'm not suggesting your opinion represents that of CCS, but the seemingly large number of bugs, the lack of response from technical support and the fact they don't seem to monitor their customer forum does seem to suggest they might share this opinion.
Don't get me wrong I'm not a CCS hater I just don't think your point that 'you get what you pay for', and 'building a compiler isn't easy you know' are valid excuses. Can you imagine if everyone thought like that? Everything would be rubbish and nothing would progress. I think this attitude comes from PIC's being used a lot by hobbyists, and whilst that probably isn't big business for Microchip, it probably represents a good proportion of CCS's customers. As such they are forced to keep their prices low, which forces them to push out products which aren't properly tested.
For me I want a compiler that works, the cost is not really the issue within reason. Has anyone got experience of another compiler that performs better than CCS's PCWH? |
|
|
Ken Johnson
Joined: 23 Mar 2006 Posts: 197 Location: Lewisburg, WV
|
|
Posted: Thu Feb 24, 2011 8:51 am |
|
|
I have been very happy with Microchip's compilers for our PIC18 and PIC24 projects (and with MPLAB). Of course there is a learning curve, and you lose the peripheral setup features that CCS provides. I believe the "student" version of these compilers is still free, and gives you everything the full version offers except the full optimization. Have a look.
Although I use the CCS compiler now only for "legacy" product support, this forum is still a great source of information - I continue to visit here often, and continue to learn from the discussions here. I only wish I could contribute more to this forum.
Hope this is helpful.
Ken |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Thu Feb 24, 2011 9:28 am |
|
|
Quote: | the fact they don't seem to monitor their customer forum |
I understand the official statement about monitoring the forum as a clear hint to the user: "If you found a bug,
report it directly to CCS support". That's O.K. for me. Also the below statement is basically O.K.:
Quote: | Your e-mail has been assigned to someone in Tech Support. The e-mail has been reviewed,
however, an answer is not yet ready. A priority has not yet been determined for this issue. |
But when the answer becomes overdue? For some bug reports, the status doesn't change after a year or two.
Quote: | I have been very happy with Microchip's compilers for our PIC18 and PIC24 projects. | I don't love the longwinded definition of chip resources, but I agree about the quality.
If I change the compiler, it's surely to Microchip. MPLAB is a must for serious harware debugging anyway. |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9241 Location: Greensville,Ontario
|
|
Posted: Thu Feb 24, 2011 9:55 am |
|
|
What I was trying to get across was that a 'microcontroller' is NOT a microcomputer,minicomputer,desktop, mainframe or whatever you want to call a machine with tons of memory,pins for every conceivable peripheral,etc.Yet, it seems everyone wants these itty bitty micros do do everything,superfast on next to zero power and of course CHEAP!
Like using a tack hammer to break up a concrete driveway. Wrong tool for the job, sure it'll work,maybe for awhile but the proper tool exists.
I've yet to see a product that went out 100%, seems the public always want another feature,could you add this or that AFTER you've tweaked code and I/O resources to the smallest footprint to maximize the 'bottom line'.Just look at the hundreds of PIC types and subtypes,let alone the other guys. Sure there 'might' be an 'ideal' or 'perfect' chip for the product,until the 'I'd like this added' comes up, and it always does.
Let's face it, the PIC has 35 instructions,far less than the Z80 does,far fewer than a PC CPU but look what people have programmed it to do.
The major problem for companies like CCS is the vast and quick entry of new varients of the PICs almost every week or two.Sure they are 'variations on a theme',but it's a real challenge to test every possible feature or function,especially combinations that one wouldn't think of.
Heck, right now I've got a PIC16F88 breadboarded with 2 Dallas DS18B20PAR and an LCD.Been running 3 days and the screen has an 'extra' character on it even though it refreshes 1 /second.Something 'quirky'? Bad data?Ultra simple circuit and code, but now I'll probably spend 3-4 days tracking down what's gone 'wrong', another few days to test, before the project can go from proof of concept to beta testing with real pcbs.
Just remember , one size doesn't fit all.....
cheers |
|
|
|
|
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
|