Merge branch 'project-chip:master' into issue_226_temp_2

.github/workflows/check-data-model-directory-updates.yaml

@@ -12,20 +12,28 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-name: Check for changes to data_model directory without a sha update 
+name: Data model directory checks
 
 on:
   pull_request:
-    paths:
-      - "data_model/**"
 
 jobs:
-  check-submodule-update-label:
-    name: Check for changes to data_model directory without a sha update
+  check-data_model-updates:
+    name: Check for updates to data model directory without SHA updates
     runs-on: ubuntu-latest
-    if: "git diff --name-only HEAD^..HEAD data_model/ | grep -q spec_sha"
+    container:
+      image: ghcr.io/project-chip/chip-build
     steps:
-      - name: Error Message
-        run: echo This pull request attempts to update data_model directory, but is missing updates to spec_sha file with the latest version of the sha. Files in the data_model directory are generated automatically and should not be updated manually.
-      - name: Fail Job
-        run: exit 1
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 2
+      - name: Check for changes to master data_model directory without a SHA update
+        run: |
+          python3 scripts/dm_xml_ci_change_enforcement.py data_model/master
+      - name: Check for changes to 1.3 data_model directory without a SHA update
+        run: |
+          python3 scripts/dm_xml_ci_change_enforcement.py data_model/1.3
+      - name: Check for changes to 1.4 data_model directory without a SHA update
+        run: |
+          python3 scripts/dm_xml_ci_change_enforcement.py data_model/1.4

examples/all-clusters-app/linux/ValveControlDelegate.cpp

@@ -38,7 +38,6 @@ DataModel::Nullable<chip::Percent> ValveControlDelegate::HandleOpenValve(DataMod
     // In this demo application, the transition is considered instant,
     // so current level is set to the requested level and current state is set to kOpen.
     currentLevel = sLevel;
-    Attributes::CurrentState::Set(kValveEndpoint, ValveConfigurationAndControl::ValveStateEnum::kOpen);
 
     return DataModel::Nullable<chip::Percent>(currentLevel);
 }
@@ -48,8 +47,6 @@ CHIP_ERROR ValveControlDelegate::HandleCloseValve()
     sLastOpenDuration = 0;
     sLevel            = 0;
     ReturnErrorOnFailure(ValveConfigurationAndControl::UpdateCurrentLevel(kValveEndpoint, sLevel));
-    ReturnErrorOnFailure(
-        ValveConfigurationAndControl::UpdateCurrentState(kValveEndpoint, ValveConfigurationAndControl::ValveStateEnum::kClosed));
     ChipLogProgress(NotSpecified, "Valve closed");
     return CHIP_NO_ERROR;
 }

examples/fabric-sync/admin/FabricAdmin.cpp

@@ -16,8 +16,10 @@
  */
 
 #include "FabricAdmin.h"
+
 #include <AppMain.h>
 #include <CommissionerMain.h>
+#include <app/server/Server.h>
 #include <bridge/include/FabricBridge.h>
 #include <controller/CHIPDeviceControllerFactory.h>
 
@@ -45,9 +47,8 @@ CHIP_ERROR FabricAdmin::Init()
     VerifyOrReturnError(engine != nullptr, CHIP_ERROR_INCORRECT_STATE);
     ReturnLogErrorOnFailure(IcdManager::Instance().Init(&sICDClientStorage, engine));
 
-    auto exchangeMgr = Controller::DeviceControllerFactory::GetInstance().GetSystemState()->ExchangeMgr();
-    VerifyOrReturnError(exchangeMgr != nullptr, CHIP_ERROR_INCORRECT_STATE);
-    ReturnLogErrorOnFailure(sCheckInHandler.Init(exchangeMgr, &sICDClientStorage, &IcdManager::Instance(), engine));
+    ReturnLogErrorOnFailure(sCheckInHandler.Init(&chip::Server::GetInstance().GetExchangeManager(), &sICDClientStorage,
+                                                 &IcdManager::Instance(), engine));
 
     ReturnLogErrorOnFailure(PairingManager::Instance().Init(GetDeviceCommissioner()));
 

scripts/dm_xml_ci_change_enforcement.py

