Skip to content

Commit

Permalink
Fix regression: state should not get updated unless bulb is on (#447)
Browse files Browse the repository at this point in the history
* Add failing test for broken behavior

* fix broken behavior
  • Loading branch information
sidoh authored Apr 23, 2019
1 parent 2f5f02c commit a050b19
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/MiLightState/GroupState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -709,15 +709,19 @@ 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));
}
}

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));
}
}
Expand Down
26 changes: 26 additions & 0 deletions test/remote/spec/state_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -380,4 +380,30 @@
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

0 comments on commit a050b19

Please sign in to comment.