diff --git a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h index 9edf13f839df44..b57ee2492122e7 100644 --- a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h +++ b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h @@ -58,7 +58,7 @@ class ThermostatDelegate : public Delegate void InitializePendingPresets() override; - CHIP_ERROR AppendToPendingPresetList(const Structs::PresetStruct::Type & preset) override; + CHIP_ERROR AppendToPendingPresetList(const PresetStructWithOwnedMembers & preset) override; CHIP_ERROR GetPendingPresetAtIndex(size_t index, PresetStructWithOwnedMembers & preset) override; diff --git a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp index 8c411cd5a9176e..7991e48323a62f 100644 --- a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp +++ b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp @@ -177,17 +177,17 @@ void ThermostatDelegate::InitializePendingPresets() } } -CHIP_ERROR ThermostatDelegate::AppendToPendingPresetList(const PresetStruct::Type & preset) +CHIP_ERROR ThermostatDelegate::AppendToPendingPresetList(const PresetStructWithOwnedMembers & preset) { if (mNextFreeIndexInPendingPresetsList < ArraySize(mPendingPresets)) { mPendingPresets[mNextFreeIndexInPendingPresetsList] = preset; - if (preset.presetHandle.IsNull()) + if (preset.GetPresetHandle().IsNull()) { // TODO: #34556 Since we support only one preset of each type, using the octet string containing the preset scenario // suffices as the unique preset handle. Need to fix this to actually provide unique handles once multiple presets of // each type are supported. - const uint8_t handle[] = { static_cast(preset.presetScenario) }; + const uint8_t handle[] = { static_cast(preset.GetPresetScenario()) }; mPendingPresets[mNextFreeIndexInPendingPresetsList].SetPresetHandle(DataModel::MakeNullable(ByteSpan(handle))); } mNextFreeIndexInPendingPresetsList++; diff --git a/src/app/clusters/thermostat-server/thermostat-delegate.h b/src/app/clusters/thermostat-server/thermostat-delegate.h index ccb690a34fba60..5f11ab59889430 100644 --- a/src/app/clusters/thermostat-server/thermostat-delegate.h +++ b/src/app/clusters/thermostat-server/thermostat-delegate.h @@ -103,7 +103,7 @@ class Delegate * @return CHIP_NO_ERROR if the preset was appended to the list successfully. * @return CHIP_ERROR if there was an error adding the preset to the list. */ - virtual CHIP_ERROR AppendToPendingPresetList(const Structs::PresetStruct::Type & preset) = 0; + virtual CHIP_ERROR AppendToPendingPresetList(const PresetStructWithOwnedMembers & preset) = 0; /** * @brief Get the Preset at a given index in the pending presets list. diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index a56c94fa49834d..9413513fd0cedd 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -38,16 +38,16 @@ namespace { * @return true If the preset is valid i.e the PresetHandle (if not null) fits within size constraints and the presetScenario enum * value is valid. Otherwise, return false. */ -bool IsValidPresetEntry(const PresetStruct::Type & preset) +bool IsValidPresetEntry(const PresetStructWithOwnedMembers & preset) { // Check that the preset handle is not too long. - if (!preset.presetHandle.IsNull() && preset.presetHandle.Value().size() > kPresetHandleSize) + if (!preset.GetPresetHandle().IsNull() && preset.GetPresetHandle().Value().size() > kPresetHandleSize) { return false; } // Ensure we have a valid PresetScenario. - return (preset.presetScenario != PresetScenarioEnum::kUnknownEnumValue); + return (preset.GetPresetScenario() != PresetScenarioEnum::kUnknownEnumValue); } /** @@ -123,7 +123,7 @@ bool MatchingPendingPresetExists(Delegate * delegate, const PresetStructWithOwne * * @return true if a matching entry was found in the presets attribute list, false otherwise. */ -bool GetMatchingPresetInPresets(Delegate * delegate, const PresetStruct::Type & presetToMatch, +bool GetMatchingPresetInPresets(Delegate * delegate, const DataModel::Nullable & presetHandle, PresetStructWithOwnedMembers & matchingPreset) { VerifyOrReturnValue(delegate != nullptr, false); @@ -143,7 +143,7 @@ bool GetMatchingPresetInPresets(Delegate * delegate, const PresetStruct::Type & } // Note: presets coming from our delegate always have a handle. - if (presetToMatch.presetHandle.Value().data_equal(matchingPreset.GetPresetHandle().Value())) + if (presetHandle.Value().data_equal(matchingPreset.GetPresetHandle().Value())) { return true; } @@ -351,53 +351,71 @@ Status ThermostatAttrAccess::SetActivePreset(EndpointId endpoint, DataModel::Nul return Status::Success; } -CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * delegate, const PresetStruct::Type & preset) +CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * delegate, const PresetStruct::Type & newPreset) { + PresetStructWithOwnedMembers preset = newPreset; if (!IsValidPresetEntry(preset)) { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } - if (preset.presetHandle.IsNull()) + if (preset.GetPresetHandle().IsNull()) { if (IsBuiltIn(preset)) { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } + // Force to be false, if passed as null + preset.SetBuiltIn(false); } else { - auto & presetHandle = preset.presetHandle.Value(); - // Per spec we need to check that: // (a) There is an existing non-pending preset with this handle. PresetStructWithOwnedMembers matchingPreset; - if (!GetMatchingPresetInPresets(delegate, preset, matchingPreset)) + if (!GetMatchingPresetInPresets(delegate, preset.GetPresetHandle().Value(), matchingPreset)) { return CHIP_IM_GLOBAL_STATUS(NotFound); } // (b) There is no existing pending preset with this handle. - if (CountPresetsInPendingListWithPresetHandle(delegate, presetHandle) > 0) + if (CountPresetsInPendingListWithPresetHandle(delegate, preset.GetPresetHandle().Value()) > 0) { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } + const auto & presetBuiltIn = preset.GetBuiltIn(); + const auto & matchingPresetBuiltIn = matchingPreset.GetBuiltIn(); // (c)/(d) The built-in fields do not have a mismatch. - // TODO: What's the story with nullability on the BuiltIn field? - if (!preset.builtIn.IsNull() && !matchingPreset.GetBuiltIn().IsNull() && - preset.builtIn.Value() != matchingPreset.GetBuiltIn().Value()) + if (presetBuiltIn.IsNull()) { - return CHIP_IM_GLOBAL_STATUS(ConstraintError); + if (matchingPresetBuiltIn.IsNull()) + { + // This really shouldn't happen; internal presets should alway have built-in set + return CHIP_IM_GLOBAL_STATUS(InvalidInState); + } + preset.SetBuiltIn(matchingPresetBuiltIn.Value()); + } + else + { + if (matchingPresetBuiltIn.IsNull()) + { + // This really shouldn't happen; internal presets should alway have built-in set + return CHIP_IM_GLOBAL_STATUS(InvalidInState); + } + if (presetBuiltIn.Value() != matchingPresetBuiltIn.Value()) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } } } - if (!PresetScenarioExistsInPresetTypes(delegate, preset.presetScenario)) + if (!PresetScenarioExistsInPresetTypes(delegate, preset.GetPresetScenario())) { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } - if (preset.name.HasValue() && !PresetTypeSupportsNames(delegate, preset.presetScenario)) + if (preset.GetName().HasValue() && !PresetTypeSupportsNames(delegate, preset.GetPresetScenario())) { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 563d6f3f2eddfc..b133b1be2c8f22 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -246,8 +246,12 @@ async def test_TC_TSTAT_4_2(self): 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")): await self.send_atomic_request_begin_command() + # Set the new preset to a null built-in value; will be replaced with false on reading + test_presets = copy.deepcopy(new_presets) + test_presets[2].builtIn = NullValue + # Write to the presets attribute after calling AtomicRequest command - status = await self.write_presets(endpoint=endpoint, presets=new_presets) + status = await self.write_presets(endpoint=endpoint, presets=test_presets) status_ok = (status == Status.Success) asserts.assert_true(status_ok, "Presets write did not return Success as expected") @@ -268,8 +272,12 @@ async def test_TC_TSTAT_4_2(self): # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() + # Set the existing preset to a null built-in value; will be replaced with true on reading + test_presets = copy.deepcopy(new_presets) + test_presets[0].builtIn = NullValue + # Write to the presets attribute after calling AtomicRequest command - await self.write_presets(endpoint=endpoint, presets=new_presets) + await self.write_presets(endpoint=endpoint, presets=test_presets) # Send the AtomicRequest commit command await self.send_atomic_request_commit_command()