|
|
View previous topic :: View next topic |
Author |
Message |
Ttelmah Guest
|
|
Posted: Wed Dec 08, 2004 3:33 am |
|
|
Some general comments.
You are explicitly initialising variables like:
char BUF_Var[7] = "";
but using #zero_ram as well. Now zero_ram, loads every RAM location with a '0', and takes a _long time_ on chips with a lot of memory. Static variables (all your buffers etc.,), are _automatically_ zeroed. Personally, I'd suggest explicitly initialising the variables you want (as you largely do), and omitting zero_ram. You are not using the watchdog, but zero_ram, classically causes a problem on chips with a lot of RAM, where it can take so long to execute that the watchdog times out.
At this point, I'd wonder if you are sure that the watchog _is_ disabled?. There have been a number of cases in the past, where particular programmers, 'ignore' the fuse settings from the code, and (for instance), some ICE units, have a seperate setting to enable/disable the watchdog, which overrides the one from the fuses statement. If the watchdog was being left enabled, wth the time taken to zero the memory, and then the time taken to explicitly initialise the arrays as well, I'd expect the code never to start...
Do you really 'need' to explicitly locate the variables shown?. Generally, #locate, should _only_ be used, if you must put a variable at a specific location, for compatibility with another routine, or in the case of the older chips, to 'localise' variables for speed reasons. On the latter 18 series chips, only the first reason really applies. The sheer amount of overriding that you are attempting, raises the possibility of conflicts with the optimiser.
The #build statement, should only be used, when you are wanting to relocate the reset/interrupt vectors for something like a bootloader. 0x00, is the default reset location, and adding this, is another complexity that might confuse the compiler (general rule with CCS, is KISS...).
Try the code, without #zero_ram, without the explicit locates, and without the #build. Verify that the watchdog is disabled on your programmer.
Best Wishes |
|
|
Guest
|
|
Posted: Wed Dec 08, 2004 3:47 am |
|
|
Originally, I didn't use build and locate but it wrong than as well. Data corruption or the controller not able to start were common problems. As you told. maybe #zero_ram was the cause of not starting because at that time I did use the watchdog timer indeed. But I still can't explain the data corruption. Now we did a revision and used the A4 silicon instead of A3. I hope that solves a lot of problems now. Testing it without the #locate and #build statements, worked. I'm not really sure though, I haven't tested it enough with the A4 to be 100% about that. That's the problem with this kind of errors, they are so unpredictable.
I'm still afraid of data corruption. I have seen it one time on with A4 but then again, maybe it was because #locate and stuff was still used then. Or maybe its something with the timer as CKielstra said. I get this warning all the time:
Interrupts disabled during call to prevent re-entrancy: @MUL1616
Don't really know what this means...
I think I can get rid of #zero_ram, but I also like to be sure if the watchdog timer is disabled, maybe I'm doing it wrong. I removed WDT and WDT128 from the #fuses and added NOWDT. I also disabled all restart_wdt() functions. Is that enough?
Thanks for helping! |
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Wed Dec 08, 2004 4:23 am |
|
|
You didn't answer the question about the exact crystal frequency. You said 10MHz while your code says 9.somethingMHz? This is important because the RS232 settings are derived from the value specified in the #use delay settings.
How are you programming the chip? We know it is not an ICD but you never told us what device you use. Like Ttelmah wrote, this might be important because some programming devices overrule the #fuse settings from your program. |
|
|
PaulK
Joined: 08 Dec 2004 Posts: 2 Location: Norfolk, UK
|
|
Posted: Wed Dec 08, 2004 8:56 am |
|
|
I agree that #locate isn't very flexible and could cause conflicts. Instead, you could define a memory area using the 'typemod' keyword. You can then put all your arrays into a section in higher memory out of the way of all the other variables.
For example:
// Define a memory area for all the buffers, ie above the 'normal' variables.
// NB Check the listing file to make sure the buffers fit into the available memory.
typemod <,,,0x200,0x5ff> BufferSect;
Then make use of it:
// PPS String output control and buffer.
// The buffer is located in the BufferSect memory area.
#define PPSOutSize 50
intu8 BufferSect PPSOutBuff[PPSOutSize];
intu8 PPSOutCount = 0 ;
intu8 PPSOutOut = 0 ;
The compiler will sort out the actual addresses within the memory area. |
|
|
Guest
|
|
Posted: Wed Dec 08, 2004 7:33 pm |
|
|
To CKielstra:
I don't know much about the hardware but I think the 10Mhz was rounded to above. The code sais:
#define CPU_CLK 9830400 // Clock 9.830400 MHz of 40000000 MHz
So I think it's not exactly 10Mhz.
>We know it is not an ICD...
Stupid me. I thought you ment something else with in-circuit debugger because the guys at work once we're making some sort of test device. But 30 cm left from me lies a nice round-blue in-circuit debugger saying "MicroChip mplab ICD 2".
I used a few large buffers (char x[255]) and someone told me I'd better made them a little bit smaller (250 for example). Does this matter? And another off-topic question, I often see the main loop uses something like this:
loop:
...do stuff
goto loop;
But from what I now, goto is quit a slow instruction because its a large jump or something. Would'nt something like
while(1){ ... do stuff }
be better? Maybe these are stupid questions but I'd like to be sure of everything with all those weird errors (although I haven't seen them anymore after disabling #locate and using the A4 silicon).
Greetings |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Wed Dec 08, 2004 9:26 pm |
|
|
Quote: | I get this warning all the time:
Interrupts disabled during call to prevent re-entrancy: @MUL1616
Don't really know what this means... |
This means that you're doing a 16-bit multiplication inside one
of your interrupt service routines, and you're also doing it
somewhere else in the program (outside of an isr).
It's normal programming practice to not do a lot of
time-consuming operations inside an isr. Based on
this, I believe that you likely have other areas of
your program that are not perfect, from a design
point of view. Somehow, these problem areas
need to be identified and fixed.
---------
The following program will give this warning message
(provide that the +EA compiler option is used):
"Interrupts disabled during call to prevent re-entrancy: @MUL1616"
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)
#int_rda
void rda_isr(void)
{
int16 a, b, c;
a = b * c;
}
//=====================
void main()
{
int16 x, y, z;
enable_interrupts(INT_RDA);
enable_interrupts(GLOBAL);
x = y * z;
while(1);
} |
|
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Thu Dec 09, 2004 2:33 am |
|
|
Let me summarize to see where we are now:
- The problem with the chip not starting was caused by #zero_ram resulting in a watchdog timeout.
- PIC18F8720 stepping A3 has some serious core problems resulting in weird behaviour.
- In trying to solve the problems you added an abundance of #locate statements, probably introducing more bugs this way.
You disabled the watchdog, replaced the chip by an A4 revision and got rid of most #locate statements and now your code seems to work.
Great!!! This normally should be the end of the thread.
Off-topic questions should be placed in a new thread. |
|
|
Guest
|
|
Posted: Thu Dec 09, 2004 4:28 am |
|
|
Please allow me to bore for the last time ;)
I knew about the less code in interrupts, the better. There's not that much code except for some flags which are set. But I believe 16 bit ints are used in one of the interrupts, I shall check that. Besides of that, I was told (I thought it was you) to check conversion errors because they can write garbage in the memory as well. Its hard to check it out with the picc compiler because it doesn't warn about invalid conversions. So, I moved all the code to c++ and looked at all the warnings. Of course, c++ is a little bit different from the compiler we use but it did point out some conversion warnings. What I found strange was that there were also warnings in stdlib.c. I can't remember the exact function but there was something like this:
Code: |
BYTE a;
a=&someVariable;
|
Maybe I'm wrong but adresses are 16 bit in our case, aren't they? So this should be potentional 'dangerous' code? Just checking it, before I mess up things.
Thanks for all the helping guys, I've learned much from this!
Rick |
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Thu Dec 09, 2004 7:18 am |
|
|
Quote: | What I found strange was that there were also warnings in stdlib.c. I can't remember the exact function but there was something like this:
BYTE a;
a=&someVariable;
|
I guess you mean stdlib.h? You checked this in v3.191, didn't you? I don't have that version, but in stdlib.h from v3.187 I can't find a line of code like this (There is no line with the single '&' symbol).
Be more specific. |
|
|
Guest
|
|
Posted: Thu Dec 09, 2004 2:17 pm |
|
|
Forget about it. I didn't read it very well (it was 3.30 in the morning!). C++ pointed out this warning:
incompatible types - from 'float *' to 'unsigned char *'
On this code ( math.h version 3.191 )
Code: |
float sqrt(float x)
{
float y, res;
BYTE *p;
#ifdef _ERRNO
if(x < 0)
{
errno=EDOM;
}
#endif
if( x<=0.0)
return(0.0);
y=x;
p=&y; // This line
(*p)=(BYTE)((((int16)(*p)) + 127) >> 1);
do {
res=y;
y+=(x/y);
(*p)--;
} while(res != y);
return(res);
}
|
But I didn't read the * before p so its a pointer so it should be allright I guess. Ok, enough for this thread. I don't think you guys can't help me much further and with a little bit off luck, I think the problems should be solved!
Thanks for helping everyone! |
|
|
Mark
Joined: 07 Sep 2003 Posts: 2838 Location: Atlanta, GA
|
|
Posted: Thu Dec 09, 2004 2:33 pm |
|
|
I took this code (modified slightly):
Code: |
/* Test version */
#include <18f6720.h>
#device *=16
/* Setup */
#fuses HS,NOPROTECT, BROWNOUT, BORV45, PUT, STVREN, NODEBUG,NOLVP, NOWRT, NOWRTD, NOWDT
#ID CHECKSUM
#ZERO_RAM
#device ADC=10
// #define CPU_CLK 9830400
#define CPU_CLK 20000000
#use DELAY(CLOCK=CPU_CLK, RESTART_WDT)
/* RS232 */
#define baudrateGPRS 57600
#define baudrateDB9 9600
#use rs232( baud=baudrateDB9, parity=N, xmit=PIN_G1, rcv=PIN_G2, ERRORS, /*RESTART_WDT,*/ BRGH1OK, stream=DB9 )
#use rs232( baud=baudrateGPRS, parity=N, xmit=PIN_C6, rcv=PIN_C7, ERRORS, /*RESTART_WDT,*/ BRGH1OK, stream=GPRS )
#byte TIMER_1_LOW = 0xFCE
#byte TIMER_1_HIGH = 0xFCF
#Build(reset=0x000)
/* Basic imports */
#include <stdlib.h>
#include <input.c>
#include <ctype.h>
#include <math.h>
#include <string.h>
#include <stdio.h>
/* Some variables */
char BUF_Var[7] = "";
char BUF_Var2[7] = "";
char BUF_Var3[7] = "";
char BUF_Value[45] = "";
char BUF_Float[15] = "";
char BUF_SR[40] = "";
char BUF_SResult[255]= "";
byte cmd0[4];
byte cmd1[3];
unsigned int8 Trig_Allarm_Count[5];
BYTE Trigger_Active[5];
BYTE CHECK_SENSOR[6];
unsigned int8 allowAdress[9];
byte Array_Xmodem_Packet[134];
/* Memory locations */
/* Bank 3*/
#LOCATE cmd1 = 0x300 // byte 3 array
#LOCATE cmd0 = 0x303 // byte 4 array
#LOCATE Trig_Allarm_Count = 0x307 // byte 5 array
#LOCATE Trigger_Active = 0x30C // byte 5 array
#LOCATE CHECK_SENSOR = 0x311 // byte 6 array
#LOCATE Buf_Var = 0x318 // char 7 array
#LOCATE Buf_Var2 = 0x31F // char 7 array
#LOCATE Buf_Var3 = 0x326 // char 7 array
#LOCATE Current_Port = 0x32D // char 7 array
#LOCATE socket_seconds = 0x334 // byte 10 array
/* Bank 5*/
#LOCATE Array_Xmodem_Packet = 0x500 // byte 134 array
#LOCATE Buf_Float = 0x587 // float 15
#locate allowAdres = 0x5C3 // byte 9 array
#LOCATE Buf_Value = 0x5CC // char 45
/* Bank 6*/
#LOCATE BUF_SResult = 0x600 // char 255
#byte RCREG2 = 0xF6E
/* Rx 2 interrupt handler */
#int_rda2
void serial_isr_DB9()
{
// Empty
} // serial_isr_DB9
/* Timer 0 interrupt */
#int_rtcc
clock_isr()
{
// Empty as well
} // Timer
void main(void)
{
while(1)
fprintf( GPRS, "What a nice program\r\n" );
}
|
And I got this output:
Quote: |
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
What a nice program
|
|
|
|
Guest
|
|
Posted: Thu Dec 09, 2004 7:20 pm |
|
|
Weird, it did absolutely nothing on my pic. But maybe that was because I removed some #locate's and global variables because otherwise it was too much code for posting. Anyway, I don't use the #locate's anymore, we got a revision to A4 silicon and I changed some other stuff with the help of all the posts here. Since then, the system seems pretty stable!
Greetings,
Rick |
|
|
|
|
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
|