[HVAC] Handle null BuiltIn field on Preset write according to spec (project-chip#35161)

* Add support for Presets attributes and commands to the Thermostat cluster

Clean up the Thermostat cluster and remove the TemperatureSetpointHoldPolicy attribute
and SetTemperatureSetpointHoldPolicy command

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn.

* Fix build error for Linux configure build of all-clusters-app

* Fix Darwin CI issues

Editorial fixes

* Restyled by clang-format

* More fixes

* Restyled by clang-format

* BUILD.gn fixes for CI

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Address review comments.

* Restyled by clang-format

* Regenerate Thermostat XML from spec

* Move atomic enum to global-enums.xml, actually

# Conflicts:
#	src/app/zap-templates/zcl/data-model/chip/global-structs.xml

* Regenerate XML and convert thermostat-server to atomic writes

* Pull in ACCapacityFormat typo un-fix

* Update Test_TC_TSTAT_1_1 to know about AtomicResponse command.

* Restyled patch

* Fix weird merge with upstream

* Fix emberAfIsTypeSigned not understanding temperature type

* Merge fixes from atomic write branch

* Relocate thermostat-manager sample code to all-clusters-common

* Fix g++ build error on linux

* Fix C formatter for long int, cast whole expression

* Sync cast fix with master

* Add thermostat-common dependency to thermostat app under linux

* Remove MatterPostAttributeChangeCallback from thermostat-manager, as it conflicts with other implementations

* Convert Atomic enums and structs to global

* Restyled patch

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Regen with alchemy 0.6.1

* Updates based on comments

* Add TC_MCORE_FS_1_3.py test implementation (project-chip#34650)

* Fix most TC-SWTCH-2.4 remaining issues (project-chip#34677)

- Move 2.4 in a better place in the file
- Add test steps properly
- Allow default button press position override

Issue project-chip#34656

Testing done:

- Test still passes on DUT with automation

* Initial test script for Fabric Sync TC_MCORE_FS_1_2 (project-chip#34675)

* Initial test script for Fabric Sync TC_MCORE_FS_1_2

* Apply suggestions from code review

Co-authored-by: C Freeman <cecille@google.com>

* Address Review Comments

* Address review comments

* Fix default timeout after other timeouts changed

* Restyled by autopep8

* Fix linter error

---------

Co-authored-by: C Freeman <cecille@google.com>
Co-authored-by: Restyled.io <commits@restyled.io>

* Test automation for FabricSync ICD BridgedDeviceBasicInfoCluster (project-chip#34628)

* WIP Bridged ICD, commissioning to both fabrics

* wip testing sending KeepActive

* wip most steps implemented

* using SIGSTOP and SIGCONT to control ICD server pausing

* Update src/python_testing/TC_BRBINFO_4_1.py

Co-authored-by: Terence Hampson <thampson@google.com>

* comments addressed

* more comments addressed

* lint pass

* Update src/python_testing/TC_BRBINFO_4_1.py

Co-authored-by: C Freeman <cecille@google.com>

* comments addressed, incl TH_SERVER configurable

* added setupQRCode and setupManualCode as options for DUT commissioning

* Restyled by autopep8

* Restyled by isort

* Update src/python_testing/TC_BRBINFO_4_1.py

Co-authored-by: Terence Hampson <thampson@google.com>

* Update src/python_testing/TC_BRBINFO_4_1.py

Co-authored-by: Terence Hampson <thampson@google.com>

* Update src/python_testing/TC_BRBINFO_4_1.py

Co-authored-by: Terence Hampson <thampson@google.com>

* comments addressed

* Restyled by autopep8

---------

Co-authored-by: Terence Hampson <thampson@google.com>
Co-authored-by: C Freeman <cecille@google.com>
Co-authored-by: Restyled.io <commits@restyled.io>

* ServiceArea test scripts (project-chip#34548)

* initial commit

* fix bugs

* fix issues reported by the linter

* fix bug in checking for unique areaDesc

* add TC 1.5

* Update src/python_testing/TC_SEAR_1_2.py

Co-authored-by: William <hicklin@users.noreply.github.com>

* Update src/python_testing/TC_SEAR_1_2.py

Co-authored-by: William <hicklin@users.noreply.github.com>

* address code review comments

* fix issue introduced by the previous commit

* address code review feedback

* Update src/python_testing/TC_SEAR_1_2.py

Co-authored-by: Kiel Oleson <kielo@apple.com>

* address code review feedback

* remove PICS checked by the TC_SEAR_1.6

* more code review updates

* Restyled by autopep8

---------

Co-authored-by: William <hicklin@users.noreply.github.com>
Co-authored-by: Kiel Oleson <kielo@apple.com>
Co-authored-by: Restyled.io <commits@restyled.io>

* Remove manual tests for Thermostat presets (project-chip#34679)

* Dump details about leaked ExchangeContexts before aborting (project-chip#34617)

* Dump details about leaked ExchangeContexts before aborting

This is implemented via a VerifyOrDieWithObject() variant of the existing
VerifyOrDie() macro that calls a DumpToLog() method on the provided object if
it exists (otherwise this is simply a no-op).

If CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE is not enabled, VerifyOrDieWithObject()
simply behaves like a plain VerifyOrDie(). DumpToLog() implementations can use
ChipLogFormatRtti to log type information about an object (usually a delegate);
if RTTI is disabled this simply outputs whether the object was null or not.

* Address review comments

* Make gcc happy and improve documentation

* Remove unused include

* Fix compile error without CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE

* Avoid unused parameter warning

* [TI] CC13x4_26x4 build fixes (project-chip#34682)

* lwip pbuf, map file, and hex creation when OTA is disabled

* added cc13x4 family define around the non OTA hex creation

* whitespace fix

* reversed custom factoy data flash with cc13x4 check

* more whitespace fixes

* [ICD] Add missing polling function to NoWifi connectivity manager (project-chip#34684)

* Add missing polling function to NoWifi connectivity manager

* Update GenericConnectivityManagerImpl_NoWiFi.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* [OPSTATE] Add Q test script for CountdownTime (project-chip#34632)

* Add Q test

* Added test to test set

* Remove unused var

* Restyled by autopep8

* Restyled by isort

* Fix name

* Use pics over other method

* Removed unused stuff

* Added pipe commands

* Fix reset

* Get example to report appropriate changes.

* WiP

* Added some comments

* Changes to make things work

* Removed dev msgs

* Missed some

* Removed dev msgs

* Straggler

* Restyled by clang-format

* Restyled by autopep8

* Restyled by isort

* Commented unused var

* Update examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp

* Fix bug

---------

Co-authored-by: Restyled.io <commits@restyled.io>

* YAML update to BRBINFO, ProductId (project-chip#34513)

* Bridged Device Information Cluster, Attribute ProductID test reflects marking as O, not X

* Update src/app/tests/suites/certification/Test_TC_BRBINFO_2_1.yaml

Co-authored-by: Terence Hampson <thampson@google.com>

* corrected pics

* corrected pics

* WIP Bridged ICD, commissioning to both fabrics

* wip testing sending KeepActive

* update to bridged-device-basic-information.xml and zap generated files

* removed unrelated file

---------

Co-authored-by: Terence Hampson <thampson@google.com>
Co-authored-by: Andrei Litvin <andy314@gmail.com>

* Fix simplified Linux tv-casting-app gn build error. (project-chip#34692)

* adding parallel execution to restyle-diff (project-chip#34663)

* adding parallel execution to restyle-diff

* using xargs to call restyle-paths

* fixing Copyright year

* restyle the restyler

* Add some bits to exercise global structs/enums to Unit Testing cluster. (project-chip#34540)

* Adds things to the Unit Testing cluster XML.
* This requires those things to be enabled in all-clusters-app,
  all-clusters-minimal-app, and one of the chef contact sensors to pass CI.
* That requires an implementation in test-cluster-server
* At which point might as well add a YAML test to exercise it all.

* [Silabs] Port platform specific Multi-Chip OTA work  (project-chip#34440)

* Pull request project-chip#1836: Cherry multi ota

Merge in WMN_TOOLS/matter from cherry-multi-ota to silabs_slc_1.3

Squashed commit of the following:

commit 4320bb46571658bc44fb82345348265def394991
Author: Michael Rupp <michael.rupp@silabs.com>
Date:   Fri May 10 14:26:07 2024 -0400

    remove some unwanted diffs in provision files

commit be160931dc600de7e7ead378b70d6a43c3945e46
Author: Michael Rupp <michael.rupp@silabs.com>
Date:   Fri May 10 14:24:25 2024 -0400

    revert changes to generator.project.mak

commit 14b6605887166e6d5284a61feb2bf407d850bdcf
Author: Michael Rupp <michael.rupp@silabs.com>
Date:   Fri May 10 13:06:12 2024 -0400

    revert NVM key changes and script changes

... and 8 more commits

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Restyled by autopep8

* remove unused libs caught by linter

* update doctree with new readmes

* rerun CI, cirque failing for unknown reasons

* fix include guards in provision examples

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <commits@restyled.io>

* Add python tests for Thermostat presets feature (project-chip#34693)

* Add python tests for Thermostat presets feature

* Restyled by autopep8

* Restyled by isort

* Update the PICS code for presets attribute

---------

Co-authored-by: Restyled.io <commits@restyled.io>

* removing unneccessary git fetch (project-chip#34698)

* Restyle patch

* Regen to fix ordering of global structs

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Return correct AtomicResponse when committing or rolling back

* Patch tests for atomic write of presets

* Fix tests to work with the new setup.

Specific changes:

* Enable SetActivePresetRequest command in all-clusters-app.
* Fix assignment of a PresetStructWithOwnedMembers to another
  PresetStructWithOwnedMembers to actually work correctly.
* Move constraint checks that happen on write from commit to write.
* Fix sending of atomic responses to not have use-stack-after-return.
* Fix PICS for the tests involved.

* Fix PICS values for atomic requests

* Remove PresetsSchedulesEditable and QueuedPreset from various places

* Restyled patch

* Restyled patch, again

* Remove PICS value for PresetsSchedulesEditable

* clang-tidy fixes

* clang-tidy fixes

* Clear associated atomic writes when fabric is removed

* Add tests for fabric removal and lockout of clients outside of atomic write

* Python linter

* Restyled patch

* Clear timer when fabric is removed

* Check for open atomic write before resetting

* Revert auto delegate declaration on lines where there's no collision

* Allow Thermostat delegate to provide timeout for atomic requests

* Relocate thermostat example code to thermostat-common

* Remove thermostat-manager code, replace with thermostat delegate

* Sync atomic write error order with spec

* Restyle patch

* Drop memset of atomic write sessions

* Add PreCommit stage to allow rollback of multiple attributes when only one fails

* Separate OnTimerExpired method, vs ResetWrite

* Method documentation

* Apply suggestions from code review

Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>

* Remove unused InWrite check

* Drop imcode alias

* Switch AtomicWriteState to enum class

* DRY up atomic write manager

* Apply suggestions from code review

Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>

* Drop duplicate doc comments

* Rename GetAtomicWriteScopedNodeId to GetAtomicWriteOriginatorScopedNodeId

* Updates based on comments

* Add MatterReportingAttributeChangeCallback calls for updated attributes

* Relocate thermostat example code to thermostat-common, and remove thermostat-manager

* Merge atomic write code back into thermostat-server

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Fix build after suggestions

* Actually track attribute IDs associated with atomic write

* Only commit presets if all attribute precommits were successful

* Fix scope on err

* Add documentation to methods

* Remove duplicate preset check.

* Move various functions into anonymous namespaces, or Thermostat namespace

* Drop impossible non-atomic attribute status after rollback

* Allow null BuiltIn field when saving Presets

* Namespace workaround for compilers on other platforms

* Fix bad merge

* Fix readability issue

* Force built-in to false on new presets

---------

Co-authored-by: Nivedita Sarkar <nivedita_sarkar@apple.com>
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Terence Hampson <thampson@google.com>
Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com>
Co-authored-by: Chris Letnick <cletnick@google.com>
Co-authored-by: C Freeman <cecille@google.com>
Co-authored-by: Douglas Rocha Ferraz <rochaferraz@google.com>
Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com>
Co-authored-by: William <hicklin@users.noreply.github.com>
Co-authored-by: Kiel Oleson <kielo@apple.com>
Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com>
Co-authored-by: Anu Biradar <104591549+abiradarti@users.noreply.github.com>
Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com>
Co-authored-by: Rob Bultman <rob.Bultman@gmail.com>
Co-authored-by: Andrei Litvin <andy314@gmail.com>
Co-authored-by: Shao Ling Tan <161761051+shaoltan-amazon@users.noreply.github.com>
Co-authored-by: Amine Alami <43780877+Alami-Amine@users.noreply.github.com>
Co-authored-by: Michael Rupp <95718139+mykrupp@users.noreply.github.com>

Author: 20 people

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;
 

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<uint8_t>(preset.presetScenario) };
+            const uint8_t handle[] = { static_cast<uint8_t>(preset.GetPresetScenario()) };
             mPendingPresets[mNextFreeIndexInPendingPresetsList].SetPresetHandle(DataModel::MakeNullable(ByteSpan(handle)));
         }
         mNextFreeIndexInPendingPresetsList++;

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.

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<ByteSpan> & 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);
     }

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()