Reset loop GPS parser

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

I faced a strange phenomenon while tried to run a gps parser. Briefly the hardwer: NEO-6M GPS module, Atmega328p, LCD1602. While the program is running, the LCD shows the time from the GPRMC message. But after some time, the uC starts reseting (in the init part of the code I have a greeting message on the LCD - this indicates me that the uC has rebooted). After some time it stucks in a reset loop. 

There is no watchdog running, so I have no idea what can cause the reset. My first guess was the amount of ram is too less for processing the GPS sentences. I tried using the freeRam() function I found on Jeelabs, but there there was no meaningful data displayed (and the compiler warn me that an adress is used as a value...). 

Do you have any idea what can cause the reset-loop, and how can I debug it? (I'm using USB-asp programmer, no debugger, and atmel studio). I include some parts of the code:

 

NMEA_parse.c:

#include "NMEA_parse.h"

struct gprmc gprmc_datas;

static void fill_data (char* param_in)
{
	uint8_t i = 0U;
	char* trunk;
	char trunk_datas[20U][10U]; 

	trunk = strtok(param_in, ",");
	while(trunk != NULL)
	{
		i++;
		if(i > 20) { i = 0; }

		strcpy(trunk_datas[i],trunk);
		trunk = strtok (NULL, ",");
	}

	if(memcmp(trunk_datas[1U],"GPRMC",6U) == 0U)
	{
		strcpy(gprmc_datas.time,trunk_datas[TIME]);
		strcpy(gprmc_datas.latitude,trunk_datas[LAT]);
		strcpy(gprmc_datas.longitude,trunk_datas[LON]);
		strcpy(gprmc_datas.date,trunk_datas[DATE]);
		strcpy(gprmc_datas.time,trunk_datas[TIME]);
	}
}

PARSE_RESULT proc_sentence (char* data_in)
{
	char buffer[100U];
	char cs_got[3U];
	uint8_t u8cs_counted = 0U;
	uint8_t cc_flag = FALSE;
	uint8_t i = 0U, j = 0U;

	if(strlen(data_in) < 100U){
		for(i = 0U; i < strlen(data_in); i++){
			if(i == 0){
				if(data_in[0U] != '$') { return D_ERROR; }
				else { ; }
			}
			else{
				if(cc_flag == TRUE){
					if(data_in[i] == 0x0DU) {}
					else{
						if(j > 3){ return CL_ERROR; }
						else if(j == 2) { cs_got[j] = '\0'; }
						else { cs_got[j] = data_in[i]; j++; }
                    }
				}
				else{
					if( (data_in[i] == '$') || (data_in[i] == 0x0A) ||
						(data_in[i] == 0x0D) || (data_in[i] == '\0')) { return S_ERROR; }
					else if(data_in[i] != '*'){
						buffer[i - 1U] = data_in[i];
						u8cs_counted ^= data_in[i];
                        }
					else { cc_flag = TRUE; buffer[i -1U] = '\0';}
				}
			}
		}
		if(strtol(cs_got,NULL,16U) == u8cs_counted){
			fill_data(buffer);
			return OK;
		}
		else { return C_ERROR; }
	}
	else { return L_ERROR; }
}

main.c:

/*
 *  gps_logger.c
 *
 *  Created: 2016. 11. 13. 21:33:16
 *  Author: Barta Tamás
 */ 

#include <avr/io.h>

#include "config_avr.h"
#include "NMEA_parse.h"
#include "UART.h"
#include "lcd1602_i2c.h"

volatile uint8_t data[100U];
volatile uint8_t counter = 0U, sentence_end_flag = FALSE;
volatile uint8_t begin_flag = 0U, end_flag = 0U, begin_temp = 0U;

ISR(USART_RX_vect)
{
    if(counter >= 100U)  { counter = 0U; }
    data[counter] = UART_Receive();
    if(data[counter] == '$')
    {
        begin_temp = counter;
    }
    else if(data[counter] == 0x0AU)
    {
        end_flag = counter;
        begin_flag = begin_temp;
        sentence_end_flag = TRUE;
    }
    counter++;
}

int main(void)
{
    uint8_t i, j, k;
    uint8_t status = D_ERROR;
    char sentence[100U];

    DDRB |= (1 << PB5);
    LcdInit(NOCURSOR, NOBLINK);
    LcdStringSend(0, 0, "Welcome");
    _delay_ms(1000);
    LcdClear();
    UART_Init(9600U);
    sei();

    while(1U)
    {
        if(sentence_end_flag == TRUE)
        {
            cli();
            if(begin_flag < end_flag)
            {
                for(i = begin_flag; i <= end_flag; i++)
                {
                    if(i >= 100) { i = 0; }
                    sentence[i - begin_flag] = data[i];
                }
                sentence[i - begin_flag] = '\0';
            }
            else
            {
                k = begin_flag;
                j = 0;
                for(i = begin_flag; i <= (end_flag + 100); i++)
                {
                    if(k == 100) { k = 0; }
                    if(j >= 100) { j = 0; }
                    sentence[j] = data[k];
                    j++; k++;
                }
                sentence[j] = '\0';
            }
            status = proc_sentence(sentence);
            strcpy(sentence,"");
            sentence_end_flag = FALSE;
            sei();
        }

        if(status == OK)
        {
            LcdClear();
            LcdWriteString(0, 0, gprmc_datas.time);
            status = D_ERROR;
        }
    }
}

 

This topic has a solution.
Last Edited: Sun. Oct 20, 2019 - 04:44 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How do you hope to process a message, when another message is concurrently being received? The usual solution is to buffer the incoming receive data in a circular buffer. The main line code pulls characters from this buffer and parses/assembles a NMEA message, processes it then rinse and repeat.

 

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

Thanks for dealing with my problem. Yes I do use a circular buffer. I mark the beginning of the message, if the $ charachter arrives, and the end of the message, if the line feed arrives on UART. Then I send it to NMEA_parse.c for processing, meanwhile the new incoming message can fill the circular buffer. I measured with scope, and the maximum time to process a sentence is 4,5 ms. 

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

But the issue is surely that your sentence extraction/parsing stuff runs between cli() and sei() so all the characters that arrive while that is running will not trigger interrupts - they'll just be lost.

 

The whole point of using interrupts is so that thing can be received "in the background" while the foreground stuff is doing its work.

 

As it's a 328 I guess it must be 2K - I'd dedicate a lot more of that to the ring buffer and use a "proper" ring buffer like the lightweight one from Dean Camera. Have the foreground code completely isolated form the ring buffer so that it just has a single extract_char_from_buffer() that it uses to build up sentences (so at first skip until you see '$' then take everything up to '\n'). Rinse and repeat.

 

Do not be tempted to use start/end markers in the ring buffer itself.

 

This way you simply have the background process that just keeps receiving and buffering characters. From time to time the foreground process will extract a whole load (like 50..60 or however long NMEA sentences are). Your only issue would be if the foregorund can't extract/parse fast enough - then the buffer would fill right up and overflow. You would need a strategy to decide how to handle that. But 4800 baud NMEA (or whatever the speed is) is so slow that you should have tons of time to parse a sentence before a whole new one has arrived.

Last Edited: Thu. Oct 10, 2019 - 09:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the advice, especially for the ring buffer from Dean Camera, I will definitely give a try to it. The cli() and sei() is a good remark. Further, do you have any idea what "mistakes" can cause a reset in general?

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

yoman91 wrote:
what "mistakes" can cause a reset in general?
Some classics:

 

1) you enable an interrupt source but have no ISR() (but probably not in this case)

 

