Use single topic for MQTT client status (LWT, birth, etc.) (#435)

* Update test for new MQTT client status approach

* Refactor About helper to support outputing JSON

* Use single topic for client status

* Web UI: Use single topic for client status

* Add platformio autogenerated file

* increase MQTT buffer from 200 -> 250 to make room for larger LWT messages

.gitignore

@@ -2,6 +2,7 @@
 .piolibdeps
 .clang_complete
 .gcc-flags.json
+.sconsign.dblite
 /web/node_modules
 /web/build
 /dist/*.bin

lib/MQTT/MqttClient.cpp

@@ -6,7 +6,11 @@
 #include <ArduinoJson.h>
 #include <WiFiClient.h>
 #include <MiLightRadioConfig.h>
-#include <AboutStringHelper.h>
+#include <AboutHelper.h>
+
+static const char* STATUS_CONNECTED = "connected";
+static const char* STATUS_DISCONNECTED = "disconnected_clean";
+static const char* STATUS_LWT_DISCONNECTED = "disconnected_unclean";
 
 MqttClient::MqttClient(Settings& settings, MiLightClient*& milightClient)
   : milightClient(milightClient),
@@ -21,6 +25,8 @@ MqttClient::MqttClient(Settings& settings, MiLightClient*& milightClient)
 }
 
 MqttClient::~MqttClient() {
+  String aboutStr = generateConnectionStatusMessage(STATUS_DISCONNECTED);
+  mqttClient->publish(settings.mqttClientStatusTopic.c_str(), aboutStr.c_str(), true);
   mqttClient->disconnect();
   delete this->domain;
 }
@@ -52,39 +58,39 @@ bool MqttClient::connect() {
     Serial.println(F("MqttClient - connecting"));
 #endif
 
-  if (settings.mqttUsername.length() > 0 && settings.mqttLwtTopic.length() > 0) {
+  if (settings.mqttUsername.length() > 0 && settings.mqttClientStatusTopic.length() > 0) {
     return mqttClient->connect(
       nameBuffer,
       settings.mqttUsername.c_str(),
       settings.mqttPassword.c_str(),
-      settings.mqttLwtTopic.c_str(),
+      settings.mqttClientStatusTopic.c_str(),
       2,
       true,
-      settings.mqttLwtMessage.c_str()
+      generateConnectionStatusMessage(STATUS_LWT_DISCONNECTED).c_str()
     );
   } else if (settings.mqttUsername.length() > 0) {
     return mqttClient->connect(
       nameBuffer,
       settings.mqttUsername.c_str(),
       settings.mqttPassword.c_str()
     );
-  } else if (settings.mqttLwtTopic.length() > 0) {
+  } else if (settings.mqttClientStatusTopic.length() > 0) {
     return mqttClient->connect(
       nameBuffer,
-      settings.mqttLwtTopic.c_str(),
+      settings.mqttClientStatusTopic.c_str(),
       2,
       true,
-      settings.mqttLwtMessage.c_str()
+      generateConnectionStatusMessage(STATUS_LWT_DISCONNECTED).c_str()
     );
   } else {
     return mqttClient->connect(nameBuffer);
   }
 }
 
 void MqttClient::sendBirthMessage() {
-  if (settings.mqttBirthTopic.length() > 0) {
-    String aboutStr = AboutStringHelper::generateAboutString(true);
-    mqttClient->publish(settings.mqttBirthTopic.c_str(), aboutStr.c_str());
+  if (settings.mqttClientStatusTopic.length() > 0) {
+    String aboutStr = generateConnectionStatusMessage(STATUS_CONNECTED);
+    mqttClient->publish(settings.mqttClientStatusTopic.c_str(), aboutStr.c_str(), true);
   }
 }
 
@@ -229,3 +235,17 @@ inline void MqttClient::bindTopicString(
   topicPattern.replace(":group_id", String(groupId));
   topicPattern.replace(":device_type", remoteConfig.name);
 }
+
+String MqttClient::generateConnectionStatusMessage(const char* connectionStatus) {
+  DynamicJsonBuffer buffer;
+  JsonObject& status = buffer.createObject();
+  status["status"] = connectionStatus;
+
+  // Fill other fields
+  AboutHelper::generateAboutObject(status, true);
+
+  String response;
+  status.printTo(response);
+
+  return response;
+}

lib/MQTT/MqttClient.h

@@ -49,6 +49,8 @@ class MqttClient {
     const uint16_t deviceId,
     const uint16_t groupId
   );
+
+  static String generateConnectionStatusMessage(const char* status);
 };
 
 #endif

lib/Settings/AboutHelper.cpp

@@ -0,0 +1,29 @@
+#include <AboutHelper.h>
+#include <ArduinoJson.h>
+#include <Settings.h>
+#include <ESP8266WiFi.h>
+
+String AboutHelper::generateAboutString(bool abbreviated) {
+  DynamicJsonBuffer buffer;
+  JsonObject& response = buffer.createObject();
+
+  generateAboutObject(response, abbreviated);
+
+  String body;
+  response.printTo(body);
+
+  return body;
+}
+
+void AboutHelper::generateAboutObject(JsonObject& obj, bool abbreviated) {
+  obj["firmware"] = QUOTE(FIRMWARE_NAME);
+  obj["version"] = QUOTE(MILIGHT_HUB_VERSION);
+  obj["ip_address"] = WiFi.localIP().toString();
+  obj["reset_reason"] = ESP.getResetReason();
+
+  if (! abbreviated) {
+    obj["variant"] = QUOTE(FIRMWARE_VARIANT);
+    obj["free_heap"] = ESP.getFreeHeap();
+    obj["arduino_version"] = ESP.getCoreVersion();
+  }
+}

lib/Settings/AboutStringHelper.h → lib/Settings/AboutHelper.h

@@ -1,11 +1,13 @@
 #include <Arduino.h>
+#include <ArduinoJson.h>
 
 #ifndef _ABOUT_STRING_HELPER_H
 #define _ABOUT_STRING_HELPER_H
 
-class AboutStringHelper {
+class AboutHelper {
 public:
   static String generateAboutString(bool abbreviated = false);
+  static void generateAboutObject(JsonObject& obj, bool abbreviated = false);
 };
 
 #endif

lib/Settings/Settings.cpp

@@ -98,9 +98,7 @@ void Settings::patch(JsonObject& parsedSettings) {
     this->setIfPresent(parsedSettings, "mqtt_topic_pattern", mqttTopicPattern);
     this->setIfPresent(parsedSettings, "mqtt_update_topic_pattern", mqttUpdateTopicPattern);
     this->setIfPresent(parsedSettings, "mqtt_state_topic_pattern", mqttStateTopicPattern);
-    this->setIfPresent(parsedSettings, "mqtt_lwt_topic", mqttLwtTopic);
-    this->setIfPresent(parsedSettings, "mqtt_lwt_message", mqttLwtMessage);
-    this->setIfPresent(parsedSettings, "mqtt_birth_topic", mqttBirthTopic);
+    this->setIfPresent(parsedSettings, "mqtt_client_status_topic", mqttClientStatusTopic);
     this->setIfPresent(parsedSettings, "discovery_port", discoveryPort);
     this->setIfPresent(parsedSettings, "listen_repeats", listenRepeats);
     this->setIfPresent(parsedSettings, "state_flush_interval", stateFlushInterval);
@@ -216,9 +214,7 @@ void Settings::serialize(Stream& stream, const bool prettyPrint) {
   root["mqtt_topic_pattern"] = this->mqttTopicPattern;
   root["mqtt_update_topic_pattern"] = this->mqttUpdateTopicPattern;
   root["mqtt_state_topic_pattern"] = this->mqttStateTopicPattern;
-  root["mqtt_lwt_topic"] = this->mqttLwtTopic;
-  root["mqtt_lwt_message"] = this->mqttLwtMessage;
-  root["mqtt_birth_topic"] = this->mqttBirthTopic;
+  root["mqtt_client_status_topic"] = this->mqttClientStatusTopic;
   root["discovery_port"] = this->discoveryPort;
   root["listen_repeats"] = this->listenRepeats;
   root["state_flush_interval"] = this->stateFlushInterval;

lib/Settings/Settings.h

@@ -167,9 +167,7 @@ class Settings {
   String mqttTopicPattern;
   String mqttUpdateTopicPattern;
   String mqttStateTopicPattern;
-  String mqttLwtTopic;
-  String mqttLwtMessage;
-  String mqttBirthTopic;
+  String mqttClientStatusTopic;
   size_t stateFlushInterval;
   size_t mqttStateRateLimit;
   size_t packetRepeatThrottleThreshold;

lib/WebServer/MiLightHttpServer.cpp

@@ -6,7 +6,7 @@
 #include <MiLightRadioConfig.h>
 #include <string.h>
 #include <TokenIterator.h>
-#include <AboutStringHelper.h>
+#include <AboutHelper.h>
 #include <index.html.gz.h>
 
 void MiLightHttpServer::begin() {
@@ -120,19 +120,7 @@ void MiLightHttpServer::onGroupDeleted(GroupDeletedHandler handler) {
 }
 
 void MiLightHttpServer::handleAbout() {
-  // DynamicJsonBuffer buffer;
-  // JsonObject& response = buffer.createObject();
-
-  // response["version"] = QUOTE(MILIGHT_HUB_VERSION);
-  // response["variant"] = QUOTE(FIRMWARE_VARIANT);
-  // response["free_heap"] = ESP.getFreeHeap();
-  // response["arduino_version"] = ESP.getCoreVersion();
-  // response["reset_reason"] = ESP.getResetReason();
-
-  // String body;
-  // response.printTo(body);
-
-  server.send(200, APPLICATION_JSON, AboutStringHelper::generateAboutString());
+  server.send(200, APPLICATION_JSON, AboutHelper::generateAboutString());
 }
 
 void MiLightHttpServer::handleGetRadioConfigs() {

platformio.ini

@@ -24,7 +24,7 @@ lib_deps_external =
   CircularBuffer@~1.2.0
 extra_scripts =
   pre:.build_web.py
-build_flags = !python .get_version.py -DMQTT_MAX_PACKET_SIZE=200 -DHTTP_UPLOAD_BUFLEN=128 -D FIRMWARE_NAME=milight-hub -Idist -Ilib/DataStructures
+build_flags = !python .get_version.py -DMQTT_MAX_PACKET_SIZE=250 -DHTTP_UPLOAD_BUFLEN=128 -D FIRMWARE_NAME=milight-hub -Idist -Ilib/DataStructures
 # -D STATE_DEBUG
 # -D DEBUG_PRINTF
 # -D MQTT_DEBUG

test/remote/spec/mqtt_spec.rb

@@ -50,28 +50,35 @@
     end
   end
 
-  context 'birth and LWT' do
+  context 'client status topic' do
     # Unfortunately, no way to easily simulate an unclean disconnect, so only test birth
-    it 'should send birth message when configured' do
-      birth_topic = "#{@topic_prefix}birth"
+    it 'should send client status messages when configured' do
+      status_topic = "#{@topic_prefix}client_status"
 
       @client.put(
         '/settings',
-        mqtt_birth_topic: birth_topic
+        mqtt_client_status_topic: status_topic
       )
 
-      seen_birth = false
+      # Clear any retained messages
+      @mqtt_client.publish(status_topic, nil)
 
-      @mqtt_client.on_message(birth_topic) do |topic, message|
-        seen_birth = true
+      seen_statuses = Set.new
+      required_statuses = %w(connected disconnected_clean)
+
+      @mqtt_client.on_message(status_topic, 20) do |topic, message|
+        message = JSON.parse(message)
+
+        seen_statuses << message['status']
+        required_statuses.all? { |x| seen_statuses.include?(x) }
       end
 
       # Force MQTT reconnect by updating settings
       @client.put('/settings', fakekey: 'fakevalue')
 
       @mqtt_client.wait_for_listeners
 
-      expect(seen_birth).to be(true)
+      expect(seen_statuses).to include(*required_statuses)
     end
   end
 
@@ -128,9 +135,6 @@
     end
 
     it 'should respect the state update interval' do
-      # Wait for MQTT to reconnect
-      @mqtt_client.on_message("#{@topic_prefix}birth") { |x, y| true }
-
       # Disable updates to prevent the negative effects of spamming commands
       @client.put(
         '/settings',
@@ -139,8 +143,6 @@
         packet_repeats: 1
       )
 
-      @mqtt_client.wait_for_listeners
-
       # Set initial state
       @client.patch_state({status: 'ON', level: 0}, @id_params)