Tried to do some better commenting in usb_serial.c, also tried to do a better job of documenting the protocol in programmer_protocol.h

main.c

@@ -19,6 +19,7 @@ int main(void)
 	DDRD |= (1 << 7);
 	PORTD &= ~(1 << 7);
 
+	// If there was a brownout detected, turn on the LED momentarily
 	if (MCUSR & (1 << BORF))
 	{
 		MCUSR = 0;
@@ -39,42 +40,5 @@ int main(void)
 		USBSerial_Check();
 	}
 
-	/*int result = SIMMElectricalTest_Run();
-
-	if (result == 0)
-	{
-		while (1)
-		{
-			_delay_ms(200);
-			PIND = (1 << 7);
-		}
-	}
-	else
-	{
-		while (1)
-		{
-			int tmpResult = result;
-
-			while (tmpResult)
-			{
-				int thisDigit = (tmpResult % 10) + 1;
-
-				while (thisDigit--)
-				{
-					PIND = (1 << 7);
-					_delay_ms(500);
-					PIND = (1 << 7);
-					_delay_ms(500);
-				}
-
-				_delay_ms(2000);
-
-				tmpResult /= 10;
-			}
-
-			_delay_ms(5000);
-		}
-	}*/
-
 	return 0;
 }

programmer_protocol.h

@@ -115,4 +115,23 @@ typedef enum BootloaderStateReply
 	BootloaderStateInProgrammer
 } BootloaderStateReply;
 
+// -------------------------  BOOTLOADER ERASE/WRITE PROTOCOL  -------------------------
+// If the command is BootloaderEraseAndWriteProgram, it will reply with CommandReplyOK
+// followed by either BootloaderEraseOK or BootloaderEraseError. At this point
+// the program can ask to write more data or finish or cancel, and then the appropriate
+// reply will be sent back to it. Works very similar to the write protocol.
+typedef enum ProgrammerBootloaderEraseWriteReply
+{
+	BootloaderWriteOK,
+	BootloaderWriteError,
+	BootloaderWriteConfirmCancel
+} ProgrammerBootloaderEraseWriteReply;
+
+typedef enum ComputerBootloaderEraseWriteRequest
+{
+	ComputerBootloaderWriteMore = 0,
+	ComputerBootloaderFinish,
+	ComputerBootloaderCancel
+} ComputerBootloaderEraseWriteRequest;
+
 #endif /* PROGRAMMER_PROTOCOL_H_ */

usb_serial/usb_serial.c

@@ -27,6 +27,7 @@ void USBSerial_Init(void)
 	USB_Init();
 }
 
+// Internal state so we know how to interpret the next-received byte
 typedef enum ProgrammerCommandState
 {
 	WaitingForCommand = 0,
@@ -35,107 +36,37 @@ typedef enum ProgrammerCommandState
 	ReadingChipsUnableSendError,
 	WritingChips
 } ProgrammerCommandState;
-
 static ProgrammerCommandState curCommandState = WaitingForCommand;
+
+// State info for reading/writing
 static uint8_t byteAddressReceiveCount = 0;
 static uint16_t curReadIndex;
 static int16_t writePosInChunk = -1;
 static uint16_t curWriteIndex = 0;
 
+// Private functions
 void USBSerial_HandleWaitingForCommandByte(uint8_t byte);
 void USBSerial_HandleReadingChipsByte(uint8_t byte);
 void USBSerial_SendReadDataChunk(void);
 void USBSerial_HandleWritingChipsByte(uint8_t byte);
 void USBSerial_ElectricalTest_Fail_Handler(uint8_t index1, uint8_t index2);
 
+// Read/write to USB serial macros -- easier than retyping
+// CDC_Device_XXX(&VirtualSerial_CDC_Interface...) every time
 #define SendByte(b) CDC_Device_SendByte(&VirtualSerial_CDC_Interface, b)
 #define ReadByte() CDC_Device_ReceiveByte(&VirtualSerial_CDC_Interface)
 #define SendData(d, l) CDC_Device_SendData(&VirtualSerial_CDC_Interface, d, l)
 
