avr uart string receive and compare

N.Winterbottom

Level: Posting Freak

Joined: Tue. Oct 1, 2002

Posts: 1653 View posts

Location: Redbourn, Herts, UK

#2

Posted by N.Winterbottom: Sat. May 23, 2020 - 11:51 AM

First you must decide how you will denote the end of the string.

A reasonable choice may be a carriage return '\r'.

Secondly; it makes for easier coding if you decide the maximum length of string you wish to transmit.

Last Edited: Sat. May 23, 2020 - 11:54 AM

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#3

Posted by clawson: Sat. May 23, 2020 - 12:01 PM

As Nigel alludes to you need some kind of loop calling getchar() repeatedly. It needs (at least) two stopping conditions. One is to stop when you have received so many characters that to receive another would overflow the receiving array (and don't forget you always need room for an additional 0x00 at the end to mark the end). The other is you need to stop when you receive some king of "end of string" marker which is often '\n' (newline) but could also be '\r' (return) or perhaps even '.' (full stop at the end of a sentence)

avrcandies

Level: Raving Lunatic

Joined: Wed. Jun 30, 2004

Posts: 5512 View posts

Location: Cincinnati Oh

#4

Posted by avrcandies: Sat. May 23, 2020 - 01:43 PM

First you must decide how you will denote the end of the string.

Using a terminator is probably 99% most common and easiest and recommended. However, depending on what you exactly do, it is not always needed. For example, you are waiting to get A P P L E or D O G ....you could check char-by-incoming-char until you get one or the other, then take the corresponding action. This sequence checking can be some form of a state machine. It is usually easier to wait for the terminator and do the checking afterwards. That also better allows checking DOG/r vs DOGGY/r & provides an easy resync point (since at some time you'll get partial or garbled incoming).

When in the dark remember-the future looks brighter than ever. I look forward to being able to predict the future!

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#5

Posted by Rajat choudhary: Sat. May 23, 2020 - 03:12 PM

Dear sir, i have already made a getchar function.suppose i am transmitting "apple\r" but how would my controller know that the complete string has been received on receiver side and also how would i store and where. I will be highly glad if you kindly suggeste me any sample code if possible.

Rjchoudhary

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#6

Posted by clawson: Sat. May 23, 2020 - 04:15 PM

Did you bother to read any of the answers so far?

N.Winterbottom

Level: Posting Freak

Joined: Tue. Oct 1, 2002

Posts: 1653 View posts

Location: Redbourn, Herts, UK

#7

Posted by N.Winterbottom: Sat. May 23, 2020 - 04:20 PM

Well I see this in your main loop:

	while (1)
	{
		char x = getChar();

	}

So you already have most of the "engine".

You could start with an empty string in which you will eventually store "apple\r". This will do.

char rxStr[16];

Clear down rxStr to begin.
You know the first characters in order if all goes well will be 'a' , 'p', 'p', 'l', 'e', '\r'
If x is NOT '\r', you need to append this character onto the end of rxStr and loop around.
If the character x just received from getChar() was '\r'. You know you've reached the end and can move on to compare the strings.

Except for error handling - this is it.

Research the functions in string.h.they will be your friends. There are many websites covering this : http://www.cplusplus.com/reference/cstring/ is one.

Last Edited: Sat. May 23, 2020 - 04:21 PM

MrKendo

Level: Hangaround

Joined: Mon. Nov 19, 2018

Posts: 470 View posts

#8

Posted by MrKendo: Sat. May 23, 2020 - 04:37 PM

I see I'm just duplicating what has been said already, but since I already wrote the reply :

I'm not going to write the code, but to give you a start,

I would have a separate message buffer, big enough for whatever the maximum message len is going to be eg.

#define MAX_MSG_LEN 10

/* +1 to allow for null termination */

char msg_buf[MAX_MSG_LEN + 1];

uint8_t msg_buf_index = 0;

When you receive the first character ('a' in this example) write it to msg_buf, then when you receive the next character ('p' in this example) write that to msg_buf, and keep going until you receive the terminating character, typically \r or \n (or until you have received MAX_MSG_LEN characters, any more characters received after this up to the terminating character will be ignored).

When you receive the terminating character, write a '\0' into msg_buf instead of the terminating character.

So msg_buf now contains 'a', 'p', 'p', 'l', 'e', '\0'

You now have a complete messgae in msg_buf and it can be handled as a null terminated string, do whatever you need to do with it, then reset msg_buf_index to 0.

Yor getchar function returns 0 if no new data is available in the rx buffer. That's probably OK if you only ever intend to receive text characters, but clearly doesn't work if you might actually want to receive a character with value 0 ie. not so good for binary data.

Last Edited: Sat. May 23, 2020 - 04:48 PM

avrcandies

Level: Raving Lunatic

Joined: Wed. Jun 30, 2004

Posts: 5512 View posts

Location: Cincinnati Oh

#9

Posted by avrcandies: Sat. May 23, 2020 - 06:56 PM

but how would my controller know that the complete string has been received on receiver side and also how would i store and where

You already have it stored in your RX buffer...do you need to keep it around further? If not, that is enough, if you can start comparing right away & don't need to worry about the buffer being overwitten.

If you are looking for apple you can do it letter by letter as they arrive

wait for an A... is it an "A" ? No--start over

yes--wait for a P... is it an "P" ? No--start over

etc

This way is more complex since if you are checking against 5 defined words, you may have to simultaneously "keep track" of decoding 5 potential matches..And possibly eliminating them until a winner is found (or possibly no match at all).

-----------------------

Better to simply set a flag when you get a terminator (like Carriage return), which tells the code to go ahead & compare the buffer segment against your list (dictionary), in some smart manner (for example check against the most common match first & save the rare for checking last).

ex:

LEDOFF

LEDON ...common

VALVEON

VALVEOFF

FWD

STOP

BLINK

ERASE ...rare

PARITYON ..rare

You might also say during decoding,1)any incomings will be ignored (hence further commands must wait for an "ok" response before sending) 2) allow enough room in the buffer so there is enough space for additional incoming segments (separated by CR), for later decodes (setting up a queue).

