Merge branch 'master' into granbery/preset_python

Author: hasty

.github/workflows/qemu.yaml

@@ -27,7 +27,7 @@ concurrency:
 
 env:
     CHIP_NO_LOG_TIMESTAMPS: true
-    
+
 jobs:
 
     qemu-esp32:
@@ -75,12 +75,7 @@ jobs:
         name: Tizen
 
         runs-on: ubuntu-latest
-        # NOTE: job temporarely disabled as it seems flaky. The flake does not result in usable
-        #       logs so the current theory is that we run out of space. This is unusual as
-        #       larger docker images succeed at bootstrap, however it needs more investigation
-        #       to detect an exact/real root cause.
-        if: false
-        # if: github.actor != 'restyled-io[bot]'
+        if: github.actor != 'restyled-io[bot]'
 
         container:
             image: ghcr.io/project-chip/chip-build-tizen-qemu:54

build/config/compiler/BUILD.gn

@@ -334,7 +334,11 @@ config("warnings_third_party") {
 }
 
 config("symbols_default") {
-  cflags = [ "-g${symbol_level}" ]
+  if (strip_symbols) {
+    cflags = [ "-s" ]
+  } else {
+    cflags = [ "-g${symbol_level}" ]
+  }
 }
 
 config("std_default") {

build/config/compiler/compiler.gni

@@ -30,11 +30,11 @@ declare_args() {
   symbol_level = 2
 
   # Enable position independent code (-fPIC).
-  enable_pic =
-      current_os == "linux" || current_os == "mac" || current_os == "android"
+  enable_pic = current_os == "linux" || current_os == "mac" ||
+               current_os == "android" || current_os == "tizen"
 
   # Enable position independent executables (-pie).
-  enable_pie = current_os == "linux"
+  enable_pie = current_os == "linux" || current_os == "tizen"
 
   # Remove unwind tables from the binary to save space.
   exclude_unwind_tables = current_os != "android"
@@ -48,6 +48,9 @@ declare_args() {
   # enable libfuzzer
   is_libfuzzer = false
 
+  # Remove all symbol table and relocation information from the binary.
+  strip_symbols = false
+
   # Generate code coverage analysis artifacts when enabled.
   use_coverage = false
 

examples/fabric-bridge-app/fabric-bridge-common/src/ZCLCallbacks.cpp

@@ -26,6 +26,7 @@ using namespace ::chip;
 using namespace ::chip::app::Clusters;
 
 #define ZCL_DESCRIPTOR_CLUSTER_REVISION (1u)
+#define ZCL_ADMINISTRATOR_COMMISSIONING_CLUSTER_REVISION (1u)
 #define ZCL_BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_REVISION (2u)
 #define ZCL_BRIDGED_DEVICE_BASIC_INFORMATION_FEATURE_MAP (0u)
 
@@ -37,9 +38,14 @@ Protocols::InteractionModel::Status emberAfExternalAttributeReadCallback(Endpoin
     AttributeId attributeId = attributeMetadata->attributeId;
 
     BridgedDevice * dev = BridgeDeviceMgr().GetDevice(endpoint);
-    if (dev != nullptr && clusterId == app::Clusters::BridgedDeviceBasicInformation::Id)
+    if (dev == nullptr)
     {
-        using namespace app::Clusters::BridgedDeviceBasicInformation::Attributes;
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
+    if (clusterId == BridgedDeviceBasicInformation::Id)
+    {
+        using namespace BridgedDeviceBasicInformation::Attributes;
         ChipLogProgress(NotSpecified, "HandleReadBridgedDeviceBasicAttribute: attrId=%d, maxReadLength=%d", attributeId,
                         maxReadLength);
 
@@ -69,6 +75,21 @@ Protocols::InteractionModel::Status emberAfExternalAttributeReadCallback(Endpoin
         return Protocols::InteractionModel::Status::Success;
     }
 
+    if (clusterId == AdministratorCommissioning::Id)
+    {
+        // TODO(#34791) This is a workaround to prevent crash. CADMIN is still reading incorrect
+        // Attribute values on dynamic endpoint as it only reads the root node and not the actual bridge
+        // device we are representing here, when addressing the issue over there we can more easily
+        // resolve this workaround.
+        if ((attributeId == AdministratorCommissioning::Attributes::ClusterRevision::Id) && (maxReadLength == 2))
+        {
+            uint16_t rev = ZCL_ADMINISTRATOR_COMMISSIONING_CLUSTER_REVISION;
+            memcpy(buffer, &rev, sizeof(rev));
+            return Protocols::InteractionModel::Status::Success;
+        }
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     return Protocols::InteractionModel::Status::Failure;
 }
 

examples/network-manager-app/linux/include/CHIPProjectAppConfig.h

@@ -18,7 +18,7 @@
 
 #pragma once
 
-#define CHIP_DEVICE_CONFIG_DEVICE_TYPE 0xFFF10010 // TODO: ID-TBD
+#define CHIP_DEVICE_CONFIG_DEVICE_TYPE 144 // 0x0090 Network Infrastructure Manager
 #define CHIP_DEVICE_CONFIG_DEVICE_NAME "Network Infrastructure Manager"
 
 // Inherit defaults from config/standalone/CHIPProjectConfig.h

scripts/build/builders/tizen.py

@@ -117,6 +117,10 @@ def __init__(self,
 
         if app == TizenApp.TESTS:
             self.extra_gn_options.append('chip_build_tests=true')
+            # Tizen test driver creates ISO image with all unit test files. So,
+            # it uses twice as much space as regular build. Due to CI storage
+            # limitations, we need to strip debug symbols from executables.
+            self.extra_gn_options.append('strip_symbols=true')
             self.build_command = 'check'
 
         if not enable_ble:

src/app/clusters/color-control-server/color-control-server.cpp

@@ -3113,15 +3113,15 @@ void ColorControlServer::levelControlColorTempChangeCommand(EndpointId endpoint)
  * - When it changes from null to any other value and vice versa. (Implicit to the QuieterReportingAttribute class)
  *
  * The QuietReportAttribute class is updated with the new value and when the report conditions are met,
- * this function will return MarkAttributeDirty::kIfChanged.
+ * this function will return MarkAttributeDirty::kYes.
  * It is expected that the user will use this return value to trigger a reporting mechanism for the attribute with the new value
  * (Which was updated in the quietReporter)
  *
  * @param quietReporter: The QuieterReportingAttribute<TYPE> object for the attribute to update.
  * @param newValue: Value to update the attribute with
  * @param isStartOrEndOfTransition: Boolean that indicatse whether the update is occurring at the start or end of a level transition
- * @return MarkAttributeDirty::kIfChanged when the attribute must be maredk dirty and be reported. MarkAttributeDirty::kNo when it
- * when it no report is needed.
+ * @return MarkAttributeDirty::kYes when the attribute must be marked dirty and be reported. MarkAttributeDirty::kNo when
+ * no report is needed.
  */
 template <typename Q, typename V>
 MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingAttribute<Q> & quietReporter, V newValue,
@@ -3132,7 +3132,7 @@ MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingA
 
     if (isStartOrEndOfTransition)
     {
-        // At the start or end of the movement/transition we must report
+        // At the start or end of the movement/transition we must report if the value changed
         auto predicate = [](const typename QuieterReportingAttribute<Q>::SufficientChangePredicateCandidate &) -> bool {
             return true;
         };
@@ -3155,7 +3155,7 @@ MarkAttributeDirty ColorControlServer::SetQuietReportAttribute(QuieterReportingA
         dirtyState                                   = quietReporter.SetValue(newValue, now, predicate);
     }
 
-    return (dirtyState == AttributeDirtyState::kMustReport) ? MarkAttributeDirty::kIfChanged : MarkAttributeDirty::kNo;
+    return (dirtyState == AttributeDirtyState::kMustReport) ? MarkAttributeDirty::kYes : MarkAttributeDirty::kNo;
 }
 
 /*
@@ -3180,7 +3180,7 @@ Status ColorControlServer::SetQuietReportRemainingTime(EndpointId endpoint, uint
     // - kMarkDirtyOnIncrement : When the value increases.
     if (quietRemainingTime[epIndex].SetValue(newRemainingTime, now) == AttributeDirtyState::kMustReport)
     {
-        markDirty = MarkAttributeDirty::kIfChanged;
+        markDirty = MarkAttributeDirty::kYes;
     }
 
     return Attributes::RemainingTime::Set(endpoint, quietRemainingTime[epIndex].value().Value(), markDirty);

src/app/clusters/level-control/level-control.cpp

@@ -545,7 +545,7 @@ static void writeRemainingTime(EndpointId endpoint, uint16_t remainingTimeMs)
             markDirty = MarkAttributeDirty::kYes;
         }
 
-        Attributes::RemainingTime::Set(endpoint, state->quietRemainingTime.value().ValueOr(0), markDirty);
+        Attributes::RemainingTime::Set(endpoint, state->quietRemainingTime.value().Value(), markDirty);
     }
 #endif // IGNORE_LEVEL_CONTROL_CLUSTER_LEVEL_CONTROL_REMAINING_TIME
 }

src/controller/CommissionerDiscoveryController.cpp

@@ -589,12 +589,36 @@ void CommissionerDiscoveryController::Cancel()
         return;
     }
     UDCClientState * client = mUdcServer->GetUDCClients().FindUDCClientState(mCurrentInstance);
-    if (client == nullptr || client->GetUDCClientProcessingState() != UDCClientProcessingState::kPromptingUser)
+
+    if (client == nullptr)
+    {
+        ChipLogError(AppServer, "UX Cancel: client not found");
+        return;
+    }
+
+    auto state = client->GetUDCClientProcessingState();
+
+    bool isCancelableState =
+        (state == UDCClientProcessingState::kPromptingUser || state == UDCClientProcessingState::kObtainingOnboardingPayload ||
+         state == UDCClientProcessingState::kWaitingForCommissionerPasscodeReady);
+
+    if (!isCancelableState)
     {
-        ChipLogError(AppServer, "UX Cancel: invalid state for cancel");
+        ChipLogError(AppServer, "UX Cancel: invalid state for cancel, state: %hhu", static_cast<uint8_t>(state));
         return;
     }
+
     client->SetUDCClientProcessingState(UDCClientProcessingState::kUserDeclined);
+
+    if (state == UDCClientProcessingState::kObtainingOnboardingPayload ||
+        state == UDCClientProcessingState::kWaitingForCommissionerPasscodeReady)
+    {
+        ChipLogDetail(AppServer, "UX Cancel: user cancelled entering PIN code, sending CDC");
+        CommissionerDeclaration cd;
+        cd.SetCancelPasscode(true);
+        mUdcServer->SendCDCMessage(cd, Transport::PeerAddress::UDP(client->GetPeerAddress().GetIPAddress(), client->GetCdPort()));
+    }
+
     mPendingConsent = false;
     ResetState();
 }

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -142,16 +142,6 @@ - (BOOL)callDelegateSynchronouslyWithBlock:(void (^)(id<MTRDeviceDelegate>))bloc
 #endif
 @end
 
-NSNumber * MTRClampedNumber(NSNumber * aNumber, NSNumber * min, NSNumber * max)
-{
-    if ([aNumber compare:min] == NSOrderedAscending) {
-        return min;
-    } else if ([aNumber compare:max] == NSOrderedDescending) {
-        return max;
-    }
-    return aNumber;
-}
-
 /* BEGIN DRAGONS: Note methods here cannot be renamed, and are used by private callers, do not rename, remove or modify behavior here */
 
 @interface NSObject (MatterPrivateForInternalDragonsDoNotFeed)
@@ -252,10 +242,6 @@ @implementation MTRDeviceClusterData {
     NSMutableDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _attributes;
 }
 
-static NSString * const sDataVersionKey = @"dataVersion";
-static NSString * const sAttributesKey = @"attributes";
-static NSString * const sLastInitialSubscribeLatencyKey = @"lastInitialSubscribeLatency";
-
 - (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute
 {
     _attributes[attribute] = value;
@@ -498,6 +484,15 @@ @implementation MTRDevice {
     NSMutableSet<MTRDeviceDelegateInfo *> * _delegates;
 }
 
+- (instancetype)initForSubclasses
+{
+    if (self = [super init]) {
+        // nothing, as superclass of MTRDevice is NSObject
+    }
+
+    return self;
+}
+
 - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
 {
     if (self = [super init]) {
@@ -1193,23 +1188,21 @@ - (void)_handleSubscriptionEstablished
 {
     os_unfair_lock_lock(&self->_lock);
 
-    // We have completed the subscription work - remove from the subscription pool.
-    [self _clearSubscriptionPoolWork];
-
-    // reset subscription attempt wait time when subscription succeeds
-    _lastSubscriptionAttemptWait = 0;
-    if (HadSubscriptionEstablishedOnce(_internalDeviceState)) {
-        [self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
-    } else {
-        MATTER_LOG_METRIC_END(kMetricMTRDeviceInitialSubscriptionSetup, CHIP_NO_ERROR);
-        [self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
+    // If subscription had reset since this handler was scheduled, do not execute "established" logic below
+    if (!HaveSubscriptionEstablishedRightNow(_internalDeviceState)) {
+        MTR_LOG("%@ _handleSubscriptionEstablished run with internal state %lu - skipping subscription establishment logic", self, static_cast<unsigned long>(_internalDeviceState));
+        return;
     }
 
-    [self _changeState:MTRDeviceStateReachable];
+    // We have completed the subscription work - remove from the subscription pool.
+    [self _clearSubscriptionPoolWork];
 
     // No need to monitor connectivity after subscription establishment
     [self _stopConnectivityMonitoring];
 
+    // reset subscription attempt wait time when subscription succeeds
+    _lastSubscriptionAttemptWait = 0;
+
     auto initialSubscribeStart = _initialSubscribeStart;
     // We no longer need to track subscribe latency for this device.
     _initialSubscribeStart = nil;
@@ -2476,6 +2469,19 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
                                },
                                ^(void) {
                                    MTR_LOG("%@ got subscription established", self);
+                                   std::lock_guard lock(self->_lock);
+
+                                   // First synchronously change state
+                                   if (HadSubscriptionEstablishedOnce(self->_internalDeviceState)) {
+                                       [self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
+                                   } else {
+                                       MATTER_LOG_METRIC_END(kMetricMTRDeviceInitialSubscriptionSetup, CHIP_NO_ERROR);
+                                       [self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
+                                   }
+
+                                   [self _changeState:MTRDeviceStateReachable];
+
+                                   // Then async work that shouldn't be performed on the matter queue
                                    dispatch_async(self.queue, ^{
                                        // OnSubscriptionEstablished
                                        [self _handleSubscriptionEstablished];

src/darwin/Framework/CHIP/MTRDeviceController.mm

@@ -31,6 +31,7 @@
 #import "MTRDeviceControllerLocalTestStorage.h"
 #import "MTRDeviceControllerStartupParams.h"
 #import "MTRDeviceControllerStartupParams_Internal.h"
+#import "MTRDevice_Concrete.h"
 #import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
 #import "MTRKeypair.h"
@@ -988,7 +989,7 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
 {
     os_unfair_lock_assert_owner(&_deviceMapLock);
 
-    MTRDevice * deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
+    MTRDevice * deviceToReturn = [[MTRDevice_Concrete alloc] initWithNodeID:nodeID controller:self];
     // If we're not running, don't add the device to our map.  That would
     // create a cycle that nothing would break.  Just return the device,
     // which will be in exactly the state it would be in if it were created

src/darwin/Framework/CHIP/MTRDevice_Concrete.h

@@ -0,0 +1,27 @@
+/**
+ *
+ *    Copyright (c) 2022-2023 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 <Foundation/Foundation.h>
+#import <Matter/MTRDevice.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface MTRDevice_Concrete : MTRDevice
+
+@end
+
+NS_ASSUME_NONNULL_END