@@ -0,0 +1,52 @@
+#!/usr/bin/env python3
+
+# Copyright (c) 2024 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+import os
+import subprocess
+import sys
+
+import click
+
+
+@click.command()
+@click.argument('dir', nargs=1, type=click.Path(exists=True))
+def check_dm_directory(dir):
+    clusters = os.path.join(dir, 'clusters')
+    device_types = os.path.join(dir, 'device_types')
+    if not os.path.isdir(clusters) or not os.path.isdir(device_types):
+        print(f"Invalid data model directory {dir}")
+        sys.exit(1)
+
+    # We are operating in a VM, and although there is a checkout, it is working in a scratch directory
+    # where the ownership is different than the runner.
+    # Adding an exception for this directory so that git can function properly.
+    subprocess.run("git config --global --add safe.directory '*'", shell=True)
+
+    def check_dir(dir):
+        cmd = f'git diff HEAD^..HEAD --name-only -- {dir}'
+        output = subprocess.check_output(cmd, shell=True).decode().splitlines()
+        if output and 'spec_sha' not in output and 'scraper_version' not in output:
+            print(f'Data model directory {dir} had changes to the following files without a corresponding update to the spec SHA')
+            print(output)
+            print("Note that the data_model directory files are automatically updated by a spec scraper and should not be manually updated.")
+            return 1
+        return 0
+
+    ret = check_dir(clusters) + check_dir(device_types)
+    sys.exit(ret)
+
+
+if __name__ == '__main__':
+    check_dm_directory()

src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp

@@ -360,9 +360,9 @@ CHIP_ERROR SetValveLevel(EndpointId ep, DataModel::Nullable<Percent> level, Data
     if (!isDelegateNull(delegate))
     {
         DataModel::Nullable<Percent> cLevel = delegate->HandleOpenValve(level);
-        if (HasFeature(ep, ValveConfigurationAndControl::Feature::kLevel))
+        if (HasFeature(ep, ValveConfigurationAndControl::Feature::kLevel) && !cLevel.IsNull())
         {
-            VerifyOrReturnError(Status::Success == CurrentLevel::Set(ep, cLevel), attribute_error);
+            UpdateCurrentLevel(ep, cLevel.Value());
         }
     }
     // start countdown
@@ -376,14 +376,28 @@ CHIP_ERROR UpdateCurrentLevel(EndpointId ep, Percent currentLevel)
     if (HasFeature(ep, ValveConfigurationAndControl::Feature::kLevel))
     {
         VerifyOrReturnError(Status::Success == CurrentLevel::Set(ep, currentLevel), CHIP_IM_GLOBAL_STATUS(ConstraintError));
-        return CHIP_NO_ERROR;
     }
-    return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
+    DataModel::Nullable<Percent> targetLevel = DataModel::NullNullable;
+    TargetLevel::Get(ep, targetLevel);
+    if (!targetLevel.IsNull() && currentLevel == targetLevel.Value())
+    {
+        targetLevel = DataModel::NullNullable;
+        TargetLevel::Set(ep, targetLevel);
+        UpdateCurrentState(ep, currentLevel == 0 ? ValveStateEnum::kClosed : ValveStateEnum::kOpen);
+    }
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR UpdateCurrentState(EndpointId ep, ValveConfigurationAndControl::ValveStateEnum currentState)
 {
     VerifyOrReturnError(Status::Success == CurrentState::Set(ep, currentState), CHIP_IM_GLOBAL_STATUS(ConstraintError));
+    DataModel::Nullable<ValveStateEnum> targetState = DataModel::NullNullable;
+    TargetState::Get(ep, targetState);
+    if (currentState == targetState.ValueOr(ValveStateEnum::kUnknownEnumValue))
+    {
+        targetState = DataModel::NullNullable;
+        TargetState::Set(ep, targetState);
+    }
     emitValveStateChangedEvent(ep, currentState);
     return CHIP_NO_ERROR;
 }

src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

@@ -871,13 +871,7 @@ - (void)invalidate
         // tear down the subscription.  We will get no more callbacks from
         // the subscription after this point.
         std::lock_guard lock(self->_lock);
-        self->_currentReadClient = nullptr;
-        if (self->_currentSubscriptionCallback) {
-            delete self->_currentSubscriptionCallback;
-        }
-        self->_currentSubscriptionCallback = nullptr;
-
-        [self _changeInternalState:MTRInternalDeviceStateUnsubscribed];
+        [self _resetSubscription];
     }
                                      errorHandler:nil];
 
