Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HVAC] Add check for per-preset-scenario limit #35310

Merged
merged 36 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
8c79a18
[HVAC] Check if number of preset scenarios exceeds maximum number of …
hasty Aug 26, 2024
4fae149
[NXP][Zephyr] Provide AP band in connection request parameters (#35181)
axelnxp Aug 28, 2024
69cd036
Plumbing for CADMIN attribute updates from fabric-admin to fabric-bri…
tehampson Aug 28, 2024
e5f99fb
Fix TC_BRBINFO_4_1 for execution on TH (#35257)
tehampson Aug 28, 2024
560350b
[Fabric-Admin] Move DeviceSynchronization from pairing command to dev…
yufengwangca Aug 28, 2024
d174d62
Add command-line argument to allow userprompt at start of ECOINFO_2_1…
tehampson Aug 28, 2024
3f02786
Testing fixes for TC_SWTCH from TE2 (#34984)
tcarmelveilleux Aug 28, 2024
69cfdf5
Add test cases for testing additional Presets write and commit constr…
nivi-apple Aug 28, 2024
97ad414
Allow TestAccessControl to run with ARL (#35262)
tleacmcsa Aug 28, 2024
1631e3b
Make zap_downloadl.py create a usable zap.app on Mac (#35242)
ksperling-apple Aug 28, 2024
7d02d7c
TBRM Tests scripts consistency with te2 fixes (#35153)
marchemi Aug 28, 2024
9565641
[HVAC] Alter Thermostat Preset tests to not rely on knowledge of the …
hasty Aug 29, 2024
d2ef1ba
Pick midpoint setpoints for new presets
hasty Aug 29, 2024
ad2a2e6
Lint fixes
hasty Aug 29, 2024
8e7c198
Apply suggestions from code review
hasty Aug 29, 2024
231e17f
Fixes from code review
hasty Aug 29, 2024
f50dc83
Apply suggestions from code review
hasty Aug 29, 2024
70daba4
Fix remaining places with hard-coded setpoints
hasty Aug 29, 2024
0e34bfa
Don't abort test if there are no built-in presets
hasty Aug 29, 2024
a9a0bd4
Remove unneeded length check
hasty Aug 29, 2024
25376bf
Fix max number of preset types
hasty Aug 29, 2024
5fd85ad
Add test for individual preset scenario limits
hasty Aug 30, 2024
53abeae
Merge remote-tracking branch 'upstream/master' into granbery/preset_s…
hasty Aug 30, 2024
a01650b
Fix lint issue
hasty Aug 30, 2024
1c71b75
Merge branch 'master' into granbery/preset_scenario_count
hasty Aug 30, 2024
a8c8865
Merge branch 'master' into granbery/preset_scenario_count
hasty Aug 30, 2024
d0a4777
Return invalid in state if we're unable to iterate over the preset ty…
hasty Aug 30, 2024
adade93
Apply suggestions from code review
hasty Aug 30, 2024
93206a3
Remove unneeded active preset setting
hasty Aug 30, 2024
82a86a5
Restyled patch
hasty Aug 30, 2024
edc0cf8
Merge branch 'master' into granbery/preset_scenario_count
hasty Aug 31, 2024
948afaf
Merge branch 'master' into granbery/preset_scenario_count
hasty Sep 1, 2024
79edbd5
Merge branch 'master' into granbery/preset_scenario_count
hasty Sep 1, 2024
d1fe36d
Suggestions from code review
hasty Sep 1, 2024
de5066b
Merge branch 'master' into granbery/preset_scenario_count
hasty Sep 12, 2024
76f4721
Merge branch 'master' into granbery/preset_scenario_count
hasty Oct 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 55 additions & 54 deletions src/app/clusters/thermostat-server/thermostat-server-presets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,69 +152,36 @@ bool GetMatchingPresetInPresets(Delegate * delegate, const DataModel::Nullable<B
}

/**
* @brief Returns the length of the list of presets if the pending presets were to be applied. The size of the pending presets list
* calculated, after all the constraint checks are done, is the new size of the updated Presets attribute since the pending
* preset list is expected to have all existing presets with or without edits plus new presets.
* This is called before changes are actually applied.
* @brief Gets the maximum number of presets allowed for a given preset scenario.
*
* @param[in] delegate The delegate to use.
*
* @return count of the updated Presets attribute if the pending presets were applied to it. Return 0 for error cases.
* @param[in] delegate The delegate to use.
* @param[in] presetScenario The presetScenario to match with.
* @param[out] count The maximum number of presets for the specified presetScenario
* @return CHIP_NO_ERROR if the maximum number was determined, or an error if not
*/
uint8_t CountNumberOfPendingPresets(Delegate * delegate)
CHIP_ERROR MaximumPresetScenarioCount(Delegate * delegate, PresetScenarioEnum presetScenario, size_t & count)
{
uint8_t numberOfPendingPresets = 0;

VerifyOrReturnValue(delegate != nullptr, 0);

count = 0;
for (uint8_t i = 0; true; i++)
{
PresetStructWithOwnedMembers pendingPreset;
CHIP_ERROR err = delegate->GetPendingPresetAtIndex(i, pendingPreset);

PresetTypeStruct::Type presetType;
auto err = delegate->GetPresetTypeAtIndex(i, presetType);
if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
{
break;
// We exhausted the list trying to find the preset scenario
return CHIP_NO_ERROR;
}
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "CountNumberOfPendingPresets: GetPendingPresetAtIndex failed with error %" CHIP_ERROR_FORMAT,
err.Format());
return 0;
return err;
}
numberOfPendingPresets++;
}

return numberOfPendingPresets;
}

/**
* @brief Checks if the presetScenario is present in the PresetTypes attribute.
*
* @param[in] delegate The delegate to use.
* @param[in] presetScenario The presetScenario to match with.
*
* @return true if the presetScenario is found, false otherwise.
*/
bool PresetScenarioExistsInPresetTypes(Delegate * delegate, PresetScenarioEnum presetScenario)
{
VerifyOrReturnValue(delegate != nullptr, false);

for (uint8_t i = 0; true; i++)
{
PresetTypeStruct::Type presetType;
auto err = delegate->GetPresetTypeAtIndex(i, presetType);
if (err != CHIP_NO_ERROR)
{
return false;
}

if (presetType.presetScenario == presetScenario)
{
return true;
count = presetType.numberOfPresets;
return CHIP_NO_ERROR;
}
}
return false;
return CHIP_NO_ERROR;
}

/**
Expand Down Expand Up @@ -410,8 +377,16 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
}
}

if (!PresetScenarioExistsInPresetTypes(delegate, preset.GetPresetScenario()))
size_t maximumPresetCount = delegate->GetNumberOfPresets();
size_t maximumPresetScenarioCount = 0;
if (MaximumPresetScenarioCount(delegate, preset.GetPresetScenario(), maximumPresetScenarioCount) != CHIP_NO_ERROR)
{
return CHIP_IM_GLOBAL_STATUS(InvalidInState);
}

if (maximumPresetScenarioCount == 0)
{
// This is not a supported preset scenario
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}

Expand All @@ -423,16 +398,42 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele
// 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.

uint8_t numberOfPendingPresets = CountNumberOfPendingPresets(delegate);
// We're going to append this preset, so let's assume a count as though it had already been inserted
size_t presetCount = 1;
size_t presetScenarioCount = 1;
for (uint8_t i = 0; true; i++)
{
PresetStructWithOwnedMembers otherPreset;
CHIP_ERROR err = delegate->GetPendingPresetAtIndex(i, otherPreset);

if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED)
{
break;
}
if (err != CHIP_NO_ERROR)
{
return CHIP_IM_GLOBAL_STATUS(InvalidInState);
}
presetCount++;
if (preset.GetPresetScenario() == otherPreset.GetPresetScenario())
{
presetScenarioCount++;
}
}

// We will be adding one more preset, so reject if the length is already at max.
if (numberOfPendingPresets >= delegate->GetNumberOfPresets())
if (presetCount > maximumPresetCount)
{
ChipLogError(Zcl, "Preset count exceeded %u: %u ", static_cast<unsigned>(maximumPresetCount),
static_cast<unsigned>(presetCount));
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.
if (presetScenarioCount > maximumPresetScenarioCount)
{
ChipLogError(Zcl, "Preset scenario count exceeded %u: %u ", static_cast<unsigned>(maximumPresetScenarioCount),
static_cast<unsigned>(presetScenarioCount));
return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
}

return delegate->AppendToPendingPresetList(preset);
}
Expand Down
28 changes: 23 additions & 5 deletions src/python_testing/TC_TSTAT_4_2.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import copy
import logging
import random
from collections import namedtuple

import chip.clusters as Clusters
from chip import ChipDeviceCtrl # Needed before chip.FabricAdmin
Expand Down Expand Up @@ -418,11 +419,6 @@ async def test_TC_TSTAT_4_2(self):
logger.info(
"Couldn't run test step 4 since there were no built-in presets")

# Send the SetActivePresetRequest command
await self.send_set_active_preset_handle_request_command(value=b'\x03')

activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle)

self.step("5")
if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")):

Expand Down Expand Up @@ -714,6 +710,28 @@ async def test_TC_TSTAT_4_2(self):
self.step("18")
if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")):

ScenarioHeadroom = namedtuple("ScenarioHeadroom", "presetScenario remaining")
# Generate list of tuples of scenarios and number of remaining presets per scenario allowed
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
presetScenarioHeadrooms = list(ScenarioHeadroom(presetType.presetScenario,
presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)) for presetType in presetTypes)

if presetScenarioHeadrooms:
# Find the preset scenario with the smallest number of remaining allowed presets
presetScenarioHeadrooms = sorted(presetScenarioHeadrooms, key=lambda psh: psh.remaining)
presetScenarioHeadroom = presetScenarioHeadrooms[0]

# Add one more preset than is allowed by the preset type
test_presets = copy.deepcopy(current_presets)
test_presets.extend([cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=presetScenarioHeadroom.presetScenario,
coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)] * (presetScenarioHeadroom.remaining + 1))

await self.send_atomic_request_begin_command()

await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ResourceExhausted)

# Clear state for next test.
await self.send_atomic_request_rollback_command()

# Calculate the length of the Presets list that could be created using the preset scenarios in PresetTypes and numberOfPresets supported for each scenario.
totalExpectedPresetsLength = sum(presetType.numberOfPresets for presetType in presetTypes)

Expand Down
Loading