| View previous topic :: View next topic |
| Author |
Message |
Requan
Joined: 11 May 2008
Posts: 74
|
| Rs232 voltmeter |
 Posted: Mon Feb 27, 2012 9:21 am |
 |
|
Dear All,
Previously i wrote programs on PICs in basic language. Now i'am learning CCS because basic has to much limitations.
Base on samples i found on forum i create program: voltmeter rs232: Program wait for rs232 data with 13 (0x0D) on the end. If we send data without 13 on the end, program skip this data.
Program send volts via rs232 and display on LCD. I created simple program on PC in c# enivorment and this soft ask for data from PIC each 100ms.
All works fine.
I am still learning so i will be thankful for any comments to help me write programs in better way.
This is program:
| Code: |
#include <16f873.h>
#device ADC=10
#fuses HS, NOWDT, NOPROTECT, BROWNOUT, PUT, NOLVP
#use delay(clock=20000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7,bits=8,parity=N,stop=1,stream = UART1)
#include "Flex_Lcd.c"
#define BUFFER_SIZE 32
BYTE buffer[BUFFER_SIZE];
BYTE next_in = 0;
float volts;
#BIT PIR1_5 = 0x0C.5
Boolean bkbhit = False; //Received data flag
#int_rda //rs232 interrupt
void serial_isr() {
int t;
buffer[next_in]=fgetc(UART1);
t=next_in;
if( buffer[next_in] != 0x0D) //wait for receive Termination char
{
bkbhit = False;
}
else
{
bkbhit = True;
}
if(++next_in==BUFFER_SIZE) // if oversize
{
next_in =0;
bkbhit = False;
}
if(next_in == 0) //get rid of the ';' here - wrong.....
{
next_in = t; // Buffer full !!
bkbhit = False;
}
}
void Measure()
{
int16 adc_value;
adc_value = read_adc();
volts = (float)(adc_value * 5)/1023.0;
printf(lcd_putc, "\fVolt=%3.2f%s", volts,"V"); //3-znaki; 2- znaki po przecinku
delay_ms(70);
}
//============================
void main()
{
enable_interrupts(global);
enable_interrupts(int_rda);
printf("\r\n\Running...\r\n");
lcd_init();
setup_adc_ports(AN0);
setup_adc(ADC_CLOCK_DIV_32);
set_adc_channel(0);
delay_us(20);
while(1)
{
Measure();
if(bkbhit)
{
PIR1_5 = 0; // turn off USART Interrupt
if ((buffer[0] =='?') & (buffer[1] == 'V'))
{
fprintf(UART1,"%3.2fV%c", volts, 0x0D);
}
else
{
fprintf(UART1,"Error");
}
memset(buffer, 0, sizeof(buffer));//clear buffer
next_in = 0;
bkbhit = False;
PIR1_5 = 1; // turn on USART Interrupt
}
}
}
|
Best Regards,
Martin |
|
 |
