Fix out SRAM compile time issue

Was able to coax enough SRAM free by rewriting the functions: ``` void logmsg_buf(const unsigned char *buf, unsigned long size); void logmsg_f(const char *format, ...); ``` used in `lib/SCSI2SD/src/firmware/network.c` from `src/ZuluSCSI_log.h`. They now use a shared buffer. There is possibly more SRAM savings if the above methods wrote directly to the global log buffer. Their debug equivalents have also been added ``` void dbgmsg_buf(const unsigned char *buf, unsigned long size); void dbgmsg_f(const char *format, ...); ``` And C macros have been added to test `g_debug_log` before calling the equivalent functions to avoid making unnecessary function calls in `lib/SCSI2SD/src/firmware/log.h`: ``` DBGMSG_BUF() DBGMSG_F() ```
ZuluSCSI · Oct 23, 2023 · 30df3aa · 30df3aa
1 parent 1f9a0c6
commit 30df3aa
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 57 deletions.
diff --git a/lib/SCSI2SD/src/firmware/log.h b/lib/SCSI2SD/src/firmware/log.h
@@ -14,14 +14,32 @@
 //
 //	You should have received a copy of the GNU General Public License
 //	along with SCSI2SD.  If not, see <http://www.gnu.org/licenses/>.
+#ifndef SCSI2SD_LOG_H
+#define SCSI2SD_LOG_H
+#include <stdbool.h>
+
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+extern bool g_log_debug;
+
 extern void logmsg_buf(const unsigned char *buf, unsigned long size);
 extern void logmsg_f(const char *format, ...);
 
+extern void dbgmsg_buf(const unsigned char *buf, unsigned long size);
+extern void dbgmsg_f(const char *format, ...);
+
+// check if debug is enabled before calling the logging function
+#define DBGMSG_BUF(buf, size) \
+	if (g_log_debug) {dbgmsg_buf(buf, size);}
+
+#define DBGMSG_F(format, ...) \
+	if (g_log_debug) {dbgmsg_f(format, __VA_ARGS__);}
+
 #ifdef __cplusplus
 }
-#endif
+#endif
+
+#endif // SCSI2SD_LOG_H
diff --git a/lib/SCSI2SD/src/firmware/network.c b/lib/SCSI2SD/src/firmware/network.c
@@ -22,8 +22,6 @@
 #include "config.h"
 #include "network.h"
 
-extern bool g_log_debug;
-
 static bool scsiNetworkEnabled = false;
 
 static uint8_t scsiNetworkPacketsInbound[NETWORK_PACKET_QUEUE_SIZE][NETWORK_PACKET_MAX_SIZE];
@@ -103,10 +101,7 @@ int scsiNetworkCommand()
 	uint8_t command = scsiDev.cdb[0];
 	uint8_t cont = (scsiDev.cdb[5] == 0x80);
 
-	if (g_log_debug)
-	{
-		logmsg_f("in scsiNetworkCommand with command 0x%02x (size %d)", command, size);
-	}
+	DBGMSG_F("------ in scsiNetworkCommand with command 0x%02x (size %d)", command, size);
 
 	switch (command) {
 	case 0x08:
@@ -139,10 +134,7 @@ int scsiNetworkCommand()
 				psize = size - 6;
 			}
 
-			if (g_log_debug)
-			{
-				logmsg_f("%s: sending packet[%d] to host of size %zu + 6", __func__, scsiNetworkPacketInboundReadIndex, psize);
-			}
+			DBGMSG_F("%s: sending packet[%d] to host of size %zu + 6", __func__, scsiNetworkPacketInboundReadIndex, psize);
 
 			scsiDev.dataLen = psize + 6; // 2-byte length + 4-byte flag + packet
 			memcpy(scsiDev.data + 6, scsiNetworkPacketsInbound[scsiNetworkPacketInboundReadIndex], psize);
@@ -161,10 +153,7 @@ int scsiNetworkCommand()
 			// more data to read?
 			scsiDev.data[5] = (scsiNetworkPacketInboundReadIndex == scsiNetworkPacketInboundWriteIndex ? 0 : 0x10);
 
-			if (g_log_debug)
-			{
-				logmsg_buf(scsiDev.data, scsiDev.dataLen);
-			}
+			DBGMSG_BUF(scsiDev.data, scsiDev.dataLen);
 		}
 
 		// DaynaPort driver needs a delay between reading the initial packet size and the data so manually do two transfers
@@ -216,11 +205,15 @@ int scsiNetworkCommand()
 		parityError = 0;
 		scsiRead(scsiDev.data, size, &parityError);
 
