Improvements in group 0 behavior (#426)

* add support for kelvin command
* Add helper for re-initing fields
* add methods for clearing fields
* Improve behavior with bulb mode
* Improve behavior with group 0 state
* Add tests

lib/MiLight/MiLightClient.cpp

@@ -429,6 +429,9 @@ void MiLightClient::update(const JsonObject& request) {
   if (request.containsKey("temperature")) {
     this->updateTemperature(request["temperature"]);
   }
+  if (request.containsKey("kelvin")) {
+    this->updateTemperature(request["kelvin"]);
+  }
   // HomeAssistant
   if (request.containsKey("color_temp")) {
     this->updateTemperature(

lib/MiLightState/GroupState.cpp

@@ -69,6 +69,15 @@ bool BulbId::operator==(const BulbId &other) {
 }
 
 GroupState::GroupState() {
+  initFields();
+}
+
+GroupState::GroupState(const JsonObject& jsonState) {
+  initFields();
+  patch(jsonState);
+}
+
+void GroupState::initFields() {
   state.fields._state                = 0;
   state.fields._brightness           = 0;
   state.fields._brightnessColor      = 0;
@@ -128,6 +137,64 @@ void GroupState::print(Stream& stream) const {
   stream.printf("State: %08X %08X\n", state.rawData[0], state.rawData[1]);
 }
 
+bool GroupState::clearField(GroupStateField field) {
+  bool clearedAny = false;
+
+  switch (field) {
+    // Always set and can't be cleared
+    case GroupStateField::COMPUTED_COLOR:
+    case GroupStateField::DEVICE_ID:
+    case GroupStateField::GROUP_ID:
+    case GroupStateField::DEVICE_TYPE:
+      break;
+
+    case GroupStateField::STATE:
+    case GroupStateField::STATUS:
+      clearedAny = isSetState();
+      state.fields._isSetState = 0;
+      break;
+
+    case GroupStateField::BRIGHTNESS:
+    case GroupStateField::LEVEL:
+      clearedAny = clearBrightness();
+      break;
+
+    case GroupStateField::COLOR:
+    case GroupStateField::HUE:
+    case GroupStateField::OH_COLOR:
+      clearedAny = isSetHue();
+      state.fields._isSetHue = 0;
+      break;
+
+    case GroupStateField::SATURATION:
+      clearedAny = isSetSaturation();
+      state.fields._isSetSaturation = 0;
+      break;
+
+    case GroupStateField::MODE:
+    case GroupStateField::EFFECT:
+      clearedAny = isSetMode();
+      state.fields._isSetMode = 0;
+      break;
+
+    case GroupStateField::KELVIN:
+    case GroupStateField::COLOR_TEMP:
+      clearedAny = isSetKelvin();
+      state.fields._isSetKelvin = 0;
+      break;
+
+    case GroupStateField::BULB_MODE:
+      clearedAny = isSetBulbMode();
+      state.fields._isSetBulbMode = 0;
+
+      // Clear brightness as well
+      clearedAny = clearBrightness() || clearedAny;
+      break;
+  }
+
+  return clearedAny;
+}
+
 bool GroupState::isSetField(GroupStateField field) const {
   switch (field) {
     case GroupStateField::COMPUTED_COLOR:
@@ -289,7 +356,11 @@ bool GroupState::setState(const MiLightStatus status) {
 }
 
 bool GroupState::isSetBrightness() const {
-  if (! state.fields._isSetBulbMode) {
+  // If we don't know what mode we're in, just assume white mode.  Do this for a few
+  // reasons:
+  //   * Some bulbs don't have multiple modes
+  //   * It's confusing to not have a default
+  if (! isSetBulbMode()) {
     return state.fields._isSetBrightness;
   }
 
@@ -304,6 +375,33 @@ bool GroupState::isSetBrightness() const {
 
   return false;
 }
+bool GroupState::clearBrightness() {
+  bool cleared = false;
+
+  if (!state.fields._isSetBulbMode) {
+    cleared = state.fields._isSetBrightness;
+    state.fields._isSetBrightness = 0;
+  } else {
+    switch (state.fields._bulbMode) {
+      case BULB_MODE_COLOR:
+        cleared = state.fields._isSetBrightnessColor;
+        state.fields._isSetBrightnessColor = 0;
+        break;
+
+      case BULB_MODE_SCENE:
+        cleared = state.fields._isSetBrightnessMode;
+        state.fields._isSetBrightnessMode = 0;
+        break;
+
+      case BULB_MODE_WHITE:
+        cleared = state.fields._isSetBrightness;
+        state.fields._isSetBrightness = 0;
+        break;
+    }
+  }
+
+  return cleared;
+}
 uint8_t GroupState::getBrightness() const {
   switch (state.fields._bulbMode) {
     case BULB_MODE_WHITE:
@@ -532,6 +630,31 @@ bool GroupState::applyIncrementCommand(GroupStateField field, IncrementDirection
   return false;
 }
 
+bool GroupState::clearNonMatchingFields(const GroupState& other) {
+#ifdef STATE_DEBUG
+  this->debugState("Clearing fields.  Current state");
+  other.debugState("Other state");
+#endif
+
+  bool clearedAny = false;
+
+  for (size_t i = 0; i < size(ALL_PHYSICAL_FIELDS); ++i) {
+    GroupStateField field = ALL_PHYSICAL_FIELDS[i];
+
+    if (other.isSetField(field) && isSetField(field) && getFieldValue(field) != other.getFieldValue(field)) {
+      if (clearField(field)) {
+        clearedAny = true;
+      }
+    }
+  }
+
+#ifdef STATE_DEBUG
+  this->debugState("Result");
+#endif
+
+  return clearedAny;
+}
+
 bool GroupState::patch(const GroupState& other) {
 #ifdef STATE_DEBUG
   other.debugState("Patching existing state with: ");
@@ -717,21 +840,21 @@ void GroupState::applyField(JsonObject& partialState, const BulbId& bulbId, Grou
       case GroupStateField::EFFECT:
         if (getBulbMode() == BULB_MODE_SCENE) {
           partialState["effect"] = String(getMode());
-        } else if (getBulbMode() == BULB_MODE_WHITE) {
+        } else if (isSetBulbMode() && getBulbMode() == BULB_MODE_WHITE) {
           partialState["effect"] = "white_mode";
         } else if (getBulbMode() == BULB_MODE_NIGHT) {
           partialState["effect"] = "night_mode";
         }
         break;
 
       case GroupStateField::COLOR_TEMP:
-        if (getBulbMode() == BULB_MODE_WHITE) {
+        if (isSetBulbMode() && getBulbMode() == BULB_MODE_WHITE) {
           partialState["color_temp"] = getMireds();
         }
         break;
 
       case GroupStateField::KELVIN:
-        if (getBulbMode() == BULB_MODE_WHITE) {
+        if (isSetBulbMode() && getBulbMode() == BULB_MODE_WHITE) {
           partialState["kelvin"] = getKelvin();
         }
         break;

lib/MiLightState/GroupState.h

@@ -49,14 +49,19 @@ class GroupState {
   GroupState(const GroupState& other);
   GroupState& operator=(const GroupState& other);
 
+  // Convenience constructor that patches defaults with JSON state
+  GroupState(const JsonObject& jsonState);
+
+  void initFields();
+
   bool operator==(const GroupState& other) const;
   bool isEqualIgnoreDirty(const GroupState& other) const;
   void print(Stream& stream) const;
 
-
   bool isSetField(GroupStateField field) const;
   uint16_t getFieldValue(GroupStateField field) const;
   void setFieldValue(GroupStateField field, uint16_t value);
+  bool clearField(GroupStateField field);
 
   bool isSetScratchField(GroupStateField field) const;
   uint16_t getScratchFieldValue(GroupStateField field) const;
@@ -73,6 +78,7 @@ class GroupState {
   bool isSetBrightness() const;
   uint8_t getBrightness() const;
   bool setBrightness(uint8_t brightness);
+  bool clearBrightness();
 
   // 8 bits
   bool isSetHue() const;
@@ -115,6 +121,10 @@ class GroupState {
   inline bool setMqttDirty();
   bool clearMqttDirty();
 
+  // Clears all of the fields in THIS GroupState that have different values
+  // than the provided group state.
+  bool clearNonMatchingFields(const GroupState& other);
+
   // Patches this state with ONLY the set fields in the other. Returns 
   // true if there were any changes.
   bool patch(const GroupState& other);

lib/MiLightState/GroupStateStore.cpp

@@ -10,26 +10,17 @@ GroupStateStore::GroupStateStore(const size_t maxSize, const size_t flushRate)
 GroupState* GroupStateStore::get(const BulbId& id) {
   GroupState* state = cache.get(id);
 
-  // Always force re-initialization of group 0 state
-  if (id.groupId == 0 || state == NULL) {
+  if (state == NULL) {
     trackEviction();
     GroupState loadedState = GroupState::defaultState(id.deviceType);
 
-    // For device types with groups, group 0 is a "virtual" group.  All devices paired with the same ID will respond
-    // to group 0.  So it doesn't make sense to store group 0 state by itself.
-    //
-    // For devices that don't have groups, we made the unfortunate decision to represent state using the fake group
-    // ID 0, so we can't always ignore group 0.
     const MiLightRemoteConfig* remoteConfig = MiLightRemoteConfig::fromType(id.deviceType);
 
     if (remoteConfig == NULL) {
       return NULL;
     }
 
-    if (id.groupId != 0 || remoteConfig->numGroups == 0) {
-      persistence.get(id, loadedState);
-    }
-
+    persistence.get(id, loadedState);
     state = cache.set(id, loadedState);
   }
 
@@ -41,27 +32,40 @@ GroupState* GroupStateStore::get(const uint16_t deviceId, const uint8_t groupId,
   return get(bulbId);
 }
 
-// save state for a bulb.  If id.groupId == 0, will iterate across all groups
-// and individually save each group (recursively)
+// Save state for a bulb.
+//
+// Notes:
+//
+// * For device types with groups, group 0 is a "virtual" group.  All devices paired with the same ID will 
+//   respond to group 0.  When state for an individual (i.e., != 0) group is changed, the state for 
+//   group 0 becomes out of sync and should be cleared.
+//
+// * If id.groupId == 0, will iterate across all groups and individually save each group (recursively)
+//
 GroupState* GroupStateStore::set(const BulbId &id, const GroupState& state) {
+  BulbId otherId(id);
   GroupState* storedState = get(id);
-  *storedState = state;
+  storedState->patch(state);
 
   if (id.groupId == 0) {
     const MiLightRemoteConfig* remote = MiLightRemoteConfig::fromType(id.deviceType);
-    BulbId individualBulb(id);
 
 #ifdef STATE_DEBUG
     Serial.printf_P(PSTR("Fanning out group 0 state for device ID 0x%04X (%d groups in total)\n"), id.deviceId, remote->numGroups);
     state.debugState("group 0 state = ");
 #endif
 
     for (size_t i = 1; i <= remote->numGroups; i++) {
-      individualBulb.groupId = i;
+      otherId.groupId = i;
 
-      GroupState* individualState = get(individualBulb);
+      GroupState* individualState = get(otherId);
       individualState->patch(state);
     }
+  } else {
+    otherId.groupId = 0;
+    GroupState* group0State = get(otherId);
+
+    group0State->clearNonMatchingFields(state);
   }
 
   return storedState;
@@ -72,6 +76,14 @@ GroupState* GroupStateStore::set(const uint16_t deviceId, const uint8_t groupId,
   return set(bulbId, state);
 }
 
+void GroupStateStore::clear(const BulbId& bulbId) {
+  GroupState* state = get(bulbId);
+
+  if (state != NULL) {
+    state->initFields();
+  }
+}
+
 void GroupStateStore::trackEviction() {
   if (cache.isFull()) {
     evictedIds.add(cache.getLru());

lib/MiLightState/GroupStateStore.h

@@ -26,6 +26,8 @@ class GroupStateStore {
   GroupState* set(const BulbId& id, const GroupState& state);
   GroupState* set(const uint16_t deviceId, const uint8_t groupId, const MiLightRemoteType deviceType, const GroupState& state);
 
+  void clear(const BulbId& id);
+
   /*
    * Flushes all states to persistent storage.  Returns true iff anything was
    * flushed.

lib/WebServer/MiLightHttpServer.cpp

@@ -27,6 +27,7 @@ void MiLightHttpServer::begin() {
   const char groupPattern[] = "/gateways/:device_id/:type/:group_id";
   server.onPatternAuthenticated(groupPattern, HTTP_PUT, [this](const UrlTokenBindings* b) { handleUpdateGroup(b); });
   server.onPatternAuthenticated(groupPattern, HTTP_POST, [this](const UrlTokenBindings* b) { handleUpdateGroup(b); });
+  server.onPatternAuthenticated(groupPattern, HTTP_DELETE, [this](const UrlTokenBindings* b) { handleDeleteGroup(b); });
   server.onPatternAuthenticated(groupPattern, HTTP_GET, [this](const UrlTokenBindings* b) { handleGetGroup(b); });
 
   server.onPatternAuthenticated("/raw_commands/:type", HTTP_ANY, [this](const UrlTokenBindings* b) { handleSendRaw(b); });
@@ -353,6 +354,24 @@ void MiLightHttpServer::handleGetGroup(const UrlTokenBindings* urlBindings) {
   sendGroupState(bulbId, stateStore->get(bulbId));
 }
 
+void MiLightHttpServer::handleDeleteGroup(const UrlTokenBindings* urlBindings) {
+  const String _deviceId = urlBindings->get("device_id");
+  uint8_t _groupId = atoi(urlBindings->get("group_id"));
+  const MiLightRemoteConfig* _remoteType = MiLightRemoteConfig::fromType(urlBindings->get("type"));
+
+  if (_remoteType == NULL) {
+    char buffer[40];
+    sprintf_P(buffer, PSTR("Unknown device type\n"));
+    server.send(400, TEXT_PLAIN, buffer);
+    return;
+  }
+
+  BulbId bulbId(parseInt<uint16_t>(_deviceId), _groupId, _remoteType->type);
+  stateStore->clear(bulbId);
+
+  server.send_P(200, APPLICATION_JSON, PSTR("true"));
+}
+
 void MiLightHttpServer::handleUpdateGroup(const UrlTokenBindings* urlBindings) {
   DynamicJsonBuffer buffer;
   JsonObject& request = buffer.parse(server.arg("plain"));

lib/WebServer/MiLightHttpServer.h

@@ -57,6 +57,7 @@ class MiLightHttpServer {
   void handleListenGateway(const UrlTokenBindings* urlBindings);
   void handleSendRaw(const UrlTokenBindings* urlBindings);
   void handleUpdateGroup(const UrlTokenBindings* urlBindings);
+  void handleDeleteGroup(const UrlTokenBindings* urlBindings);
   void handleGetGroup(const UrlTokenBindings* urlBindings);
 
   void handleRequest(const JsonObject& request);

platformio.ini

@@ -24,7 +24,8 @@ 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=200 -DHTTP_UPLOAD_BUFLEN=128 -D FIRMWARE_NAME=milight-hub -Idist -Ilib/DataStructures 
+# -D STATE_DEBUG
 # -D DEBUG_PRINTF
 # -D MQTT_DEBUG
 # -D MILIGHT_UDP_DEBUG