it's a lot simpler for your first program to assume one command will come in, you will decode & if valid, take action, THEN being finished with that, another command may thereafter be received. So you don't allow any overlapping, making it very straightforward. Upon detecting the terminator, the buffer will only contain either one command, or else be assumed to contain junk. Also in this simpler case, you don't even need a circular buffer, you can just reset the pointer to the beginning & refill buffer with another incoming (though a circular is fine too).

When in the dark remember-the future looks brighter than ever. I look forward to being able to predict the future!

Last Edited: Sat. May 23, 2020 - 07:04 PM

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#10

Posted by Rajat choudhary: Sun. May 24, 2020 - 10:34 AM

#include <avr/io.h>
#define F_CPU 8000000
#define BAUD 9600
#define RATE ((F_CPU/16/BAUD)-1)
#include <util/delay.h>
#include <string.h>
#include <stdio.h>
#include <avr/interrupt.h>
#define LED_ON  PORTD |= (1<<PIND6);
#define LED_OFF6  PORTD &= ~(1<<PIND6);
#define LED_OFF5  PORTD &= ~(1<<PIND5);
#define toggel_led6  PORTD ^=(1<<PD6);
#define toggel_led5  PORTD ^=(1<<PD5);

volatile int a=0,b=0,c=0,m=0,n=0;

// #define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
// #define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#define RX_BUFFER_SIZE  128


char rxBuffer[RX_BUFFER_SIZE];
char compBuffer[RX_BUFFER_SIZE+1];
unsigned int rx_wr_index;
char Reply_CallReady[RX_BUFFER_SIZE]="apple\0";
uint8_t rxReadPos=0;
uint8_t rxWritePos=0;

char getChar(void);
char peekChar(void);

void T2delay()
{
	while(TIFR&0x01==0);
	TCCR2=0;
}

void T2_init()
{
	TCCR2 = (1<<CS22)|(1<<CS21)|(1<<CS20);
	TCNT2 = 0xB2;
}

int main(void)
{
	DDRD |=(1<<PIND6)|(1<<PIND5);
	TIMSK=(1<<TOIE2);
	T2_init();
	
	UBRRH |= (RATE>>8);
	UBRRL |= RATE;
	
	UCSRB |= (1<<RXEN)|(1<<RXCIE);
	UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);
	sei();
	while (1)
	{
		compBuffer[RX_BUFFER_SIZE+1] = getChar();
		if (strcmp(Reply_CallReady,compBuffer) ==0)
		{
			LED_ON;
		}
		else 
		{
			LED_OFF6;

		}

	}
	
}



char peekChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
	}
	return ret;
}

char getChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
		rxReadPos++;
		if (rxReadPos>=RX_BUFFER_SIZE)
		{
			rxReadPos=0;
		}
	}
	return ret;
}

ISR(USART_RXC_vect)
{
	rxBuffer[rxWritePos]=UDR;
	
	rxWritePos++;
	if (rxWritePos>=RX_BUFFER_SIZE)
	{
		rxWritePos=0;
	}
}

sir i am still unable to store and compare the string it does not working.kindly help me to get the solution if possible.

Rjchoudhary

Kartman

Level: 10k+ Postman

Joined: Thu. Dec 30, 2004

Posts: 24415 View posts

Location: Melbourne,Australia

#11

Posted by Kartman: Sun. May 24, 2020 - 11:06 AM

what is this line doing?

compBuffer[RX_BUFFER_SIZE+1] = getChar();

you need to write a function that will collect the characters into a string and return when you detect a carriage return character. Once you have a string, then you can do a comparison.

MrKendo

Level: Hangaround

Joined: Mon. Nov 19, 2018

Posts: 470 View posts

#12

Posted by MrKendo: Sun. May 24, 2020 - 12:11 PM

If you are going to build up the receive message in a separate linear buffer (your compBuffer), which I think is a good way to do it, you have to use your existing getChar function to get each new character one at a time and append that character onto the end of compBuffer, until you get a \r (or whatever character you wish to use to terminate each message), at which point append a '\0'. You now have a complete message as a null terminated string. At this point you do the strcmp(compBuffer, "apple") or whatever.

You will need to keep track of where to write next into compBuffer, so have a separate index for this

uint8_t compBufferPos = 0;

This starts out as 0. Increment as you append each new character. Reset to 0 after you have handled a complete message ready for the start of the next message.

Your compBuffer doesn't needto be the same size as the uart rx circular buffer.

Your rx circular buffer probably doesn't need to be as big as 128.

Your compBuffer needs to be big enough to hold the longest individual message.

avrcandies

Level: Raving Lunatic

Joined: Wed. Jun 30, 2004

Posts: 5512 View posts

Location: Cincinnati Oh

#13

Posted by avrcandies: Sun. May 24, 2020 - 02:51 PM

As the bare-bones getting started try this:

STEP A : set the buffer pointer to the beginning

Turn on the rcv interrupt (RXIE) bit

----

Receive your chars into the buffer, looking for CR & simply increase the pointer

