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

UART again
Goto page 1, 2  Next
 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
Jean FOUGERON



Joined: 30 Nov 2012
Posts: 110
Location: France

View user's profile Send private message Visit poster's website

UART again
PostPosted: Mon May 19, 2014 9:51 am     Reply with quote

One year ago ckielstra and Ttelmah helped me a lot to write a receive procedure, which works perfectly untill now.

The message to be received is as follow
aa fc nn nn 04 xx xx crc CRC
where :
aa is the adress of the set
fc is a function code the set has to proceed
nn nn is a transaction number
xx xx are the datas linked used with the function code
crc CRC is ... as they say themselves

Code:
#INT_RDA
void Receive()
{
char c;


   c=getc();
   if(I_UART_In_Pos==0 && c==I_Adresse_Box)         // The take in account only if I am the correct set                                                
   {                                       
      INStr[I_UART_In_Pos++]=c;               // store the character
      B_UART_OK=FALSE;                                
   }

   else if(I_UART_In_Pos>0 && I_UART_In_Pos<UART_MAX)
     {
        INStr[I_UART_In_Pos++]=c;                     
        B_UART_OK=(I_UART_In_Pos>=UART_MAX);         // is the buffer full ?
   }
   
   L_Cycle_UART=1;                           // en prĂ©vision du Time_out
}


It works, I think, because the set alway waits for a message beginning by its adress and because there was only one set connected at the same time

But, when have to pilot more than one set at the same time (32 in fact) I want to use a broadcast adress, which I fix to 0

So the sets have to wait for their adress AND adress 0

Code:
if(I_UART_In_Pos==0 &&( c==I_Adresse_Box || c==0x00))


But adress 0 can be anywhere in the message, so the set which does not react to the first character because it is not its adress (so I_UART_In_Pos stays =0) will react to the first zero seen in the message

for example, if the message is 01 03 00 03 04 00 0D 8B B3, the set 01 will react OK but the set 02 will react to the 3rd character of the message as it must not.

As the messages are always the same length, I thought I could use a Time out system : If set 02 react to the 3rd character, it will not have the 9 characters at the end of the message, so if I re-initialisate I_UART_In_Pos to 0 before the next message it could be good. Could'nt it ?

So I have introduced a test in a Timer procedure

Code:
#int_TIMER1
void Lit_Switches()                           // every 13 milliseconds
{

   ... some code ...
   
   if(I_UART_In_Pos>0)
   {
      I_UART_TimeUp++;
      if(I_UART_TimeUp>=8)                  // 8 x 13ms = 104ms, it will occurs after the end of the message wich is 75ms
      {   
         I_UART_TimeUp=0;
         //I_UART_In_Pos=0;
      }   
   }      
}


But it does not work, and I do not understand why yet

Do you have any idea to help me find a solution to this issue ?
ckielstra



Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands

View user's profile Send private message

PostPosted: Mon May 19, 2014 10:53 am     Reply with quote

To avoid your problem many protocols start with a fixed sequence of several known bytes. The chance of this special start sequence occurring in the middle of your message is very small and is a good filter for detecting start of the message.
Disadvantage is that you have to send extra bytes, this takes time and power.

Your solution with a time-out is another approach used in many systems.

One reason for it not working is this line:
Code:
         //I_UART_In_Pos=0;
Remove the comments.

Another potential problem is that you are waiting 8 character times before you reset the receiver. How much time do you have between your messages? When this is less than 8 * 13ms = 104ms then you will miss the start of the next message. Not a really big problem, but you can optimize this a bit when you reset I_UART_TimeUp after every received character. Then you can reset the receiver when for example 4 characters are missing.
Something like this:
Code:
#define BROADCAST_ADDRESS    (0x00)            // Using a define makes your code easier to understand.

#INT_RDA
void Receive()
{
   char c;

   c=getc();
   if ((I_UART_In_Pos==0)
       && ((c==I_Adresse_Box) || (c==BROADCAST_ADDRESS)))   // Accept only if I am the correct set
   {                                       
      INStr[I_UART_In_Pos++]=c;               // store the character
      B_UART_OK=FALSE;                               
   }
   else if ((I_UART_In_Pos>0) && (I_UART_In_Pos<UART_MAX))
   {
        INStr[I_UART_In_Pos++]=c;                     
        B_UART_OK=(I_UART_In_Pos>=UART_MAX);         // is the buffer full ?
   }
   
   L_Cycle_UART=1;                           // en prĂ©vision du Time_out
   I_UART_TimeUp=0;                          // reset the receiver time-out.
}




