 |
 |
| View previous topic :: View next topic |
| Author |
Message |
zjcheah
Guest
|
| I've written a monster! help with interrupts and globals |
 Posted: Mon Nov 14, 2005 4:13 am |
 |
|
I'm a student currently doing industrial training at a company that manufactures LED displays and I've been assigned to develop firmware for a certain hardware setup.
OK I've hardly ever used header files before, so I think I've gotten a little too deep now. I have some software which will send a string of characters over the serial (with a certain protocol, which is why u see me storing certain characters over the serial) , and my PIC+display will compare the characters with 'bitmap' patterns stored in the ROM (the tables which contain the bitmap patterns is called tables.h and i won't include it here.
After which it finishes comparing the characters and storing the bit patterns it will start to display the stuff on the screen. Also since I have managed to successfully get the display to work, my boss asked me to improve the program and add pages, where we can store multiple 'pages' of data on the PIC and choose which page we wish to display at the moment by sending the request through the software. Currently I'm trying only 2 pages but it doesn't work. I see some parts of my memory being overwritten (I think) because the strings that appear on the LED display isn't really garbage, but some strings that shouldn't be seen at that moment.
This is my main
| Code: |
#if defined(__PCM__)
#include <16F877A.h>
#DEVICE *=16
#fuses HS,NOWDT,NOPROTECT,NOLVP, NOBROWNOUT
#use delay(clock=20000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7, parity =N, bits=8)
#endif
#if defined(__PCH__)
#include <18F452.h>
#DEVICE *=16
#fuses HS,NOWDT,NOPROTECT,NOLVP, NOBROWNOUT
#use delay(clock=20000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7, parity =N, bits=8)
#endif
#include <functions.h> //contains the function definitions
#int_rda // this is the interrupt service routine------------------------
void serial_isr() {
grabString();
intepretString();
}//-------------------------------------------------------------------------
void main() {
char
intepretString();
enable_interrupts(global);
enable_interrupts(int_rda);
do{
showDisplay();
} while (1);
//------------------------------------------------------------------------------
} //end of main
|
and this is the functions.h file
| Code: |
#define data PIN_C5
#define clock PIN_C3
#define strobe PIN_C0
//#use fast_io(D)
//#use fast_io(C)
#define maxLinePixel 120
#define maxChar (maxLinePixel/6)
#define pixelInBytes (maxLinePixel/8)
#define maxPage 8
#include <tables.h> //this file contains the bitmap tables
#include <string.h>
typedef struct {
char line[15];
} dLine;
//these are my global variables------------------------------------------
int counter, tableChoice, bitCounter, bitPosition,tester,flag;
char pixelRow,temp,pixel,onRow,tableIncrement,page,sendType ;
char enteredData[maxChar]={'0'};
char recPageOne[maxChar]={'0'}; //recStringX stores strings of page X
char recPageTwo[maxChar]={'0'};
char recString[maxChar];
char pageStore[maxPage]={'0','0','0','0','0','0','0','\0'}; // stores the page data , i.e. pages that the user have chosen to display, up to 8 pages
//user defined datatypes-----------------------------------------------------
dLine upperpixelLine[4]; // try split the pixel Lines into 2 parts upper and lower
dLine lowerpixelLine[4]; //into arrays of strings/struct upperpixelLine[x].line would be pixel line x
/*these 2 are the data structures that i constructed to shift thru the shift registers when going thru the LED display, the display is 120 bits wide and 7 bits high (20 chars, each char is 5 bits wide + 1 space so that makes 120/(5+1))*/
void grabString(){ //this is the part that stores the info from the PC
int rcvPointer=0;
flag=0;
getc();
getc();
getc();
sendType=getc();
page=getc();
while(flag==0){
recString[rcvPointer]=getc();
if(recString[rcvPointer]==0x1B) flag=1;
rcvPointer++;
}
if(sendType==0x0F) {
for(counter=0;counter<15;counter++) getc();
if(page==0x30) strcpy(recPageOne, recString); //if page =0
if(page==0x31) strcpy(recPageTwo, recString); //if page =1
}
if(sendType==0x08) {
for(counter=0;counter<3;counter++) getc();
pageStore[0]=page;
}
}
void intepretString(){//this is the part that compares the chars with the bitmap pattern-------
if(pageStore[0]==0x30) strcpy(enteredData, recPageOne); //if page=0
if(pageStore[0]==0x31) strcpy(enteredData, recPageTwo); //if page=1
for(counter=0;counter<maxChar;counter++){
if(enteredData[counter]<128) tableChoice =4; //table4
if(enteredData[counter]<96) tableChoice =3; //table 3
if(enteredData[counter]<64) tableChoice =2; //table2
if(enteredData[counter]<32) tableChoice =1; //table1
//shift_right (address, bytes, value)
//store the value from the ROM table into temp
//temp=table1[tableIncrement][tester]);
//then shift it out 6 times from temp, the bits that are shifted out we wanna shift inside our pixelLInes
//for(6 times)
//shift_right(&upperpixelLine[tester],15,shift_right(&temp,1,1/0 it doesn't matter here));
switch (tableChoice){
case 1: tableIncrement =enteredData[counter] ; //this can be left out and tableIncrement can
//be set as enteredData[counter] in case1, but I did it for the sake of uniformity
for(tester=0;tester<4;tester++){
temp=table1[tableIncrement][tester];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&upperpixelLine[tester],15,shift_right(&temp,1,0));
//&upperpixelLine[tester] would be the address of the first string i.e. the first pixel row
}
temp=table1[tableIncrement][tester+4];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&lowerpixelLine[tester],15,shift_right(&temp,1,0));
}
}
break;
case 2: tableIncrement= enteredData[counter]-32;
for(tester=0;tester<4;tester++){
temp=table2[tableIncrement][tester];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&upperpixelLine[tester],15,shift_right(&temp,1,0));
}
temp=table2[tableIncrement][tester+4];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&lowerpixelLine[tester],15,shift_right(&temp,1,0));
}
}
break;
case 3: tableIncrement= enteredData[counter]-64;
for(tester=0;tester<4;tester++){
temp=table3[tableIncrement][tester];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&upperpixelLine[tester],15,shift_right(&temp,1,0));
}
temp=table3[tableIncrement][tester+4];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&lowerpixelLine[tester],15,shift_right(&temp,1,0));
}
}
break;
case 4: tableIncrement= enteredData[counter]-96;
for(tester=0;tester<4;tester++){
temp=table4[tableIncrement][tester];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&upperpixelLine[tester],15,shift_right(&temp,1,0));
}
temp=table4[tableIncrement][tester+4];
for(bitCounter=0;bitCounter<6;bitCounter++) {
shift_right(&lowerpixelLine[tester],15,shift_right(&temp,1,0));
}
}
break;
}
}
for(counter=0;counter<20;counter++) recString[counter]=0x20; //empty the string to avoid residue (using Space)
}
void showDisplay(){
setup_adc_ports(no_analogs);
//set_tris_d(0x00);
//set_tris_c(0xD6);
// i can't seem to set this stuff before declaring any variables
// the only way i can think of is to set this right before throwing the data onscreen
// and finish all data manipulation before this moment
//-------------------------------------------------------------------------------
onRow=0b01000000;
for(pixelRow=0;pixelRow<7;pixelRow++){
output_low(strobe);
for(counter=0;counter<15;counter++){
if (pixelRow==0) temp=upperpixelLine[0].line[counter];
if (pixelRow==1) temp=upperpixelLine[1].line[counter];
if (pixelRow==2) temp=upperpixelLine[2].line[counter];
if (pixelRow==3) temp=upperpixelLine[3].line[counter];
if (pixelRow==4) temp=lowerpixelLine[0].line[counter];
if (pixelRow==5) temp=lowerpixelLine[1].line[counter];
if (pixelRow==6) temp=lowerpixelLine[2].line[counter];
for (pixel=0;pixel<8;pixel++){
output_low(clock);
output_bit(data, shift_right(&temp,1,0));
output_high(clock);
};
};
output_high(strobe);
output_D(onRow); //turning on the particular row
delay_us(200); //adjust the brightness
output_D(0x00); //turn it off
onRow=onRow/2;
} //end of pixelRow for loop
}
|
I know the code looks massive, I'm not asking you guys to read everything but to advise me on how come my variables are being overwritten? |
|
 |