Requan
Joined: 11 May 2008
Posts: 74
|
|
| Posted: Thu Mar 01, 2012 2:14 am |
|
|
It is nice to hear...
If You design system PC - uP, You can add CRC check.
Quick and good solution is to apply xor, so Your CRC = byte1 ^ byte2 ^ byte3 ^...
++++++++++++++++++++
Previous post was from a spambot.
Post removed.
Sorry.
- Forum Moderator
++++++++++++++++++++ |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Mon Mar 05, 2012 5:18 pm |
|
|
I'd like to say as politely as i can, that the serial ISR shown here should NOT be copied - as the way it handles bkbhit is rather illogical, and can cause trouble under several circumstances.
bkbhit should ONLY report if characters are in the receive buffer or not,
AND the question of what to DO with characters received should be handled by a high level parser, OUTSIDE the ISR , that removes characters in sequence and decides what to do with them.
Resetting the buffer is a VERY poor practice.
The author would do well to study EX_SISR ( which this was obviously cribbed from) and better understand that it was done the way it was for a VERY good reason. And that this alteration weakens the function of the ISR in a big way. Clearing the circular buffer and restarting after a removal further weakens it , as after a message is pulled for LCD display - the DELAY in sending to the LCD could allow a NEW message to come and then be cancelled when the buffer is reset. This defeats the primary purpose of the circular buffer ISR.
I do understand what the author WANTS to do - but really - it would be much simpler and better to BKBHIT as it was intended , and detect a carriage with a variable perhaps. That would be better instead of the crazy tap dance that is coded now, and then HEW to the proper circular nature of the buffer as EX_SISR shows.
In short - this is an example of how to take a very well done - high function, SIMPLE bit of code - and then weaken it in every way. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Mon Mar 05, 2012 8:04 pm |
|
|
Some things to watch with the ex_sisr.c file:
1. It doesn't indicate that the buffer size is very important to the timing of the ISR. The % operation is only fast when the value is a power of 2 (which the example does use...32). If a buffer size that isn't a power of 2 is used, then the % operator is as slow as a divide, which isn't great for an ISR
2. It doesn't use the ERRORS keyword with the #use rs232() call. Granted, the reads happen in an ISR, so it probably should be ok, but if you ever disable interrupts for too long that could cause you trouble and lock up the UART. |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Mon Mar 05, 2012 8:43 pm |
|
|
i've done it this way for quite a while now
buffer can be ANY size - no magic numbers etc
no speed impairment
| Code: |
#int_rda
void serial_isr() {
int t;
buffer[next_in]=getc();
t=next_in;
next_in=(next_in+1); // these 2 lines instead of modulo
if (next_in==BUFFER_SIZE) next_in=0;
if(next_in==next_out) next_in=t; // Buffer full !!
}
|
|
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Mon Mar 05, 2012 9:30 pm |
|
|
| Yep, I was referencing the ex_sisr.c method, which uses the % operator (at least in the version that comes with 4.124-4.130). You were mentioning how the method it used was better, and I was just making sure he understood that using it that way means he has to consider buffer size in order to get the % operation down to a single AND instruction rather than a DIV instruction, which takes a lot longer. I tend to use the method you just highlighted. |
|
|
Requan
Joined: 11 May 2008
Posts: 74
|
|
| Posted: Sat Mar 10, 2012 4:01 pm |
|
|
| Code: |
#int_rda t
void serial_isr() {
int t;
buffer[next_in]=getc();
t=next_in;
next_in=(next_in+1); // these 2 lines instead of modulo
if (next_in==BUFFER_SIZE) next_in=0;
if(next_in==next_out) next_in=t; // Buffer full !!
}
|
Yes this code I had on the beginning and this is very poor solution. When I simulate situation: First I send incomplete date (it could happen in real life). Next, I send complete data, and in buffer I had all of it. So I add termination character (I communicated with a lot of factory equipments via rs232 and all of it had termination characters).
This code I tested during 1 week and ask every 100ms and nothing happen. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Mon Apr 09, 2012 7:49 pm |
|
|
| I've used the code asmboy posted with no functional issues on real hardware. The buffer is supposed to have all of it. That's how the buffer works. You need to read the data out with bgetc() to get it out of the buffer. Don't try to access the buffer array directly. |
|
|
Requan
Joined: 11 May 2008
Posts: 74
|
|
| Posted: Sun Apr 15, 2012 1:00 am |
|
|
| Quote: | | I've used the code asmboy posted with no functional issues on real hardware. The buffer is supposed to have all of it. That's how the buffer works. You need to read the data out with bgetc() to get it out of the buffer. Don't try to access the buffer array directly. |
This example is good when you receive data with the same length.
I wrote test program: program received data and sent it back. When I sent data from PC very quickly and with different length, PIC sometimes sent back data with mistake (because when eg buffer had length 5 byte and I sent data 2 bytes it still wait for 3 bytes)
So when you receive data with different length - use command:" receive until buffer is full" it does not make sense because sometimes data length is less then buffer.
Now you understand why I apply termination char (0x0D). |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Sun Apr 15, 2012 6:35 am |
|
|
That's not what the buffer is doing at all. It handles any length. bkbhit returns true when ANY number of bytes are available...not just the size of the buffer. The buffer / ISR method doesn't "wait" for any amount of data. It just collects it as it comes in. It doesn't care how long the data is aside from needing to be big enough to handle that data, but it doesn't wait for a specific length.
You'll need to show an example (fully compilable) where the buffer cares about length and waited until it was full, because the code provided doesn't do that. |
|
|
Requan
Joined: 11 May 2008
Posts: 74
|
|
| Posted: Sun Apr 15, 2012 10:48 am |
|
|
 jeremiah