2) something "eats" memory slowly over time (a "memory leak") and when it all runs out the stack starts to get corrupted so that eventually the CPU does a RET to an invalid address (often 0)

 

3) a pointer is corrupted then used with corrupt value - especially bad with function pointers.

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

Another one to consider, and the C programmers can help me with the exact term...

 

When you have a variable that is shared between your Main Loop code and your ISR code it has to be declared volatile, (or whatever...).

 

(Closely related to Cliff's #3 above).

 

JC

 

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

If all you want is the time, just get the time (I'm going by what you have in main- looks like only time needed). Something like-

 

//headers, etc.
#include <stdbool.h>          //true/false

#define TIMESIZE 7            //6chars + 0 terminate

volatile char time[TIMESIZE]; //store time HHMMSS\0
volatile bool newtime;        //signal new time available

int main(void)
{
    //init stuff

    char buf[TIMESIZE]; //to store copy of time for lcd use

    while(1){
        if( newtime == false ) continue;//no new time yet

        //there is plenty of time to copy the buffer
        //(whatever update rate gps set to- 1Hz, 2Hz, 5Hz, etc)
        //so cli/sei not really needed, but doesn't hurt either

        cli();                          //cli so isr does not modify while copy
        newtime = false;                //clear flag
        for( uint8_t i = 0; i < TIMESIZE; buf[i] = time[i] ); //copy
        sei();                          //done copy

        LcdClear();
        LcdWriteString(0, 0, buf);
    }
}

//|------------| 13ms+ @9600 baud
//$GPRMC,225446,A,4916.45,N,12311.12,W,000.5,054.7,191194,020.3,E*68

ISR(USART_RX_vect)
{
    //msg to match
    //                 01234567
    static char msg[]="$GPRMC,xxxxxx"; //assuming don't need any decimal time if its there (.000)
    //                        789012   //since its probably not of much use
    //                        012345

    static char buf[TIMESIZE]; //6 (0-5) + \0 = 7
    static uint8_t idx;        //index into msg

    char c = UART_Receive();   //get rx char

    char m = msg[idx];         //get current msg char

    if( m == 0 ){              //end of msg
        idx = 0;               //reset index
        buf[TIMESIZE-1] = 0;   //0 terminate
        for( uint8_t i = 0; i < TIMESIZE; time[i] = buf[i] ); //copy to time
        newtime = true;        //signal for main loop
        return;
    }
    if( m == 'x'){              //in the 'x's, getting time
        buf[idx-TIMESIZE] = c;  //7-12 -> 0-5, store time char
        idx++;                  //next
        if( c < '0' || c > '9' ) idx = 0; //not valid char, start over
        return;                 //done
    }
    //checking for msg match
    //if match, inc to next char, else no match start over at msd[0]
    idx = (m == c) ? idx+1 : 0;
}

The example may not be correct and may have errors, but the idea is there (I didn't try to compile it). The simple validation (checking for 0-9) will be plenty good- if you can get a valid $GPRMC, followed by 6 characters that are 0-9, there is a pretty good chance its ok so no need to be trying to figure out a checksum.

 

gps -> isr buf -> [time buf] -> main/tmp buf -> lcd use

that's a total of 21 bytes for buffer use.

 

Maybe you eventually want more than time, but by starting simple you can get a handle on problems earlier and easier. Then work your way into more complicated code as needed.

 

Just providing an alternate idea that may be useful, or maybe not.

 

 

 

 

 

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

if you are only interested in time ( your code also seems to be doing something with long and lat though)

then just start "logging"  characters when you see a $ sign. 

After you then have received the 3rd character you check if that was an R ( IIRC this character is unique in the segment header )

If not, then stop receiving as it is not the segment you want and thus wait for the next start of frame character.

If it is the correct character receive until the carriage return or Linefeed character has been received and notify the main program loop that you have new data.

After that you normally have about 1 second to process that data, so you should have time enough.

 

A more general approach might be reserving 9 segments of 85 bytes in memory. IIRC a GPS segment is max 82 characters long, so I reserved a bit extra space to be safe.

Also there are maximum 9 sentences in a complete 1 second message.

After you have received the start of frame just record in a segment. Then when the end of segment is received notify the main loop what buffer has been filled and jump to the start of the next buffer.

Again, you should have more than enough time to copy and process the data from the buffer to a struct that holds all the data. The ISR can then be kept nice very small.

 

 

 

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

Thanks for the tips. For my purpose the time itself is not enough, because the final goal is a GPS logger which saves data to SD card via FatFS. The time in my application is fine for debugging, because it is changing from second to second, and I can see whether the processing of the sentence was finished or not. 

 

I tried the LightweightRingBuffer.h from Dean Camera. I really pleased with it, did a sample program and it was working. Now I sketched a basic version in the main software. Something is wrong in the NMEA parsing or in the LCD. Since I'm using the LCD library for many years, I guess the problem is in the NMEA parsing. If I put the "i = 0" to line 41 (which is necessary), it keeps resetting. I did not have time to debug, because I was too tired, I want to continue today. 

RingBuff_t Buffer;
volatile uint8_t buffercount;
volatile entence_end_flag = FALSE;

ISR(USART_RX_vect)
{   
    uint8_t data_in = UART_Receive(); 
    RingBuffer_Insert(&Buffer, data_in);
    
	if(data_in == 0x0AU) /* Line Feed */
	{
        buffercount = RingBuffer_GetCount(&Buffer);
        sentence_end_flag = TRUE;
	}
}

int main(void)
{
	uint8_t i = 0;
	uint8_t status = D_ERROR;
	char sentence[100U];

    //Inits
    UART_Init(9600U);
    LCDInit(LS_NONE);
    
    LCDWriteStringXY(0, 0, "Welcome");
    _delay_ms(1000);
    LCDClear();   
	sei();
	
	while(1)
	{         
		if(sentence_end_flag == TRUE)
		{   
            while(buffercount--)
            {
                sentence[i++] = RingBuffer_Remove(&Buffer);
            }
            sentence[i] = '\0';
            i = 0;
            		
			status = proc_sentence(sentence);
            strcpy(sentence,"");
            LCDWriteIntXY(0, 1, status, 1);
			sentence_end_flag = FALSE;
		}
        
		if(status == OK)
		{
            LCDClear();
            LCDWriteStringXY(0, 0, gprmc_datas.time);
			status = D_ERROR;
		}  
	}
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1
	if(data_in == 0x0AU) /* Line Feed */
	{
        buffercount = RingBuffer_GetCount(&Buffer);
        sentence_end_flag = TRUE;
	}

You still aren't getting it. You don't do things like 0xAU (which is much easier to read if you simply use '\n'!) testing in the ISR. The ONLY thing the ISR does is the RingBuffer_Insert(). Later in main() you have something like:

char sentence[SOME_SIZE_ENOUGH_FOR_LONGEST_SENTENCE];

int main(void) {
    while (1) {
        readSentence();
        processSentence();
    }
}

void readSentence() {
    uint8_t i = 0;
    char c;
    // first align to statr of next sentence..
    do {
        while(RingBuffer_IsEmpty()); // wait until chars arrive
        c = RingBuffer_Remove();
    } while (c != '$');
    // then just read chars to "sentence[]" until \n...
    do {
        sentence[i++] = c;
        while(RingBuffer_IsEmpty()); // wait until chars arrive 
        c = RingBuffer_Remove();
    } while (c != '\n');
    sentence[i] = 0;
}

I just did this to show structure - I'm not a great fan of having sentence[] global and functions that have no parameters in fact.

 

The fact is that the sentence[] is decoupled from the actual ring buffer. The ISR() just gets on in the background stuffing characters into the buffer as they arrive and does nothing more. In a completely asynchronous process the readSentence() pulls characters from the buffer when it can. I started with a loop to just waste characters until '$' is found. That is simply to align with the start of the first whole sentence (in case you come in "part way through"). It then just keeps sucking characters out of the ring buffer and building the sentence until it gets to the '\n' (new line) at the end. Then the function returns and processSentence() is used to act upon the whole extracted sentence. Meanwhile, in the background characters continue to arrive and go into the ring. When the next call to readSentence() starts the "current read position" in the buffer should be at the next '$' after the last '\n' already so that first loop will immediately pick up the next '$' and fall though, that is then stored to the buffer and it keeps extracting and building the sentence.

 

If the buffer "runs out" at any stage it will simply sit in the while(RingBuffer_IsEmpty()) loop until the interrupt in the background pushes more received characters into it.

 

So if a healthy buffer has built up then readSentence() may process and return very quickly but, otherwise, the majority of the time will be sat waiting for the characters until the final '\n' arrives allowing it to complete and allow processSentenece() to execute.

 

Hope this is clear.

 

BTW, I just typed the code above - I made no attempt to compile or test it so I could have something horrendously wrong - up to you to debug that!

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

Now it became clear what did you want to suggest me some posts ago. I try to form my mentality, thinking in this way. Thanks. 

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

Ok, I tried this lightweight ringbuffer, but it seems does not do what it should. I changed the GPS module to an USB-serial-converter, to make debug easier. So, I save the incoming data in the ISR like you suggested:

ISR(USART_RX_vect)
{   
    RingBuffer_Insert(&Buffer, UART_Receive());
}

To avoid optimization, I included a volatile statement, in the ringbuffer header file: 

typedef struct
{
	RingBuff_Data_t  Buffer[BUFFER_SIZE]; /**< Internal ring buffer data, referenced by the buffer pointers. */
	RingBuff_Data_t* In; /**< Current storage location in the circular buffer */
	RingBuff_Data_t* Out; /**< Current retrieval location in the circular buffer */
	RingBuff_Count_t Count;
} volatile RingBuff_t;

The main program is this:

int main(void)
{
	uint8_t i = 0, j = 0;
    char c;

    UART_Init(9600);
    LCDInit(LS_NONE);
    
    LCDWriteStringXY(0, 0, "welcome");
    LCDClear();
	
    _delay_ms(1000); 
	sei();
	
	while(1)
	{   
        while (c != '\n')
        {
            while(RingBuffer_IsEmpty(&Buffer)); // wait until chars arrive
            {
                c = RingBuffer_Remove(&Buffer);
                LCDGotoXY(j++, 0);
                LCDByte(c, 1);
            }
        }
		j = 0;	
	}
}

So basically I'm testing the ringbuffer function. In realterm I start typing letters ("A", "B", "C", ...), but in the LCD the first 2 characters are different from the input, it is actually like this: "°". After that some characters are matches, but the joy doesn't last long, the input chars changes to "°" and "2". Can you give me some advice where can I start debugging? I double checked the UART library and LCD library, seems there is no error in there. Can the atomic blocks which the ringbuffer using compatible with ISR?

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

When you do the ringbuffer remove, how do you know if there is a char available or not?

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

From the RingBuffer_IsEmpty function. Return as defined in the header: Boolean true if the buffer contains no free space, false otherwise. In my understanding value is true if there is data in it, and false if it is empty. 

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

Why is there a call to UART_Receive() in the ISR? What is that and why isn't it as simple as a read of UDR? (You wouldn't have entered the ISR if UDR didn't have a character ready)

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

yoman91 wrote:

To avoid optimization, I included a volatile statement, in the ringbuffer header file: 

typedef struct
{
	RingBuff_Data_t  Buffer[BUFFER_SIZE]; /**< Internal ring buffer data, referenced by the buffer pointers. */
	RingBuff_Data_t* In; /**< Current storage location in the circular buffer */
	RingBuff_Data_t* Out; /**< Current retrieval location in the circular buffer */
	RingBuff_Count_t Count;
} volatile RingBuff_t;

I do not think this is the correct way to define things volatile......

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

>I do not think this is the correct way to define things volatile......

 

That qualifier can be placed either before the struct keyword or after the struct closing brace and will have the same outcome. When you start dealing with pointers that have qualifiers, then it takes more thought and you have two sides of the * with different meaning. The volatile qualifier could also be moved closer to its use- where the Buffer is defined, but it doesn't matter in this case so may as well put on the struct so its not forgotten when a buffer is declared/defined.

 

You can move the qualifier around to check it out-

https://godbolt.org/z/5BltZj

 

 

In the isr you are inserting data into the buffer when it may be full. This buffer is not designed for that, so you need to check it first, and if its full you cannot insert the rx data. I would rather the buffer take care of itself where it simply returns a bool- insert returns true if there was room, remove returns true if there was something to return (and the argument passed is by reference). Then the buffer cannot be screwed up because you forgot to check it first. There is no need to change what you have, but that buffer code is simple enough to change if you ever wanted to (move the checks into the insert/remove and change the insert return value and argument type).

 

You also have an uninitialized 'char c;' which in this case you have a chance it will be an '\n' since it is not initialized, and will then never make it to the code that uses the lcd. Its just something to watch out for, and the compiler will usually complain about it when it can be a problem so listen to its warnings.

 

One more thing, you have a while loop that is disguised to look like it has a body but it does not- I would lose the curly braces there as it is not the body of the while loop.

 

None of these are your immediate problem, but they will be in the future.

 

 

If was trying to troubleshoot this, I would send the data back out to the pc to see, not the lcd where anything less than perfect leads to what you have now. You can then also send various gps sentences one at a time from the pc, let your code do its thing, and send back out to the pc. You can then deal in 'slow motion' and use a debugger if you have one, without a flood of data coming in. Once you get things squared away by using only the pc, then move over to getting data from the gps and also again send back out to the pc. One step at a time, and eventually you will have what you want.

 

An idea similar to this for echoing sentences back to the pc-

https://godbolt.org/z/xtuppN

this is c++ example usart code I had from another thread, and just tacked on some code in main to send gps sentences back out to pc.

The incoming test data doesn't have to be actual gps type data at first, just send something from the pc that starts with '$' and ends with \r\n, then send it back to the pc. Or whatever method you have going on, just send something back to the pc so you can see what is happening. I would be happy once I could get the gps data to echo back to the pc in real time, and have it doing that reliably for a day or so (or at least a while). Then I would move on to the next step and would most likely not have to revisit the buffer code again when my new problems show up since it was already thoroughly tested.

 

Kind of a long post for not saying much. I'm not sure if any of this is useful, so use if you can, ignore if you want. 

 

 

Last Edited: Mon. Oct 14, 2019 - 07:35 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dean's RingBuffer does not need ANY modification - he's already considered issues such as volatility. 

 

Even if you were going to make it volatile then do it at the point of instantiation. So:

     RingBuffer_t Buffer;

becomes:

     volatile RingBuffer_t Buffer;

 

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

Surely this application has been done to death with Arduino?

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

Your remark is correct, I don't need an extra statement (while ( !(UCSR0A & (1<<RXC0)) )), because the interrupt flag is set only if the char is ready.

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

Thank you for the notices, I try to keep in mind the advices. 

 

The first attempt I tried to define in volatile was this way: 

volatile RingBuffer_t Buffer;

But the compilar raised some warnings that the function discards volatile variable. Ok, then I did it the way you can see it in my code. I checked the code without volatile as well, nothing changed. Clawson right in this topic, seems the ringbuffer is well optimized for interrupt handling. 

You also have an uninitialized 'char c;'...

You are right, but the compiler did not show me warnings, maybe I should set some flags to be more sensitive on warning messages.

If was trying to troubleshoot this, I would send the data back out to the pc to see,

Good point. Since I'm not using the GPS receiver anymore for debugging but the USB-serial converter, it is feasable, so I will give it a try.

I tried to debug on a cheap JTAG ICE debugger with AVR Studio 4.x. I saved the UART data to a variable, and then I passed this variable to the ringbuffer. In this case the GPS module was used, not the USB-serial converter. As I remember the behavior was more or less the same (some good characters, some fake, etc).. I have to repeat this. 

 

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

Does the mega328 have an external crystal connected, configured and operating correctly? Otherwise, this might explain the problems you re having.

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

I migrated everything to the Atmega32A debugger, removed the LCD and using simply the USB serial converter and RealTerm to debug. The code cannot be simplier:

#define F_CPU 8000000UL
#include <avr/io.h>
#include "UART.h"
#include "LightweightRingBuff.h"
RingBuff_t Buffer;

ISR(USART_RXC_vect)
{   
    RingBuffer_Insert(&Buffer, UDR);
}

int main(void)
{
    char c;
    UART_Init(9600);
    sei();
	
    while(1)
    {
        while(RingBuffer_IsEmpty(&Buffer));
        {
            c = RingBuffer_Remove(&Buffer);
            UART_Transmit(c);
        } 
    }
}

I sent the "Welcome" message multiple times, and captured the result, which is the following: "  lcomeWelcomeWelc3meWel‚meWe oůe˙  #oMfţ  e"

I changed the chrystal from 8 Mhz to 9216000Hz, but nothing changed. Checked the fuses as well, but nothing changed. 

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

Show uart_init(). How is the 9600 you pass converted into a UBRR value? It must take F_CPU into account? But is the value it uses the actual CPU frequency?
.
If you changed crystal but nothing changed then CKSEL is probably set to intRC

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

When I changed the crystal from 8 Mhz to 9.216 MHz, I changed F_CPU too. And the fuses accordingly. For the 9.216 MHz I used the following fuses (high: 0x99, low: 0xff), for 8 MHz (high: 0x99, low: 0xfd). 

 

Uart Init for Atmega32A: 

void UART_Init(uint32_t baud_rate)
{
	uint8_t ubrr = ((F_CPU / (baud_rate * 16U)) - 1U);
	
	UBRRH = (uint8_t)(ubrr>>8);
	UBRRL = (uint8_t)ubrr;
	
	UCSRB = (1<<RXEN) | (1<<TXEN) | (1<<RXCIE);
	UCSRC = (1<<URSEL) | (1<<UCSZ1) | (1<<UCSZ0);
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

are you absolutely sure the defined "ubrr" register has to be only 8 bits?????

 

I would change that stuff with " UBRR = ((F_CPU ? ........ ) -1)"  then the compiler will sort the writing order for you as there is a catch there somewhere in that you need to write one before the other.

 

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

Ok, defining ubrr as a 8 bit variable can cause difficulties, if I want to use low baud rates on high frequency. The result of the equation is 51 (8 MHz, 9600 baud rate), so I don't think this is the problem in this application. Anyway, I will correct it, thanks. 

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

I'd forget all the fancy interrupts and ring buffer and so on until you have some robust UART link in place using the very simplest synchronous reading and writing. Clearly you are suffering from #3 in my signature!

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

While I was searching the Lightweightringbuffer topic in the forum, I realized a very big mistake in my code. Initialization of the ringbuffer is missing frown Tonight I will continue the investigation. 

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK, final conclusion: The lightweight ringbuffer works perfectly as it is, no changes needed. The problem was the user, shame on me, but after I properly inited the ringbuffer, it started to work perfectly. Now I can step further.