rwyoung
Joined: 12 Nov 2003
Posts: 563
Location: Lawrence, KS USA
|
|
| Posted: Mon Nov 14, 2005 9:57 am |
|
|
1) What chip are you using?
2) Compiler version?
3) Consider doing MUCH LESS work in your RDA interrupt service routine.
My first guess is that your IRQ is trying to do too much processing. The RDA should just get the incoming characters from the UART and put them into a software FIFO. Then in your main() you can run a statemachine that pulls data from the FIFO (if available) and processes it.
_________________
Rob Young
The Screw-Up Fairy may just visit you but he has crashed on my couch for the last month! |
|
|
asmallri
Joined: 12 Aug 2004
Posts: 1634
Location: Perth, Australia
|
|
| Posted: Mon Nov 14, 2005 10:13 am |
|
|
Had a giggle when I looked at the code. I remember when I was at Uni being told to keep the mainline as clean and short as possible. My tutor would have loved your main :-)
An alternative use of the interrupt capability would be to set up a serial ring buffer to capture serial data. Use a head and tail pointer to indicate next free location for storing (head) and another (tail) for the next location to be read. Other than during initialization, head is only ever adjusted in the interrupt handler | Code: | | ++Head%=Buffer_length |
Tail is only ever adjusted outside of the ISR.
When Head <> Tail there is something in the buffer. Perform all your string processing in the mainline (with appropriate functions).
_________________
Regards, Andrew
http://www.brushelectronics.com/software
Home of Ethernet, SD card and Encrypted Serial Bootloaders for PICs!! |
|
|
mcafzap
Joined: 07 Sep 2003
Posts: 46
Location: Manchester, UK
|
|
|
zjcheah
Guest
|
| Thanks but I have a question |
| Posted: Mon Nov 14, 2005 10:23 pm |
|
|
Rereading my program above, I realized some typo errors, please ignore them.
To rwyoung:
I'm currently using PIC16F877A but might move on to PIC18F452 which is which you see 2 device declarations on top of the main(). But I haven't studied the datasheet on PIC18Fxxx yet.
My compiler version is 3.235
To asmallri:
 yup at first my main() was really bloated which is why i created the header files. I don't really understand what is a serial buffer ring. Care to explain to me? I've only being doing this stuff for 3 weeks (since I started my training in this company) so my knowledge is pretty shallow.
