UART problems on Atmega164a

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

Hi,

 

I have to correct someone code, but I'm not so good in this. As I understand there are problem with UART receiving and/or sending.

 

Can someone help me with this please?

 

 

 

uart.hpp file

 

#ifndef UART_H_
#define UART_H_


#ifndef UART_BAUDRATE
    #warning "You should define UART_BAUDRATE in ini file"
    #define UART_BAUDRATE 19200UL
#endif 

#define F_CPU 14745600L

#define BAUD_PRESCALE F_CPU/16/UART_BAUDRATE-1

void uart_init(uint16_t ubrr);
void uart_send(unsigned int);
void sendstring(char *strin);
void dec_print(int16_t inp);
void hex_print(uint32_t inp);
void bin_print(unsigned char inp);
void cr(void);
unsigned char UART_Receive(void);
#endif /* UART_H_ */

uart.cpp file

#include <Arduino.h>

void uart_init(unsigned int ubrr)
{
	//Setup baudrate
	UBRR0H = (unsigned char)(ubrr >> 8);
	UBRR0L = (unsigned char)ubrr;
	//Enable receiver, transmitter and receive interrupt
	UCSR0B = _BV(RXEN0) | _BV(TXEN0) | _BV(RXCIE0);
	//Set frame format: 8data
	UCSR0C = _BV(UCSZ01) | _BV(UCSZ00);
	//UCSR0B &= ~_BV(UCSZ02);
}

void uart_send(unsigned int data)
{
	//Wait for empty transmit buffer
	while (!(UCSR0A & _BV(UDRE0)));
	//Put data into buffer, sends the data
	UDR0 = data;
}

void sendstring(char *strin)
{
	int k = strlen(strin);
	for (int i = 0; i < k; i++)
	{
		while ((UCSR0A & _BV(UDRE0)) == 0);
		UDR0 = strin[i];
	}
}

void dec_print(int16_t inp)
{
	char mas[8];
	if (inp < 0)
	{
		uart_send('-');
		inp = abs(inp);
	}
	itoa(inp, mas, 10);
	sendstring(mas);
	//uart_send(13);
}

void bin_print(unsigned char inp)
{
	for (unsigned char i = 8; i > 0; i--)
	{
		if (inp & _BV(i - 1))
		{
			uart_send('1');
		}
		else
		{
			uart_send('0');
		}
	}
}

void hex_print(uint32_t inp)
{
	char mas[15];
	itoa(inp, mas, 16);
	sendstring("0x");
	sendstring(mas);
}
void cr(void) { uart_send('\r'); }

unsigned char UART_Receive(void)
{
	//Wait for data to be received
	while (!(UCSR0A & (1 << RXC0)));
	//Get and return received data from buffer
	return UDR0;
}

 

Last Edited: Tue. Nov 10, 2020 - 09:57 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

AleksejsM wrote:
I have to correct someone code

Why do you have to?

 

AleksejsM wrote:
As I understand there are problem with UART receiving and/or sending

What "problem(s)", exactly?

 

AleksejsM wrote:

#include <Arduino.h>

Surely, Arduino already comes with all this stuff ready made & working?

 

EDIT

 

https://www.arduino.cc/reference/en/language/functions/communication/serial/

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...
Last Edited: Tue. Nov 10, 2020 - 10:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

AleksejsM wrote:

#include <Arduino.h>

Since this is arduino code, you should not be talking directly with the hardware, but using arduino I/O (serial() functions)

 

Jim

 

 

(Possum Lodge oath) Quando omni flunkus, moritati.

"I thought growing old would take longer"

 

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

awneil wrote:

Why do you have to?

We have some segment displays and a code provided by some guy, but now this guy don't want to support his code and make some data changes. In that situation we asked him for source code. And thats we got :(

 

awneil wrote:
What "problem(s)", exactly?

There are no comming data back to serial port either I just try to send just a test line.

 

Jim

ki0bk wrote:
Since this is arduino code, you should not be talking directly with the hardware, but using arduino I/O (serial() functions)

Then I must to remake all the code. This will be in future...

But now I need to finde what's wrong in this part

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


