Split received string and save in an array

Go To Last Post
7 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi all,

I am stuck for a few days with my code, so i need some help.

My goal is to receive a string via UART, split the string in pieces by the comma and save the pieces in an array. To receive characters via UART i use the UART library van Peter Fleury. With the function uart_getc() i can pick the characters out of the ring buffer, which i place in an array. Then i copy the array into a string. Then i split the string in pieces with the function str_split (which i did not create by myself). Then i show the pieces of the string on a display. So far i got that working, that is the code below:

 

#define F_CPU 16000000UL
#include <stdlib.h>
#include <avr/io.h>
#include <assert.h>
#include <avr/interrupt.h> // for interrupts
#include <avr/pgmspace.h>
#include <util/delay.h>  // for delays
#include <string.h>   // for tokenize string
#include <stddef.h>

#include "uart.h"
#include "lcd.h"

/* define CPU frequency in Mhz here if not defined in Makefile */
#ifndef F_CPU
#define F_CPU 16000000UL
#endif

/* 2400 baud */
#define UART_BAUD_RATE      2400

extern void lcd_backlight(char on);     //not in lcd.h
char** str_split(char* a_str, const char a_delim); // splits string
void split_received_string();         //fill array with characters and convert to a string 

int main(void)
{
 uart_init( UART_BAUD_SELECT(UART_BAUD_RATE,F_CPU) ); //initialize the uart
 sei();             //enable interrupts
 lcd_init(LCD_ON_DISPLAY);        //initialize lcd
 lcd_backlight(1);          //lcd_backlight on


    while(1)
    { 
  split_received_string();
 }
}

void split_received_string()
{
    unsigned int c;   
    char myArray[30];
    memset(myArray, 0, 30);  //set array to zero
    int i = 0;
    int x = 0;
    
    c = uart_getc();   //get character
    _delay_ms(100);    //Need delay otherwise the lcd shows weird symbols??
    
    if(c & UART_NO_DATA)  //If there's no data return function
    {
     return;
    }

    while(!(c & UART_NO_DATA)) //while there is data, fill array with characters
    {
     myArray[i] = c;
     i++;
     c = uart_getc();
    }

    x = sizeof(myArray);
    char* string = malloc(30);
    memcpy(string, myArray, x); //copy array in string
    string[x] = '\0';
   
   lcd_gotoxy(0,0);
   lcd_puts(string);   //Display string
   
   char** tokens;
   tokens = str_split(string, ','); //Split string by comma

   lcd_gotoxy(0,1);
   lcd_puts(tokens[0]);
   lcd_gotoxy(10,1);
   lcd_puts(tokens[1]);
   lcd_gotoxy(0,2);
   lcd_puts(tokens[2]);
   lcd_gotoxy(10,2);
   lcd_puts(tokens[3]);
}

char** str_split(char* a_str, const char a_delim)
{
    char** result    = 0;
    size_t count     = 0;
    char* tmp        = a_str;
    char* last_comma = 0;
    char delim[2];
    delim[0] = a_delim;
    delim[1] = 0;

    /* Count how many elements will be extracted. */
    while (*tmp)
    {
        if (a_delim == *tmp)
        {
            count++;
            last_comma = tmp;
        }
        tmp++;
    }

    /* Add space for trailing token. */
    count += last_comma < (a_str + strlen(a_str) - 1);

    /* Add space for terminating null string so caller
       knows where the list of returned strings ends. */
    count++;

    result = malloc(sizeof(char*) * count);

    if (result)
    {
        size_t idx  = 0;
        char* token = strtok(a_str, delim);

        while (token)
        {
            assert(idx < count);
            *(result + idx++) = strdup(token);
            token = strtok(0, delim);
        }
        assert(idx == count - 1);
        *(result + idx) = 0;
    }

    return result;
}

But i have to receive 6 strings, and in the end of the program i want to do some calculations with the strings. So i want to edit the function split_received_string() so that i can give a name of an array and that the function saves the pieces of the string in that given array. i have tried things like:

int main(void)
{
    split_received_string(localarray);
}

void split_received_string(char** tokens)
{
    tokens = ....
}

But i can't get it working. Also you see my knowledge of C is very basic and maybe this is a step too high, but i have to get it working!:)

Also i don't understand why i need a delay of 100ms in the split_received_string() function?

So if someone can help me out, or have some advise. Please help me out! Any help would be highly appreciated.

 

Boott

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Before re-inventing the wheel, have you studied the facilities available in the Standard 'C' Library...?

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Instead of reallocating memory, why not just create an array of 6 pointers, then use strtok() to find the comma and set the pointers to the start of the strings?

 

Also;

split_received_string() should be two functions.

The first one will get the string and put it into a buffer.

the second will actually split the string

 

as written split_received_string() will need the string to be sent all at once, any gap will get a partial string I believe.It should look for a terminating charactor or something to know when it's done receiving.

Keith Vasilakes