Code:
#define RECEIVE_TIMEOUT    4                  // 4 x 13ms = 52ms. After receiving no new characters for this time we reset the receiver logic.

#int_TIMER1
void Lit_Switches()                           // every 13 milliseconds
{

   ... some code ...
   
   if(I_UART_In_Pos>0)
   {
      I_UART_TimeUp++;
      if (I_UART_TimeUp >= RECEIVE_TIMEOUT)
      {   
         I_UART_TimeUp=0;
         I_UART_In_Pos=0;
      }   
   }     
}
asmboy



Joined: 20 Nov 2007
Posts: 2128
Location: albany ny

View user's profile Send private message AIM Address

PostPosted: Mon May 19, 2014 5:32 pm     Reply with quote

Rather than the ISR + timer route you are struggling with, have you thought about simply catching ALL the chars in a standard circular serial receive buffer
and sorting out what is in the buffer in MAIN(); at your leisure ??

Far simpler really and very fast in and out of the ISR -
with a big enough circular receive buffer - you can sort out what you have
in the foreground and never risk a dropped character.

Your timeout-timing can then be a foreground process as well, possibly not even needing a ISR for timer service either.
Jean FOUGERON



Joined: 30 Nov 2012
Posts: 110
Location: France

View user's profile Send private message Visit poster's website

PostPosted: Tue May 20, 2014 2:16 am     Reply with quote

Thanks friends, for your message

To ckielstra

Quote:
One reason for it not working is this line:
Code:
//I_UART_In_Pos=0;
Remove the comments.


I comment because it does not work on my panel. I have a spy on the time out procedure and I observe that it loses one shot on two
I mean that the supervisor queries 2 sets, nbr 01 and nbr 02. On my panel, which is nbr 01, reacts to query 01, sleep query 02 SLEEP query 01 sleep query 02 and reacts query 01
The time out procedure should go on RECEIVE_TIMEOUT x 13 ms on each query 02, but it goes on 2,4 seconds (query repetition time : approx 0,5s)

And for the time beeing, I have not understood yet what happens

To asmboy

Quote:
with a big enough circular receive buffer - you can sort out what you have
in the foreground and never risk a dropped character.


You mention a good proposal, I will study today also, so I will have two possibilities if I need.
The supervisor queries either :
aa 03 nn nn 04 xx xx crc crc where aa is 01 to 20 (32 sets maxi on the line) (03 is the JBus command to read many registers)
or 00 06 nn nn 04 xx xx crc crc which is the brodcast command (06 is the Jbus command to write a register)

So I could look for the sequence (I_Adresse_Box 03 or 00 06) && (5th character ==04) ?
That means I store 5 characters, if OK I store the 4 followers, if not I left shift the 5 characters to accept a new 5th one

Do you think it could be a correct method ?
Ttelmah



Joined: 11 Mar 2010
Posts: 19367

View user's profile Send private message

PostPosted: Tue May 20, 2014 2:55 am     Reply with quote

Have to agree.

Thing is that a well written circular buffer, even at quite a small size, then allows you to take more than one character time between things being handled. A 'classic' problem would be if (for instance) at the end of line, the code then performs calculations to check the CRC, followed by some other processing of the data. It is always quite easy for this to take more than a character time, and then things start to go wrong....

I tend to use a 'per character' parser, which whenever a character arrives, uses a state machine that is stepping forwards through the expected values and actions for each, performing the required operations.

Now, I have to suggest that you 'reconsider' the format if you can. The problem you are seeing, is fundamental, with any message where the same value is used potentially inside a message, and in an address or control. Though a 'timeout' may allow a form of recovery, this doesn't appear to be guaranteed, since what happens if two successive messages both contain the same address?.
Using some form of unique marker, is the best way to avoid problems. As a 'suggestion', consider setting your UART to 9bit mode, and setting bit 9, in the address only. The cost is 11 bits per byte sent, versus the current 10 bits, so just 10 percent. Then your handler can just look for bit 9 being set, and when it is, use this as the 'address' marker. Nice thing then is that even if a byte is lost, or an extra one received so a count goes faulty, you can still recover. The bytes received with the count 'wrong' can be rejected when the CRC doesn't work, and the new start of packet location identified.

