Merge pull request #463 from sidoh/simple_mqtt_client_status

Add support for simple MQTT client status messages

README.md

@@ -292,6 +292,8 @@ To receive updates when the MQTT client connects or disconnects from the broker,
 {"status":"disconnected_unclean","firmware":"milight-hub","version":"1.9.0-rc3","ip_address":"192.168.1.111","reset_reason":"External System"}
 ```
 
+If you wish to have the simple messages `connected` and `disconnected` instead of the above environmental data, configure `simple_mqtt_client_status` to `true` (or set Client Status Message Mode to "Simple" in the Web UI).
+
 ## UDP Gateways
 
 You can add an arbitrary number of UDP gateways through the REST API or through the web UI. Each gateway server listens on a port and responds to the standard set of commands supported by the Milight protocol. This should allow you to use one of these with standard Milight integrations (SmartThings, Home Assistant, OpenHAB, etc.).

lib/MQTT/MqttClient.cpp

@@ -237,14 +237,23 @@ inline void MqttClient::bindTopicString(
 }
 
 String MqttClient::generateConnectionStatusMessage(const char* connectionStatus) {
-  StaticJsonDocument<1024> json;
-  json["status"] = connectionStatus;
+  if (settings.simpleMqttClientStatus) {
+    // Don't expand disconnect type for simple status
+    if (0 == strcmp(connectionStatus, STATUS_CONNECTED)) {
+      return connectionStatus;
+    } else {
+      return "disconnected";
+    }
+  } else {
+    StaticJsonDocument<1024> json;
+    json["status"] = connectionStatus;
 
-  // Fill other fields
-  AboutHelper::generateAboutObject(json, true);
+    // Fill other fields
+    AboutHelper::generateAboutObject(json, true);
 
-  String response;
-  serializeJson(json, response);
+    String response;
+    serializeJson(json, response);
 
-  return response;
+    return response;
+  }
 }

lib/MQTT/MqttClient.h

@@ -50,7 +50,7 @@ private:
     const uint16_t groupId
   );
 
-  static String generateConnectionStatusMessage(const char* status);
+  String generateConnectionStatusMessage(const char* status);
 };
 
 #endif

lib/Settings/Settings.cpp

@@ -83,6 +83,7 @@ void Settings::patch(JsonObject parsedSettings) {
   this->setIfPresent(parsedSettings, "mqtt_update_topic_pattern", mqttUpdateTopicPattern);
   this->setIfPresent(parsedSettings, "mqtt_state_topic_pattern", mqttStateTopicPattern);
   this->setIfPresent(parsedSettings, "mqtt_client_status_topic", mqttClientStatusTopic);
+  this->setIfPresent(parsedSettings, "simple_mqtt_client_status", simpleMqttClientStatus);
   this->setIfPresent(parsedSettings, "discovery_port", discoveryPort);
   this->setIfPresent(parsedSettings, "listen_repeats", listenRepeats);
   this->setIfPresent(parsedSettings, "state_flush_interval", stateFlushInterval);
@@ -205,6 +206,7 @@ void Settings::serialize(Print& stream, const bool prettyPrint) {
   root["mqtt_update_topic_pattern"] = this->mqttUpdateTopicPattern;
   root["mqtt_state_topic_pattern"] = this->mqttStateTopicPattern;
   root["mqtt_client_status_topic"] = this->mqttClientStatusTopic;
+  root["simple_mqtt_client_status"] = this->simpleMqttClientStatus;
   root["discovery_port"] = this->discoveryPort;
   root["listen_repeats"] = this->listenRepeats;
   root["state_flush_interval"] = this->stateFlushInterval;

lib/Settings/Settings.h

@@ -90,6 +90,7 @@ public:
     httpRepeatFactor(1),
     listenRepeats(3),
     discoveryPort(48899),
+    simpleMqttClientStatus(false),
     stateFlushInterval(10000),
     mqttStateRateLimit(500),
     packetRepeatThrottleThreshold(200),
@@ -150,6 +151,7 @@ public:
   String mqttUpdateTopicPattern;
   String mqttStateTopicPattern;
   String mqttClientStatusTopic;
+  bool simpleMqttClientStatus;
   size_t stateFlushInterval;
   size_t mqttStateRateLimit;
   size_t packetRepeatThrottleThreshold;

test/remote/spec/mqtt_spec.rb

@@ -44,22 +44,21 @@ RSpec.describe 'State' do
   end
 
   context 'client status topic' do
-    # Unfortunately, no way to easily simulate an unclean disconnect, so only test birth
-    it 'should send client status messages when configured' do
-      status_topic = "#{@topic_prefix}client_status"
-
-      @client.put(
-        '/settings',
-        mqtt_client_status_topic: status_topic
-      )
+    before(:all) do
+      @status_topic = "#{@topic_prefix}client_status"
+      @client.patch_settings(mqtt_client_status_topic: @status_topic)
+    end
 
+    it 'should send client status messages when configured' do
       # Clear any retained messages
-      @mqtt_client.publish(status_topic, nil)
+      @mqtt_client.publish(@status_topic, nil)
 
+      # Unfortunately, no way to easily simulate an unclean disconnect, so only test birth
+      # and forced disconnect
       seen_statuses = Set.new
       required_statuses = %w(connected disconnected_clean)
 
-      @mqtt_client.on_message(status_topic, 20) do |topic, message|
+      @mqtt_client.on_message(@status_topic, 20) do |topic, message|
         message = JSON.parse(message)
 
         seen_statuses << message['status']
@@ -73,6 +72,30 @@ RSpec.describe 'State' do
 
       expect(seen_statuses).to include(*required_statuses)
     end
+
+    it 'should send simple client status message when configured' do
+      @client.patch_settings(simple_mqtt_client_status: true)
+
+      # Clear any retained messages
+      @mqtt_client.publish(@status_topic, nil)
+
+      # Unfortunately, no way to easily simulate an unclean disconnect, so only test birth
+      # and forced disconnect
+      seen_statuses = Set.new
+      required_statuses = %w(connected disconnected)
+
+      @mqtt_client.on_message(@status_topic, 20) do |topic, message|
+        seen_statuses << message
+        required_statuses.all? { |x| seen_statuses.include?(x) }
+      end
+
+      # Force MQTT reconnect by updating settings
+      @client.patch_settings(fakekey: 'fakevalue')
+
+      @mqtt_client.wait_for_listeners
+
+      expect(seen_statuses).to include(*required_statuses)
+    end
   end
 
   context 'commands and state' do

test/remote/spec/settings_spec.rb

@@ -23,6 +23,19 @@ RSpec.describe 'Settings' do
     @client.clear_auth!
   end
 
+  context 'keys' do
+    it 'should persist known settings keys' do
+      {
+        'simple_mqtt_client_status' => [true, false]
+      }.each do |key, values|
+        values.each do |v|
+          @client.patch_settings({key => v})
+          expect(@client.get('/settings')[key]).to eq(v)
+        end
+      end
+    end
+  end
+
   context 'POST settings file' do
     it 'should clobber patched settings' do
       file = Tempfile.new('espmh-settings.json')

web/src/js/script.js

@@ -171,6 +171,16 @@ var UI_FIELDS = [ {
     type: "string",
     tab: "tab-mqtt"
   }, {
+    tag:   "simple_mqtt_client_status",
+    friendly: "Client Status Messages Mode",
+    help: "In simple mode, only the strings 'connected' and 'disconnected' will be published.  In detailed mode, data about the version, IP address, etc. will be included.",
+    type: "option_buttons",
+    options: {
+      true: "Simple",
+      false: "Detailed"
+    },
+    tab: "tab-mqtt"
+  }, {
     tag:   "radio_interface_type",
     friendly: "Radio interface type",
     help: "2.4 GHz radio model. Only change this if you know you're not using an NRF24L01!",