Check status return after sending a command to ESP module

2 files changed, 84 insertions, 22 deletions

diff --git a/src/Networking/ESP8266WiFi/WifiFirmwareUploader.cpp b/src/Networking/ESP8266WiFi/WifiFirmwareUploader.cpp
index b7849109..b0ce3b00 100644
--- a/src/Networking/ESP8266WiFi/WifiFirmwareUploader.cpp
+++ b/src/Networking/ESP8266WiFi/WifiFirmwareUploader.cpp
@@ -47,8 +47,18 @@ const uint8_t ESP_CHECKSUM_MAGIC = 0xef;
 // Messages corresponding to result codes, should make sense when followed by " error"
 const char * const resultMessages[] =
 {
-	"no",
-	"timeout",
+	"no",											// 0
+	"unknown", "unknown", "unknown", "unknown",		// 0x01 to 0x04
+	"invalidMessage",								// 0x05
+	"failedToAct",									// 0x06
+	"invalidCrc",									// 0x07
+	"flashWriteError",								// 0x08
+	"flashReadError",								// 0x09
+	"unknown",										// 0x0a
+	"deflateError",									// 0x0b
+	"unknown", "unknown", "unknown", "unknown",		// 0x0c to 0x0f
+
+	"timeout",										// 0x10
 	"comm write",
 	"connect",
 	"bad reply",
@@ -190,7 +200,7 @@ inline void WifiFirmwareUploader::WriteByteSlip(uint8_t b) noexcept
 //
 // If an error occurs, return a negative value.  Otherwise, return the number
 // of bytes in the response (or zero if the response was not the standard "two bytes of zero").
-WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::readPacket(uint8_t op, uint32_t *valp, size_t& bodyLen, uint32_t msTimeout) noexcept
+WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::readPacket(uint8_t op, uint32_t *valp, size_t& bodyLen, uint32_t *status, uint32_t msTimeout) noexcept
 {
 	enum class PacketState
 	{
@@ -208,7 +218,7 @@ WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::readPacket(uint8_t o
 	uint16_t hdrIdx = 0;
 	bodyLen = 0;
 	uint16_t bodyIdx = 0;
-	uint8_t respBuf[2];
+	alignas(4) uint8_t respBuf[4];
 
 	// wait for the response
 	uint16_t needBytes = 1;
@@ -322,6 +332,11 @@ WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::readPacket(uint8_t o
 		return EspUploadResult::respHeader;
 	}
 
+	if (status != nullptr)
+	{
+		*status = (bodyLen == 2) ? *(const uint16_t*)respBuf : *(const uint32_t*)respBuf;
+	}
+
 	return EspUploadResult::success;
 }
 
@@ -383,22 +398,57 @@ void WifiFirmwareUploader::sendCommand(uint8_t op, uint32_t checkVal, const uint
 	}
 }
 
+// Calculate the number of bytes to erase
+/*static*/ uint32_t WifiFirmwareUploader::getEraseSize(uint32_t offset, uint32_t size) noexcept
+{
+#if WIFI_USES_ESP32
+	return size;
+#else
+	// This works around a bug in the ESP8266 ROM
+    const uint32_t sectors_per_block = 16;
+    const uint32_t sector_size = 4 * 1024;
+    const uint32_t num_sectors = (size + sector_size - 1) / sector_size;
+    const uint32_t start_sector = offset / sector_size;
+
+    uint32_t head_sectors = sectors_per_block - (start_sector % sectors_per_block);
+    if (num_sectors < head_sectors)
+    {
+        head_sectors = num_sectors;
+    }
+
+    return (num_sectors < 2 * head_sectors)
+    		? (num_sectors + 1) / 2 * sector_size
+    			: (num_sectors - head_sectors) * sector_size;
+#endif
+}
+
 // Send a command to the attached device together with the supplied data, if any, and get the response
 WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::doCommand(uint8_t op, const uint8_t *data, size_t dataLen, uint32_t checkVal, uint32_t *valp, uint32_t msTimeout) noexcept
 {
 	sendCommand(op, checkVal, data, dataLen);
 	size_t bodyLen;
-	EspUploadResult stat = readPacket(op, valp, bodyLen, msTimeout);
-	if (stat == EspUploadResult::success && bodyLen != 2)
+	uint32_t status;
+	EspUploadResult stat = readPacket(op, valp, bodyLen, &status, msTimeout);
+	if (stat == EspUploadResult::success)
 	{
-		stat = EspUploadResult::badReply;
+		if (!(bodyLen == 2 || bodyLen == 4))
+		{
+			stat = EspUploadResult::badReply;
+		}
+		else if ((status & 0xFF) != 0)
+		{
+			stat = (EspUploadResult)((status >> 8) & 0xFF);
+			if (reprap.Debug(moduleWiFi))
+			{
+				debugPrintf("opcode %u returned length %u status 0x%" PRIx32 "\n", op, bodyLen, status);
+			}
+		}
 	}
 
 	return stat;
 }
 
-// Send a synchronising packet to the serial port in an attempt to induce
-// the ESP8266 to auto-baud lock on the baud rate.
+// Send a synchronising packet to the serial port in an attempt to induce the ESP8266 to auto-baud lock on the baud rate.
 WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::Sync(uint16_t timeout) noexcept
 {
 	uint8_t buf[36];
@@ -416,7 +466,7 @@ WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::Sync(uint16_t timeou
 	for (int i = 0; i < 10 && stat == EspUploadResult::respHeader; ++i)
 	{
 		size_t bodyLen;
-		stat = readPacket(ESP_SYNC, nullptr, bodyLen, timeout);
+		stat = readPacket(ESP_SYNC, nullptr, bodyLen, nullptr, timeout);
 	}
 
 	if (stat == EspUploadResult::success)
@@ -425,8 +475,8 @@ WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::Sync(uint16_t timeou
 		for (;;)
 		{
 			size_t bodyLen;
-			EspUploadResult rc = readPacket(ESP_SYNC, nullptr, bodyLen, defaultTimeout);
-			if (rc != EspUploadResult::success || bodyLen != 2)
+			EspUploadResult rc = readPacket(ESP_SYNC, nullptr, bodyLen, nullptr, defaultTimeout);
+			if (rc != EspUploadResult::success || !(bodyLen == 2 || bodyLen == 4))
 			{
 				break;
 			}
@@ -471,7 +521,7 @@ WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::flashSpiSetParameter
 WifiFirmwareUploader::EspUploadResult WifiFirmwareUploader::flashSpiAttach() noexcept
 {
 	const uint32_t buf[2] = { 0, 0 };
-	return doCommand(ESP_SPI_SET_PARAMS, (const uint8_t*)buf, sizeof(buf), 0, nullptr, defaultTimeout);
+	return doCommand(ESP_SPI_ATTACH, (const uint8_t*)buf, sizeof(buf), 0, nullptr, defaultTimeout);
 }
 
 #endif
@@ -661,7 +711,7 @@ void WifiFirmwareUploader::Spin() noexcept
 	case UploadState::erasing2:
 		if (millis() - lastAttemptTime >= blockWriteInterval)
 		{
-			uploadResult = DoErase(uploadAddress, fileSize);
+			uploadResult = DoErase(uploadAddress, getEraseSize(uploadAddress, fileSize));
 			if (uploadResult == EspUploadResult::success)
 			{
 				MessageF("Uploading file...\n");
diff --git a/src/Networking/ESP8266WiFi/WifiFirmwareUploader.h b/src/Networking/ESP8266WiFi/WifiFirmwareUploader.h
index b574315d..e1332e92 100644
--- a/src/Networking/ESP8266WiFi/WifiFirmwareUploader.h
+++ b/src/Networking/ESP8266WiFi/WifiFirmwareUploader.h
@@ -26,24 +26,35 @@ public:
 	static const uint32_t WebFilesAddress = 0x00100000;
 
 private:
-	static const uint32_t defaultTimeout = 500;				// default timeout in milliseconds
+	static const uint32_t defaultTimeout = 500;					// default timeout in milliseconds
 	static const uint32_t syncTimeout = 1000;
 	static const unsigned int retriesPerBaudRate = 9;
 	static const unsigned int retriesPerReset = 3;
 	static const uint32_t connectAttemptInterval = 50;
 	static const uint32_t resetDelay = 500;
-	static const uint32_t blockWriteInterval = 15;			// 15ms is long enough, 10ms is mostly too short
+	static const uint32_t blockWriteInterval = 15;				// 15ms is long enough, 10ms is mostly too short
 	static const uint32_t blockWriteTimeout = 200;
-	static const uint32_t eraseTimeout = 15000;				// increased from 12 to 15 seconds because Roland's board was timing out
-	static const unsigned int percentToReportIncrement = 5;	// how often we report % complete
+	static const uint32_t eraseTimeout = 15000;					// increased from 12 to 15 seconds because Roland's board was timing out
+	static const unsigned int percentToReportIncrement = 5;		// how often we report % complete
 	static const uint32_t systemParametersAddress = 0x3FE000;	// the address of the system + user parameter area that needs to be cleared when changing SDK version
 	static const uint32_t systemParametersSize = 0x2000;		// the size of the system + user parameter area
 
-	// Return codes - this list must be kept in step with the corresponding messages
-	enum class EspUploadResult
+	// Return codes
+	// *** This list must be kept in step with the corresponding messages! ***
+	enum class EspUploadResult : uint8_t
 	{
 		success = 0,
-		timeout,
+
+		// The following are status codes return by the ESP
+		invalidMessage = 0x05,
+		failedToAct = 0x06,
+		invalidCrc = 0x07,
+		flashWriteError = 0x08,
+		flashReadError = 0x09,
+		deflateError = 0x0b,
+
+		// The remainder are our own status codes
+		timeout = 0x10,
 		connect,
 		badReply,
 		fileRead,
@@ -70,12 +81,13 @@ private:
 	void WriteByteRaw(uint8_t b) noexcept;
 	void WriteByteSlip(uint8_t b) noexcept;
 	void flushInput() noexcept;
-	EspUploadResult readPacket(uint8_t op, uint32_t *valp, size_t& bodyLen, uint32_t msTimeout) noexcept;
+	EspUploadResult readPacket(uint8_t op, uint32_t *valp, size_t& bodyLen, uint32_t *status, uint32_t msTimeout) noexcept;
 	void writePacket(const uint8_t *data, size_t len) noexcept;
 	void writePacketRaw(const uint8_t *buf, size_t len) noexcept;
 	void writePacket(const uint8_t *hdr, size_t hdrLen, const uint8_t *data, size_t dataLen) noexcept;
 	void writePacketRaw(const uint8_t *hdr, size_t hdrLen, const uint8_t *data, size_t dataLen) noexcept;
 	void sendCommand(uint8_t op, uint32_t checkVal, const uint8_t *data, size_t dataLen) noexcept;
+	static uint32_t getEraseSize(uint32_t offset, uint32_t size) noexcept;
 	EspUploadResult doCommand(uint8_t op, const uint8_t *data, size_t dataLen, uint32_t checkVal, uint32_t *valp, uint32_t msTimeout) noexcept;
 	EspUploadResult Sync(uint16_t timeout) noexcept;
 	EspUploadResult flashBegin(uint32_t addr, uint32_t size) noexcept;