Skip to content

Commit

Permalink
Move the constraint check for number of presets not to exceed the num…
Browse files Browse the repository at this point in the history
…ber of presets supported from the atomic write commit command handling to the write request
  • Loading branch information
nivi-apple committed Aug 23, 2024
1 parent 9520bef commit 353fefe
Showing 1 changed file with 22 additions and 19 deletions.
41 changes: 22 additions & 19 deletions src/app/clusters/thermostat-server/thermostat-server-presets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,28 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}

// Before adding this preset to the pending presets, if the expected length of the pending presets' list
// exceeds the total number of presets supported, return RESOURCE_EXHAUSTED. Note that the preset has not been appended yet.

// Increment number of pending presets by 1 to account for this preset.
uint8_t totalExpectedCount = ++CountNumberOfPendingPresets(delegate);

uint8_t numberOfPresetsSupported = delegate->GetNumberOfPresets();

if (numberOfPresetsSupported == 0)
{
ChipLogError(Zcl, "AppendPendingPreset: Failed to get NumberOfPresets");
return CHIP_IM_GLOBAL_STATUS(InvalidInState);
}

if (numberOfPresetsSupported > 0 && totalExpectedCount > numberOfPresetsSupported)

This comment has been minimized.

Copy link
@hasty

hasty Aug 23, 2024

Contributor

This is an unsigned number and you just checked if it was zero, so the first part of this is always true.

{
return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
}

// TODO #34556 : Check if the number of presets for each presetScenario exceeds the max number of presets supported for that
// scenario. We plan to support only one preset for each presetScenario for our use cases so defer this for re-evaluation.

return delegate->AppendToPendingPresetList(preset);
}

Expand Down Expand Up @@ -504,25 +526,6 @@ Status ThermostatAttrAccess::PrecommitPresets(EndpointId endpoint)
}
}

uint8_t totalCount = CountNumberOfPendingPresets(delegate);

uint8_t numberOfPresetsSupported = delegate->GetNumberOfPresets();

if (numberOfPresetsSupported == 0)
{
ChipLogError(Zcl, "emberAfThermostatClusterCommitPresetsSchedulesRequestCallback: Failed to get NumberOfPresets");
return Status::InvalidInState;
}

// If the expected length of the presets attribute with the applied changes exceeds the total number of presets supported,
// return RESOURCE_EXHAUSTED. Note that the changes are not yet applied.
if (numberOfPresetsSupported > 0 && totalCount > numberOfPresetsSupported)
{
return Status::ResourceExhausted;
}

// TODO: Check if the number of presets for each presetScenario exceeds the max number of presets supported for that
// scenario. We plan to support only one preset for each presetScenario for our use cases so defer this for re-evaluation.
return Status::Success;
}

Expand Down

0 comments on commit 353fefe

Please sign in to comment.