Store UART Data in Char Array and display to PC

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

Hi,

Board : ATMEGA328P

I have just started a project to build a GUI in VB which sends commands in format "X12345#"

where X is start character, 5 digit data (0 to 65535) and # is data end character

 

I am using UART receive interrupt. My code also contains a TIMER1 interrupt(FOR PWM generation) but i am disabling it.

Problem: when i send "A12345#", i am not getting the same value in my PC (attached snap below).

 

Intention to send back to PC is just to verify my data for parsing. Actually I want to convert the string received to meaningful register value to be fed to OCR1A register.

 

Please guide me.

Thank you for reading.

Data send and receive snapshot in PC

 

Code (Attached also)

#define F_CPU 16000000
#define BAUD 9600
#define MYUBRR F_CPU/16/BAUD-1
#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>
#include <stdlib.h>
#include<stdio.h>
//storage variables
bool toggle1 = false;
bool command_ready=false;

unsigned char buffer[16];
volatile char data_in[200];
volatile unsigned char data_count=0;

unsigned int TimerData;
unsigned int oldTimerData;

void uartInit(unsigned int ubrr){
	/*Set baud rate */
	UBRR0H = (unsigned char)(ubrr>>8);
	UBRR0L = (unsigned char)ubrr;
	
	/*Enable receiver and transmitter. Receive interrupt enable*/
	UCSR0B = (1<<RXCIE0) | (1<<RXEN0)|(1<<TXEN0);
	/* Set frame format: 8data, 1stop bit */
	UCSR0C = (0<<USBS0)|(3<<UCSZ00);
}

void uartTxByte(unsigned char byte){
	/* UCSRC0 TX buffer ready? */
	while (!(UCSR0A &(1<<UDRE0)));	//WAIT TILL DATA IS RECEIVED AND BUFFER IS EMPTY
	/* transmit data */
	UDR0 = byte;
	//SHOULD WE WAIT TILL BYTE IS TRANSMITTED? will observe after complete code configuration
	//while (!(UCSR0A &(1<<TXC0)));
}

void uart_sTxString(char * str) {
	//cli();
	while (*str) {
		uartTxByte(*str++);
	}
	//sei();
}
int main(){
	//CONVIGURE UART
	uartInit(MYUBRR);
	
	//CONFIGURE ADC
	ADMUX |=(1<<REFS0);	//REFERENCE VOLTAGE OF 5V, CHANNEL A0
	
	//set pins as outputs
	DDRB |=(1<<PINB1) |(1<<PINB5);

	cli();//stop interrupts
	oldTimerData=0;
	TCCR1A = 0;// set entire TCCR1A register to 0
	TCCR1B = 0;// same for TCCR1B
	TCNT1  = 0;//initialize counter value to 0
	OCR1A = 65535;
	TCCR1A |=(1<<WGM11) | (1<<WGM10) |(0<<COM1A1) | (1<<COM1A0);	//TOGGLE ON COMPARE MATCH
	TCCR1B |= (1<<WGM13) |(1 << WGM12);	//FAST PWM MODE 15
	ADCSRA |(1<<ADEN) |(1<<ADSC) |(1<<ADIE);	//start adc, INTERRUPT ENABLE
	TCCR1B |= (0 << CS12) | (1 << CS11) | (1 << CS10);	//start timer with no prescalar
	// enable timer compare interrupt
	TIMSK1 |= (0 << OCIE1A);
	sei();//allow interrupts

	while(1){
		itoa(data_count, buffer, 10);
		if (data_count==7){
			uart_sTxString(data_in);
			uart_sTxString(buffer);
			data_count=0;
		}
	}	
}
//ISR(TIMER1_COMPA_vect){//timer1 interrupt 1Hz toggles pin 13 (LED)
////generates pulse wave of frequency 1Hz/2 = 0.5kHz (takes two cycles for full wave- toggle high then toggle low)
  //if (toggle1){
    //PORTB |=(1<<PINB5);
    //toggle1 = 0;
  //}
  //else{
    //PORTB &=~(1<<PINB5);
    //toggle1 = 1;
  //}
	////FOR DEBUG ONLY. To be moved to proper function out of ISR SCOPE
	////itoa(OCR1A, buffer, 10);
	////uart_sTxString(buffer);
	//
