 |
 |
| View previous topic :: View next topic |
| Author |
Message |
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
| Avoiding GOTO/LABEL in coding... |
 Posted: Fri Jan 30, 2015 4:59 pm |
 |
|
I need to read the following data from serial :
| Code: |
First Char (Any 4 letters) | Second Char (Any 4 letters) | Value
R | A | 0000 to 4095 (in ASCII)
G | B | 0000 to 4095 (in ASCII)
B | C | 0000 to 4095 (in ASCII)
W | D | 0000 to 4095 (in ASCII)
Example data from serial (all in ASCII) --> RA2544GA0032BA0000WA4000
|
In state machine programming that code would looks like (ugly):
| Code: |
int8 packetcolor,packetstep,packetchannel,packetvalue;
int1 canread;
#INT_RDA
void serial_isr() {
int8 rxchar;
rxchar=getc();
start: //label for start of the sorting
if (packetstep == 0){ //first step read the first char (R/G/B or W)
switch (rxchar){
case 'R' :packetcolor=1;
packetstep = 1;
break;
case 'G' :packetcolor=2;
packetstep = 1;
break;
case 'B' :packetcolor=3;
packetstep = 1;
break;
case 'W' :packetcolor=4;
packetstep = 1;
break;
default:packetstep = 0; //stay at 0 if not RGBW (to restart trying capturing the first char)
break;
}
}
if (packetstep == 2){ //second step try to read the second char
switch (rxchar){
case 'A' :packetchannel=1;
packetstep = 3;
break;
case 'B' :packetchannel=2;
packetstep = 3;
break;
case 'C' :packetchannel=3;
packetstep = 3;
break;
case 'D' :packetchannel=4;
packetstep = 3;
break;
default:packetstep = 0;
Goto start;//stay at 0 and restart the sorting (second chance catching the first char [RGBW] if not ABCD) by going at the beginning of the interrupt
break;
}
}
if (packetstep == 3){ //third step getting values
switch (rxchar){
case '0' : //store number
packetstep = 4;
break;
case '1' ://store number
packetstep = 4;
break;
case '2' ://store number
packetstep = 4;
break;
case '3' ://store number
packetstep = 4;
break;
default:packetstep = 0; //Not a valid number so restart
Goto start;//stay at 0 and restart the sorting (second chance catching the first char [RGBW] if not a number) by going at the beginning of the interrupt
break;
}
}
[...]
Flag the canread to true when everything is finished capturing then read the vars!
}
|
Code is not complete (in fact it's an aberration  )
It's just something that I've written on a piece of paper didn't even try to compile it yet!
I could skip the goto and declare invalid data but I would loose the entire packet until I catch the right first character.
Any solution to chicken or egg problem?
PS: I know it's bad to do this in the INT_RDA interrupt it's just I don't trust the built-in serial buffer from CCS (overwrite new data first) and KISS for testing.
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
 |
Mike Walne
Joined: 19 Feb 2004
Posts: 1785
Location: Boston Spa UK
|
| Re: Avoiding GOTO/LABEL in coding... |
| Posted: Fri Jan 30, 2015 5:51 pm |
|
|
| ELCouz wrote: |
I could skip the goto and declare invalid data but I would loose the entire packet until I catch the right first character.
|
Can you explain more clearly what you want to happen on a restart.
Is there not a problem with second characters having possible hex values?
Mike
EDIT My first thought was to use a data array and only one loop, not 6 code versions for each character. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
| Re: Avoiding GOTO/LABEL in coding... |
| Posted: Fri Jan 30, 2015 5:54 pm |
|
|
| Mike Walne wrote: | | ELCouz wrote: |
I could skip the goto and declare invalid data but I would loose the entire packet until I catch the right first character.
|
Can you explain more clearly what you want to happen on a restart.
Is there not a problem with second characters having possible hex values?
Mike |
Sorry if it's not clear 
I want to retry capturing the first char hence the goto label (at top of the interrupt)
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
Mike Walne
Joined: 19 Feb 2004
Posts: 1785
Location: Boston Spa UK
|
|
| Posted: Fri Jan 30, 2015 5:57 pm |
|
|
You mean re-try with the character you've just tested?
Mike |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19496
|
|
| Posted: Sat Jan 31, 2015 1:57 am |
|
|
It's classic (and easy) to do.
Given your values are just alpha, have an multi dimensional array with the characters in. "RGBW" for the first row.
Have the 'row number' as a _static_ variable. Set it to zero when declared.
Read the character.
Enclose code in a do loop. do{}while(TRUE);
If row==0.
Then just use a for loop through the column characters in the row. If the character matches 'packetcolor', is set to the column number+1, and the row is incremented. 'Break' out of the loop. If no match also break out.
If row==1.
Next time in since row is incremented, it now looks through the second row in the array. Again if there is a match, 'packetchannel' is set to column+1, and the row is incremented. Again break out of the loop. If however there is no match, simply set 'row' back to 0, and execute a 'continue', which means it'll execute the loop again, with row==0, and repeat the first comparison.
If row == 2, process the first digit of the value, and increment row. Break.
If row == 3, process the second digit of the value. etc...
When you get to the end of the value, set your flag, and remember to set row back to 0.
Just put a core together using this:
| Code: |
char first[]="RGBW";
char second[]="ABCD";
#inline
int8 find(char what, char * where)
{
int8 seek; //routine to search for a single character in a four char array
for (seek=0;seek<4;seek++)
{
if (where[seek]==what)
return seek+1;
}
return 0;
}
#inline
int16 fast_ten(int16 x)
{
int16 temp; //fastest way to multiply by 10 - also doesn't use mult
temp=x*2;
return (temp*4)+temp;
}
#INT_RDA
void rx_char(void)
{
static int8 row=0;
static int16 build_val;
int8 temp;
int8 rxchar;
rxchar=getc();
do
{
if (row==0)
if ((temp=find(rxchar,first))==0) //test if zero, and save as well
break;
else
{
packetcolor=temp;
row++; //next time check for channel
break;
}
if (row==1)
if ((temp=find(rxchar,second))==0)
{
row=0; //force the re-scan
continue; //re-scan the character
}
else
{
packetchannel=temp;
row++; //now looking for number
build_val=0;
break; //exit
}
else
{
//Now all the numbers can be handled the same
if (isdigit(rxchar))
{
build_val=fast_ten(build_val);
build_val=build_val+(rxchar-'0');
break; //exit
}
else
{
//here we have an non numeric, so value complete
packet_value=build_val; //adjust variable names etc here to suit
flag=TRUE;
row=0;
continue; //and scan the text character again
}
}
} while(TRUE);
}
|
You'll get a warning about doing an assignment in a test, but this is just me avoiding having to transfer the value twice. I put the numeric result into 'packet_value' - don't know what your variable is called, and just set 'flag'. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sat Jan 31, 2015 5:26 pm |
|
|
| Ttelmah wrote: | It's classic (and easy) to do.
Given your values are just alpha, have an multi dimensional array with the characters in. "RGBW" for the first row.
Have the 'row number' as a _static_ variable. Set it to zero when declared.
Read the character.
Enclose code in a do loop. do{}while(TRUE);
If row==0.
Then just use a for loop through the column characters in the row. If the character matches 'packetcolor', is set to the column number+1, and the row is incremented. 'Break' out of the loop. If no match also break out.
If row==1.
Next time in since row is incremented, it now looks through the second row in the array. Again if there is a match, 'packetchannel' is set to column+1, and the row is incremented. Again break out of the loop. If however there is no match, simply set 'row' back to 0, and execute a 'continue', which means it'll execute the loop again, with row==0, and repeat the first comparison.
If row == 2, process the first digit of the value, and increment row. Break.
If row == 3, process the second digit of the value. etc...
When you get to the end of the value, set your flag, and remember to set row back to 0.
Just put a core together using this:
| Code: |
char first[]="RGBW";
char second[]="ABCD";
#inline
int8 find(char what, char * where)
{
int8 seek; //routine to search for a single character in a four char array
for (seek=0;seek<4;seek++)
{
if (where[seek]==what)
return seek+1;
}
return 0;
}
#inline
int16 fast_ten(int16 x)
{
int16 temp; //fastest way to multiply by 10 - also doesn't use mult
temp=x*2;
return (temp*4)+temp;
}
#INT_RDA
void rx_char(void)
{
static int8 row=0;
static int16 build_val;
int8 temp;
int8 rxchar;
rxchar=getc();
do
{
if (row==0)
if ((temp=find(rxchar,first))==0) //test if zero, and save as well
break;
else
{
packetcolor=temp;
row++; //next time check for channel
break;
}
if (row==1)
if ((temp=find(rxchar,second))==0)
{
row=0; //force the re-scan
continue; //re-scan the character
}
else
{
packetchannel=temp;
row++; //now looking for number
build_val=0;
break; //exit
}
else
{
//Now all the numbers can be handled the same
if (isdigit(rxchar))
{
build_val=fast_ten(build_val);
build_val=build_val+(rxchar-'0');
break; //exit
}
else
{
//here we have an non numeric, so value complete
packet_value=build_val; //adjust variable names etc here to suit
flag=TRUE;
row=0;
continue; //and scan the text character again
}
}
} while(TRUE);
}
|
You'll get a warning about doing an assignment in a test, but this is just me avoiding having to transfer the value twice. I put the numeric result into 'packet_value' - don't know what your variable is called, and just set 'flag'. |
Thank you very much for the help Ttelmah!
Very clean programming
However... ain't the while(TRUE) gonna jam the whole RDA interrupt (less time you spent in interrupt the better it is normally)?
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sat Jan 31, 2015 6:38 pm |
|
|
Code work but it won't capture the whole data Ttelmah...
Example data: RA3000GA2000BA1000WA0000
Example code (replicating the problem):
| Code: |
void main() {
enable_interrupts(INT_RDA);
enable_interrupts(GLOBAL);
printf("PIC18F2550 Ready!\n\r");
while(true) {
if (canread){
printf("Got a packet! :) ... Color: %u Channel: %u Value: %Lu \n\r",packetcolor,packetchannel,packetvalue);
canread = false;
}
}
}
|
Will output this:
| Code: | | Got a packet! :) ... Color: 4 Channel: 1 Value: 0 |
Not the four packet!
Will only display the last "packet" !
EDIT: the only change I did in your code is replacing FLAG by canread int1 var!
EDIT2: maybe I need a buffer because RDA is too fast for printf!
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19496
|
|
| Posted: Sun Feb 01, 2015 1:56 am |
|
|
No more so than the goto.
Follow the paths. If it finds the character it _breaks_ (so comes straight out of the loop). Only if it doesn't find the character on the second route, or it hits an alpha character when waiting for the number, does it execute a 'continue' (so loops back and tries the character again).
OK. See you have tried it.
I expected what you are now seeing, but it was not clear from your description how the numbers are sent. Are they _always_ four digits?.
As written, it won't complete the number till it sees a _non numeric_ character.
So as soon as another packet follows it'll complete. If you are confident that there are always four digits, simply count instead. So:
| Code: |
#INT_RDA
void rx_char(void)
{
static int8 row=0;
static int16 build_val;
static int8 digits;
int8 temp;
int8 rxchar;
rxchar=getc();
do
{
if (row==0)
if ((temp=find(rxchar,first))==0) //test if zero, and save as well
break;
else
{
packetcolor=temp;
row++; //next time check for channel
break;
}
if (row==1)
if ((temp=find(rxchar,second))==0)
{
row=0; //force the re-scan
continue; //re-scan the character
}
else
{
packetchannel=temp;
row++; //now looking for number
build_val=0;
digits=4; //four digits to receive
break; //exit
}
else
{
//Now all the numbers can be handled the same
build_val=fast_ten(build_val);
build_val=build_val+(rxchar-'0');
if (--digits)
break; //exit if not zero
else
{
//here we have received four digits
packet_value=build_val; //adjust variable names etc here to suit
flag=TRUE;
row=0;
break; //and exit
}
break;
}
} while(TRUE);
}
|
This will exit after four digits, but will _not_ test that the values are numeric. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sun Feb 01, 2015 7:32 am |
|
|
| Ttelmah wrote: | No more so than the goto.
Follow the paths. If it finds the character it _breaks_ (so comes straight out of the loop). Only if it doesn't find the character on the second route, or it hits an alpha character when waiting for the number, does it execute a 'continue' (so loops back and tries the character again).
OK. See you have tried it.
I expected what you are now seeing, but it was not clear from your description how the numbers are sent. Are they _always_ four digits?.
As written, it won't complete the number till it sees a _non numeric_ character.
So as soon as another packet follows it'll complete. If you are confident that there are always four digits, simply count instead. So:
{code_removed_to_shorten_the_quote}
This will exit after four digits, but will _not_ test that the values are numeric. |
Yes exactly! Exactly 4 digits all the time!
You code works nicely however I still can't catch all the "packets" !
Your new code based on counting exactly detect and store the packet when it reach the last digit... but the output still is read once!
Same serial example: RA3000GA2000BA1000WA0000
Still output:
| Code: |
Color: 2 Channel: 1 Value: 0
|
Normally I should get this output:
| Code: |
Color: 1 Channel: 1 Value: 3000
Color: 2 Channel: 1 Value: 2000
Color: 3 Channel: 1 Value: 1000
Color: 4 Channel: 1 Value: 0
|
I've tried with the built-in CCS serial buffer --> #use rs232(baud=115200, UART1,errors,RECEIVE_BUFFER = 64)
and throwing your whole RDA code into the kbhit statement in the main loop instead.
I get nothing anymore.
I think the problem is the RDA loop set the flag canread too quickly for the main loop to catch it!
Please correct me if i'm wrong!
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9221
Location: Greensville,Ontario
|
|
| Posted: Sun Feb 01, 2015 7:42 am |
|
|
hmm...
Wondering have you tried the CCS ex_sisr.c example program to use a buffer for the data, then parse the buffer?
Also, there might be a 'glitch' in the 'buffer' option in the use rs232(...) depending on the compiler version.
just ideas...
Jay |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sun Feb 01, 2015 7:45 am |
|
|
| temtronic wrote: |
Also, there might be a 'glitch' in the 'buffer' option in the use rs232(...) depending on the compiler version.
just ideas...
Jay |
Right now I'm on V5.035, I plan to upgrade later when it will stabilize the last 2 version were bad!
| Quote: | | Wondering have you tried the CCS ex_sisr.c example program to use a buffer for the data, then parse the buffer? |
I thought it was the same as the built-in buffer!
Of course, I will try it thanks! 
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sun Feb 01, 2015 7:55 am |
|
|
Thanks it work!!
Now the output is:
| Code: |
Color: 1 Channel: 1 Value: 3000
Color: 2 Channel: 1 Value: 2000
Color: 3 Channel: 1 Value: 1000
Color: 4 Channel: 1 Value: 0
|
Everything work normally
Seems built-in buffer was buggy... in fact it was driving me nuts!
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19496
|
|
| Posted: Sun Feb 01, 2015 8:53 am |
|
|
The built in buffer will throw away the _latest_ character if it overflows. Ex_sisr throws the _oldest_. It's a very important difference, and has been commented on here.
You can't use the built in buffer with you using a serial interrupt. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sun Feb 01, 2015 8:57 am |
|
|
| Ttelmah wrote: | The built in buffer will throw away the _latest_ character if it overflows. Ex_sisr throws the _oldest_. It's a very important difference, and has been commented on here.
You can't use the built in buffer with you using a serial interrupt. |
I did remove the RDA interrupt (and enables) since the built-in manage it itself!
I've seen that you noticed long time ago CCS for this buffer silly problem (way it treats the old vs new data)
However I wonder why it didn't changed as today?
What are the advantage of this, why they still want to leave it that way?
Thank you both Ttelmah and temtronic for your great support!
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
|
|
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