-		if (g_log_debug)
+		if (parityError)
 		{
-			logmsg_f("%s: read packet from host of size %zu - %d (parity error %d)", __func__, size, (cont ? 4 : 0), parityError);
-			logmsg_buf(scsiDev.data, size);
+			DBGMSG_F("------ %s: read packet from host of size %zu - %d (parity error raised: %d)", __func__, size, (cont ? 4 : 0), parityError);
 		}
+		else
+		{
+			DBGMSG_F("------ %s: read packet from host of size %zu - %d", __func__, size, (cont ? 4 : 0));
+		}
+		DBGMSG_BUF(scsiDev.data, size);
 
 		if (cont)
 		{
@@ -252,11 +245,9 @@ int scsiNetworkCommand()
 		parityError = 0;
 		scsiRead(scsiDev.data, size, &parityError);
 
-		if (g_log_debug)
-		{
-			logmsg_f("%s: adding multicast address %02x:%02x:%02x:%02x:%02x:%02x", __func__,
+		DBGMSG_F("%s: adding multicast address %02x:%02x:%02x:%02x:%02x:%02x", __func__,
 			  scsiDev.data[0], scsiDev.data[1], scsiDev.data[2], scsiDev.data[3], scsiDev.data[4], scsiDev.data[5]);
-		}
+
 
 		platform_network_add_multicast_address(scsiDev.data);
 
@@ -268,10 +259,8 @@ int scsiNetworkCommand()
 		// toggle interface
 		if (scsiDev.cdb[5] & 0x80)
 		{
-			if (g_log_debug)
-			{
-				logmsg_f("%s: enable interface", __func__);
-			}
+
+			DBGMSG_F("%s: enable interface", __func__);
 			scsiNetworkEnabled = true;
 			scsiNetworkPacketInboundWriteIndex = 0;
 			scsiNetworkPacketInboundReadIndex = 0;
@@ -280,10 +269,7 @@ int scsiNetworkCommand()
 		}
 		else
 		{
-			if (g_log_debug)
-			{
-				logmsg_f("%s: disable interface", __func__);
-			}
+			DBGMSG_F("%s: disable interface", __func__);
 			scsiNetworkEnabled = false;
 		}
 		break;
@@ -304,10 +290,7 @@ int scsiNetworkCommand()
 
 	// custom wifi commands all using the same opcode, with a sub-command in cdb[2]
 	case SCSI_NETWORK_WIFI_CMD:
-		if (g_log_debug)
-		{
-			logmsg_f("in scsiNetworkCommand with wi-fi command 0x%02x (size %d)", scsiDev.cdb[2], size);
-		}
+		DBGMSG_F("------ in scsiNetworkCommand with wi-fi command 0x%02x (size %d)", scsiDev.cdb[2], size);
 
 		switch (scsiDev.cdb[2]) {
 		case SCSI_NETWORK_WIFI_CMD_SCAN:
@@ -405,11 +388,8 @@ int scsiNetworkCommand()
 			parityError = 0;
 			scsiRead((uint8_t *)&req, sizeof(req), &parityError);
 
-			if (g_log_debug)
-			{
-				logmsg_f("%s: read join request from host:", __func__);
-				logmsg_buf(scsiDev.data, size);
-			}
+			DBGMSG_F("%s: read join request from host:", __func__);
+			DBGMSG_BUF(scsiDev.data, size);
 
 			platform_network_wifi_join(req.ssid, req.key);
 
@@ -433,8 +413,7 @@ int scsiNetworkEnqueue(const uint8_t *buf, size_t len)
 
 	if (len + 4 > sizeof(scsiNetworkPacketsInbound[0]))
 	{
-		if (g_log_debug)
-			logmsg_f("%s: dropping incoming network packet, too large (%zu > %zu)", __func__, len, sizeof(scsiNetworkPacketsInbound[0]));
+		DBGMSG_F("%s: dropping incoming network packet, too large (%zu > %zu)", __func__, len, sizeof(scsiNetworkPacketsInbound[0]));
 		return 0;
 	}
 
@@ -454,8 +433,7 @@ int scsiNetworkEnqueue(const uint8_t *buf, size_t len)
 
 	if (scsiNetworkPacketInboundWriteIndex == scsiNetworkPacketInboundReadIndex)
 	{
-		if (g_log_debug)
-			logmsg_f("%s: dropping packets in ring, write index caught up to read index", __func__);
+		DBGMSG_F("%s: dropping packets in ring, write index caught up to read index", __func__);
 	}
 
 	return 1;

diff --git a/platformio.ini b/platformio.ini
@@ -166,7 +166,7 @@ build_flags =
     -DZULUSCSI_DAYNAPORT
 ; These take a large portion of the SRAM and can be adjusted
     -DLOGBUFSIZE=8192
-    -DPREFETCH_BUFFER_SIZE=7168
+    -DPREFETCH_BUFFER_SIZE=8192
     -DSCSI2SD_BUFFER_SIZE=57344
     ; This controls the depth of 2 x NETWORK_PACKET_MAX_SIZE (1520 bytes)
     ; For example a queue size of 10 would be 10 x 2 x 1520 = 30400 bytes

diff --git a/src/ZuluSCSI_log.cpp b/src/ZuluSCSI_log.cpp
@@ -28,6 +28,7 @@
 #include <stdarg.h>
 
 const char *g_log_firmwareversion = ZULU_FW_VERSION " " __DATE__ " " __TIME__;
+
 bool g_log_debug = true;
 
 // This memory buffer can be read by debugger and is also saved to zululog.txt
@@ -198,35 +199,73 @@ const char *log_get_buffer(uint32_t *startpos, uint32_t *available)
     return result;
 }
 
-void logmsg_f(const char *format, ...)
+// TODO write directly global log buffer to save some memory
+static char shared_log_buf[1500 * 3];
+
+// core method for variadic printf like logging
+static void log_va(bool debug, const char *format, va_list ap)
 {
-    static char out[2048];
+    vsnprintf(shared_log_buf, sizeof(shared_log_buf), format, ap);
+    if (debug)
+    {
+        dbgmsg(shared_log_buf);
+    }
+    else
+    {
+        logmsg(shared_log_buf);
+    }
+}
 
+void logmsg_f(const char *format, ...)
+{
     va_list ap;
     va_start(ap, format);
-    vsnprintf(out, sizeof(out), format, ap);
+    log_va(false, format, ap);
     va_end(ap);
+}
 
-    logmsg(out);
+void dbgmsg_f(const char *format, ...)
+{
+    if (!g_log_debug)
+        return;
+    va_list ap;
+    va_start(ap, format);
+    log_va(true, format, ap);
+    va_end(ap);
 }
 
-void logmsg_buf(const unsigned char *buf, unsigned long size)
+// core method for logging a data buffer into a hex string
+void log_hex_buf(const unsigned char *buf, unsigned long size, bool debug)
 {
-    static char tmp[1500 * 3];
     static char hex[] = "0123456789abcdef";
     int o = 0;
 
     for (int j = 0; j < size; j++) {
-        if (o + 3 >= sizeof(tmp))
+        if (o + 3 >= sizeof(shared_log_buf))
             break;
 
         if (j != 0)
-            tmp[o++] = ' ';
-        tmp[o++] = hex[(buf[j] >> 4) & 0xf];
-        tmp[o++] = hex[buf[j] & 0xf];
-        tmp[o] = 0;
+            shared_log_buf[o++] = ' ';
+        shared_log_buf[o++] = hex[(buf[j] >> 4) & 0xf];
+        shared_log_buf[o++] = hex[buf[j] & 0xf];
+        shared_log_buf[o] = 0;
     }
+    if (debug)
+        dbgmsg(shared_log_buf);
+    else
+        logmsg(shared_log_buf);
+}
 
-    logmsg_f("%s", tmp);
+void logmsg_buf(const unsigned char *buf, unsigned long size)
+{
+    log_hex_buf(buf, size, false);
+}
+
+void dbgmsg_buf(const unsigned char *buf, unsigned long size)
+{
+    if (!g_log_debug)
+        return;
+    log_hex_buf(buf, size, true);
 }
 
+
diff --git a/src/ZuluSCSI_log.h b/src/ZuluSCSI_log.h
@@ -36,7 +36,7 @@ uint32_t log_get_buffer_len();
 const char *log_get_buffer(uint32_t *startpos, uint32_t *available = nullptr);
 
 // Whether to enable debug messages
-extern bool g_log_debug;
+extern "C" bool g_log_debug;
 
 // Firmware version string
 extern const char *g_log_firmwareversion;
@@ -107,8 +107,17 @@ extern "C" {
 #endif
 // Log long hex string
 void logmsg_buf(const unsigned char *buf, unsigned long size);
+
+// Log long hex string
+void dbgmsg_buf(const unsigned char *buf, unsigned long size);
+
 // Log formatted string
 void logmsg_f(const char *format, ...);
+
+// Log formatted string
+void dbgmsg_f(const char *format, ...);
+
+
 #ifdef __cplusplus
 }
 #endif