Array in Proteus using WinAVR C size issue

Question

I have a lab assignment which needs me to use an Atmega328P to do an ADC, and with USART, transmit the digital value to a MILFORD-4X20-BKP LCD display. The LCD needs to display the value in a 2 byte format (decimal, 0-255) on the first line, and a word format (4 bytes, 0-1023) on the third line.

I was successful in doing this, but because I was unsure of the array sizes, I initially had them all big enough to not have an issue. When I changed it to what I believe was necessary, I had a weird bug. It's the weird symbol shown below (or I guess at the bottom). The symbol in that position would depend on the potentiometer value.

So here is my thinking. I allocated 36 (+1 for pos 0) positions to the buff, which was sent to the LCD. I allocated 3 to buff2 for the word value (4 n positions) and finally 4 for buff1 for the 2 bytes value (5 n positions)

buff[36]; buff1[4]; buff2[3];

3n positions for the word value works, but when I put 4n for the 2byte value, the bug appears. See the first picture.

The bug also appears in the form of a portion of the 0-255 value appearing at the end of line 3, depending on different array values of buff and buff1. The second photo has buff[37], buff1[2], buff2[3]

Last note, if I change the value to buff1[5], the bug disappears..but why? The array size for 2 bytes should be less than that for 4 bytes.

The weird bug

The LCD

I'm doing my best at explaining my issue, but don't know if I'm clear enough. I know I am having my arrays cross over into one another's memory address, but I don't see how and where.

/*
 * Serial Lcd.c
 * 
 * Use's a 4x20 serial LCD display.
 *
 * Adapted by Phil J to suit Atmega328P: 15/2/2015 (corrected Usart_Rx Int Vector address ref. for 328)
 *
 * Editted by Tomi Fodor
 *
 */ 

#define F_CPU 16000000UL
#define BAUDRATE 9600 - change to External 16MHz crystal on MCU

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include "stdlib.h"
#include "USART.h"

// Global Variables
// Note the use of the volatile keyword to ensure that the compiler knows that these variables can be changed at 
// any time, including by the ISR

volatile int i=0; 
volatile uint16_t buffer[]; // 20 place array
volatile char buff[36];     // var sent out value
volatile char buff1[4];     // var for the pot value / 4 ***** HAS TO BE AT LEAST 4 FOR SOME REASON (5 w/o bug), SHOULD BE FINE AT 2
volatile char buff2[3];     // var for the actual pot value
volatile uint16_t StrRxFlag=0;
volatile int Ana, Bell;     // pot value

int main(void)
{
   buff[4]=' ';buff[5]='P';buff[6]='o';buff[7]='t';buff[8]=' ';buff[9]='V';buff[10]='a';buff[11]='l';buff[12]='(';buff[13]='D';buff[14]=')'; // constants to be displayed
   _delay_ms(500);

   ADCSRA = (1<<ADEN)|(1<<ADPS2)|(1<<ADPS1);    // Enables the ADC, sets the ADC to use the division factor 64 for the ADC clock 
   USART_interrupt_init();
   USART_putstring("Ready ");           // Send String to the LCD
// USART_putstring(buff3);                  
   USART_send('\r');                // Send carriage return
// USART_send('\n');                // Send linefeed
   _delay_ms(500);              // Allows for the LCD module to initialize

   while(1)
      {
     USART_send(254);       // LCD control mode
     USART_send(0);         // LCD HOME command
     USART_send(254);
     USART_send(1);         // LCD CLEAR SCREEN
     buff[0] = ' ';         // Required for offset of display
     buff[4] = ' ';         // Signifies terminator of pot
     ADCSRA |= (1<<ADSC);       // Starts A-D conversion
     while (ADCSRA & (1<<ADSC));    // Wait till A-D conversion is complete
     Ana = ADCW/4;          // Get A-D result
     Bell = ADCW;           // Get actual A-D result
     itoa(Ana,buff1,10);        // Creats the dec value of the Analogue value [stdlib.h]
     itoa(Bell,buff2,10);       // actual

     if (buff1[1] == '\0')      // If only 1 digit
        {
           buff[1] = ' ';       // Not hundreds
           buff[2] = ' ';       // Not tens
           buff[3] = buff1[0];  // Place in single digit
        }
     else if(buff1[2] == '\0')  // If only 2 digits
        {
           buff[1] = ' ';       // Not hundreds
           buff[2] = buff1[0];  // Shift
           buff[3] = buff1[1];  // Shift
        }
     else
        {
           buff[1] = buff1[0];  // Shift
           buff[2] = buff1[1];  // Shift
           buff[3] = buff1[2];  // Shift
        }

     for(i=0;i<25;i++)
        {
           buff[i+15] = ' ';
        }
buff[25]=' ';buff[26]='P';buff[27]='o';buff[28]='t';buff[29]=' ';buff[31]='V';buff[32]='a';buff[33]='l';buff[34]='(';buff[35]='D';buff[36]=')'; // constants to be displayed

     if (buff2[1] == '\0')      // If only 1 digit
        {
           buff[21] = ' ';      // Not thousands
           buff[22] = ' ';      // Not hundreds
           buff[23] = ' ';      // Not tens
           buff[24] = buff2[0]; // Place in single digit
        }
     else if(buff2[2] == '\0')  // If only 2 digits
        {
           buff[21] = ' ';      // Not thousands
           buff[22] = ' ';      // Not hundreds
           buff[23] = buff2[0]; // Shift
           buff[24] = buff2[1]; // Shift
        }
     else if(buff2[3] == '\0')  // If only 3 digits
        {
           buff[21] = ' ';      // Not thousands
           buff[22] = buff2[0]; // Shift
           buff[23] = buff2[1]; // Shift
           buff[24] = buff2[2]; // Shift
        }
     else
        {
           buff[21] = buff2[0]; // Shift
           buff[22] = buff2[1]; // Shift
           buff[23] = buff2[2]; // Shift
           buff[24] = buff2[3]; // Shift
        }

          USART_putstring(buff);
          USART_send('\r'); 
         _delay_ms(500);
    }
}