Best Wishes
Jean FOUGERON



Joined: 30 Nov 2012
Posts: 110
Location: France

View user's profile Send private message Visit poster's website

PostPosted: Tue May 20, 2014 3:18 am     Reply with quote

Thanks Ttelmah

Quote:
Now, I have to suggest that you 'reconsider' the format if you can


But I can't, the supervisor is an existing product I cannot change, I have to do with Crying or Very sad
Ttelmah



Joined: 11 Mar 2010
Posts: 19367

View user's profile Send private message

PostPosted: Tue May 20, 2014 3:55 am     Reply with quote

Is there a guaranteed time gap between the messages?.

If so, then this can be used as the marker. Reset a timer to a value that will not reach it's limit, between standard characters, each time the character is received. Then if the timer interrupt is called, you are in the gap between messages.

Unless there is a guaranteed time marker, then the message is really badly designed (I'm not even sure 'designed' would be the applicable word....). Without something to distinguish the start of message, you degenerate into 'trial and error'. You'd have to use the count approach, with a state machine, and when the limit count is reached, check the CRC. If this is OK, accept the message, and pull the address from the expected byte. If it isn't, throw away the first character from the sequence being checked, and loop back to checking. There is still a tiny chance of accepting a wrong message, but this is as close as you'll get.
temtronic



Joined: 01 Jul 2010
Posts: 9174
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Tue May 20, 2014 5:22 am     Reply with quote

Is the 5th character always '04' ?
If so, using a circular buffer makes it possible,though 'messy'.

1) capture a datastream.
2) locate the '04'.
3) create a new buffer 'test_buffer[]',filling it with the previous 4 bytes before the '04' and those after to the crc bytes.
4) run a 'CRC checking' function to confirm/deny this 'test_buffer' has the correct data.
5) if correct, continue your program, parsing address,command,etc.
6) if NOT correct, go back to step 1, try again.If your 'raw data' buffer is large enough,just find the next '04' and retest.

This might work but ONLY if '04' is always the 5th element of the datastream.

As others have said you really NEED a 'delimiter', a 'reference' or 'starting point' in order to properly capture and deal with the data.

hth
jay
jeremiah



Joined: 20 Jul 2010
Posts: 1328

View user's profile Send private message

PostPosted: Tue May 20, 2014 6:32 am     Reply with quote

Another "messy" way would be to do the ISR buffer they are talking about and in the main do a multi-pass CRC check until you find a valid CRC, then go back and parse the message to see if it is for your address or the broadcast address

So if you had the byte stream:


b00 b01 b02 b03 b04 b05 b06 b07 b08 b09 b0a b0b b0c b0d b0e b0f b10

So the first pass would use b0-b8, checking versus the CRC bytes in b7 and b8. If the computed CRC matches those, go back and look b0 to see if it is your address or a broadcast address.

If not, look at b01-b09, assuming b8 and b9 are the CRC bytes. If this doesn't pass, do b02-b0a, etc.

This is tricky because it is a lot of processing of the same data, so you'll have to figure out how to manage processing time, getting the data out of the circular buff, and possibly adding new bytes to the end of your saved copy if new ones come in while you processing earlier ones.

This isn't a 100% solution, but since you cannot change the supervisor, it might make due. Really the best solution is to talk with your management and do a new revision on the supervisor firmware and add new parts to the protocol (or rewrite it).


Last edited by jeremiah on Tue May 20, 2014 8:03 am; edited 1 time in total
Jean FOUGERON



Joined: 30 Nov 2012
Posts: 110
Location: France

View user's profile Send private message Visit poster's website

PostPosted: Tue May 20, 2014 7:13 am     Reply with quote

Yes the 5th is always 0x04 because the message is always 9 bytes long and the MODBUS standart implies the succession of set adress, function code, content. In content is included remaining bytes quantity in the message, which counts data by itself = 2 bytes plus CRC = 2 bytes -> total 4 bytes
So, the 5th is always 0x04

Taking that in account, I presently am testing a solution like as follow, according to asmboy idea