+// Should be called periodically in the main loop
 void USBSerial_Check(void)
 {
-	/*if (USB_DeviceState == DEVICE_STATE_Configured)
+	// If we're configured, read a byte (if one is available) and process it
-	{
-		if (CDC_Device_BytesReceived(&VirtualSerial_CDC_Interface))
-		{
-			uint8_t rb = (uint8_t)CDC_Device_ReceiveByte(&VirtualSerial_CDC_Interface);
-
-			if (rb == 'i')
-			{
-				struct ChipID chips[4];
-				ExternalMem_IdentifyChips(chips);
-				char tmp[20];
-				uint32_t data = ExternalMem_ReadCycle(0);
-
-				int x;
-				for (x = 0; x < 4; x++)
-				{
-					sprintf(tmp, "IC%d: M%02X, D%02X\r\n", x+1, chips[x].manufacturerID, chips[x].deviceID);
-					CDC_Device_SendString(&VirtualSerial_CDC_Interface, tmp);
-				}
-
-				sprintf(tmp, "%08lX\r\n", data);
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, tmp);
-			}
-			else if (rb == 'e')
-			{
-				ExternalMem_EraseChips(ALL_CHIPS);
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, "Erased\r\n");
-			}
-			else if (rb == 'r')
-			{
-				uint32_t result = ExternalMem_ReadCycle(0);
-				char tmp[20];
-				sprintf(tmp, "%08lX\r\n", result);
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, tmp);
-			}
-			else if (rb == 'w')
-			{
-				uint32_t address = 0;
-				uint32_t x;
-				uint32_t y;
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, "Writing...\r\n");
-				CDC_Device_Flush(&VirtualSerial_CDC_Interface);
-				for (y = 0; y < 512UL*1024UL / (READ_CHUNK_SIZE_BYTES/4); y++)
-				{
-					for (x = 0; x < READ_CHUNK_SIZE_BYTES/4; x++)
-					{
-						ExternalMem_WriteByteToChips(address++, 0x12345678, ALL_CHIPS);
-					}
-				}
-				//ExternalMem_WriteByteToChips(0, 0x12345678UL, ALL_CHIPS);
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, "Wrote\r\n");
-			}
-			else if (rb == 't')
-			{
-				int result = SIMMElectricalTest_Run();
-
-				char tmp[20];
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, "SIMM electrical test complete: ");
-				sprintf(tmp, "%d errors\r\n", result);
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, tmp);
-			}
-			else if (rb == 'a')
-			{
-				uint32_t x;
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, "Reading...\r\n");
-				CDC_Device_Flush(&VirtualSerial_CDC_Interface);
-
-				for (x = 0; x < 512UL*1024UL; x++)
-				{
-					ExternalMem_ReadCycle(x);
-				}
-
-				CDC_Device_SendString(&VirtualSerial_CDC_Interface, "Finished\r\n");
-			}
-		}
-	}*/
-
 	if (USB_DeviceState == DEVICE_STATE_Configured)
 	{
-		// Check for commands, etc...
 		int16_t recvByte = ReadByte();
 
+		// Did we get a byte? If so, hand it off to the correct handler
+		// function based on the current state
 		if (recvByte >= 0)
 		{
 			switch (curCommandState)
@@ -153,24 +84,30 @@ void USBSerial_Check(void)
 		}
 	}
 
+	// And do the periodic CDC and USB tasks...
 	CDC_Device_USBTask(&VirtualSerial_CDC_Interface);
 	USB_USBTask();
 }
 