Thanks for explanation.
Please correct me if i wrong: so i have to in main program check bkbhit status and read data from byte by byte until i read 0x0D? |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Sun Apr 15, 2012 12:59 pm |
|
|
| Requan wrote: | jeremiah
Thanks for explanation.
Please correct me if i wrong: so i have to in main program check bkbhit status and read data from byte by byte until i read 0x0D? |
Yep. Think of it this way. Most PICs have a serial buffer on them, but it is typically very small (the chips I use have only a 4 byte buffer). Using this method, you can drastically increase the size of the buffer AND receive them as soon as they are sent to the PIC, but you don't have to work with them until you are ready.
| Code: |
#define BUFFER_SIZE 64 //much larger buffer
BYTE buffer[BUFFER_SIZE];
BYTE next_in = 0;
BYTE next_out = 0;
//this function moves incoming serial data to the circular buffer
#int_rda
void serial_isr() {
int t;
buffer[next_in]=getc();
t=next_in;
next_in=(next_in+1); // these 2 lines instead of modulo
if (next_in==BUFFER_SIZE) next_in=0;
if(next_in==next_out) next_in=t; // Buffer full !!
}
#define bkbhit (next_in!=next_out)
BYTE bgetc() {
BYTE c;
while(!bkbhit) ;
c=buffer[next_out];
if(next_out >= (BUFFER_SIZE-1) ){
next_out = 0;
}else{
next_out++;
}
return (c);
}
void main(){
BYTE data;
while(TRUE){
if(bkbhit){
data = bgetc();
switch(data){
//do stuff with the data here
}
}
}
}
|
instead of the switch statement, you could also just save them to an array if you prefer that method. The point is, bkbhit() tells you there is at least one byte in the buffer, but there could be more. Having the bigger buffer store them in the ISR and using bkbhit() and bgetc() to look at the data later gives you some breathing room to work with the data when your code is ready.
The macro for bkbhit:
| Code: |
#define bkbhit (next_in != next_out)
|
returns true as long as there is data in the buffer. The time when next_in == next_out only occurs when the buffer is empty. I know the comment in ISR says "full", but notice the next line that immediately changes next_in to be the previous value, so outside the ISR, next_in will never equal next_out unless the buffer is empty. So bkbhit returns FALSE if the buffer is empty or TRUE if there is any amount of data in the buffer.
That explain it more clearly? |
|
|
Requan
Joined: 11 May 2008
Posts: 74
|
|
| Posted: Wed May 02, 2012 11:34 am |
|
|
Hi Jeremiah,
Thanks for Your time again.
Yes, now its clear.
I tried to wrote code:
- receiver bytes and send it back - it worked
- wrote data in to buffer RX and check with pattern - it worked only when i received proper data because I clear index "i" only when I received proper message, so how to detect end of transmision?
Please look at my code and advise me:
| Code: |
int i = 0;
BYTE RX[32];
do
{
if(bkbhit)
{
RX[i] = bgetc();//put data into buffer
putc(RX[i]);//display data
i++;
}
if(RX[0] == '?' && RX[1] == 'V')
{
printf("OK");
i = 0;
next_in = 0;
next_out = 0;
memset(RX, 0, sizeof(RX));//clear buffer
}
} while (TRUE);
|
|
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1345
|
|
| Posted: Wed May 02, 2012 1:56 pm |
|
|
First of all, don't mess with next_in or next_out. That can mess you up since the ISR and bgetc() are the only methods supposed to touch those.
In order to detect end of transmission, you have to decide what character is the end of transmission and check for that. You can try a state machine. There are plenty of threads on how to make one. |
|
|
Requan
Joined: 11 May 2008
Posts: 74
|
|
| Posted: Wed May 02, 2012 3:05 pm |
|
|
Yes, i tried to use CR character:
| Code: |
if(bkbhit)
{
while(RX[i] != 0x0D)
{
RX[i] = bgetc();//put data into buffer
putc(RX[i]);//display data
i++;
}
}
if(RX[0] == '?' && RX[1] == 'V')
{
printf("OK");
i = 0;
memset(RX, 0, sizeof(RX));//clear buffer
}
|
It should work, i don't understand what i did wrong? |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