in the interrupt, if you see a cr, set a flag and disable the RX irq enable (actually the RX irq enable bit could serve as the flag)\

If the buffer reaches the end before CR, something went wrong...reset the pointer to the beginning (the next CR will resync).

in main:

if your CR RX flag is set, you are ready to scan the buffer, from beginning to CR, looking for comparisons

Once that is finished, go to step A, & wait for the next message

Drawbacks (which can be minor), are that:

You will miss any incoming during your checking. Messages will likely be processed within a fraction of a millisecond...so a message overlap might be rare (depending on your message separations).

Since there is no message queue, you need to check the flag often to minimize delay.

You can get this much easily working, then you can upgrade to a circular buffer, copying the message segment to a different buffer, adding more buffer error detections, etc.

Even with a circ buff, there should be message handshaking, otherwise even a buffered setup could eventually overflow (if msg processing is slowed or delayed).

When in the dark remember-the future looks brighter than ever. I look forward to being able to predict the future!

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#14

Posted by Rajat choudhary: Tue. May 26, 2020 - 07:18 AM

#include <avr/io.h>
#define F_CPU 8000000
#define BAUD 9600
#define RATE ((F_CPU/16/BAUD)-1)
#include <util/delay.h>
#include <string.h>
#include <stdio.h>
#include <avr/interrupt.h>
#define LED_ON  PORTD |= (1<<PIND6);
#define LED_OFF6  PORTD &= ~(1<<PIND6);
#define LED_OFF5  PORTD &= ~(1<<PIND5);
#define toggel_led6  PORTD ^=(1<<PD6);
#define toggel_led5  PORTD ^=(1<<PD5);

volatile int a=0,b=0,c=0,m=0,n=0;
volatile unsigned char j=0;
volatile unsigned char string[15],x,i =0;

#define RX_BUFFER_SIZE  128

char rxBuffer[RX_BUFFER_SIZE];
unsigned int rx_wr_index;
char Reply_CallReady[15]="apple";
uint8_t rxReadPos=0;
uint8_t rxWritePos=0;

char getChar(void);
char peekChar(void);

void T2delay()
{
	while(TIFR&0x01==0);
	TCCR2=0;
}

void T2_init()
{
	TCCR2 = (1<<CS22)|(1<<CS21)|(1<<CS20);
	TCNT2 = 0xB2;
}

unsigned char *UART_RX_STR(void)
{
	x= getChar();
	while (x != '\0')
	{
		string[i++] = x;
	}
		return string;

}

int main(void)
{
	DDRD |=(1<<PIND6)|(1<<PIND5);
	TIMSK=(1<<TOIE2);
	T2_init();

	UBRRH |= (RATE>>8);
	UBRRL |= RATE;

	UCSRB |= (1<<RXEN)|(1<<RXCIE);
	UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);
	sei();
	char c;
	while (1)
	{
			//c= getChar();
			UART_RX_STR();
					if (strcmp(string,Reply_CallReady) ==0)
					{
						LED_ON;

					}
					else
					{
						LED_OFF6;
					}

	}

}

char peekChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
	}
	return ret;
}

char getChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
		rxReadPos++;
		if (rxReadPos>=RX_BUFFER_SIZE)
		{
			rxReadPos=0;
		}
	}
	return ret;
}

ISR(USART_RXC_vect)
{
	rxBuffer[rxWritePos]=UDR;

	rxWritePos++;
	if (rxWritePos>=RX_BUFFER_SIZE)
	{
		rxWritePos=0;
	}
}

hello sir, now i have made an array to store the string but still it is not working, can you please check my code and help me to find the fault i have made in this.

Rjchoudhary

Last Edited: Tue. May 26, 2020 - 07:26 AM

Kartman

Level: 10k+ Postman

Joined: Thu. Dec 30, 2004

Posts: 24415 View posts

Location: Melbourne,Australia

#15

Posted by Kartman: Tue. May 26, 2020 - 07:29 AM

unsigned char *UART_RX_STR(void)
{
	x= getChar();
	while (x != '\0')
	{
		string[i++] = x;
	}
		return string;

}

Even after your change, this function does not make sense - unless you want to fill memory with the value of x.

Have you ever considered what you want to do is so common that there is a standard function for it? gets() perhaps. Google it.

To help you along, here's this: https://www.nongnu.org/avr-libc/...

A little more Googling will find you examples of how to use printf() and scanf(). You'll probably find these standard functions helpful as well.

Last Edited: Tue. May 26, 2020 - 07:31 AM

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#16

Posted by clawson: Tue. May 26, 2020 - 08:11 AM

Also why does UART_RX_STRING

a) have a name in all upper case - usually that suggests a macro not a function?

b) have a return value of char * but then the invocation completely ignores this?

c) return a global?

Seems to me you have some basic C concepts to learn before you can move to application implementation.

Oh and just write out on paper what you think an "RX_STRING" function needs to do first so you have a clear picture. THEN write the accompanying C code.

Something like:

"keep calling getchar until the character returned is '\n' (or whatever end marker is to be used), while not at the end put character into buffer passed by the caller. If number of received characters reaches length of buffer then stop. At hte end mark the last byte of the string with 0x00"

N.Winterbottom

Level: Posting Freak

Joined: Tue. Oct 1, 2002

Posts: 1653 View posts

Location: Redbourn, Herts, UK

#17

Posted by N.Winterbottom: Tue. May 26, 2020 - 09:14 AM

I think we freaks have overloaded you with much conflicting advice.

