Publish blank messages when state is deleted. This clears retained MQTT state (#430)

lib/WebServer/MiLightHttpServer.cpp

@@ -115,6 +115,10 @@ void MiLightHttpServer::onSettingsSaved(SettingsSavedHandler handler) {
   this->settingsSavedHandler = handler;
 }
 
+void MiLightHttpServer::onGroupDeleted(GroupDeletedHandler handler) {
+  this->groupDeletedHandler = handler;
+}
+
 void MiLightHttpServer::handleAbout() {
   // DynamicJsonBuffer buffer;
   // JsonObject& response = buffer.createObject();
@@ -368,6 +372,10 @@ void MiLightHttpServer::handleDeleteGroup(const UrlTokenBindings* urlBindings) {
   stateStore->clear(bulbId);
 
   server.send_P(200, APPLICATION_JSON, PSTR("true"));
+
+  if (groupDeletedHandler != NULL) {
+    this->groupDeletedHandler(bulbId);
+  }
 }
 
 void MiLightHttpServer::handleUpdateGroup(const UrlTokenBindings* urlBindings) {

lib/WebServer/MiLightHttpServer.h

@@ -10,6 +10,7 @@
 #define MAX_DOWNLOAD_ATTEMPTS 3
 
 typedef std::function<void(void)> SettingsSavedHandler;
+typedef std::function<void(const BulbId& id)> GroupDeletedHandler;
 
 const char TEXT_PLAIN[] PROGMEM = "text/plain";
 const char APPLICATION_JSON[] = "application/json";
@@ -30,6 +31,7 @@ public:
   void begin();
   void handleClient();
   void onSettingsSaved(SettingsSavedHandler handler);
+  void onGroupDeleted(GroupDeletedHandler handler);
   void on(const char* path, HTTPMethod method, ESP8266WebServer::THandlerFunction handler);
   void handlePacketSent(uint8_t* packet, const MiLightRemoteConfig& config);
   WiFiClient client();
@@ -72,6 +74,7 @@ protected:
   Settings& settings;
   GroupStateStore*& stateStore;
   SettingsSavedHandler settingsSavedHandler;
+  GroupDeletedHandler groupDeletedHandler;
   ESP8266WebServer::THandlerFunction _handleRootPage;
 
 };

src/main.cpp

@@ -280,10 +280,23 @@ void wifiExtraSettingsChange() {
   settings.save();
 }
 
+// Called when a group is deleted via the REST API.  Will publish an empty message to
+// the MQTT topic to delete retained state
+void onGroupDeleted(const BulbId& id) {
+  if (mqttClient != NULL) {
+    mqttClient->sendState(
+      *MiLightRemoteConfig::fromType(id.deviceType),
+      id.deviceId,
+      id.groupId,
+      ""
+    );
+  }
+}
+
 void setup() {
   Serial.begin(9600);
   String ssid = "ESP" + String(ESP.getChipId());
-  
+
   // load up our persistent settings from the file system
   SPIFFS.begin();
   Settings::load(settings);
@@ -303,7 +316,7 @@ void setup() {
   // that change is only on the development branch so we are going to continue to use this fork until
   // that is merged and ready.
   wifiManager.setSetupLoopCallback(handleLED);
-  
+
   // Allows us to have static IP config in the captive portal. Yucky pointers to pointers, just to have the settings carry through
   wifiManager.setSaveConfigCallback(wifiExtraSettingsChange);
 
@@ -374,6 +387,7 @@ void setup() {
 
   httpServer = new MiLightHttpServer(settings, milightClient, stateStore);
   httpServer->onSettingsSaved(applySettings);
+  httpServer->onGroupDeleted(onGroupDeleted);
   httpServer->on("/description.xml", HTTP_GET, []() { SSDP.schema(httpServer->client()); });
   httpServer->begin();
 

test/remote/lib/mqtt_client.rb

@@ -14,7 +14,7 @@ class MqttClient
   def disconnect
     @client.disconnect
   end
-  
+
   def reconnect
     @client.disconnect
     @client.connect
@@ -47,13 +47,18 @@ class MqttClient
     on_message(sub_topic, timeout) do |topic, message|
       topic_parts = topic.split('/')
 
+      begin
+        message = JSON.parse(message)
+      rescue JSON::ParserError => e
+      end
+
       yield(
         {
           id: topic_parts[2].to_i(16),
           type: topic_parts[3],
           group_id: topic_parts[4].to_i
         },
-        JSON.parse(message)
+        message
       )
     end
   end
@@ -69,12 +74,16 @@ class MqttClient
         end
       rescue Timeout::Error => e
         puts "Timed out listening for message on: #{topic}"
-        puts e.backtrace.join("\n")
+        raise e
       rescue BreakListenLoopError
       end
     end
   end
 
+  def publish(topic, state = {})
+    @client.publish(topic, state.to_json)
+  end
+
   def patch_state(id_params, state = {})
     @client.publish(
       "#{@topic_prefix}commands/#{id_topic_suffix(id_params)}",

test/remote/spec/mqtt_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe 'State' do
     @updates_topic = "#{@topic_prefix}updates/:device_id/:device_type/:group_id"
 
     @client.put(
-      '/settings', 
+      '/settings',
       mqtt_server: ENV.fetch('ESPMH_MQTT_SERVER'),
       mqtt_username: ENV.fetch('ESPMH_MQTT_USERNAME'),
       mqtt_password: ENV.fetch('ESPMH_MQTT_PASSWORD'),
@@ -26,12 +26,30 @@ RSpec.describe 'State' do
       type: 'rgb_cct',
       group_id: 1
     }
+    @client.delete_state(@id_params)
 
     @mqtt_client = MqttClient.new(
       *%w(SERVER USERNAME PASSWORD).map { |x| ENV.fetch("ESPMH_MQTT_#{x}") } << @topic_prefix
     )
   end
 
+  context 'deleting' do
+    it 'should remove retained state' do
+      @client.patch_state(@id_params, status: 'ON')
+
+      seen_blank = false
+
+      @mqtt_client.on_state(@id_params) do |topic, message|
+        seen_blank = (message == "")
+      end
+
+      @client.delete_state(@id_params)
+      @mqtt_client.wait_for_listeners
+
+      expect(seen_blank).to eq(true)
+    end
+  end
+
   context 'birth and LWT' do
     # Unfortunately, no way to easily simulate an unclean disconnect, so only test birth
     it 'should send birth message when configured' do
@@ -115,7 +133,7 @@ RSpec.describe 'State' do
 
       # Disable updates to prevent the negative effects of spamming commands
       @client.put(
-        '/settings', 
+        '/settings',
         mqtt_update_topic_pattern: '',
         mqtt_state_rate_limit: 500,
         packet_repeats: 1