+// If we're in the "waiting for command" state, handle the command...
 void USBSerial_HandleWaitingForCommandByte(uint8_t byte)
 {
 	switch (byte)
 	{
+	// Asked to enter waiting mode -- we're already there, so say OK.
 	case EnterWaitingMode:
 		SendByte(CommandReplyOK);
 		curCommandState = WaitingForCommand;
 		break;
+	// Asked to do the electrical test. Reply OK, and then do the test,
+	// sending whatever replies necessary
 	case DoElectricalTest:
 		SendByte(CommandReplyOK);
 		SIMMElectricalTest_Run(USBSerial_ElectricalTest_Fail_Handler);
 		SendByte(ProgrammerElectricalTestDone);
 		curCommandState = WaitingForCommand;
 		break;
+	// Asked to identify the chips in the SIMM. Identify them and send reply.
 	case IdentifyChips:
 	{
 		struct ChipID chips[4];
@@ -185,91 +122,120 @@ void USBSerial_HandleWaitingForCommandByte(uint8_t byte)
 		SendByte(ProgrammerIdentifyDone);
 		break;
 	}
+	// Asked to read a single byte from each SIMM. Change the state and reply.
 	case ReadByte:
 		curCommandState = ReadingByteWaitingForAddress;
 		byteAddressReceiveCount = 0;
 		SendByte(CommandReplyOK);
 		break;
+	// Asked to read all four chips. Set the state, reply with the first chunk
 	case ReadChips:
 		curCommandState = ReadingChips;
 		curReadIndex = 0;
 		SendByte(CommandReplyOK);
 		USBSerial_SendReadDataChunk();
 		break;
+	// Erase the chips and reply OK. (TODO: Sometimes erase might fail)
 	case EraseChips:
 		ExternalMem_EraseChips(ALL_CHIPS);
 		SendByte(CommandReplyOK);
 		break;
+	// Begin writing the chips. Change the state, reply, wait for chunk of data
 	case WriteChips:
 		curCommandState = WritingChips;
 		curWriteIndex = 0;
 		writePosInChunk = -1;
 		SendByte(CommandReplyOK);
 		break;
+	// Asked for the current bootloader state. We are in the program right now,
+	// so reply accordingly.
 	case GetBootloaderState:
 		SendByte(CommandReplyOK);
 		SendByte(BootloaderStateInProgrammer);
 		break;
+	// Enter the bootloader. Wait a bit, then jump to the bootloader location.
 	case EnterBootloader:
 		SendByte(CommandReplyOK);
+
+		// Done with the USB interface -- the bootloader will re-initialize it.
 		USB_Disable();
 
-		// Disable interrupts...
+		// Disable interrupts so nothing weird happens...
 		cli();
 
-		// Wait a little bit to let everything settle and let the program close the port after the USB disconnect
+		// Wait a little bit to let everything settle and let the program
+		// close the port after the USB disconnect
 		_delay_ms(2000);
 
-		// Now run the bootloader
+		// And, of course, go into the bootloader.
 		__asm__ __volatile__ ( "jmp 0xE000" );
 		break;
+	// Enter the programmer. We're already there, so reply OK.
 	case EnterProgrammer:
 		// Already in the programmer
 		SendByte(CommandReplyOK);
 		break;
+	// We don't know what this command is, so reply that it was invalid.
 	default:
 		SendByte(CommandReplyInvalid);
 		break;
 	}
 }
 
+// If we're in the "reading chips" state, handle the incoming byte...
 void USBSerial_HandleReadingChipsByte(uint8_t byte)
 {
+	// The byte should be a reply from the computer. It should be either:
+	// 1) ComputerReadOK -- meaning it got the chunk we just sent
+	// or
+	// 2) ComputerReadCancel -- meaning the user canceled the read
 	switch (byte)
 	{
 	case ComputerReadOK:
+		// If they have confirmed the final data chunk, let them know
+		// that they have finished, and enter command state.
 		if (curReadIndex >= (CHIP_SIZE / (READ_CHUNK_SIZE_BYTES/4)))
 		{
 			SendByte(ProgrammerReadFinished);
 			curCommandState = WaitingForCommand;
 		}
-		else
+		else // There's more data left to read, so read it and send it to them!
 		{
 			SendByte(ProgrammerReadMoreData);
 			USBSerial_SendReadDataChunk();
 		}
 		break;
 	case ComputerReadCancel:
+		// If they've canceled, let them know we got their request and go back
+		// to "waiting for command" state
 		SendByte(ProgrammerReadConfirmCancel);
 		curCommandState = WaitingForCommand;
 		break;
 	}
 }
 
+// Read the next chunk of data from the SIMM and send it off over the serial.
 void USBSerial_SendReadDataChunk(void)
 {
-	// TODO: How do I send an error back to the device?
+	// Here's a buffer we will use to read the next chunk of data.
-	// Maybe the device, when it tries to request the next data chunk,
+	// It's static because the stack is NOT big enough for it. If I start
-	// will get an ERROR response instead of an "OK" response?
+	// running low on RAM, I could pull this out of the function and share it
-
+	// with other functions, but I'm not bothering with that for now.
 	static union
 	{
 		uint32_t readChunks[READ_CHUNK_SIZE_BYTES / 4];
 		uint8_t readChunkBytes[READ_CHUNK_SIZE_BYTES];
 	} chunks;
 
-	ExternalMem_Read(curReadIndex * (READ_CHUNK_SIZE_BYTES/4), chunks.readChunks, READ_CHUNK_SIZE_BYTES/4);
+	// Read the next chunk of data, send it over USB, and make sure
-	uint8_t retVal = SendData((const char *)chunks.readChunkBytes, READ_CHUNK_SIZE_BYTES);
+	// we sent it correctly.
+	ExternalMem_Read(curReadIndex * (READ_CHUNK_SIZE_BYTES/4),
+			chunks.readChunks, READ_CHUNK_SIZE_BYTES/4);
+	uint8_t retVal = SendData((const char *)chunks.readChunkBytes,
+			READ_CHUNK_SIZE_BYTES);
+
+	// If for some reason there was an error, mark it as such. Otherwise,
+	// increment our pointer so we know the next chunk of data to send.
 	if (retVal != ENDPOINT_RWSTREAM_NoError)
 	{
 		curCommandState = ReadingChipsUnableSendError;
@@ -280,20 +246,30 @@ void USBSerial_SendReadDataChunk(void)
 	}
 }
 