Code:
#define UART_MAX_NEW 18
#define UART_MESSAGE_LENGTH 9
int UART_Cursor=0;
char UART_INStr[UART_MAX_NEW];


In the UART RECEIVE Interrupt, I store the successives char in an array, and store the next empty location in UART_Cursor variable
If UART_Cursor is higher than 8, that means a complete message length has been received, so I position B_UART_OK flag


Code:
#INT_RDA
void Receive()
{
char c;

   c=getc();
   
   UART_INStr[UART_Cursor++]=c;
   
   B_UART_OK=(UART_Cursor>=UART_MESSAGE_LENGTH);
   if(B_UART_OK) Spy_HI();         
                                    
}


I still have to manage when accidentally UART_Cursor is higher than UART_MAX_NEW


In main procedure, if B_UART_OK is positioned, I test whether the 9 bytes are a correct message possible or not

Code:
void main()
{
  //.../...
 
   while(TRUE)
   {
      //.../...                     
     
      if(B_UART_OK)
      {   
         if(IsMessageOK())
         {
            GetData();
         }
      }

      //.../...
   }
}


The validation of the message is as follow

is the first == BROADCAST_ADDRESS and the 2nd == function code 0x06 or the first == Adress_box and the 2nd == function code 0x03
is the 5th == MODBUS_DATA_LENGTH == 0x04

If yes, the message is a valid message and I treat it (I presently test CRC in treatment)
I first position the UART_Cursor to the correct position : -1 position if the message is not correct, - message length if it is correct

Code:
Boolean IsMessageOK()
{
int I;
Boolean B_OK=FALSE;
   
   Spy_MI();
                                                // 8 char at least are in UART_INStr
   if(UART_INStr[0]==BROADCAST_ADDRESS)         // Broadcast, so I need it
   {
      
      if(UART_INStr[1]==MODBUS_WRITE_ONE)         // write one register
      {
         if(UART_INStr[4]==MODBUS_DATA_LENGTH)         // correct data length
         {
            B_OK=TRUE;
         }   
      }      
   }
   else if(UART_INStr[0]==I_Adresse_Box)         // It's me !
   {
      
      if(UART_INStr[1]==MODBUS_READ_ANY)         // read many registers
      {
         Spy_MA();
         if(UART_INStr[4]==MODBUS_DATA_LENGTH)         // correct data length
         {
            
            B_OK=TRUE;
         }   
      }
   }   

   if(B_OK)
   {
      for(I=0; i<9;OUTStr[I]=UART_INStr[I]);                     //copy to treated string
      for(I=0;i<UART_MESSAGE_LENGTH;UART_INStr[I]=UART_INStr[UART_MESSAGE_LENGTH+I]);   //left shift UART_MESSAGE_LENGTH=9 char
      UART_Cursor=UART_Cursor-UART_MESSAGE_LENGTH;
   }
   else
   {
      for(I=0;i<UART_Cursor;UART_INStr[I]=UART_INStr[++I]);   //left shift 1 char
      UART_Cursor--;
   }      
   B_UART_OK=(UART_Cursor>8);
   
   return B_OK;         
}


But, still something goes wrong I haven't yet understood Evil or Very Mad
jeremiah



Joined: 20 Jul 2010
Posts: 1328

View user's profile Send private message

PostPosted: Tue May 20, 2014 8:06 am     Reply with quote

Code:

 for(I=0; i<9;OUTStr[I]=UART_INStr[I]); 
for(I=0;i<UART_MESSAGE_LENGTH;UART_INStr[I]=UART_INStr[UART_MESSAGE_LENGTH+I]);   //left shift UART_MESSAGE_LENGTH=9 char


where does "i" increment in these for loops? Looks like this would loop forever. Also, you shouldn't mix case I and i can get dangerous to manage and it isn't very readable in that form anyways.
Jean FOUGERON



Joined: 30 Nov 2012
Posts: 110
Location: France

View user's profile Send private message Visit poster's website

PostPosted: Tue May 20, 2014 10:52 am     Reply with quote

Quote:
where does "i" increment in these for loops?

True
Stupid of me !! Twisted Evil
read

Code:
for(i=0; i<9;OUTStr[i]=UART_INStr[i++]);                     //copy to treated string
for(i=0;i<UART_MESSAGE_LENGTH;UART_INStr[i]=UART_INStr[UART_MESSAGE_LENGTH+i++]);   //left shift UART_MESSAGE_LENGTH=9 char
Ttelmah