I have fixed my program and managed to get my pages to work by doing less stuff in my ISR  I think that the problem in the first place.
BTW can anyone inform me why? I do sort of remember reading somewhre that we need to limit the things that the ISR does because of stack but I'm pretty sure my program didn't exceed the stack usage previously. What about the RAM pages? Is the ISR limited to using 1 page/bank of RAM?
currently I have limited my program to 20 chars and I can only store 4 pages, I should be able to store much more considering that I have more RAM available but the compiler says that I'm out of RAM.
These are my current global variables.
| Code: |
typedef struct {
char line[15];
} dLine;
//variables that change-------------------------------------------------------------
int counter,flag,serialDetected;
char temp;
char recPageOne[maxChar]={'0'}; //recStringX stores the strings of page X
char recPageTwo[maxChar]={'0'};
char recPageThree[maxChar]={'0'};
char recPageFour[maxChar]={'0'};
//char recPageFive[maxChar]={'0'};
char recString[maxChar+22];
char pageStore[maxPage]={'0','0','0','0','0','0','0','\0'}; // stores the page data , i.e. pages that the user have chosen to display
//user defined datatypes-----------------------------------------------------
dLine upperpixelLine[4]; // try split the pixel Lines into 2 parts upper and lower
dLine lowerpixelLine[4]; //into arrays of strings/struct upperpixelLine[x].line would be pixel line x
|
As you can see, I'm trying to declare an extra recPageFive but I'm out of RAM at this point. |
|
|
asmallri
Joined: 12 Aug 2004
Posts: 1634
Location: Perth, Australia
|
| Re: Thanks but I have a question |
| Posted: Mon Nov 14, 2005 10:57 pm |
|
|
| zjcheah wrote: |
To asmallri:
yup at first my main() was really bloated which is why i created the header files. I don't really understand what is a serial buffer ring. Care to explain to me? I've only being doing this stuff for 3 weeks (since I started my training in this company) so my knowledge is pretty shallow.
|
A ring buffer is a continous buffer of length 'N'. When your application has written to the last location in the buffer, it then points back to the beginning of the buffer. In other words the buffer is a "ring". You need two pointers, on that points to the head (where you will store the next byte your receive) and a tail which points to the last location that was read from. When head == tail then you have either nothing in the buffer or you have overflowed the buffer. What you do on overflow depends on your intended error handling approach. The most speed efficent ring buffer is a length of 256 bytes using byte (int8) pointers. This was you only ever have to increment pointers - you go not need to reset then to zero when you reach the top because they automatically overflow top 0.
| Quote: | | BTW can anyone inform me why? I do sort of remember reading somewhre that we need to limit the things that the ISR does because of stack but I'm pretty sure my program didn't exceed the stack usage previously. What about the RAM pages? Is the ISR limited to using 1 page/bank of RAM? |
Interrupt allow you to capture real time events. If you are inside the interrupt handler and another of these events occurs you could miss it. Say for example your application received the string ABC. When the first character arrives an interrupt occurs. Let's assume you spend a lonog time in the handler - meanwhile another character arrives into the UART - the UART will flag another character has arrived by setting the RXIF flag but it cannot interrtup the handler as the PIC's interrupt mechanism is not reentrant. So far no problem. if you handler exists because it has finished processing the first character the PIC is immediately interrupted and the second character is processed. But what happens if you were still in the handler as a result of the first character being received when the third character arrives - in this case the UART will overflow and the second character is lost.
An interrrupt handler should be kept as short as possible to prevent this type of problem. A common mistake you see in RS232 interrupt handlers is where dianostic or development code has been added like follows
my_char = getc(); // get character from the UART
printf("%c\r\n", my_char);
In this (poor) example, in the interrupt handler we receive one character and print a message of three characters which means that by the time we aer printing the second character we are potentially missing any input.
_________________
Regards, Andrew
http://www.brushelectronics.com/software
Home of Ethernet, SD card and Encrypted Serial Bootloaders for PICs!! |
|
|
zjcheah
Guest
|
| thoughts |
| Posted: Tue Nov 15, 2005 12:31 am |
|
|
Since I can't declare an array of 256 elements in the RAM of PIC16Fxxx i suppose to implement the serial buffer ring of that size would require me to reserve some memory locations to avoid the compiler automatically using them?
I still haven't successfully used the #rom, #locate directives at the moment, my experiments with them turned out garbage on my LED display, but I will continue trying later. The main problem that I have is the help file that comes with the compiler doesn't include enough examples  wish CCS would add more examples for newbies like me.
Thanks for your lengthy explanation on the interrupts Andrew, i understand the part about the interrupt missing any potential input.
However, the implications of calling functions and variables located all over the RAM is the part that I'm not sure about. To be specific:
Is the ISR limited to using 1 page/bank of RAM?
since microcontroller hardware is so limited (relative to a full fledged PC), i know that sometimes you can't simply call functions or refer to variables that are 'too far away' in memory.[/b] |
|
|
asmallri
Joined: 12 Aug 2004
Posts: 1634
Location: Perth, Australia
|
| Re: thoughts |
| Posted: Tue Nov 15, 2005 1:14 am |
|
|
| zjcheah wrote: | Since I can't declare an array of 256 elements in the RAM of PIC16Fxxx i suppose to implement the serial buffer ring of that size would require me to reserve some memory locations to avoid the compiler automatically using them?
I still haven't successfully used the #rom, #locate directives at the moment, |
| Code: | | byte Rx_BaseM[RxMQsize]; #locate Rx_BaseM = 0x0400 // serial receive ring buffer |
| Quote: | However, the implications of calling functions and variables located all over the RAM is the part that I'm not sure about. To be specific:
Is the ISR limited to using 1 page/bank of RAM? |
No it is not limited to a single bank of RAM.
_________________
Regards, Andrew
http://www.brushelectronics.com/software
Home of Ethernet, SD card and Encrypted Serial Bootloaders for PICs!! |
|
|
rwyoung
Joined: 12 Nov 2003
Posts: 563
Location: Lawrence, KS USA
|
| Re: thoughts |
| Posted: Tue Nov 15, 2005 8:29 am |
|
|
| zjcheah wrote: |
Is the ISR limited to using 1 page/bank of RAM?
[/b] |
No but if your ISR needs to bop around memory and keep changing pages, then it will run a bit slower than if you keep everthing in a single page.
Your RDA ISR doesn't necessarily need a FIFO that will hold the entire message. It only needs a FIFO that will hold enough of the message so that nothing gets dropped. Your main should be pulling data from the FIFO and processing it, storing more data if necessary.
As a general rule with microcontrollers, do as little as possible inside the ISR. Also, you can speed up the time into and out of the ISR using #priority or by writing your own global interrupt handler.
There is a very good example of how to write a RDA irq in the examples supplied with the compiler. Something like int_rda.c or similar name. All the examples are described in the manual.
If you dont' have the manual (and you should) you can download a fresh copy from CCS
_________________
Rob Young
The Screw-Up Fairy may just visit you but he has crashed on my couch for the last month! |
|
|
jecottrell
Joined: 16 Jan 2005
Posts: 559
Location: Tucson, AZ
|
|
| Posted: Tue Nov 15, 2005 9:09 am |
|
|
I'm surprised nobody has mentioned the "standard" implementation of a ring buffer for serial receives? Take a look at one of my recent posts and you'll see the standard code and ISR. It is a great starting place and is easily modified for any protocol you might want to implement. The thread will also help you understand what's happening. You'll also see the fairly brief ISR (typically an ISR will set a flag to enable a mainline function, you'll see that in #int_rda.)
http://www.ccsinfo.com/forum/viewtopic.php?t=24923
Good Luck,
John |
|
|
asmallri
Joined: 12 Aug 2004
Posts: 1634
Location: Perth, Australia
|
|
| Posted: Tue Nov 15, 2005 9:33 am |
|
|
| jecottrell wrote: | I'm surprised nobody has mentioned the "standard" implementation of a ring buffer for serial receives? Take a look at one of my recent posts and you'll see the standard code and ISR. It is a great starting place and is easily modified for any protocol you might want to implement. The thread will also help you understand what's happening. You'll also see the fairly brief ISR (typically an ISR will set a flag to enable a mainline function, you'll see that in #int_rda.)
http://www.ccsinfo.com/forum/viewtopic.php?t=24923
Good Luck,
John |
The "standard code" has a potential bug which may intermittently result in a lost character or, more likely, an overflow occuring when the buffer was meant to halt.
| Code: | #int_rda
void serial_isr()
{
int t;
buffer[next_in]=getc();
t = next_in;
next_in = (next_in+1) % BUFFER_SIZE;
if(next_in == next_out)
next_in = t; // Buffer full !!
}
byte bgetc()
{
byte c;
while(!bkbhit) ;
c = buffer[next_out];
next_out=(next_out+1) % BUFFER_SIZE;
return(c);
} |
The interrupt handler checks against next_out. Next_out is manipulated in bgetc() and this maniplulation process can and will take several CPU cycles. If is possible that an interrupt occurs while next_out is being manipulated with the % operator in which case the value of next_out could be undefined. The next_out manipulation should be enclosed in a "disable RD_ISR / enable Rd_ISR" sequence. It is possible (but unlikely) that the compiler may do this for you.
Note that the efficiency of this "standard implementation" could be enhanced significantly if the buffer was 256 characters deep. In this case no modulus operation is required inside or outside the ISR.
_________________
Regards, Andrew
http://www.brushelectronics.com/software
Home of Ethernet, SD card and Encrypted Serial Bootloaders for PICs!!
Last edited by asmallri on Tue Nov 15, 2005 7:33 pm; edited 1 time in total |
|
|
zjcheah
Guest
|
| Thanks |
| Posted: Tue Nov 15, 2005 7:22 pm |
|
|
Thanks for all the help guys =)
It's great to see such an active user support forum.
To asmallri:
| Code: | | #locate Rx_BaseM = 0x0400 |
I suppose 0x0400 can't be used for the PIC16Fxxx? since it only has a 9 bit address.
Also can we do this: (instead of using byte, use char)
| Code: |
char text[20];
#locate text 0x0180h
// that would be the first location in bank 1 am I correct?
|
Anyways I have moved on to trying to make my text come in with some shifting patterns, so I guess that would be my last question. |
|
|
zjcheah
Guest
|
| Thanks |
| Posted: Tue Nov 15, 2005 7:26 pm |
|
|
Thanks for all the help guys =)
It's great to see such an active user support forum.
To asmallri:
| Code: | | #locate Rx_BaseM = 0x0400 |
I suppose 0x0400 can't be used for the PIC16Fxxx? since it only has a 9 bit address.
Also can we do this: (instead of using byte, use char)
| Code: |
char text[20];
#locate text 0x0180h
// that would be the first location in bank 1 am I correct?
|
Anyways I have moved on to trying to make my text come in with some shifting patterns, so I guess that would be my last question. |
|
|
asmallri
Joined: 12 Aug 2004
Posts: 1634
Location: Perth, Australia
|
| Re: Thanks |
| Posted: Tue Nov 15, 2005 8:32 pm |
|
|
| zjcheah wrote: | Thanks for all the help guys =)
It's great to see such an active user support forum.
To asmallri:
| Code: | | #locate Rx_BaseM = 0x0400 |
I suppose 0x0400 can't be used for the PIC16Fxxx? since it only has a 9 bit address. |
Correct - I was just giving an example, in this case from a PIC18F452
[/quote]
Also can we do this: (instead of using byte, use char)
| Code: |
char text[20];
#locate text 0x0180h
// that would be the first location in bank 1 am I correct?
|
Yes but for a PIC16F you would use 0xA0 as 0x180 maps to the indirect pseudo register.
[/quote]
_________________
Regards, Andrew
http://www.brushelectronics.com/software
Home of Ethernet, SD card and Encrypted Serial Bootloaders for PICs!! |
|
|
zjcheah
Guest
|
| -_- Thanks again Andrew |
| Posted: Wed Nov 16, 2005 1:12 am |
|
|
| Quote: | Also can we do this: (instead of using byte, use char)
Code:
char text[20];
#locate text 0x0180h
// that would be the first location in bank 1 am I correct?
Yes but for a PIC16F you would use 0xA0 as 0x180 maps to the indirect pseudo register. |
yes, I just looked at the datasheet and realized I was looking at the indirect adressing diagram. How stupid must I look!
Thanks alot guys ! |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