Firstly; have you done any programming in C on your Windows PC ? You will find writing and testing software a whole lot easier if you can do this. You can enter your "LEDON" command at the keyboard instead of using Serial Comms if you wish.
You can write your command processor using the string handling functions exclusively. Although not optimal for size, this will be your fastest route to a working program.
I'd hoped My post #7 would lead you to write an inner and outer loop. the inner loop does the getChar() thing; appending any received characters into the "received command" string and when '\r' is received it executes the right command. The outer loop clears down the "received command" string and waits for the next command.

Here are my #7 notes paraphrased into Structured English / C code: [The C code being what you've already written]

void main (void)
{
   char rxCmd[16];

   while (1) {
        clear down rxCmd to empty string; // you can use strcpy for this or set the 1st character to 0
       while (1) {

           char c = getChar();

           if (c == '\r') {

               if (strcmp(rxCmd, "LEDON") == 0) {
                    LED_ON();
                   break;
               }

               if (strcmp(rxCmd, "LEDOFF") == 0) {
                    LED_OFF();
                   break;
               }

            }
           else if (c != 0) {
                concatenate c onto rxCmd; // You can use strncat for this. Read http://www.cplusplus.com/reference/cstring/
            }
       }
   }

}

Error handling is missing but essentially that's all there is to it.

Last Edited: Tue. May 26, 2020 - 09:17 AM

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#18

Posted by clawson: Tue. May 26, 2020 - 10:30 AM

My take on it:

void getString(char * buf, int len) {
    int idx = 0;
    while(1) {
        c = getChar();
        // if it is RET or NL..
        if ((c == '\r') || (c == '\n')) {
            break;
        }
        // else store the received character
        buf[idx] = c;
        // increment the index
        idx++;
        // if reached the end (with room for final 0x00) break
        if (idx == (len - 1)) {
            break;
        }
    }
    // mark the end of the
    buf[idx] = 0x00;
}

int main{void) {
    char rxString[14];
    while (1) {
        getString(rxString, 14);
        if (strcmp(rxString, "LEDON") == 0) {
            LED_ON();
        }
        else if (strcmp(rxString, "LEDOFF") == 0) {
            LED_OFF();
        }
    }
}

EDIT: added "== 0" to strcmp()s - thanks Mr Kendo!

Last Edited: Tue. May 26, 2020 - 12:07 PM

MrKendo

Level: Hangaround

Joined: Mon. Nov 19, 2018

Posts: 470 View posts

#19

Posted by MrKendo: Tue. May 26, 2020 - 10:51 AM

The OP's getChar function is non blocking and returns 0 if the uart rx buffer is empty, so I would add one extra modification to #18

void getString(char * buf, int len) {
    int idx = 0;
    while(1) {
        c = getChar();
        if (c == '\0') {  <======== here
            continue;
        }
        if ((c == '\r' etc.))

EDIT

oh, and the usual typo with strcmp, should be

if (strcmp(rxString, "LEDON") == 0)

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#20

Posted by Rajat choudhary: Tue. May 26, 2020 - 11:25 AM

#ifndef F_CPU
#define F_CPU 8000000UL // 16Mhz clock speed
#endif
#include <avr/io.h>
#include <util/delay.h>
//#define USART_BAUDRATE 9600
#define BAUD_PRESCALE (((F_CPU / (USART_BAUDRATE * 16UL))) -1)

void UART_init(long USART_BAUDRATE)
{
	UCSRB |=(1<<RXEN)|(1<<TXEN);//TURN ON TRANSMISSION AND RECEPTION.
	UCSRC |=(1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);//USE 8 BIT CHARACTER.
	UBRRL |=(BAUD_PRESCALE);//LOAD LOWER 8 BITS OF THE BAUD RATE VALUE.
	UBRRH |=(BAUD_PRESCALE>>8);//LOAD UPPER 8 BIT.
}
unsigned char UART_Rxchar()
{
	while((UCSRA & (1<<RXC))==0);//wait till data is received.
	return(UDR);//RETURN THE BYTE.
}
void UART_Txchar(char ch)
{
	while (!(UCSRA & (1<<UDRE)));//wait for empty transmit buffer.
	UDR=ch;
	
}
void UART_sendstring(char *str)
{
	unsigned char j=0;
	while(str[j]!='\0')     // send string till null.
	{
		UART_Txchar(str[j]);
		j++;
	}
}
void lcd_cmd(char cm)
{
	char a=cm;
	PORTA=a;
	PORTB=(0<<PD0)|(0<<PD1)|(1<<PD2);
	_delay_ms(50);
	PORTB=(0<<PD0)|(0<<PD1)|(0<<PD2);
}
void lcd_data(char dat)
{
	char b=dat;
	PORTA=b;
	PORTB=(1<<PD0)|(0<<PD1)|(1<<PD2);
	_delay_ms(50);
	PORTB=(1<<PD0)|(0<<PD1)|(0<<PD2);
	_delay_ms(50);
}


int main()
{
	DDRB=0xFF;
	DDRA=0xFF;
	UART_init(9600);
	UART_sendstring("LEDON");
	UART_sendstring('\r');
	while(1)
	{

	}
}

this is the code what i have made for transmitting the string is it correct.

Rjchoudhary

MrKendo

Level: Hangaround

Joined: Mon. Nov 19, 2018

Posts: 470 View posts

#21

Posted by MrKendo: Tue. May 26, 2020 - 12:09 PM

Rajat choudhary wrote:

this is the code what i have made for transmitting the string is it correct.

Why not test it and see.

You seem to have dropped the uart rx circular buffer and rx interrupt in favor of a simple blocking Uart_Rxchar.

Thta's probably for the best.

Use your new Uart_Rxchar with clawson's code from #18 and you have something that should work (or at least not be far off).

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#22

Posted by Rajat choudhary: Wed. May 27, 2020 - 07:40 AM

#include <avr/io.h>
#define F_CPU 8000000
#define BAUD 9600
#define RATE ((F_CPU/16/BAUD)-1)
#include <util/delay.h>
#include <string.h>
#include <stdio.h>
#include <avr/interrupt.h>
#define LED_ON  PORTD |= (1<<PIND6);
#define LED_OFF  PORTD &= ~(1<<PIND6);
#define LED_OFF5  PORTD &= ~(1<<PIND5);
#define toggel_led6  PORTD ^=(1<<PD6);
#define toggel_led5  PORTD ^=(1<<PD5);

volatile int a=0,b=0,c=0,m=0,n=0;
volatile unsigned char j=0;
volatile unsigned char string[15],x,i =0;

#define RX_BUFFER_SIZE  128

char rxBuffer[RX_BUFFER_SIZE];
unsigned int rx_wr_index;
char Reply_CallReady[15]="apple\0";
uint8_t rxReadPos=0;
uint8_t rxWritePos=0;

char getChar(void);
char peekChar(void);

void T2delay()
{
	while(TIFR&0x01==0);
	TCCR2=0;
}

void T2_init()
{
	TCCR2 = (1<<CS22)|(1<<CS21)|(1<<CS20);
	TCNT2 = 0xB2;
}



void getString(char * buf, int len) {
	int idx = 0;
	while(1)
	{
		c = getChar();
		// if it is RET or NL..
		if ((c == '\r') || (c == '\n'))
		{
			break;
		}
		// else store the received character
		buf[idx] = c;
		// increment the index
		idx++;
		// if reached the end (with room for final 0x00) break
		if (idx == (len - 1))
		{
			break;
		}
	}
	// mark the end of the
	buf[idx] = 0x00;
}

int main(void)
{
	DDRD |=(1<<PIND6)|(1<<PIND5);
	TIMSK=(1<<TOIE2);
	T2_init();

	UBRRH |= (RATE>>8);
	UBRRL |= RATE;

	UCSRB |= (1<<RXEN)|(1<<RXCIE);
	UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);
	sei();
	char c;
	char rxString[14];
	while (1)
	{

     getString(rxString, 14);
     if (strcmp(rxString, "LEDON") == 0) {
	     LED_ON;
     }
     else if (strcmp(rxString, "LEDOFF") == 0) {
	     LED_OFF;
     }

	}

}

char peekChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
	}
	return ret;
}

char getChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
		rxReadPos++;
		if (rxReadPos>=RX_BUFFER_SIZE)
		{
			rxReadPos=0;
		}
	}
	return ret;
}

ISR(USART_RXC_vect)
{
	rxBuffer[rxWritePos]=UDR;

	rxWritePos++;
	if (rxWritePos>=RX_BUFFER_SIZE)
	{
		rxWritePos=0;
	}
}

hello sir, this is the edited code with the function you gave me and still it is not working,it is really embarrassing why i am unable to do this.

#ifndef F_CPU
#define F_CPU 8000000UL // 16Mhz clock speed
#endif
#include <avr/io.h>
#include <util/delay.h>
//#define USART_BAUDRATE 9600
#define BAUD_PRESCALE (((F_CPU / (USART_BAUDRATE * 16UL))) -1)

void UART_init(long USART_BAUDRATE)
{
	UCSRB |=(1<<RXEN)|(1<<TXEN);//TURN ON TRANSMISSION AND RECEPTION.
	UCSRC |=(1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);//USE 8 BIT CHARACTER.
	UBRRL |=(BAUD_PRESCALE);//LOAD LOWER 8 BITS OF THE BAUD RATE VALUE.
	UBRRH |=(BAUD_PRESCALE>>8);//LOAD UPPER 8 BIT.
}
unsigned char UART_Rxchar()
{
	while((UCSRA & (1<<RXC))==0);//wait till data is received.
	return(UDR);//RETURN THE BYTE.
}
void UART_Txchar(char ch)
{
	while (!(UCSRA & (1<<UDRE)));//wait for empty transmit buffer.
	UDR=ch;
	
}
void UART_sendstring(char *str)
{
	unsigned char j=0;
	while(str[j]!='\0')     // send string till null.
	{
		UART_Txchar(str[j]);
		j++;
	}
}
void lcd_cmd(char cm)
{
	char a=cm;
	PORTA=a;
	PORTB=(0<<PD0)|(0<<PD1)|(1<<PD2);
	_delay_ms(50);
	PORTB=(0<<PD0)|(0<<PD1)|(0<<PD2);
}
void lcd_data(char dat)
{
	char b=dat;
	PORTA=b;
	PORTB=(1<<PD0)|(0<<PD1)|(1<<PD2);
	_delay_ms(50);
	PORTB=(1<<PD0)|(0<<PD1)|(0<<PD2);
	_delay_ms(50);
}


int main()
{
	DDRB=0xFF;
	DDRA=0xFF;
	UART_init(9600);
	UART_sendstring("LEDON\r");
	
	while(1)
	{

	}
}

this is my transmitting code to transmit "LEDON".

i have provided an attachment of the out put what i am getting on Proteus.

Attachment(s):

IMG_20200527_123803.jpg

Rjchoudhary

Last Edited: Wed. May 27, 2020 - 07:43 AM

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#23

Posted by clawson: Wed. May 27, 2020 - 08:30 AM

You forget the other bit that Mr Kendo suggested. That is:

