 |
 |
| View previous topic :: View next topic |
| Author |
Message |
weg22
Joined: 08 Jul 2005
Posts: 91
|
| help with INT_RDA |
 Posted: Thu Jan 11, 2007 8:56 am |
 |
|
Hi all,
I have an ultrasonic sensor that outputs RS232 data in the following format:
Rxyz and then a carriage return
where xyz is a 3 digit number that represents distance. I'm using a global buffer to store the data in and then in my main program, I assign another char array to that buffer. I have the global buffer as 8 bytes to make sure that I have at least one valid distance in the array. That is, the worst case scenario for a distance of 123 would be:
123xR123
where x represents the carriage return. Just making sure this is the most efficient way to do this, becauase it still seems like I'm missing a char every now and again.
| Code: |
#include <18F8722.h>
#include <stdlib.h>
#include <math.h>
#fuses HS,NOWDT,NOPROTECT,PUT,NOLVP
#use delay(clock=10000000)
#use rs232(baud=9600, xmit=PIN_G1, rcv=PIN_G2, stream=sonar, DISABLE_INTS)
#use rs232(baud=9600, xmit=PIN_A4, rcv=PIN_A5, stream=PC, DISABLE_INTS)
char c;
char sonarBuf[8];
int i=0, m=0;
#INT_RDA2 // RS232 interrupt for sonar
void sonarRda_isr(void)
{
c = fgetc(sonar);
c = (char)c;
if(m==8) m = 0;
sonarBuf[m] = c;
m = m + 1;
}
#define LED PIN_F0
main()
{
char sonarData[8];
char sonarVal[3];
int sonar_raw=0;
output_high(LED); delay_ms(1500);
output_low(LED);
// enable interrupts
enable_interrupts(INT_RDA2);
enable_interrupts(GLOBAL);
while(1)
{
i=0;
// assign sonarData array to buffer
for(i=0; i<8; i++) {sonarData[i] = sonarBuf[i];}
i=0;
// search sonarData array for 'R' and then place next 3 bytes in sonarVal
for(i=0; i<5; i++)
{
if(sonarData[i]=='R')
{
// make sure we don't have any erroneous data
if(sonarData[i+1]=='R' || sonarData[i+2]=='R' || sonarData[i+3]=='R')
i=i;
else
{
sonarVal[0] = sonarData[i+1];
sonarVal[1] = sonarData[i+2];
sonarVal[2] = sonarData[i+3];
sonar_raw = atoi(sonarVal);
}
}
}
fprintf(PC, "%u\r\n", sonar_raw);
}
} // end of main
|
|
|
 |
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Thu Jan 11, 2007 10:34 am |
|
|
| Code: | | #use rs232(baud=9600, xmit=PIN_G1, rcv=PIN_G2, stream=sonar, DISABLE_INTS) |
Are you sure you want to use DISABLE_INTS for the hardware UART? It is a valid keyword, but I can only think of very obscure reasons to use it on the hardware UART.
A bug: The atoi() function requires the string to be zero terminated. You don't do this.
Your use of a circular buffer is very inventive, but why not make life easier on yourself and use a linear buffer?
Using a linear buffer you wouldn't do anything in the Receive Interrupt until the reception of the 'R' character, after that you process the next three incomming digits.
A possible implementation (it compiles but isn't tested): | Code: | #include <18F8722.h>
#fuses HS,NOWDT,NOPROTECT,PUT,NOLVP
#use delay(clock=10000000)
#use rs232(baud=9600, xmit=PIN_G1, rcv=PIN_G2, stream=sonar)
#use rs232(baud=9600, xmit=PIN_A4, rcv=PIN_A5, stream=PC, DISABLE_INTS)
#include <ctype.h>
enum ReceiveState {WAIT_FOR_R, GET_NUMBER};
int8 ReceivedVal = 0;
Boolean ReceivedNewVal = FALSE;
#INT_RDA2 // RS232 interrupt for sonar
void sonarRda_isr(void)
{
static ReceiveState State = WAIT_FOR_R;
static int8 DigitCount;
static int8 IncompleteVal;
char c;
c = fgetc(sonar);
switch (State)
{
WAIT_FOR_R:
if (c == 'R')
{
State = GET_NUMBER;
DigitCount = 0;
IncompleteVal = 0;
}
break;
GET_NUMBER:
if (isdigit(c))
{
DigitCount++;
IncompleteVal = (IncompleteVal * 10) + c - '0';
if (DigitCount == 3)
{
ReceivedVal = IncompleteVal;
ReceivedNewVal = TRUE;
State = WAIT_FOR_R;
}
}
else
State = WAIT_FOR_R;
break;
default: // We should never get here
State = WAIT_FOR_R;
}
}
void main(void)
{
while (1)
{
if (ReceivedNewVal)
{
fprintf(PC, "%u\r\n", ReceivedVal);
ReceivedNewVal = FALSE;
}
}
}
|
|
|
|
Ttelmah
Guest
|
|
| Posted: Thu Jan 11, 2007 10:39 am |
|
|
I wouldn't be so complex!...
Though I am one of the primary proponents of keeping the interrupt handler quick and short, it is important to understand that some things in computing terms are very quick/short, whicle some other things are slow. Simple 'tests', are quick. So there is nothing to stop you looking for the 'R' in the interrupt handler. Add 'errors' to your hardware handler code, and get rid of the 'disable_ints' keyword on the hardware stream. Why aren't you using the second hardware port?. The disable_ints, on the software port, may well cause problems....
Code your interrupt with something like:
| Code: |
int1 newval=false;
char sonarBuff[3];
#INT_RDA2 // RS232 interrupt for sonar
void sonarRda_isr(void)
{
int8 c; //you want this to be temporary, and local
static int8 state=0; //This needs to survive multiple calls.
c = fgetc(sonar);
switch (state) {
case 0:
if (c=='R')
state=1;
break;
case 1:
sonarbuff[0] = c;
state=2;
break;
case 2:
sonarbuff[1] = c;
state=3;
break;
case 3:
sonarbuff[2] = c;
state=4;
newval=true;
break;
case 4:
if (c=='\r') state=0;
break;
}
}
//Then in your main code:
char SonarVal[4];
if (newval) {
disable_interrupts(INT_RDA2);
sonarVal[0] = sonarBuff[0];
sonarVal[1] = sonarBuff[1];
sonarVal[2] = sonarBuff[2];
newval=false;
enable_interrupts(INT_RDA2);
SonarVal[3]='\0';
sonar_raw = atoi(sonarVal);
}
|
Now the interrupt handler, looks me complex, but will actually be faster than the current system!. The reason is that direct accesses to a fixed location in an array, are done as a single memory transfer, when accessing a location indexed by a variable, involves taking the value of the variable, loading this into the indirect access registers, and then accessing through this. What happens, is that the code looks for the 'R', and when this is seen transfer the subsequent bytes in turn into a buffer. When this is finished, a flag 'newval' is set to tell the main code that there has been an update. The code then looks for the carriage return, and then once more starts looking for 'R'. The switch, does not have a default state, which in a 18 chip, means it will be coded as a table jump (and will therefore be quite fast). In the main, if 'newval' is set, simply transfer the data into the working registers, and then clear newval. During the transfer, to prevent the possibility of an update, the interrupts are for a few cycles disabled. Note the need to terminate 'SonarVal', with a '\0'. Otherwise depending what is after this in memory, the atoi call, make waste a lot of work, and give the wrong value. Four storage locations ar therefore needed for the string.
Best Wishes |
|
|
Ttelmah
Guest
|
|
| Posted: Thu Jan 11, 2007 10:40 am |
|
|
Great minds thinking alike Ckielstra!.
Best Wishes |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Thu Jan 11, 2007 10:42 am |
|
|
No offense, but I can't see why you're attacking the problem in this way. It seems like you're trying to make things much more difficult for yourself. Your buffer only needs to hold 3 values - the 3 ascii digits you receive from the sensor.
This is how I would approach the problem:
| Code: | int1 new_range_inbound = FALSE;
int1 new_range_assembled = FALSE;
int8 ascii_range[3];
int8 rxd_character;
int8 num_chars = 0;
// new stuff
int8 ascii_range_copy[3];
#int_RDA
RDA_isr() {
int8 i;
rxd_character = getc(sonar);
if (!new_range_inbound && rxd_character == 'R') { // start of a new range
new_range_inbound = TRUE;
num_chars = 0;
// if the sensor outputs a variable # of characters, then here is where you would clear the ascii_range[] array
}
else if (new_range_inbound && rxd_character == 0x0d) { // range has now been transfered
new_range_inbound = FALSE;
new_range_assembled = TRUE;
for (i = 0; i < 3; i++) {
ascii_range_copy[i] = ascii_range[i];
}
}
else if (new_range_inbound) {
ascii_range[num_chars++] = rxd_character;
}
}
void main() {
while (TRUE) {
if (new_range_assembled) {
new_range_assembled = FALSE;
// sort through ascii_range_ copy[] array to convert to a number (either int8 or int16 depending on the sensor)
// and do whatever you need to do with the data
}
}
} |
Last edited by newguy on Thu Jan 11, 2007 11:52 am; edited 2 times in total |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Thu Jan 11, 2007 11:22 am |
|
|
Wow, no response for 1.5 hours and then 3 quality answers within 8 minutes of each other. 
Hi Ttelmah, our solutions are indeed almost identical !
I was in doubt whether it would be more economical to create 4 states like you did, than they would have been identical.  |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Thu Jan 11, 2007 11:30 am |
|
|
| ckielstra wrote: | Wow, no response for 1.5 hours and then 3 quality answers within 8 minutes of each other.
Hi Ttelmah, our solutions are indeed almost identical !
I was in doubt whether it would be more economical to create 4 states like you did, than they would have been identical. |
Two were quality. Mine had a bug which I just fixed.  |
|
|
weg22
Joined: 08 Jul 2005
Posts: 91
|
|
| Posted: Thu Jan 11, 2007 11:47 am |
|
|
Ttelmah (or anyone),
I ran your code and by itself, it works perfectly! However, capturing this data from the sensor is a very small portion of my code. I have a ton of other things going on (e.g. INT_TIMER0, INT_TIMER2, INT_CCP, and over 400 lines of main code). This is originally why I had DISABLE_INTS defined in the rs232 line.
When incorporating your code (that works independedntly) into mine, I get the following values for a distance of 28 inches:
28, 28, 28, 2, 2, 2, 28, 28, 28 (pattern repeats...)
I think what's going on is that:
(1) it fills sonarbuff with 3 valid bytes of distance data
(2) sets newVal to true
(3) starts refilling sonarBuff with more bytes of data
(4) before it finishes filling sonarBuff, main sets sonarVal=sonarBuff
It's just a guess as to the source of the problem. Any help would be appreciated.
Thanks,
-weg |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Thu Jan 11, 2007 11:57 am |
|
|
| I just posted a modification to the code I posted earlier. This mod simply copies the contents of the buffer into another buffer. This way the processor has extra time to actually interpret the received data without it being overwritten by new inbound characters. |
|
|
Mark
Joined: 07 Sep 2003
Posts: 2838
Location: Atlanta, GA
|
|
| Posted: Thu Jan 11, 2007 12:08 pm |
|
|
| For Ttelmah's code, I would move the disable_interrupts(INT_RDA2) statement into the interrupt handler. There is the possiblity of over writing your data if you do not process it fast enough. You should also check to make sure that you did not get a transmission error. If you do, then go to state 0 and wait for the 'R'. |
|
|
weg22
Joined: 08 Jul 2005
Posts: 91
|
|
| Posted: Thu Jan 11, 2007 12:28 pm |
|
|
Mark,
I'm not sure how to check for transmission errors? Can you please explain?
Thanks,
-weg |
|
|
Mark
Joined: 07 Sep 2003
Posts: 2838
Location: Atlanta, GA
|
|
| Posted: Thu Jan 11, 2007 12:35 pm |
|
|
| I like to handle the errors myself by looking at the PIC registers. However, Ttelmah suggested that you add the ERRORS to you #use rs232 statement. This causes the errors to be placed in the RS232_ERRORS variable. Refer to the manual for more details. My concern would be to be receiving data, get and an overflow or parity error and then receive other bytes. This would cause an invalid reading that could have been detected by checking for errors. |
|
|
weg22
Joined: 08 Jul 2005
Posts: 91
|
|
| Posted: Thu Jan 11, 2007 4:38 pm |
|
|
Finally got it working!! Just wanted to thank everyone for all of their input. The solution was based off of Ttelmah's code, but with a modification based on ckielstra's code (actually, the "isdigit" function). If anyone cares, the INT_RDA routine is below:
| Code: |
#INT_RDA2 // RS232 interrupt for sonar
void sonarRda_isr(void)
{
int8 c;
static int8 state=0;
c = fgetc(sonar);
switch (state)
{
case 0:
if (c=='R') state=1;
break;
case 1:
if(isdigit(c)) {sonarBuff[0] = c; state=2;} // make sure it's a number
else state=0;
break;
case 2:
if(isdigit(c)) {sonarBuff[1] = c; state=3;} // make sure it's a number
else state=0;
break;
case 3:
if(isdigit(c)) {sonarBuff[2] = c; state=4; newval=true;} // make sure it's a #
else state=0;
break;
case 4:
if (c=='\r') state=0;
break;
}
}
|
Last edited by weg22 on Thu Jan 11, 2007 4:39 pm; edited 2 times in total |
|
|
Ttelmah
Guest
|
|
| Posted: Thu Jan 11, 2007 4:38 pm |
|
|
Invalid readings, is only half the problem with an error. If an overrun error happens, and is not handled, the receive UART, will stop working. The 'ERRORS' command, adds a recovery for this into the getc command.
Are you sure the sonar, does actually return three digits every time?. If not, all the codes will fail.
I'd guess, that in fact the interrupt for the '8', is getting missed. The code then writes the carriage return into the next cell, and on the next time through, the '2' is read by the main code. This implies that either there is an interrupt handler that takes longer than two character times to complete, or a block of code in the main, with interrupts disabled for the same time. The latter is probably the most likely. Look for any routines used in any interrupt handler, that are also used in the main code.
The setup for a switch, takes almost the identical time needed to access an array entry.
Answers here, are sometimes like busses!. I remember one thread, where I think eight functionally identical replies arrived in a few minutes!...
Best Wishes |
|
|
treitmey
Joined: 23 Jan 2004
Posts: 1094
Location: Appleton,WI USA
|
|
| Posted: Thu Jan 11, 2007 5:49 pm |
|
|
I guess I'll give you my 2 cent idea.
And this is only if there is some idle time between messages.
IF there is an idle time between the messages,
I'd make a timer to check for that idle time and
consider that as a message.
Then I could look for a error or malformed message or whatever type of error and if it occurs, and I could reset my buffer.
That way I could hope to receive an update
from the sonar before my car hits a tree.
woops ((sonar)) perhaps that should be my sub sits a tanker. : ) |
|
|
|
|
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
