From c50d8db2e7982505a415894792f92eb1f31cc6f5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felix=20D=C3=B6rre?= <felix@dogcraft.de>
Date: Sat, 14 Oct 2023 10:51:13 +0000
Subject: [PATCH] Dispatch activity display into the main loop

Before this change, picowota would ignore data on the tcp connection if it is
sent immediately after the connection was opened.

The reason why that happens happens:
- Client connects
- tcp_comm_client_init is called
- the gpio led turns on (tcp_comm.c#L461)
- While communicating with the cyw43 to enable the led, network packages are
  discovered that need to be processed, incl. the initial opcode.
- the recv-callback is not set yet, so the callback invocation is skipped
- tcp_comm_client_init continues to set up callbacks, the initial data is lost.

To resolve this, the code that turns the LED on and off is dispatched into the
mainloop, so it is never called from within a networking callback.
---
 main.c     | 21 ++++++++++++++++++++-
 tcp_comm.c | 16 +++++++++++-----
 tcp_comm.h |  2 +-
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/main.c b/main.c
index 80b892e..89f93ee 100644
--- a/main.c
+++ b/main.c
@@ -69,6 +69,7 @@ enum event_type {
 	EVENT_TYPE_REBOOT = 1,
 	EVENT_TYPE_GO,
 	EVENT_TYPE_SERVER_DONE,
+	EVENT_TYPE_ACTIVITY
 };
 
 struct event {
@@ -80,6 +81,9 @@ struct event {
 		struct {
 			uint32_t vtor;
 		} go;
+		struct {
+			bool active;
+		} activity;
 	};
 };
 
@@ -562,6 +566,18 @@ static bool should_stay_in_bootloader()
 	return !gpio_get(BOOTLOADER_ENTRY_PIN) || wd_says_so;
 }
 
+static void handle_activity(bool active)
+{
+	struct event ev = {
+		.type = EVENT_TYPE_ACTIVITY,
+		.activity = {
+			.active = active,
+		},
+	};
+
+	queue_try_add(&event_queue, &ev);
+}
+
 static void network_deinit()
 {
 #if PICOWOTA_WIFI_AP == 1
@@ -636,7 +652,7 @@ int main()
 		&reboot_cmd,
 	};
 
-	struct tcp_comm_ctx *tcp = tcp_comm_new(cmds, sizeof(cmds) / sizeof(cmds[0]), CMD_SYNC);
+	struct tcp_comm_ctx *tcp = tcp_comm_new(cmds, sizeof(cmds) / sizeof(cmds[0]), CMD_SYNC, handle_activity);
 
 	struct event ev = {
 		.type = EVENT_TYPE_SERVER_DONE,
@@ -659,6 +675,9 @@ int main()
 				picowota_reboot(ev.reboot.to_bootloader);
 				/* Should never get here */
 				break;
+			case EVENT_TYPE_ACTIVITY:
+				cyw43_arch_gpio_put (0, ev.activity.active);
+				break;
 			case EVENT_TYPE_GO:
 				tcp_comm_server_close(tcp);
 				network_deinit();
diff --git a/tcp_comm.c b/tcp_comm.c
index ea754d1..badf996 100644
--- a/tcp_comm.c
+++ b/tcp_comm.c
@@ -60,6 +60,7 @@ struct tcp_comm_ctx {
 	const struct comm_command *const *cmds;
 	unsigned int n_cmds;
 	uint32_t sync_opcode;
+	void (*activity_func)(bool);
 };
 
 #define COMM_BUF_OPCODE(_buf)       ((uint32_t *)((uint8_t *)(_buf)))
@@ -267,7 +268,7 @@ static err_t tcp_comm_client_close(struct tcp_comm_ctx *ctx)
 {
 	err_t err = ERR_OK;
 
-	cyw43_arch_gpio_put (0, false);
+	ctx->activity_func(false);
 	ctx->conn_state = CONN_STATE_CLOSED;
 
 	if (!ctx->client_pcb) {
@@ -450,17 +451,21 @@ static void tcp_comm_client_err(void *arg, err_t err)
 	ctx->client_pcb = NULL;
 	ctx->conn_state = CONN_STATE_CLOSED;
 	ctx->rx_bytes_needed = 0;
-	cyw43_arch_gpio_put (0, false);
+	ctx->activity_func(false);
 }
 
 static void tcp_comm_client_init(struct tcp_comm_ctx *ctx, struct tcp_pcb *pcb)
 {
 	ctx->client_pcb = pcb;
+	ctx->rx_bytes_received = 0;
+	ctx->rx_bytes_needed = 0;
+	ctx->rx_start_offs = 0;
+	ctx->tx_bytes_sent = 0;
+	ctx->tx_bytes_remaining = 0;
 	tcp_arg(pcb, ctx);
 
-	cyw43_arch_gpio_put (0, true);
-
 	tcp_comm_sync_begin(ctx);
+	ctx->activity_func(true);
 
 	tcp_sent(pcb, tcp_comm_client_sent);
 	tcp_recv(pcb, tcp_comm_client_recv);
@@ -522,7 +527,7 @@ err_t tcp_comm_listen(struct tcp_comm_ctx *ctx, uint16_t port)
 }
 
 struct tcp_comm_ctx *tcp_comm_new(const struct comm_command *const *cmds,
-		unsigned int n_cmds, uint32_t sync_opcode)
+				  unsigned int n_cmds, uint32_t sync_opcode, void (*activity_func)(bool) )
 {
 	struct tcp_comm_ctx *ctx = calloc(1, sizeof(struct tcp_comm_ctx));
 	if (!ctx) {
@@ -538,6 +543,7 @@ struct tcp_comm_ctx *tcp_comm_new(const struct comm_command *const *cmds,
 	ctx->cmds = cmds;
 	ctx->n_cmds = n_cmds;
 	ctx->sync_opcode = sync_opcode;
+	ctx->activity_func = activity_func;
 
 	return ctx;
 }
diff --git a/tcp_comm.h b/tcp_comm.h
index e6783ff..133bad2 100644
--- a/tcp_comm.h
+++ b/tcp_comm.h
@@ -29,7 +29,7 @@ err_t tcp_comm_server_close(struct tcp_comm_ctx *ctx);
 bool tcp_comm_server_done(struct tcp_comm_ctx *ctx);
 
 struct tcp_comm_ctx *tcp_comm_new(const struct comm_command *const *cmds,
-		unsigned int n_cmds, uint32_t sync_opcode);
+				  unsigned int n_cmds, uint32_t sync_opcode, void (*activity_func)(bool));
 void tcp_comm_delete(struct tcp_comm_ctx *ctx);
 
 #endif /* __TCP_COMM_H__ */