void getString(char * buf, int len) {
    int idx = 0;
    while(1) {
        c = getChar();
        if (c == '\0') {  <======== here
            continue;
        }
        if ((c == '\r' etc.))

As he pointed out getchar() does not wait for a character to become available. If one is not it returns 0x00 so the code needs to just keep calling getChar() in this case. "continue" means "go back to the start of the loop".

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#24

Posted by Rajat choudhary: Wed. May 27, 2020 - 09:38 AM (Reply to #23)

#include <avr/io.h>
#define F_CPU 8000000
#define BAUD 9600
#define RATE ((F_CPU/16/BAUD)-1)
#include <util/delay.h>
#include <string.h>
#include <stdio.h>
#include <avr/interrupt.h>
#define LED_ON  PORTD |= (1<<PIND6);
#define LED_OFF  PORTD &= ~(1<<PIND6);
#define LED_OFF5  PORTD &= ~(1<<PIND5);
#define toggel_led6  PORTD ^=(1<<PD6);
#define toggel_led5  PORTD ^=(1<<PD5);

volatile int a=0,b=0,c=0,m=0,n=0;
volatile unsigned char j=0;
volatile unsigned char string[15],x,i =0;

#define RX_BUFFER_SIZE  128


char rxBuffer[RX_BUFFER_SIZE];
unsigned int rx_wr_index;
char Reply_CallReady[15]="apple\0";
uint8_t rxReadPos=0;
uint8_t rxWritePos=0;

char getChar(void);
char peekChar(void);

void T2delay()
{
    while(TIFR&0x01==0);
    TCCR2=0;
}

void T2_init()
{
    TCCR2 = (1<<CS22)|(1<<CS21)|(1<<CS20);
    TCNT2 = 0xB2;
}



void getString(char * buf, int len) {
    int idx = 0;
    while(1) 
    {
        c = getChar();
        
                if (c == '\0') 
                {  
                    continue;
                }
        // if it is RET or NL..
        if ((c == '\r') || (c == '\n')) 
        {
            break;
        }
        // else store the received character
        buf[idx] = c;
        // increment the index
        idx++;
        // if reached the end (with room for final 0x00) break
        if (idx == (len - 1)) 
        {
            break;
        }
    }
    // mark the end of the
    buf[idx] = 0x00;
}



int main(void)
{
    DDRD |=(1<<PIND6)|(1<<PIND5);
    TIMSK=(1<<TOIE2);
    T2_init();
    
    UBRRH |= (RATE>>8);
    UBRRL |= RATE;
    
    UCSRB |= (1<<RXEN)|(1<<RXCIE);
    UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);
    sei();
    char c;
    char rxString[14];
    char buffer[100];
    char sensor1[]="LEDON";
    while (1)
    {
     getString(rxString, 14);
     if (strcmp(rxString, "LEDON") == 0) {
         LED_ON;
     }
     else if (strcmp(rxString, "LEDOFF") == 0) {
         LED_OFF;
     }


    }
    
}

char peekChar(void)
{
    char ret='\0';
    if (rxReadPos != rxWritePos)
    {
        ret=rxBuffer[rxReadPos];
    }
    return ret;
}

char getChar(void)
{
    char ret='\0';
    if (rxReadPos != rxWritePos)
    {
        ret=rxBuffer[rxReadPos];
        rxReadPos++;
        if (rxReadPos>=RX_BUFFER_SIZE)
        {
            rxReadPos=0;
        }
    }
    return ret;
}

ISR(USART_RXC_vect)
{
    rxBuffer[rxWritePos]=UDR;
    
    rxWritePos++;
    if (rxWritePos>=RX_BUFFER_SIZE)
    {
        rxWritePos=0;
    }
}

sorry sir actually it seems i have post a wrong code this is the actual code that i made.

Rjchoudhary

MrKendo

Level: Hangaround

Joined: Mon. Nov 19, 2018

Posts: 470 View posts

#25

Posted by MrKendo: Wed. May 27, 2020 - 09:39 AM

While you're at it, make sure c is defined locally ie. change

c = getChar();

to

char c = getChar();

At the moment you are relying on c having been defined somewhere else

Rajat choudhary wrote:

volatile int a=0,b=0,c=0,m=0,n=0;

Having file scope variables like this with name like a,b,c is not a good idea :)

EDIT

Also, I would make the rx write position volatile

uint8_t rxWritePos=0;

Last Edited: Wed. May 27, 2020 - 09:44 AM

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#26

Posted by Rajat choudhary: Fri. May 29, 2020 - 08:09 AM (Reply to #25)

#include <avr/io.h>

#define F_CPU 8000000
#define BAUD 9600
#define RATE ((F_CPU/16/BAUD)-1)

#include <util/delay.h>
#include <string.h>
#include <stdio.h>
#include <avr/interrupt.h>

#define LED_ON  PORTD |= (1<<PIND6);
#define LED_OFF  PORTD &= ~(1<<PIND6);
#define LED_OFF5  PORTD &= ~(1<<PIND5);
#define toggel_led6  PORTD ^=(1<<PD6);
#define toggel_led5  PORTD ^=(1<<PD5);

#define RX_BUFFER_SIZE  128

char rxBuffer[RX_BUFFER_SIZE];
char Reply_CallReady[]="LEDON";
uint8_t rxReadPos=0;
volatile rxWritePos=0;

char getChar(void);
char peekChar(void);

void getString(char * buf, int len) {
	int idx = 0;
	while(1)
	{
		char c = getChar();

		if (c == '\0')
		{
			continue;
		}
		// if it is RET or NL..
		if ((c == '\r') || (c == '\n'))
		{
			break;
		}
		// else store the received character
		buf[idx] = c;
		// increment the index
		idx++;
		// if reached the end (with room for final 0x00) break
		if (idx == (len - 1))
		{
			break;
		}
	}
	// mark the end of the
	buf[idx] = 0x00;
}

int main(void)
{
	DDRD |=(1<<PIND6)|(1<<PIND5);

	UBRRH |= (RATE>>8);
	UBRRL |= RATE;

	UCSRB |= (1<<RXEN)|(1<<RXCIE);
	UCSRC |= (1<<URSEL)|(1<<UCSZ0)|(1<<UCSZ1);
	sei();
	char rxString[14];
	while (1)
	{
		 getString(rxString, 14);
		 if (strcmp(rxString,"LEDON") == 0) {
			 LED_ON;
		 }
		 else if (strcmp(rxString, "LEDOFF") == 0) {
			 LED_OFF;
		 }

	}

}

char peekChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
	}
	return ret;
}