@@ -938,8 +932,8 @@ - (void)_triggerResubscribeWithReason:(NSString *)reason nodeLikelyReachable:(BO
         }
         readClientToResubscribe->TriggerResubscribeIfScheduled(reason.UTF8String);
     } else if (_internalDeviceState == MTRInternalDeviceStateSubscribing && nodeLikelyReachable) {
-        // If we have reason to suspect that the node is now reachable and we haven’t established a
-        // CASE session yet, let’s consider it to be stalled and invalidate the pairing session.
+        // If we have reason to suspect that the node is now reachable and we haven't established a
+        // CASE session yet, let's consider it to be stalled and invalidate the pairing session.
         [self._concreteController asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) {
             auto caseSessionMgr = commissioner->CASESessionMgr();
             VerifyOrDie(caseSessionMgr != nullptr);
@@ -1164,7 +1158,7 @@ - (void)_handleSubscriptionError:(NSError *)error
     [self _doHandleSubscriptionError:error];
 }
 
-- (void)_doHandleSubscriptionError:(NSError *)error
+- (void)_doHandleSubscriptionError:(nullable NSError *)error
 {
     assertChipStackLockedByCurrentThread();
 
@@ -1305,7 +1299,13 @@ - (void)_handleResubscriptionNeededWithDelay:(NSNumber *)resubscriptionDelayMs
 
     // Change our state before going async.
     [self _changeState:MTRDeviceStateUnknown];
-    [self _changeInternalState:MTRInternalDeviceStateResubscribing];
+
+    // If we have never had a subscription established, stay in the Subscribing
+    // state; don't transition to Resubscribing just because our attempt at
+    // subscribing failed.
+    if (HadSubscriptionEstablishedOnce(self->_internalDeviceState)) {
+        [self _changeInternalState:MTRInternalDeviceStateResubscribing];
+    }
 
     dispatch_async(self.queue, ^{
         [self _handleResubscriptionNeededWithDelayOnDeviceQueue:resubscriptionDelayMs];
@@ -2444,19 +2444,29 @@ - (void)_resetSubscriptionWithReasonString:(NSString *)reasonString
         MTR_LOG("%@ subscription reset disconnecting ReadClient and SubscriptionCallback", self);
 
         std::lock_guard lock(self->_lock);
-        self->_currentReadClient = nullptr;
-        if (self->_currentSubscriptionCallback) {
-            delete self->_currentSubscriptionCallback;
-        }
-        self->_currentSubscriptionCallback = nullptr;
 
-        [self _doHandleSubscriptionError:nil];
+        [self _resetSubscription];
+
         // Use nil reset delay so that this keeps existing backoff timing
         [self _doHandleSubscriptionReset:nil];
     }
                                      errorHandler:nil];
 }
 
+- (void)_resetSubscription
+{
+    assertChipStackLockedByCurrentThread();
+    os_unfair_lock_assert_owner(&_lock);
+
+    _currentReadClient = nullptr;
+    if (_currentSubscriptionCallback) {
+        delete _currentSubscriptionCallback;
+        _currentSubscriptionCallback = nullptr;
+    }
+
+    [self _doHandleSubscriptionError:nil];
+}
+
 #ifdef DEBUG
 - (void)unitTestResetSubscription
 {
@@ -4322,11 +4332,31 @@ - (BOOL)_deviceHasActiveSubscription
 
 - (void)_deviceMayBeReachable
 {
-    MTR_LOG("%@ _deviceMayBeReachable called", self);
+    MTR_LOG("%@ _deviceMayBeReachable called, resetting subscription", self);
     // TODO: This should only be allowed for thread devices
-    [_deviceController asyncDispatchToMatterQueue:^{
-        [self _triggerResubscribeWithReason:@"SPI client indicated the device may now be reachable"
-                        nodeLikelyReachable:YES];
+    [self._concreteController asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) {
+        // Reset all of our subscription/session state and re-establish it all
+        // from the start.  Reset our subscription first, before tearing
+        // down the session, so we don't have to worry about the
+        // notifications from the latter coming through async and
+        // complicating the situation.  Unfortunately, we do not want to
+        // hold the lock when destroying the session, just in case it still
+        // ends up calling into us somehow, so we have to break the work up
+        // into two separate locked sections...
+        {
+            std::lock_guard lock(self->_lock);
+            [self _clearSubscriptionPoolWork];
+            [self _resetSubscription];
+        }
+
+        auto caseSessionMgr = commissioner->CASESessionMgr();
+        VerifyOrDie(caseSessionMgr != nullptr);
+        caseSessionMgr->ReleaseSession(commissioner->GetPeerScopedId(self->_nodeID.unsignedLongLongValue));
+
+        std::lock_guard lock(self->_lock);
+        // Use _ensureSubscriptionForExistingDelegates so that the subscriptions
+        // will go through the pool as needed, not necessarily happen immediately.
+        [self _ensureSubscriptionForExistingDelegates:@"SPI client indicated the device may now be reachable"];
     } errorHandler:nil];
 }