From b4aaddb816e58f57ae65a4db3c5239da1ae38017 Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Windows)" <rob@synchro.net>
Date: Wed, 30 Aug 2023 18:00:02 -0700
Subject: [PATCH] Remove MQTT message publishing from mqtt_connect_callback()

A follow-up to commit 81d4575e

Although I was not able to successfully reproduce the problem that Ree
reported with his commit (even when changing the SCFG->Networks->MQTT->Publish
QOS to 1: At least once) on Windows, I do see how this problem could
theoretically happen. And like Ree said in the follow-up comment on the MR
"maybe these two lines should have stayed in mqtt_startup", they don't really
belong in the connection callback.

The "client" topics only needs to be cleared upon startup or recycle (by
publishing a null message) and it would be bad to clear these topics whenever
the broker was reconnected (the server's clients didn't magically disconnect).
So these "client" topic-clearing publishes are now only done during startup
(again).

The "recycle" topics don't really need to be published to here at all. I
think I only did this for cases where someone published a non-null message to
the topic and its stale message would remain afterward, appearing in MQTT
browsers (like MQTT explorer) long after the server had recycled. The real
solution to this cosmetic issue is to only publish null (0-length) messages to
the "recycle" topics in the first place.
---
 src/sbbs3/mqtt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/sbbs3/mqtt.c b/src/sbbs3/mqtt.c
index d6d9f42d55..c4604b51b0 100644
--- a/src/sbbs3/mqtt.c
+++ b/src/sbbs3/mqtt.c
@@ -482,8 +482,6 @@ static void mqtt_connect_callback(struct mosquitto* mosq, void* cbdata, int rc)
 			mqtt_subscribe(mqtt, TOPIC_BBS, str, sizeof(str), "exec");
 			mqtt_subscribe(mqtt, TOPIC_BBS, str, sizeof(str), "call");
 		}
-		mqtt_pub_noval(mqtt, TOPIC_SERVER, "recycle");
-		mqtt_pub_noval(mqtt, TOPIC_SERVER, "client");
 		mqtt_subscribe(mqtt, TOPIC_SERVER, str, sizeof(str), "recycle");
 		mqtt_subscribe(mqtt, TOPIC_HOST, str, sizeof(str), "recycle");
 	}
@@ -611,6 +609,7 @@ int mqtt_startup(struct mqtt* mqtt, scfg_t* cfg, struct startup* startup, const
 				result = mqtt_connect(mqtt, /* bind_address: */NULL);
 				if(result == MQTT_SUCCESS) {
 					lprintf(lputs, LOG_INFO, "MQTT broker-connect (%s:%d) successful", cfg->mqtt.broker_addr, cfg->mqtt.broker_port);
+					mqtt_pub_noval(mqtt, TOPIC_SERVER, "client");
 				} else {
 					lprintf(lputs, LOG_ERR, "MQTT broker-connect (%s:%d) failure: %d", cfg->mqtt.broker_addr, cfg->mqtt.broker_port, result);
 					mqtt_close(mqtt);
-- 
GitLab