char getChar(void)
{
	char ret='\0';
	if (rxReadPos != rxWritePos)
	{
		ret=rxBuffer[rxReadPos];
		rxReadPos++;
		if (rxReadPos>=RX_BUFFER_SIZE)
		{
			rxReadPos=0;
		}
	}
	return ret;
}

ISR(USART_RXC_vect)
{
	rxBuffer[rxWritePos]=UDR;

	rxWritePos++;
	if (rxWritePos>=RX_BUFFER_SIZE)
	{
		rxWritePos=0;
	}
}

receiver code.....

#include <avr/io.h>

#define F_CPU 8000000
#define BAUD  9600
#define BRC   ((F_CPU/16/BAUD)-1)
#define TX_BUFFER_SIZE 128

#include <util/delay.h>
#include <string.h>
#include <avr/interrupt.h>

char serialBuffer[TX_BUFFER_SIZE];
uint8_t serialReadPos=0;
uint8_t serialWritePos=0;

void appendSerial(char c);
void serialWrite(char  c[]);

int main(void)
{
	UBRRH = (BRC >> 8);
	UBRRL =  BRC;

	UCSRB = (1 << TXEN)  | (1 << TXCIE);
	UCSRC = (1 << URSEL) | (1 << UCSZ1) | (1 << UCSZ0);

	sei();

	char txstr[14]="LEDON\r";

	serialWrite(txstr);

	while (1)
	{

	}
}

void appendSerial(char c)
{
	serialBuffer[serialWritePos] = c;
	serialWritePos++;

	if (serialWritePos >= TX_BUFFER_SIZE)
	{
		serialWritePos = 0;
	}
}

void serialWrite(char c[])
{
	for (uint8_t i = 0; i < strlen(c); i++)
	{
		appendSerial(c[i]);
	}
	if (UCSRA & (1 << UDRE))
	{
		UDR = 0;
	}
}

ISR(USART_TXC_vect)
{
    if (serialReadPos != serialWritePos)
    {
		UDR = serialBuffer[serialReadPos];
		serialReadPos++;

		if (serialReadPos >= TX_BUFFER_SIZE)
		{
			serialReadPos++;
		}
    }

}

transmitter code.........

transmitter is doing well.i have checked on virtual terminal(Proteus).

sir, this the best i have done with your kind help, but it is still not working please correct me where i am doing wrong.it will be really appreciable.

THANK YOU.

Rjchoudhary

Last Edited: Fri. May 29, 2020 - 08:10 AM

awneil

Level: 10k+ Postman

Joined: Fri. Jul 1, 2005

Posts: 19952 View posts

Location: Basingstoke, Hampshire, UK

#27