This doesn't look like much of any sort of program, just a bunch of routines to set some normal IO things up...what critical tasks should the program be doing? 

If you just wan to talk with the uart, there are a million working examples.  Why not use one & trash this, you wil lbe 10 steps ahead, using your time to do something WITH the uart (rather than getting it to work).

 

 

Then I must to remake all the code. This will be in future...   

 

 

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

FAQ#3 in my sig....

#define F_CPU 14745600L

I wonder!?

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

Yeah, I'm really using 14,7456MHz quartz

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

AleksejsM wrote:
Yeah, I'm really using 14,7456MHz quartz
You may have the quartz attached but is it activated? (kind of my point).

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

clawson wrote:

AleksejsM wrote:

Yeah, I'm really using 14,7456MHz quartz

 

You may have the quartz attached but is it activated? (kind of my point).

 

You thinking about activating with fuses? Extended 0xFF, High 0xD8, Low 0xFF

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

There are several things to "tidy up" in your code.

 

1.  use #error for inappropriate BAUDRATE

2.  remove RXCIE from init()

3.  hex_print() has a 32-bit argument but you call a 16-bit signed itoa()

 

There is not much point in posting your code without a test suite.

e.g. call each function with suitable test values.   e.g. typical, boundary limit,  illegal,  ...,  short strings, empty strings, long strings, ...

 

Something simple like this can just print to the Serial Terminal.   And you can compare the actual to the expected with your eyes.

A rigorous test suite would automate the actual with expected comparison.   And report any failure.    (and award a cup of tea for success)

 

David.

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

AleksejsM wrote:
You thinking about activating with fuses? Extended 0xFF, High 0xD8, Low 0xFF

 

Try this instead:

Exteded:0xFF, High:0xD8, Low:0xF7

 

This will enable Full Swing oscillator.  You had enabled External Crystal Oscillator which is looking for an externally generated square wave.

 

Jim

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

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

AleksejsM wrote:

void uart_send( unsigned int data )

That should be char ?

 

Would also be more helpful to name it uart_sendchar() or suchlike - so that it's immediately clear what it's sending.

 

AleksejsM wrote:

void uart_send( unsigned int data )
{
	//Wait for empty transmit buffer
	while (!(UCSR0A & _BV(UDRE0)));
	//Put data into buffer, sends the data
	UDR0 = data;
}

void sendstring( char *strin )
{
	int k = strlen(strin);
	for (int i = 0; i < k; i++)
	{
		while ((UCSR0A & _BV(UDRE0)) == 0);
		UDR0 = strin[i];
	}
}

You have duplicated code there - that's never a good thing.

 

sendstring() should call uart_send()

 

Also, for consistency, name it uart_sendstring()

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

jgmdesign wrote:
You had enabled External Crystal Oscillator which is looking for an externally generated square wave

see Tip #4 in my signature, below, for the difference  between a crystal, and a crystal oscillator:

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

awneil wrote:

jgmdesign wrote:
You had enabled External Crystal Oscillator which is looking for an externally generated square wave

see Tip #4 in my signature, below, for the difference  between a crystal, and a crystal oscillator:

Thanks Andy! Thats how I figured it out....... :)

Jim

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

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

awneil wrote:
That should be char ?

Yes you're correct.

 

 

david.prentice

david.prentice wrote:
There is not much point in posting your code without a test suite.

Here are my other code part main.cpp:

#include <Arduino.h>
#include <led_driver.hpp>
#include <uart.hpp>
#include <math.h>

#define AIRTEMP 0x64 // 100 - Air Temp
#define ROADTEMP 0x65 // 101 - Road Temp
#define WINDSPEED 0x0190 // 400 - Wind Speed

#define DATA_TYPE_FLOAT 0x16

#define DATA_RECIEVED 0x07
#define WAITING 0
  
volatile uint8_t inbyte;
volatile uint8_t status = WAITING;

uint32_t raw32_wind_speed;
uint32_t raw32_air_temp;
uint32_t raw32_road_temp;

uint16_t temp1,temp2;

