CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

Rs232 voltmeter

 
Post new topic   Reply to topic    CCS Forum Index -> Code Library
View previous topic :: View next topic  
Author Message
Requan



Joined: 11 May 2008
Posts: 74

View user's profile Send private message

Rs232 voltmeter
PostPosted: Mon Feb 27, 2012 9:21 am     Reply with quote

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

View user's profile Send private message

PostPosted: Thu Mar 01, 2012 2:14 am     Reply with quote

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

View user's profile Send private message AIM Address

PostPosted: Mon Mar 05, 2012 5:18 pm     Reply with quote

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: 1314

View user's profile Send private message

PostPosted: Mon Mar 05, 2012 8:04 pm     Reply with quote

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

View user's profile Send private message AIM Address

PostPosted: Mon Mar 05, 2012 8:43 pm     Reply with quote

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: 1314

View user's profile Send private message

PostPosted: Mon Mar 05, 2012 9:30 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Sat Mar 10, 2012 4:01 pm     Reply with quote

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: 1314

View user's profile Send private message

PostPosted: Mon Apr 09, 2012 7:49 pm     Reply with 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.
Requan



Joined: 11 May 2008
Posts: 74

View user's profile Send private message

PostPosted: Sun Apr 15, 2012 1:00 am     Reply with quote

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: 1314

View user's profile Send private message

PostPosted: Sun Apr 15, 2012 6:35 am     Reply with quote

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

View user's profile Send private message

PostPosted: Sun Apr 15, 2012 10:48 am     Reply with quote

Arrow 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: 1314

View user's profile Send private message

PostPosted: Sun Apr 15, 2012 12:59 pm     Reply with quote

Requan wrote:
Arrow 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

View user's profile Send private message

PostPosted: Wed May 02, 2012 11:34 am     Reply with quote

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: 1314

View user's profile Send private message

PostPosted: Wed May 02, 2012 1:56 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Wed May 02, 2012 3:05 pm     Reply with quote

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?
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> Code Library All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
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