//}

 // Interrupt service routine for the ADC completion
 ISR(ADC_vect){
	 OCR1A=ADC*64;
	// uartsend(*buffer);
}

ISR(USART_RX_vect){
//while (!(UCSR0A &(1<<RXC0)));	//Check if data transmitter buffer is ready
data_in[data_count] = UDR0;
data_count+=1;
}

 

 

 

Attachment(s): 

-

Srikanth Chilivery

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

Could it be a VB bug?

Please check if the same result can be obtained with teraterm etc.

 

Is the 328p definitely running at 16MHz with an external crystal?

You can check it with a 1Hz LED blink.

 

Do you have a debugger?

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

Unless you are 100% sure you PC program is perfect, you need to use terraterm, Bray's terminal, or similar.  You don't wan tot have potential proble,ms in 2 softwares at the same time.

 

Use a known working terminal program, then you know you can concentrate on the AVR code.

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

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

Your method of receiving the data is clunky. The end character is commonly a carriage return and/or line feed. 

Your code would need to check for a valid start character to synchronise to the incoming data.

 

Who uses VB these days? It was out of date 20 years ago! There's much easier and better alternatives these days. Eg nodejs or python.

 

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

Who uses VB these days?

As in beer or as in Visual Basic?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

Seems VB as a beer is becoming less popular it seems. Seems joe public has a more developed taste!

 

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

uart_sTxString(data_in); is a problem waiting to happen, because the received 'string' in data_in[] is never explicitlty terminated with a null ('\0').

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

I managed to simulate some of your code and found something to explain some of what you're seeing.

 

You perform an itoa() constantly inside your while loop.

itoa is computationally expensive in CPU time because it'll be using divides and those are expensive on AVR.

So during the execution of itoa where data_count < 7 there is enough time for the remainder of the characters to be received via your ISR().

After the itoa completes; (data_count == 7) becomes true, so you now print the strings, but itoa has not run with data_count == 7

 

Solution: Just move the itoa() inside the IF block

    while(1) {
        if (data_count==7){
            itoa(data_count, buffer, 10);
            uart_sTxString(data_in);
            uart_sTxString(buffer);
            data_count=0;
        }
    }	
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you very much for responding and guiding me on itoa function. Actually i found it through google.

I used it temporarily to see if i got the values correct. I have removed and checked by only transferring data_in value. It works. perfectly.

I checked the received data in Teraterm ( I will have to work on the PC code).

 

Now I am thinking to parse this data. Please guide me a way.

 

 

Ex.

1. ATMEGA receive "A65534#" (no carriage return and line feed).

2. After receiving 7 bytes start parsing data.

3. Compare the first character and decide which register to update . "A" for updating the OCR1A register value.

4. Then i want to convert String 65534 to unsigned integer 65534 and set the register value OCR1A with this new value.

 

My understanding of the flow is to loop through data_in array. Use a Switch selection for the first character.

 

But i am not able to understand how do i convert the numeric string to numeric.

 

Please guide me with the fundamentals of parsing.

 

kabasan wrote:

Could it be a VB bug?

Please check if the same result can be obtained with teraterm etc.

 

Is the 328p definitely running at 16MHz with an external crystal?

You can check it with a 1Hz LED blink.

 

Do you have a debugger?

No i dont have debugger.

-

Srikanth Chilivery

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

srikanth977 wrote:

2. After receiving 7 bytes start parsing data.

 

No. Start parsing the moment you see your start character. Write a simple state machine, something like this...

 

State 1 = wait for start, move to state 2 once 'A' has been received.

State 2 = wait for first character, move to state 3 when a character has been received as long as the character is valid else go back to state 1

State 3 = receive 5 characters, move to state 4 once we have 5 valid characters, else go to state 1

State 4 = wait for next character, move to state 5 if '#' received, else state 1

State 5 = process the received value and go to state 1.

 

Why a state machine? Because you can deal with errors as they happen.

#1 Hardware Problem? https://www.avrfreaks.net/forum/...

#2 Hardware Problem? Read AVR042.

#3 All grounds are not created equal

#4 Have you proved your chip is running at xxMHz?

#5 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand."

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

No. Start parsing the moment you see your start character. Write a simple state machine, something like this...

Maybe, maybe not...say you wanted to handle the following commands

SETRPM1234 (SETRPM value)

