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.
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
}
}
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)