Firmware engineer

Minnesota

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

My "goto site" for looking up anything in the standard C library is cplusplus.com - yeah I know the name implies C++ not C but the fact is that C is a subset of C++ so it includes the C documentation even if C++ prefers to call the header <cstring> rather than <string.h>. One of the great things about the site is that they don't just dryly explain the function but there's always a very useful example as in this case  for strtok():

 

http://www.cplusplus.com/referen...

 

(and yes they do use <string.h> in that as well!).

 

I wonder how many times C programmers end up reinventing the wheel because they've never had a good read through the standard library documentation. For me it'd always be the first place to look and only if I really couldn't find anything offering what I wanted would I then consider my own implementation - in fact even before that I'd probably Google because something like splitting/parsing strings has got to be such a common requirement that even if libc doesn't offer anything there's bound to be 50 good implementations out there on the 'net. The only downside of 'net code is you can never tell the provenance - it can be a work of pure genius or a complete crock of poo - you need a bit of C experience to be able to spot one from the other - if in doubt ask on a forum (probably stackoverlfow.com) and you'll get experts who know what's good (and what isn't).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thankyou guys for your reply!

Before re-inventing the wheel, have you studied the facilities available in the Standard 'C' Library...?

I did have a look at the facilities in the standard C library but i will give it a better try this time.   

Instead of reallocating memory, why not just create an array of 6 pointers, then use strtok() to find the comma and set the pointers to the start of the strings?

 

Also;

split_received_string() should be two functions.

The first one will get the string and put it into a buffer.

the second will actually split the string

 

as written split_received_string() will need the string to be sent all at once, any gap will get a partial string I believe.It should look for a terminating charactor or something to know when it's done receiving

I have wrote 2 functions now as you suggested.  

My "goto site" for looking up anything in the standard C library is cplusplus.com - yeah I know the name implies C++ not C but the fact is that C is a subset of C++ so it includes the C documentation even if C++ prefers to call the header <cstring> rather than <string.h>. One of the great things about the site is that they don't just dryly explain the function but there's always a very useful example as in this case  for strtok():

 

http://www.cplusplus.com/reference/cstring/strtok/

 

(and yes they do use <string.h> in that as well!).

 

I wrote the functions with the help of your link. So this is my code now:

void receive_string();     //fill array with characters and convert to a string
void split_string(char* str);           //split string in tokens
char string[30];                                //a global array
char* myArray[30];        //a global array

int main(void)
{
 uart_init( UART_BAUD_SELECT(UART_BAUD_RATE,F_CPU) ); //initialize the uart
 sei();             //enable interrupts
 lcd_init(LCD_ON_DISPLAY);        //initialize lcd
 lcd_backlight(1);          //lcd_backlight on
 
 char* Array1[30];

    while(1)
    { 
  receive_string();  //Receives string in the global char string
  split_string(string); //Splits the string and saves the pieces in myArray
  
  memcpy(Array1, myArray, sizeof(myArray)); //Makes a copy of myArray
  
  lcd_gotoxy(0,1);
  lcd_puts(Array1[0]);
  lcd_gotoxy(10,1);
  lcd_puts(Array1[1]);
  lcd_gotoxy(0,2);
  lcd_puts(Array1[2]);
  lcd_gotoxy(10,2);
  lcd_puts(Array1[3]);
 }
}

void receive_string()
{
  unsigned int c;  
  int i = 0; 
  char localArray[30];
  memset(localArray, 0, 30);  //set array to zero
    
  c = uart_getc();   //get character
  _delay_ms(50);    //Need delay otherwise the lcd shows weird symbols??
    
  if(c & UART_NO_DATA)  //If there's no data return function
  {
   return;
  }

  while(!(c & UART_NO_DATA)) //while there is data, fill array with characters
  {
   localArray[i] = c;
   i++;
   c = uart_getc();
  }

  memcpy(string, localArray, sizeof(localArray)); //copy array in string
  string[sizeof(localArray)] = '\0';

  return;

}

void split_string(char* str)
{
 char* pch;
 int i = 0;
 
 pch = strtok (str, "+,");
 while (pch != NULL)
 {
  myArray[i] = pch;
  pch = strtok (NULL, "+,");
  i++;
 }
 
 return;
}

So i think this is way better then the code i had before and it works. But i still don't get why i need a delay in the receive_string function? If i send a bigger string in, i need a longer delay to view the tokens well on the lcd. Otherwise i get some weird characters. Does somebody have a idea?

And if you have any advise about my code, please let me know!

Thanks for the help so far!   

 

Boott

Last Edited: Thu. Nov 20, 2014 - 12:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But i still don't get why i need a delay in the receive_string function?

As you don't appear to have shown uart_getc() only you can know! My guess is that the routine is not correctly blocking on RXC in the UART status register so the delay simply gives enough time for RXC to have become set anyway. How about showing the source of uart_getc()?

 

BTW do you know of "do { ... } while (...)" as an alternative to "while(...) { ... }"? That saves you having to put the first character receive outside the while loop. As it stands it seems a bit severe to return from receive_string() if the first character has the UART_NO_DATA bit set!

 

Even then it's a bit severe to just break the loop because you get no data?!? I'd make the stopping condition either that you have received N bytes (assuming you know N) or that a terminator such a ',', ' ', '\r' or '\n' is received. ('\n' being the common choice).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for your reply.

 

As you don't appear to have shown uart_getc() only you can know! My guess is that the routine is not correctly blocking on RXC in the UART status register so the delay simply gives enough time for RXC to have become set anyway. How about showing the source of uart_getc()?

I don't understand exactly what you mean, but here is the uart_getc() function. This function is in the Peter Fleury library.

/*************************************************************************
Function: uart_getc()
Purpose:  return byte from ringbuffer  
Returns:  lower byte:  received byte from ringbuffer
          higher byte: last receive error
**************************************************************************/
unsigned int uart_getc(void)
{    
    unsigned char tmptail;
    unsigned char data;


    if ( UART_RxHead == UART_RxTail ) {
        return UART_NO_DATA;   /* no data available */
    }
    
    /* calculate /store buffer index */
    tmptail = (UART_RxTail + 1) & UART_RX_BUFFER_MASK;
    UART_RxTail = tmptail; 
    
    /* get data from receive buffer */
    data = UART_RxBuf[tmptail];
    
    data = (UART_LastRxError << 8) + data;
    UART_LastRxError = 0;
    return data;

}/* uart_getc */

And this is the receive interrupt.

ISR(UART1_RECEIVE_INTERRUPT)
/*************************************************************************
Function: UART1 Receive Complete interrupt
Purpose:  called when the UART1 has received a character
**************************************************************************/
{
    unsigned char tmphead;
    unsigned char data;
    unsigned char usr;
    unsigned char lastRxError;
 
 
    /* read UART status register and UART data register */ 
    usr  = UART1_STATUS;
    data = UART1_DATA;
    
    /* */
    lastRxError = (usr & (_BV(FE1)|_BV(DOR1)) );
        
    /* calculate buffer index */ 
    tmphead = ( UART1_RxHead + 1) & UART_RX_BUFFER_MASK;
    
    if ( tmphead == UART1_RxTail ) {
        /* error: receive buffer overflow */
        lastRxError = UART_BUFFER_OVERFLOW >> 8;
    }else{
        /* store new index */
        UART1_RxHead = tmphead;
        /* store received data in buffer */
        UART1_RxBuf[tmphead] = data;
    }
    UART1_LastRxError |= lastRxError;   
}

I hope you can help me with this.

BTW do you know of "do { ... } while (...)" as an alternative to "while(...) { ... }"? That saves you having to put the first character receive outside the while loop. As it stands it seems a bit severe to return from receive_string() if the first character has the UART_NO_DATA bit set!

 

Even then it's a bit severe to just break the loop because you get no data?!? I'd make the stopping condition either that you have received N bytes (assuming you know N) or that a terminator such a ',', ' ', '\r' or '\n' is received. ('\n' being the common choice).

 

I tried to replace the while loop with the do while loop:

void receive_string()
{
  unsigned int c = 0;  
  int i = 0; 
  char localArray[30];
  memset(localArray, 0, 30);  //set array to zero
    
  do 
  { 
   c = uart_getc();
   _delay_ms(10);
   localArray[i] = c;
   i++;
   
  } while (!(c & UART_NO_DATA));
  

  memcpy(string, localArray, sizeof(localArray)); //copy array in string
  string[sizeof(localArray)] = '\0';
  
  lcd_gotoxy(0,0);
  lcd_puts(string);

  return;

}

Then the string displayed on the lcd is correct. But if i split the string and display the tokens, the first token contains weird symbols? So thats weird. Because i don't have that problem when i use my while loop. But the reason i return the function when there is NO DATA, is because in the total program i work with a switch case statement. In the case where the microcontroller needs to receive a string, he also controls a stepmotor. Like this

case 3:  start_motor();
      receive_string();
      split_string(string);
      memcpy(Array2, myArray, sizeof(myArray)); //Makes a copy of myArray
      if(counter_motor>= 2000) // Run for 5 sec
      {
       counter_motor = 0;
       step++;
      }
   break;

i'm controlling the stepmotor with timers. So in the function start_motor() i set the right timers, and when the timer reaches a compare value, he fires a interrupt where he add one to counter_motor. Before counter_motor reach 2000, the microcontroller receives a string. So the receive function can not wait till there is data, cause he also have to check the if statement. Thats the reason i return the function when there is no data. And i have picked the UART_NO_DATA out of the uart_getc() function. 

 

I thought i use the UART_NO_DATA because then i don't have to look for a terminator. Because the UART_NO_DATA tells if there is new data or not.

 

Thanks for your time.

 

Boott