Posted by awneil: Fri. May 29, 2020 - 08:19 AM (Reply to #26)

Rajat choudhary wrote:

not working

tells us nothing.

http://www.catb.org/esr/faqs/smart-questions.html#code

What, exactly, did you expect that it should do ?
What, exactly, is it actually doing ?
What testing / debugging / investigation have you done to find where it's going wrong?

You say you have Proteus - so have you used that to step through the code, and watch what is happening?

https://www.bbc.co.uk/bitesize/guides/zgmpr82/revision/6

https://www.avrfreaks.net/commen...

Top Tips:

How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
"Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
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
Difference between a crystal, and a crystal oscillator: https://www.avrfreaks.net/comment...
When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

Last Edited: Fri. May 29, 2020 - 08:25 AM

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#28

Posted by Rajat choudhary: Fri. May 29, 2020 - 08:43 AM (Reply to #27)

sir i have tested the transmitting code and it is doing well but when i am trying to read the string and comparing with the help of strcmp function to be equal with my array and perform led on or off, it is not working.

but if i check for

if(strcmp(rxString,"LEDON") != 0)

then the led keeps on continuously......

so here i am stuck to find the solution.i am asking you for any suggestion or solution.

Rjchoudhary

awneil

Level: 10k+ Postman

Joined: Fri. Jul 1, 2005

Posts: 19952 View posts

Location: Basingstoke, Hampshire, UK

#29

Posted by awneil: Fri. May 29, 2020 - 09:02 AM (Reply to #28)

So have you examined the rxString variable to see what it contains at that point ?

Again, you have Proteus - use it!

EDIT

Have you checked the documentation of strcmp() to be sure that you're using it correctly?

eg, http://www.cplusplus.com/reference/cstring/strcmp/

To check what value is actually being returned, try writing it like this:

int result = strcmp( rxString, "LEDON" );

if( result  != 0 )

then you can inspect the value of result - so see if it's what you were expecting ...

Top Tips:

How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
"Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
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
Difference between a crystal, and a crystal oscillator: https://www.avrfreaks.net/comment...
When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

Last Edited: Fri. May 29, 2020 - 09:06 AM

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#30

Posted by clawson: Fri. May 29, 2020 - 09:59 AM

If you are looking for "LEDON" surely it needs to be a test "== 0" not "!= 0" ??

strcmp() returns 0 when the strings match and some non-0 otherwise.

MrKendo

Level: Hangaround

Joined: Mon. Nov 19, 2018

Posts: 470 View posts

#31

Posted by MrKendo: Fri. May 29, 2020 - 11:03 AM

This won't be the problem, but I would fix it anyway

Rajat choudhary wrote:

volatile rxWritePos=0;

you need the type still

    volatile uint8_t rxWritePos;

You should be getting at least a compiler warning about this (it probably defaults to type int).

You will now have to debug the problem.

Whatever characters you think you are sending should end up in the uart rx buffer. It is 128 in size so the data won't get overwritten until you have sent more than 128 characters so you should be able to see the data there. You should then be able to see what ends up in the rxString array. And so on.

N.Winterbottom

Level: Posting Freak

Joined: Tue. Oct 1, 2002

Posts: 1653 View posts

Location: Redbourn, Herts, UK

#32

Posted by N.Winterbottom: Fri. May 29, 2020 - 11:17 AM

I cannot see anything wrong with your code except this anomaly which from reset won't cause a problem:

	UBRRH |= (RATE>>8);
	UBRRL |= RATE;

Should be:

	UBRRH = (RATE>>8);
	UBRRL = RATE;

or better as a single line and let the compiler do the shifting:

	UBRR = RATE;

This is an error but not the bug - Time to hit the Proteus debugger I think.

You say Proteus has a Virtual Terminal - Why not ditch the AVR Transmitter Side and simply use the terminal to type your commands.

Last Edited: Fri. May 29, 2020 - 11:20 AM

awneil

Level: 10k+ Postman

Joined: Fri. Jul 1, 2005

Posts: 19952 View posts

Location: Basingstoke, Hampshire, UK

#33

Posted by awneil: Fri. May 29, 2020 - 11:31 AM (Reply to #32)

N.Winterbottom wrote:

I cannot see anything wrong with your code

Other than what I hinted in #29 and clawson stated explicitly in #30 ?

Time to hit the Proteus debugger

Indeed

ditch the AVR Transmitter Side and simply use the terminal

+1

Always minimise the number of unknowns!

As Kartman, in his tutorial, wrote:

Ask yourself “how many problems do I want to solve at once”? The answer should be one. Trying to solve more than one problem at a time is a recipe for failure.

https://www.avrfreaks.net/forum/help-it-doesnt-work

Top Tips:

How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
"Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
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
Difference between a crystal, and a crystal oscillator: https://www.avrfreaks.net/comment...
When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

N.Winterbottom

Level: Posting Freak

Joined: Tue. Oct 1, 2002

Posts: 1653 View posts

Location: Redbourn, Herts, UK

#34

Posted by N.Winterbottom: Fri. May 29, 2020 - 01:18 PM

awneil wrote:

Other than what I hinted in #29 and clawson stated explicitly in #30 ?

No - Rajat's #28 was an experiment; his strcmp tests are good and always have been.

His "AVR Transmitter" only ever transmits one command. Perhaps the first character gets garbled.

Typing commands at the terminal will be instructive.

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100203 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#35

Posted by clawson: Fri. May 29, 2020 - 01:24 PM

N.Winterbottom wrote:

his strcmp tests are good and always have been.

post #10 wrote:

		if (strcmp(Reply_CallReady,compBuffer) ==0)

post #14 wrote:

					if (strcmp(string,Reply_CallReady) ==0)

etc. etc. until you reach:

post #28 wrote:

if(strcmp(rxString,"LEDON") != 0)

So are you sure "good and always have been" is the right phrase to use? Something has gone radically wrong when "== 0" became "!= 0" *unless* the intention really is to test that the string is NOT "LEDON" ?!?

js

Level: Moderator

Joined: Wed. Mar 28, 2001

Posts: 29715 View posts

Location: Sydney, Australia (Gum trees, Koalas and Kangaroos, No Edelweis, but plenty of Corona beer)

#36

Posted by js: Fri. May 29, 2020 - 09:12 PM

I don't see any connection with a Studio problem here, moving to the AVR forum.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

Kunalgupta

Level: Hangaround

Joined: Tue. Nov 19, 2019

Posts: 124 View posts

Location: new delhi ,india

#37

Posted by Kunalgupta: Fri. May 29, 2020 - 10:03 PM

char rxstring0(char buff[])
{ int i=0;
   char myValue;
do
{
       myValue = UART_RxChar0();
       if(myValue!='>')
{
       buff[i]=myValue;
       UART_TxChar0(myValue);
       i++;
} else
       {
           buff[i]='\0';
       break;
       }
}while( !(UCSR1A & (1<<RXC1)));
   printString0("\n");
   printString0(buff);

}

this works perfectly ...here used '>' as line end marker.

now your string is stored in buff variable

for output u can see this..

https://drive.google.com/file/d/...

Kunal Gupta

github.com/gkunalupta

Last Edited: Fri. May 29, 2020 - 10:04 PM

Rajat choudhary

Level: Wannabe

Joined: Tue. Feb 25, 2020

Posts: 58 View posts

#38

Posted by Rajat choudhary: Sat. May 30, 2020 - 08:15 AM

thank you sir.With your kind help now i have got the solution....

Rjchoudhary

AVR Freaks