//ISR(USART0_RX_vect) - not for 328
ISR(USART_RX_vect)              //this is the right vector ref, not above
{   
   buffer[i]=UDR0;              //Read USART data register
   if(buffer[i++]=='\r')            //check for carriage return terminator and increment buffer index
      { 
     // if terminator detected
     StrRxFlag=1;           //Set String received flag 
     buffer[i-1]=0x00;      //Set string terminator to 0x00
     i=0;               //Reset buffer index
      }
}

Answer 1

Your cited problem is likely due to an non-null terminated string when using calling USART_putstring(buff);. C strings by definition require the last character to be \0 ( NULL ). Example:

Given char string[5];

|h|e|r|e|\0| is legal
|h|e|r|e|s| |a| |b|u|g| is a populated buffer, but not a string

There are places in your example where you are writing a non NULL character to the last element of a buffer. For example buff is created as an array with 36 elements:

volatile char buff[36];     // var sent out value

In the line:

buff[25]=' ';buff[26]='P';buff[27]='o';buff[28]='t';buff[29]=' ';buff[31]='V';buff[32]='a';buff[33]='l';buff[34]='(';buff[35]='D';buff[36]=')'; // constants to be displayed

index 35 (the last legal index) is populated with the D character. Index 36 (not legal) is populated with ), and should result in a run-time error at the very least. By definition then, because it is not NULL terminated, it is not a string, and using it as one will lead to Undefined Behavior.

Here also, buf2 is created with 3 elements:

volatile char buff2[3];     // var for the actual pot value 

But on this line, the index 3 is used: (only 0-2 are valid)

else
        {
           buff[21] = buff2[0]; // Shift
           buff[22] = buff2[1]; // Shift
           buff[23] = buff2[2]; // Shift
           buff[24] = buff2[3]; // Shift <<< only buff2[0] - buff2[2] are legal
        }

These errors will compile, but will result in an out-of-bound pointer at run-time.

This variable has an undefined size, and should have flagged an error:

volatile uint16_t buffer[];  // 20 place array

The assumption is that you intended it to be:

volatile uint16_t buffer[20];  // 20 place array

Later, you use it here:

ISR(USART_RX_vect)              //this is the right vector ref, not above
{   
   buffer[i]=UDR0;   

Because I am not sure what C standard you are working to (i.e. if its something other than ANSI C) I do not know if your environment would flag an undefined size for an int array at compile time. But since the rest of your array sizes are defined, this one seems suspicious.

Also, I see that you have i defined as:

volatile int i=0;

Are the bounds of i well known? Is it possible for i to ever go beyond the value 19? (assuming the array was at some point initialized)