+// Handles a received byte from the computer while we're in the "writing chips"
+// mode.
 void USBSerial_HandleWritingChipsByte(uint8_t byte)
 {
+	// A buffer we use to store the incoming data. This, too, could be shared
+	// with other functions if I end up running out of RAM. Again, I'm not
+	// bothering with that yet, but this could easily be shared with the
+	// read function.
 	static union
 	{
 		uint32_t writeChunks[WRITE_CHUNK_SIZE_BYTES / 4];
 		uint8_t writeChunkBytes[WRITE_CHUNK_SIZE_BYTES];
 	} chunks;
 
+	// This means we have just started the entire process or just finished
+	// a chunk, so see what the computer has decided for us to do.
 	if (writePosInChunk == -1)
 	{
 		switch (byte)
 		{
+		// The computer asked to write more data to the SIMM.
 		case ComputerWriteMore:
 			writePosInChunk = 0;
+			// Make sure we don't write past the capacity of the chips.
 			if (curWriteIndex < CHIP_SIZE / (WRITE_CHUNK_SIZE_BYTES/4))
 			{
 				SendByte(ProgrammerWriteOK);
@@ -301,26 +277,31 @@ void USBSerial_HandleWritingChipsByte(uint8_t byte)
 			else
 			{
 				SendByte(ProgrammerWriteError);
+				// TODO: Enter waiting for command mode?
 			}
 			break;
+		// The computer said that it's done writing.
 		case ComputerWriteFinish:
-			// Just to confirm that we finished writing...
 			SendByte(ProgrammerWriteOK);
 			curCommandState = WaitingForCommand;
 			break;
+		// The computer asked to cancel.
 		case ComputerWriteCancel:
 			SendByte(ProgrammerWriteConfirmCancel);
 			curCommandState = WaitingForCommand;
 			break;
 		}
 	}
-	else
+	else // Interpret the incoming byte as data to write to the SIMM.
 	{
+		// Save the byte, and check if we've filled up an entire chunk
 		chunks.writeChunkBytes[writePosInChunk++] = byte;
 		if (writePosInChunk >= WRITE_CHUNK_SIZE_BYTES)
 		{
-			// Write this data
+			// We filled up the chunk, write it out and confirm it, then wait
-			ExternalMem_Write(curWriteIndex * (WRITE_CHUNK_SIZE_BYTES/4), chunks.writeChunks, WRITE_CHUNK_SIZE_BYTES/4, ALL_CHIPS);
+			// for the next command from the computer!
+			ExternalMem_Write(curWriteIndex * (WRITE_CHUNK_SIZE_BYTES/4),
+					chunks.writeChunks, WRITE_CHUNK_SIZE_BYTES/4, ALL_CHIPS);
 			SendByte(ProgrammerWriteOK);
 			curWriteIndex++;
 			writePosInChunk = -1;
@@ -328,18 +309,17 @@ void USBSerial_HandleWritingChipsByte(uint8_t byte)
 	}
 }
 
+// Whenever an electrical test failure occurs, this handler will be called
+// by it. It sends out a failure notice followed by indexes of the two
+// shorted pins.
 void USBSerial_ElectricalTest_Fail_Handler(uint8_t index1, uint8_t index2)
 {
-	// Sends out a failure notice -- followed by indexes of the two shorted pins
 	SendByte(ProgrammerElectricalTestFail);
 	SendByte(index1);
 	SendByte(index2);
 }
 
-
+// LUFA event handler for when the USB configuration changes.
-
-
-/** Event handler for the library USB Configuration Changed event. */
 void EVENT_USB_Device_ConfigurationChanged(void)
 {
 	bool ConfigSuccess = true;
@@ -347,7 +327,7 @@ void EVENT_USB_Device_ConfigurationChanged(void)
 	ConfigSuccess &= CDC_Device_ConfigureEndpoints(&VirtualSerial_CDC_Interface);
 }
 
-/** Event handler for the library USB Control Request reception event. */
+// LUFA event handler for when a USB control request is received
 void EVENT_USB_Device_ControlRequest(void)
 {
 	CDC_Device_ProcessControlRequest(&VirtualSerial_CDC_Interface);