New to Arduino and programming

I recently succeeded in communicating with the ATMEL 328P using USART. However, because of my lack of programming experience, I have no idea how to take the commands I am sending it from the PC, and make them form a useful string. Also the general format of the data I am sending it is very inconsistent when the Arduino relays it back to the PC. It can change substantially by changing the code in the smallest way.

I'm very inexperienced but enrolled in an embedded systems class, with the only programming experience I have being a semester of Matlab, 2.5 years ago. We are not to use the included Arduino library functions, instead we have to make our own. So if my main program is right, maybe it's in one of my functions.

In this screenshot I sent the command "PC0=0" and the data comes back very inconsistent, I also included some of the code that I'm sure is not right:

For some reason, that I can't figure out, it keeps exiting the loop when there should still be received bytes to be read. This is causing it to keep repeating "Enter Input" over and over. I don't know what's wrong.

Like I mentioned earlier, changing the code even slightly, makes the data come back entirely different.
Here I changed the index in a way so it would not repeat the code. I then sent the command PC2=1

Main Program Code:

int main()
{
	init();
	 
	set_BAUDRATE(9600);  //9600 or 115200
	InitUsart0Module();
	enable_TransmitBuffer();
	enable_ReceiveBuffer();
	 
	EnableGlobalInterrupts();
	TIMER2_init();
	 
	PORTC_DDR = ( BIT0 | BIT1 | BIT2 );
	PORTC_DATA = ~PORTC_DATA;	//Turn all on
	 
	PORTB_DDR =  ( BIT0 | BIT1 );
	 
	index = 0;
	 
	printhis( "...Initialized\n");
	 
	while(1)
	{
		char dataByte;
		uint8_t count = 0;
		
		if ( index == 0 )
		{
			printhis( "\nEnter Input: ");
			index = 1;
		}
		
		if ( index == 2 )
		{	
			while (( ReceivedBytes() != 0) && (count < 6) )
			{
				dataByte = GetNextReceivedByte();
				The_Command[count] = dataByte;	
				count++;
			}
			
			PORTB_DATA = ~(BIT0);
			index = 3;
		}
		
		if ( index == 3 )
		{
			The_Command[6] = 0;	
			snprintf((char *)message, SERIAL_MESSAGE,"\nENTERED:  %s\n", The_Command);
			mySerialWrite(message);
			index = 0;
		}	
	}
	return 0;
}

The commands, after I figure out how to make the arduino interpret them properly, will be used to toggle LED's on Port C pins 0, 1, and 2 from the PC. Port B LED's are just there for debugging purposes, because many times I'm not sure if a function was even being called like it was supposed to.

My Interrupt service routines for this program:

//USART RECEIVE INTERRUPT ON DATA RECEIVED
ISR(USART_RX_vect)
{
	PORTB_DATA |= BIT0; 
	index = 2;
	
	if (( UCSR0A_REG & UCSR0A_RX_MASK ) != UCSR0A_RX_EMPTY )
	{
		g_receiveBuffer[g_receiveHead++] = UDR0_REG; 
		
		if ( g_receiveHead > USART_RECEIVE_BUFFER_SIZE )
		{
			g_receiveHead = 0;	
		}
	}
}	

//USART TRANSMIT INTERRUPT ON DATA REGISTER EMPTY
ISR(USART_UDRE_vect)
{
	// The data register is empty so we need to load it with the next ASCII character
	// to transmit or disable interrupts
	if (g_transmitTail == g_transmitHead)
	{
		DisableUsartTxInterrupt();
	}
	else
	{
		// load register, initiate transfer
		UDR0_REG = g_transmitBuffer[g_transmitTail];
		 
		// increment and wrap tail
		g_transmitTail++;
		 
		if (g_transmitTail == USART_BUFFER_SIZE)
		{
			g_transmitTail = 0;
		}
	}
}

//TIMER/COUNTER2 INTERRUPT ON COMPARE MATCH A
// for my custom delay function
ISR(TIMER2_COMPA_vect, ISR_NOBLOCK)
{
	g_msecCount = g_msecCount + 1;
}