ISR(USART0_RX_vect){
    cli();
	inbyte = UDR0;
	if (inbyte == 0x08){
		inbyte = UART_Receive();
		if(inbyte == 0){

			uint16_t function = UART_Receive();			
			function |= UART_Receive() << 8;
			
			switch (function)
			{
				case AIRTEMP: 
					if(UART_Receive() == DATA_TYPE_FLOAT){
						status |= 0x01;
						raw32_air_temp = UART_Receive();
						raw32_air_temp |= (uint32_t) UART_Receive() << 8;
						raw32_air_temp |= (uint32_t) UART_Receive() << 16;
						raw32_air_temp |= (uint32_t) UART_Receive() << 24;
					}	
				break;
				
				case ROADTEMP: 
					if(UART_Receive() == DATA_TYPE_FLOAT){
						status |= 0x02;
						raw32_road_temp = UART_Receive();
						raw32_road_temp |= (uint32_t) UART_Receive() << 8;
						raw32_road_temp |= (uint32_t) UART_Receive() << 16;
						raw32_road_temp |= (uint32_t) UART_Receive() << 24;		
					}		
				break;

				case WINDSPEED: 
					if(UART_Receive() == DATA_TYPE_FLOAT){
						status |= 0x04;
						raw32_wind_speed = UART_Receive();
						raw32_wind_speed |= (uint32_t) UART_Receive() << 8;
						raw32_wind_speed |= (uint32_t) UART_Receive() << 16;
						raw32_wind_speed |= (uint32_t) UART_Receive() << 24;	
					}	
				break;
								
				default:
				break;	
			}	
		}
	}
sei();
};

int real_air, real_road, real_wind;
int update_counter = 0;

void infinite_test(){
	while(1){
		real_air ++;
		display_results(real_air ,real_air, real_air);
		delay(300);
	}
}

void setup(){

	uart_send("Hello Meteo!");

    led_driver_init(20);
    display_Print3Dec(888, false);
	display_Print3Dec(888, false);
    display_Print3Dec(888, true);
	delay(3000);
	display_results(333, 0, 0);
    uart_init(BAUD_PRESCALE);
	uart_send(BAUD_PRESCALE);

    sei();
}

void loop(){

	if(status == DATA_RECIEVED){
		delay(50);
		cli();

		float wind_f = 0;
		float road_f = 0;
		float air_f = 0;

		memcpy(&wind_f, &raw32_wind_speed, 4);
		memcpy(&air_f, &raw32_air_temp, 4);
		memcpy(&road_f, &raw32_road_temp, 4);

		real_air = (int16_t) (air_f * 10);
		real_road = (int16_t) (road_f * 10);
		real_wind = (int16_t) (wind_f * 10);
			
		dec_print(real_air); 
		uart_sendstring(" ");	
		dec_print(real_road); 
		uart_sendstring(" ");	
		dec_print(real_wind); 
		uart_send(13);

		sei();
		status = WAITING;
		display_results(real_air, real_road, real_wind);
		update_counter = 0;
	}
    delay(50);
	update_counter++;
	if(update_counter >= 100 && status == WAITING){ // 5 seconds
		update_counter = 0;
		display_results(real_air, real_road, real_wind);
	}
}

 

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

I suggest that you write a specific test suite for the UART functions.    Then you will be able to re-use them in other projects with confidence.

 

I see that you have an ISR() for RXCIE but you should never call cli() or sei() from within an ISR()

 

Seriously,   if you are using an Arduino core it comes with an interrupt driven Serial class.    And has methods for doing most things that you need.

The major failing is formatted numbers.    So I use sprintf() and Serial.print() the resultant string.

 

Note that an ISR() normally fills a Receive buffer and leaves.  And the Serial.read() empties the buffer.

Your ISR() is very prone to failure.    What happens if the correct byte does not arrive?   Your ISR() will just hang forever.

 

I strongly recommend following the Arduino style (if using Arduino classes and hardware)

The Arduino Serial code is good.  Most Arduino functions work very well.    Don't believe the intellectual snobs on the Internet.

 

If there is something you are unsure of,  ask.    e.g. if you think that it could run better.

 

Remember that God gave you printf, Serial, ...

It is always wise to use proven library code whenever possible.

 

And if you think of better ways to do things,   write a test suite to check everything is working correctly.

 

David.