STOPMTR   (stop motor)

RUNMTR 

REVMTR

FWDMTR

MOTCUR?  (report the motor current)

VSYS?  (report system volts)

VMOT? (report motor volts)

VSIG1? (report signal 1 voltage)

 

Then it might be easier to get the whole thing & then search through a dictionary (the commands).  Of course, even that can be done char-by-char reception with a dictionary, though trickier (like a recursive trre search0.

So then, for example, if you get  VSG, you already know there can be no match, where as VS still has 2 possibilities.

 

 

 

 

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

Last Edited: Sun. Jun 28, 2020 - 04:45 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Brian Fairchild wrote:

srikanth977 wrote:

2. After receiving 7 bytes start parsing data.

 

No. Start parsing the moment you see your start character. Write a simple state machine, something like this...

 

State 1 = wait for start, move to state 2 once 'A' has been received.

State 2 = wait for first character, move to state 3 when a character has been received as long as the character is valid else go back to state 1

State 3 = receive 5 characters, move to state 4 once we have 5 valid characters, else go to state 1

State 4 = wait for next character, move to state 5 if '#' received, else state 1

State 5 = process the received value and go to state 1.

 

Why a state machine? Because you can deal with errors as they happen.

 

I was thinking of state machine logic. but I fear it will be a long code. Will this be ok? I mean a good practice? Can u share your experience how shall u approach the whole process.

Ex. I want to send value 12345 to OCR1A register.

 

avrcandies wrote:

No. Start parsing the moment you see your start character. Write a simple state machine, something like this...

Maybe, maybe not...say you wanted to handle the following commands

SETRPM1234 (SETRPM value)

STOPMTR   (stop motor)

RUNMTR 

REVMTR

FWDMTR

MOTCUR?  (report the motor current)

VSYS?  (report system volts)

VMOT? (report motor volts)

VSIG1? (report signal 1 voltage)

 

Then it might be easier to get the whole thing & then search through a dictionary (the commands).  Of course, even that can be done char-by-char reception with a dictionary, though trickier (like a recursive trre search0.

So then, for example, if you get  VSG, you already know there can be no match, where as VS still has 2 possibilities.

You are right, I am trying to control a motor. By checking each and every value i fear code wont be effective. So i wanted to learn what best approach is generally applied.

-

Srikanth Chilivery

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

srikanth977 wrote:

1. ATMEGA receive "A65534#" (no carriage return and line feed).

2. After receiving 7 bytes start parsing data.

3. Compare the first character and decide which register to update . "A" for updating the OCR1A register value.

4. Then i want to convert String 65534 to unsigned integer 65534 and set the register value OCR1A with this new value.

 

Consider what happens when you receive a corrupted message from the PC (it will happen). If you count bytes how will you resynchronise the messages?

#1 Hardware Problem? https://www.avrfreaks.net/forum/...

#2 Hardware Problem? Read AVR042.

#3 All grounds are not created equal

#4 Have you proved your chip is running at xxMHz?

#5 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand."

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

What's the purpose of the # at the end?

If it is to mark the end of a message, the alternative to state machine is to accumulate the received characters into a buffer and keep going until you get the #, or until you have run out of space in the buffer.

When you do get the #, write a null char (value 0) instead of '#' so you now have a null terminated string. Then process the whole message. Having it in the form of a null terminated string means you can use all the standard string handling functions.

You know what the longest expected message is, let's say it is 10 chars without the #.

So have a buufer of size 11, to allow for the '# come null' on the end.

If you have already received 10 chars, and the next one isn't a # you have various choices what to do.

Simplest thing is probably  just discard any extra chars, keep going until you do get a #.

When you do finally get a # you can either process what you have ended up with, or just ignore the whole thing.

The main thing is you will always be synchronising to the start of what should be a new message.

 

There have been lots of threads on this kind of thing.

It's nothing new!

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

When you do get the #

And it is better if you just use a CR reception to trigger the ending, so you can type some command and hit enter/return (CR)...then the RCV handler knows it's time to do something with the info.

You could use #, but that is a strange way of typing & won't start a new line if your are using a terminal to test things out.  Actually, maybe some terminals would let you set # as a newline char!   

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

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

srikanth977 wrote:

I was thinking of state machine logic. but I fear it will be a long code. Will this be ok? I mean a good practice? Can u share your experience how shall u approach the whole process.

Ex. I want to send value 12345 to OCR1A register.

 

Long code is not bad practice if it adds value/functionality (e.g, if it performs validation checks, it is adding value), unnecessarily complex code is. For this particular function, I doubt the code will be very long either. A simple way to do it would go something like this for me:

Use a couple of global boolean flags, say readyToReceiveNumber and readyToExecuteCommand, both initialized as false. Also initialize a global array index variable, say uint8_t i and a command switch variable char x.

The code might look something like this:

void uart_rx_isr()
{
    char c=UDR0;
    if(readyToExecuteCommand)
        return;     //ignore incoming if a command is not yet executed.
    if(readyToReceiveNumber)
    {
        if(c>='0' && c<='9')    //check if the character is a digit
            data_in[i++]=c;     //if valid, store it.
        else
        {
            if(c=='#')      //if '#', time to execute the command.
            {
                data_in[i]='\0';    //null terminate the string.
                readyToExecuteCommand=true;
                readyToReceiveNumber=false;
            }
            else
                readyToReceiveNumber=false;     //else reset.
        }
    }
    else     //waiting for the first character.
    {
        if(c>='A' && c<='F')        //check for whatever your criteria is for a valid command.
        {
            x=c;        //store the command letter in the switch variable.
            readyToReceiveNumber=true;
            i=0;
        }
    }
}


int main()
{
    //initializations as required.
    while(true)
    {
        if(readyToExecuteCommand)
        {
            uint16_t value = atoi(data_in);    //requires a null terminated string.
            switch(x)
            {
                case 'A':
                    OCR1A = value;
                    break;
                case 'B':
                    //other commands follow.
            }
            readyToExecuteCommand=false;
        }
    }
}

This code isn't tested but should give you the idea. Neither the starting character nor the ending # go into the received string.  I've written the validation checks in the ISR since there's nothing very time consuming there, but you might want to move it out to main (use another flag to process the data). 

Good luck.

-Sam

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

Samay, your code is calling out for a finite state machine. Having flags does not really scale well - makes the code hard to write and understand.

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

For comparison, the 'accumulate it all into a buffer then process it' method would look something like this.

This is asuming a simple blocking non interrupt uart get_char. Easy to adapt to having an interrupt driven uart rx with circular buffer.

This is using '#' as terminating character, but like it said in #15, in reality you may as well use CR or LF.

 

static void process_message (const char *buf)
{
    if (buf[0] == 'A')
    {
        long val = atol(buf + 1);
        if (val > UINT16_MAX)
        {
            /* doesn't fit in uint16, ignore or truncate */
        }
        else
        {
            OCR1A = (uint16_t)val;
        }
    }
    /* else etc. */
}

#define MAX_MSG_LEN 6

int main(void)
{
    char msg_buf[MAX_MSG_LEN + 1];
    uint8_t mb_index = 0;

    while (1)
    {
        char c = uart_get_char();
        if (c == '#')
        {
            msg_buf[mb_index] = '\0';
            process_message(msg_buf);
            mb_index = 0;
        }
        else if (mb_index < MAX_MSG_LEN)
        {
            msg_buf[mb_index++] = c;
        }
    }
}

 

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

I'll vote for dispensing with the state machine idea here.

 

Command processors tend to grow & grow as projects near completion and whilst extending state machines is relativley easy, modifying their behaviour often means starting over again.

 

Instead I'll present my favoured solution from a previous AVRfreaks post some years ago:

 

https://www.avrfreaks.net/forum/alternative-case-and-nested-if-statements

 

It involves collecting the entire command line then parsing it to determine the command to run and sub-parsing further to obtain command arguments.

 

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

Guys I found a way and its working now.

I got this idea from my engineering college project.

 

1. First i decided that i will send HEX string. If i want to send integer value 4367 , its hex will be 110F,

 

2. Enclosed the above data in "*110F#\r" // I am sure that i will always send 7 characters. (Start character *, 16BIT VALUE IN HEX, End character #, carriage return)

   Instead of choosing integer (variable length) i fixed to HEX in 4 character format.

 

3. As soon as i received UART RX interrupt, i added these characters to character array and counted the number of bytes received.

 

4. In a while loop

while(1){
		DRV_WriteValue=0;
		if((data_count==7) && (data_in[0]=='*') & (data_in[5]=='#')&&(data_in[6]=='\r')){	//I received 6 bytes and my first character is '*' and last character is '#' then process
			uartTxByte('W');	//send a wait byte.
			uart_StopReceiver();	//stop the receiving UART
			data_count=0;		//RESET THE DATA_COUNT VALUE

			//EXTRACTION PROCESS
			b1=data_in[1] -( (data_in[1]>=65) ? 55 : 48);
			b2=data_in[2] -( (data_in[2]>=65) ? 55 : 48);
			b3=data_in[3] -( (data_in[3]>=65) ? 55 : 48);
			b4=data_in[4] -( (data_in[4]>=65) ? 55 : 48);

			Reg_WriteValue = (b1 <<12) |(b2<<8) |(b3<<4) | b4;
			OCR1A=Reg_WriteValue;

			//just for simulation fun
			for (i=0;i<=5;i++){
				data_in[i]='\0';
			}
			//flushed any data buffer if present
			uart_Flush();	//clearing buffer before enabling receiver
			uartTxByte('B');	//send a BEGIN byte.
			uart_StartReceiver(); //restarted UART with interrupt

		}
	}

 

I defined as unsigned char,

unsigned char b1,b2,b3,b4;

MrKendo wrote:

This is using '#' as terminating character, but like it said in #15, in reality you may as well use CR or LF.

As per your suggestion I included a CR character. May be I will study more about good practices of UART communications.

 

I used state machine logic in PC and sent final ready value to micro controller.

 

Presently in the code i applied to OCR1A register for checking.

 

Now one small issue, I wanted to show OCR1A value on uart (as I said i don't have a debugger) is there any other method to get it correct. itoa function seems to be wrong (when i have OCR1A=65535, i get -1 on screen in teraterm)

-

Srikanth Chilivery

Last Edited: Mon. Jun 29, 2020 - 06:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

65535 is -1 as an integer (signed)

try utoa for unsigned integer

David (aka frog_jr)

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

First i decided that i will send HEX string. If i want to send integer value 4367 , its hex will be 110F,

Why?  That seem "ok" , but wouldn't you usually like see decimal values on your terminal?   For example, you set the room temperature to 75 degrees ...do you want to see "75" or  "4B"  on the terminal screen??

 

Now, in some cases hex may be preferred  (maybe, such as creating a debugger terminal interface) , but I think you'd usually prefer decimal.

Hex is also more compact, since 3 hex digits (FFF) can represent 4095, which takes 4 decimal characters.

 

In any case (hex or decimal) you will actually send separate ASCII characters .... like 9  1  3  4    or   2  3  A  E 

You could send straight binary instead (even more compact), but that will be a be messy mistake that you will regret.

 

 

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

Last Edited: Tue. Jun 30, 2020 - 04:04 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Just an aside but in something like:

			b1=data_in[1] -( (data_in[1]>=65) ? 55 : 48);

I can't help thinking it would tell the reader more about what is going on if you used:

			b1 = data_in[1] -( (data_in[1] >= 'A') ? ('A' - 10) : '0');

Just a thought.

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

avrcandies wrote:

First i decided that i will send HEX string. If i want to send integer value 4367 , its hex will be 110F,

Why?  That seem "ok" , but wouldn't you usually like see decimal values on your terminal?   For example, you set the room temperature to 75 degrees ...do you want to see "75" or  "4B"  on the terminal screen??

 

Now, in some cases hex may be preferred  (maybe, such as creating a debugger terminal interface) , but I think you'd usually prefer decimal.

Hex is also more compact, since 3 hex digits (FFF) can represent 4095, which takes 4 decimal characters.

 

In any case (hex or decimal) you will actually send separate ASCII characters .... like 9  1  3  4    or   2  3  A  E 

You could send straight binary instead (even more compact), but that will be a be messy mistake that you will regret.

 

 

 

My GUI shows numeric values, PC back end logic to hex and applied state machine in PC and sent finally 16BIT = 4 CHARACTER fixed DATA.

-

Srikanth Chilivery

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

My GUI shows numeric values, PC back end logic to hex and applied state machine in PC and sent finally 16BIT = 4 CHARACTER fixed DATA.

What about when you use Terraterm or Putty?  Make it easy on yourself, you won't regret it. 

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