My custom USART Receive Functions:

//Get next Byte Received
uint8_t GetNextReceivedByte(void)
{
	uint8_t receivedByte = 0;

	if ( ReceivedBytes() > 0 )
	{
		receivedByte = g_receiveBuffer[g_receiveTail++];
		 
		if ( g_receiveTail >= USART_RECEIVE_BUFFER_SIZE )
		{
			g_receiveTail = 0;
		}
	}
	return receivedByte;
}

//Check for bytes waiting to be read
uint8_t ReceivedBytes (void)
{
	uint8_t receiveHead;
	uint8_t receivedBytes = 0;
	 
	DisableGlobalInterrupts();
	receiveHead = g_receiveHead;
	EnableGlobalInterrupts();

	if ( receiveHead > g_receiveTail )
	{
		receivedBytes = receiveHead - g_receiveTail;
	}
	if ( receiveHead < g_receiveTail )
	{
		// Check to see if the head has bit the tail
		receivedBytes = receiveHead + ( USART_RECEIVE_BUFFER_SIZE - g_receiveTail );
	}

	return receivedBytes;
}

Of course, there's many more functions I didn't post because I'm pretty sure they are right and this post would get really long.

Hi, my first impression, in terms of coding style, is that redefining known register names and bit values is bad practice. I already spent some time and effort learning and remembering what UCSR0A is, and redefining it as UCSR0A_REG may perhaps help you remember it's a register but only confuses me (I didn't find the definition anyway, so I had to guess it was nothing new). It will probably confuse you as well once you get used to the "official" names.

Anyway, the main problem is that at 9600 baud a char takes slightly more than 1msec to arrive, and since the processor is much faster in reading them, after 1 or 2 while() cycles ReceivedBytes() will return 0, even if other characters are about to arrive. The solution is to loop (under index==2) until you receive a terminating char (e.g., a newline), or a predefined number of chars.

Yea I wanted to use official registers, but unfortunately the teacher makes us redefine them all since they won't always be defined for us.

I had a suspicion what you said was happening, but was unsure of a solution. What you said helps a lot!

I'm unsure how to go about detecting a terminating char or predefined number of characters. My attempt at predefining the number of characters was in the conditions for the while loop, to put ( count < 6 ), but that didn't work. I'll have to look deeper into those tomorrow, it's getting pretty late.

The misunderstanding that is often seen with serial is that when you "send" a string of serial all the data arrives at once, in one big chunk. That doesn't happen - it arrives one letter at a time. Your program may see the first letter, or the first couple of letters, depending on how its timing coincides with the sending timing.

The normal way of handling serial is to keep reading until you find a specific "terminating" character. This is usually character number 13 ("\r" in C parlance) which is the Return key on your keyboard.

You would normally read one character and, if it isn't character 13, add it to an array. Only when you find character 13 do you then act on the contents of the array.

Wait for character available
Read character
While character != 13
  If array not full
    Add character to array
    Increment array pointer
  End If
  Wait for character available
  Read character
End While
Act on array contents (maybe using sscanf, or strcmp etc)

Thank You!! It works now. But I hit another snag.

I'm not sure how to reset the string, as you can see it still has the characters that were not rewritten. But that's nothing to really worry about because all the commands are exactly 6 characters long.

My BIG problem is acting on the array contents. I can't do strcmp, I'm not sure how else to compare them. I also can't pass the command into the COMMANDER function. But if I could just figure out how to get it to do strcmp, I'd eliminate the function and just put it in the loop.

Make your arrays char instead of uint8_t. uint8_t is basically "unsigned char". Either than or manually cast all your array to (char *) when you call the functions. I'd go with the former, as it's only one change :wink:

And make sure your arrays are properly null terminated. I tend to write a null after each incoming character as they arrive:

g_command[count] = receivedByte;
count++
g_command[count] = 0;

That results in, as the characters arrive:

H\0
He\0
Hel\0
Hell\0
Hello\0

and strcmp etc will work properly.

I couldn't have done it with out both your help, thank you!

:slight_smile:
(and it also looks nice)