Sfoglia il codice sorgente

Fix regression: state should not get updated unless bulb is on (#447)

* Add failing test for broken behavior

* fix broken behavior
Chris Mullins 6 anni fa
parent
commit
a050b19f49
2 ha cambiato i file con 32 aggiunte e 2 eliminazioni
  1. 6 2
      lib/MiLightState/GroupState.cpp
  2. 26 0
      test/remote/spec/state_spec.rb

+ 6 - 2
lib/MiLightState/GroupState.cpp

@@ -709,7 +709,10 @@ void GroupState::patch(const GroupState& other) {
   for (size_t i = 0; i < size(ALL_PHYSICAL_FIELDS); ++i) {
     GroupStateField field = ALL_PHYSICAL_FIELDS[i];
 
-    if (other.isSetField(field)) {
+    // Conditions:
+    //   * Only set anything if field is set in other state
+    //   * Do not patch anything other than STATE if bulb is off
+    if (other.isSetField(field) && (field == GroupStateField::STATE || isOn())) {
       setFieldValue(field, other.getFieldValue(field));
     }
   }
@@ -717,7 +720,8 @@ void GroupState::patch(const GroupState& other) {
   for (size_t i = 0; i < size(ALL_SCRATCH_FIELDS); ++i) {
     GroupStateField field = ALL_SCRATCH_FIELDS[i];
 
-    if (other.isSetScratchField(field)) {
+    // All scratch field updates require that the bulb is on.
+    if (isOn() && other.isSetScratchField(field)) {
       setScratchFieldValue(field, other.getScratchFieldValue(field));
     }
   }

+ 26 - 0
test/remote/spec/state_spec.rb

@@ -380,4 +380,30 @@ RSpec.describe 'State' do
       expect(state['kelvin']).to eq(90)
     end
   end
+
+  context 'state updates while off' do
+    it 'should not affect persisted state' do
+      @client.patch_state({'status' => 'OFF'}, @id_params)
+      state = @client.patch_state({'hue' => 100}, @id_params)
+
+      expect(state.count).to eq(1)
+      expect(state).to include('status')
+    end
+
+    it 'should not affect persisted state using increment/decrement' do
+      @client.patch_state({'status' => 'OFF'}, @id_params)
+
+      10.times do
+        @client.patch_state(
+          { commands: ['level_down', 'temperature_down'] },
+          @id_params
+        )
+      end
+
+      state = @client.get_state(@id_params)
+
+      expect(state.count).to eq(1)
+      expect(state).to include('status')
+    end
+  end
 end