Joined: 11 Mar 2010
Posts: 19367

View user's profile Send private message

PostPosted: Tue May 20, 2014 2:39 pm     Reply with quote

If this is Modbus, then the time gap is defined.

Frame start, is 3.5 characters or more of silence. Well defined, and easy to test for.
Jean FOUGERON



Joined: 30 Nov 2012
Posts: 110
Location: France

View user's profile Send private message Visit poster's website

PostPosted: Wed May 21, 2014 7:10 am     Reply with quote

I have implemented the following, and it seems it works (at least since this morning)

I have one set in the lab
If I configure it in adress 01 it answers, and it acts when broadcast (adress 0)
Idem if I configure it in adress 2
... and 3, and 4 ...

Youpiii ! Very Happy

Code:
#INT_RDA
void Receive()
{
char c;

   c=getc();

   if (I_UART_Cursor<=UART_MAX) INStr[I_UART_Cursor++]=c;      // avoid overflow

   B_UART_OK=(I_UART_Cursor>=UART_MESSAGE_LENGTH);               // received at least one length of message                              
}


B_UART_OK is ON when we received 9 bytes (which is the message length)
then in main routine

Code:
void main()
{
   initialisation();
 
   while(TRUE)
   {
     //.../...
     
      if(B_UART_OK)
      {   
         isMessageOK();
      }
     
      //.../...
 
   }
}


Then the routine which verify whether thes 9 chars are a real message or not

Code:
Boolean IsMessageOK()
{
int i;
Boolean B_OK=FALSE;

                                                // UART_MESSAGE_LENGTH char at least are in INStr
   if(INStr[4]==MODBUS_DATA_LENGTH)         // correct data length
   {
      if((INStr[0]==BROADCAST_ADDRESS) && (INStr[1]==MODBUS_WRITE_ONE))   // Broadcast procedure and  write one register
      {
         B_OK=TRUE;
      }      
      else if((INStr[0]==I_Adresse_Box) && (INStr[1]==MODBUS_READ_ANY))   // It's me ! and read many registers   
      {
         B_OK=TRUE;
      }   
   }   
   
   if(B_OK)
   {
      for(i=0;i<UART_MESSAGE_LENGTH;OUTStr[i]=INStr[i++]);                     //copy to treated string
      for(i=0;i<UART_MAX-UART_MESSAGE_LENGTH;INStr[i]=INStr[UART_MESSAGE_LENGTH+i++]);   //left shift UART_MESSAGE_LENGTH=9 char
      I_UART_Cursor=I_UART_Cursor-UART_MESSAGE_LENGTH;
      GetData();                  // Proceed
   }
   else
   {
      for(I=0;i<I_UART_Cursor;INStr[I]=INStr[++I]);   //left shift 1 char
      I_UART_Cursor--;
   }
         
   B_UART_OK=(I_UART_Cursor>=UART_MESSAGE_LENGTH);         //Stops parsing, if needed

   return B_OK;         
}



I test whether char 4 is 0x04, which has to be.
if yes, I tes whether char 0 is my adress or broadcast and if yes if the command (char 1) is the corresponding command
if yes, I copy the UART_MESSAGE_LENGTH char in a string OUTStr I then will analyse to proceed
and I shift UART_MESSAGE_LENGTH char the reception array INStr


If no, I left shift one char to do it again
I also update UART_Cursor in order to store the next received char at the correct position.
I update B_UART_OK. If everything is normal, UART_Cursor =0 so B_UART_OK=FALSE

Normally the next message will arrive in approx 500ms

Do you think there still is some risk in this code ?
ezflyr



Joined: 25 Oct 2010
Posts: 1019
Location: Tewksbury, MA

View user's profile Send private message

PostPosted: Wed May 21, 2014 7:20 am     Reply with quote

Hi,

Well, is the '0x04' totally unique, or might it also appear as part of the 'data' or 'CRC' later on? If this character is not 100% unique, then yes, your method is still flawed....

As Ttelmah pointed out, the interval between messages in Modbus is well defined, and easy to test for. It seems to me that this is the only unique thing that occurs in your communications protocol. Why not use it?

John
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Goto page 1, 2  Next
Page 1 of 2

 
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