update

src/controller/python/BUILD.gn

@@ -253,7 +253,6 @@ chip_python_wheel_action("chip-core") {
         "chip/discovery/library_handle.py",
         "chip/discovery/types.py",
         "chip/exceptions/__init__.py",
-        "chip/icd/__init__.py",
         "chip/interaction_model/__init__.py",
         "chip/interaction_model/delegate.py",
         "chip/internal/__init__.py",
@@ -309,7 +308,6 @@ chip_python_wheel_action("chip-core") {
     "chip.credentials",
     "chip.crypto",
     "chip.utils",
-    "chip.icd",
     "chip.discovery",
     "chip.exceptions",
     "chip.internal",

src/controller/python/ChipDeviceController-ScriptBinding.cpp

@@ -152,9 +152,8 @@ PyChipError pychip_DeviceController_SetDSTOffset(int32_t offset, uint64_t validS
 PyChipError pychip_DeviceController_SetDefaultNtp(const char * defaultNTP);
 PyChipError pychip_DeviceController_SetTrustedTimeSource(chip::NodeId nodeId, chip::EndpointId endpoint);
 PyChipError pychip_DeviceController_SetCheckMatchingFabric(bool check);
-PyChipError pychip_DeviceController_SetIcdRegistrationParameters(chip::Controller::DeviceCommissioner * devCtrl, bool enabled,
-                                                                 uint8_t * icdSymmetricKeyOrNull, uint64_t icdCheckInNodeIdOrZero,
-                                                                 uint64_t icdMonitoredSubjectOrZero, uint32_t icdStayActiveMsec);
+struct IcdRegistrationParameters;
+PyChipError pychip_DeviceController_SetIcdRegistrationParameters(bool enabled, const IcdRegistrationParameters * params);
 PyChipError pychip_DeviceController_ResetCommissioningParameters();
 PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
 PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::DeviceCommissioner * devCtrl, const char * peerAddrStr,
@@ -580,42 +579,51 @@ PyChipError pychip_DeviceController_SetCheckMatchingFabric(bool check)
     return ToPyChipError(CHIP_NO_ERROR);
 }
 
-PyChipError pychip_DeviceController_SetIcdRegistrationParameters(chip::Controller::DeviceCommissioner * devCtrl, bool enabled,
-                                                                 uint8_t * icdSymmetricKeyOrNull, uint64_t icdCheckInNodeIdOrZero,
-                                                                 uint64_t icdMonitoredSubjectOrZero, uint32_t icdStayActiveMsec)
+struct IcdRegistrationParameters
+{
+    uint8_t * symmetricKey;
+    size_t symmetricKeyLength;
+    uint64_t checkInNodeId;
+    uint64_t monitoredSubject;
+    uint32_t stayActiveMsec;
+};
+
+PyChipError pychip_DeviceController_SetIcdRegistrationParameters(bool enabled, const IcdRegistrationParameters * params)
 {
     if (!enabled)
     {
+        sCommissioningParameters.SetICDRegistrationStrategy(ICDRegistrationStrategy::kIgnore);
         return ToPyChipError(CHIP_NO_ERROR);
     }
 
-    sCommissioningParameters.SetICDRegistrationStrategy(ICDRegistrationStrategy::kBeforeComplete);
-
-    if (icdSymmetricKeyOrNull != nullptr)
+    if (params == nullptr)
     {
-        memcpy(sICDSymmetricKey, icdSymmetricKeyOrNull, sizeof(sICDSymmetricKey));
+        return ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT);
     }
-    else
+
+    if (params->symmetricKey == nullptr || params->symmetricKeyLength != sizeof(sICDSymmetricKey))
     {
-        chip::Crypto::DRBG_get_bytes(sICDSymmetricKey, sizeof(sICDSymmetricKey));
+        return ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT);
     }
-    if (icdCheckInNodeIdOrZero == 0)
+
+    if (params->checkInNodeId == 0)
     {
-        icdCheckInNodeIdOrZero = devCtrl->GetNodeId();
+        return ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT);
     }
-    if (icdMonitoredSubjectOrZero == 0)
+    if (params->monitoredSubject == 0)
     {
-        icdMonitoredSubjectOrZero = icdCheckInNodeIdOrZero;
+        return ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT);
     }
-    // These Optionals must have values now.
-    // The commissioner will verify these values.
+
+    memcpy(sICDSymmetricKey, params->symmetricKey, sizeof(sICDSymmetricKey));
     sCommissioningParameters.SetICDSymmetricKey(ByteSpan(sICDSymmetricKey));
-    if (icdStayActiveMsec != 0)
+    if (params->stayActiveMsec != 0)
     {
-        sCommissioningParameters.SetICDStayActiveDurationMsec(icdStayActiveMsec);
+        sCommissioningParameters.SetICDStayActiveDurationMsec(params->stayActiveMsec);
     }
-    sCommissioningParameters.SetICDCheckInNodeId(icdCheckInNodeIdOrZero);
-    sCommissioningParameters.SetICDMonitoredSubject(icdMonitoredSubjectOrZero);
+    sCommissioningParameters.SetICDCheckInNodeId(params->checkInNodeId);
+    sCommissioningParameters.SetICDMonitoredSubject(params->monitoredSubject);
+    sCommissioningParameters.SetICDRegistrationStrategy(ICDRegistrationStrategy::kBeforeComplete);
 
     return ToPyChipError(CHIP_NO_ERROR);
 }

src/controller/python/chip/ChipDeviceCtrl.py

@@ -35,18 +35,19 @@
 import enum
 import json
 import logging
+import secrets
 import threading
 import time
 import typing
-from ctypes import (CDLL, CFUNCTYPE, POINTER, byref, c_bool, c_char, c_char_p, c_int, c_int32, c_size_t, c_uint8, c_uint16,
-                    c_uint32, c_uint64, c_void_p, create_string_buffer, pointer, py_object, resize, string_at)
+from ctypes import (CDLL, CFUNCTYPE, POINTER, Structure, byref, c_bool, c_char, c_char_p, c_int, c_int32, c_size_t, c_uint8,
+                    c_uint16, c_uint32, c_uint64, c_void_p, create_string_buffer, pointer, py_object, resize, string_at)
 from dataclasses import dataclass
 
 import dacite
 
 from . import FabricAdmin
 from . import clusters as Clusters
-from . import discovery, icd
+from . import discovery
 from .clusters import Attribute as ClusterAttribute
 from .clusters import ClusterObjects as ClusterObjects
 from .clusters import Command as ClusterCommand
@@ -103,10 +104,17 @@ class NOCChain:
 
 @dataclass
 class ICDRegistrationParameters:
-    symmetricKey: typing.Optional[bytes] = None
-    checkInNodeId: typing.Optional[int] = None
-    monitoredSubject: typing.Optional[int] = None
-    stayActiveMs: typing.Optional[int] = None
+    symmetricKey: typing.Optional[bytes]
+    checkInNodeId: typing.Optional[int]
+    monitoredSubject: typing.Optional[int]
+    stayActiveMs: typing.Optional[int]
+
+    class CStruct(Structure):
+        _fields_ = [('symmetricKey', c_char_p), ('symmetricKeyLength', c_size_t), ('checkInNodeId',
+                                                                                   c_uint64), ('monitoredSubject', c_uint64), ('stayActiveMsec', c_uint32)]
+
+    def to_c(self):
+        return ICDRegistrationParameters.CStruct(self.symmetricKey, len(self.symmetricKey), self.checkInNodeId, self.monitoredSubject, self.stayActiveMs)
 
 
 @_DeviceAvailableCallbackFunct
@@ -134,6 +142,74 @@ def _IssueNOCChainCallbackPythonCallback(devCtrl, status: PyChipError, noc: c_vo
         nocChain = NOCChain(nocBytes, icacBytes, rcacBytes, ipkBytes, adminSubject)
     devCtrl.NOCChainCallback(nocChain)
 
+
+# Methods for ICD
+class ScopedNodeId(Structure):
+    _fields_ = [("nodeId", c_uint64), ("fabricIndex", c_uint8)]
+
+    def __hash__(self):
+        return self.nodeId << 8 | self.fabricIndex
+
+    def __str__(self):
+        return f"({self.fabricIndex}:{self.nodeId:16x})"
+
+
+_OnCheckInCompleteFunct = CFUNCTYPE(None, ScopedNodeId)
+
+_OnCheckInCompleteWaitListLock = threading.Lock()
+_OnCheckInCompleteWaitList = dict()
+
+
+@_OnCheckInCompleteFunct
+def _OnCheckInComplete(scopedNodeId: ScopedNodeId):
+    callbacks = []
+    with _OnCheckInCompleteWaitListLock:
+        callbacks = list(_OnCheckInCompleteWaitList.get(scopedNodeId, set()))
+
+    for callback in callbacks:
+        callback(scopedNodeId)
+
+
+def RegisterOnActiveCallback(scopedNodeId: ScopedNodeId, callback: Callable[None, [OnCheckInCompleteParams]]):
+    ''' Registers a callback when the device with given (fabric index, node id) becomes active.
+
+    Does nothing if the callback is already registered.
+    '''
+    with _OnCheckInCompleteWaitListLock:
+        waitList = _OnCheckInCompleteWaitList.get(scopedNodeId, set())
+        waitList.add(callback)
+        _OnCheckInCompleteWaitList[scopedNodeId] = waitList
+
+
+def UnregisterOnActiveCallback(scopedNodeId: ScopedNodeId, callback: Callable[None, [OnCheckInCompleteParams]]):
+    ''' Unregisters a callback when the device with given (fabric index, node id) becomes active.
+
+    Does nothing if the callback has not been registered.
+    '''
+    with _OnCheckInCompleteWaitListLock:
+        _OnCheckInCompleteWaitList.get(scopedNodeId, set()).remove(callback)
+
+
+async def WaitForCheckIn(scopedNodeId: ScopedNodeId, timeoutSeconds: float):
+    ''' Waits for a device becomes active.
+
+    Returns:
+        - A future, completes when the device becomes active.
+    '''
+    eventLoop = asyncio.get_running_loop()
+    future = eventLoop.create_future()
+
+    def OnCheckInCallback(nodeid):
+        eventLoop.call_soon_threadsafe(lambda: future.done() or future.set_result(None))
+
+    RegisterOnActiveCallback(scopedNodeId, OnCheckInCallback)
+
+    try:
+        async with asyncio.timeout(timeoutSeconds):
+            await future
+    finally:
+        UnregisterOnActiveCallback(scopedNodeId, OnCheckInCallback)
+
 # This is a fix for WEAV-429. Jay Logue recommends revisiting this at a later
 # date to allow for truly multiple instances so this is temporary.
 
@@ -280,7 +356,7 @@ def HandleCommissioningComplete(nodeid, err):
             else:
                 logging.warning("Failed to commission: {}".format(err))
 
-            self.DisableIcdRegistration()
+            self.DisableICDRegistration()
             self.state = DCState.IDLE
             self._ChipStack.callbackRes = err
             self._ChipStack.commissioningEventRes = err
@@ -867,7 +943,7 @@ def deviceAvailable(self, device, err):
 
         return DeviceProxyWrapper(returnDevice, self._dmLib)
 
-    async def WaitForActive(self, nodeid, stayActiveDurationMs=30000):
+    async def WaitForActive(self, nodeid, *, timeoutSeconds=30.0, stayActiveDurationMs=30000):
         ''' Waits a LIT ICD device to become active. Will send a StayActive command to the device on active to allow human operations.
 
         nodeId: Node ID of the LID ICD
@@ -876,7 +952,7 @@ async def WaitForActive(self, nodeid, stayActiveDurationMs=30000):
         Returns:
             - StayActiveResponse on success
         '''
-        await icd.WaitForCheckIn(self._fabricIndex, nodeid)
+        await WaitForCheckIn(ScopedNodeId(self._fabricIndex, nodeid), timeoutSeconds=timeoutSeconds)
         return await self.SendCommand(nodeid, 0, GeneratedObjects.IcdManagement.Commands.StayActiveRequest(stayActiveDuration=stayActiveDurationMilliseconds))
 
     async def GetConnectedDevice(self, nodeid, allowPASE: bool = True, timeoutMs: int = None):
@@ -1668,7 +1744,7 @@ def _InitLib(self):
 
             self._dmLib.pychip_DeviceController_SetIcdRegistrationParameters.restype = PyChipError
             self._dmLib.pychip_DeviceController_SetIcdRegistrationParameters.argtypes = [
-                c_void_p, c_bool, c_char_p, c_uint64, c_uint64, c_uint32
+                c_bool, c_void_p
             ]
 
             self._dmLib.pychip_DeviceController_ResetCommissioningParameters.restype = PyChipError
@@ -1874,6 +1950,11 @@ def _InitLib(self):
             self._dmLib.pychip_DeviceController_SetIpk.argtypes = [c_void_p, POINTER(c_char), c_size_t]
             self._dmLib.pychip_DeviceController_SetIpk.restype = PyChipError
 
+            self._dmLib.pychip_CheckInDelegate_SetOnCheckInCompleteCallback.restype = None
+            self._dmLib.pychip_CheckInDelegate_SetOnCheckInCompleteCallback.argtypes = [_OnCheckInCompleteFunct]
+
+            self._dmLib.pychip_CheckInDelegate_SetOnCheckInCompleteCallback(_OnCheckInComplete)
+
 
 class ChipDeviceController(ChipDeviceControllerBase):
     ''' The ChipDeviceCommissioner binding, named as ChipDeviceController
@@ -2022,33 +2103,36 @@ def SetCheckMatchingFabric(self, check: bool):
             lambda: self._dmLib.pychip_DeviceController_SetCheckMatchingFabric(check)
         ).raise_on_error()
 
-    def EnableICDRegistration(self, parameters: typing.Optional[ICDRegistrationParameters] = None):
+    def GenerateICDRegistrationParameters(self):
+        ''' Generates ICD registration parameters for this controller. '''
+        return ICDRegistrationParameters(
+            secrets.token_bytes(16),
+            self._nodeId,
+            self._nodeId,
+            30)
+
+    def EnableICDRegistration(self, parameters: ICDRegistrationParameters):
         ''' Enables ICD registration for the following commissioning session.
 
         Args:
             parameters: A ICDRegistrationParameters for the parameters used for ICD registration, or None for default arguments.
         '''
         if parameters is None:
-            parameters = ICDRegistrationParameters()
-        if parameters.symmetricKey is not None:
-            if len(symmetricKey) != 16:
-                raise ValueError("symmetricKey should be 16 bytes")
-        if parameters.checkInNodeId is None:
-            parameters.checkInNodeId = self._nodeId
-        if parameters.monitoredSubject is None:
-            parameters.monitoredSubject = checkInNodeId
+            raise ValueError("ICD registration parameter required.")
+        if len(parameters.symmetricKey) != 16:
+            raise ValueError("symmetricKey should be 16 bytes")
 
         self.CheckIsActive()
         self._ChipStack.Call(
             lambda: self._dmLib.pychip_DeviceController_SetIcdRegistrationParameters(
-                self.devCtrl, True, parameters.symmetricKey, parameters.checkInNodeId, parameters.monitoredSubject, parameters.stayActiveMs)
+                True, pointer(parameters.to_c()))
         ).raise_on_error()
 
     def DisableICDRegistration(self):
         ''' Disables ICD registration. '''
         self.CheckIsActive()
         self._ChipStack.Call(
-            lambda: self._dmLib.pychip_DeviceController_SetIcdRegistrationParameters(self.devCtrl, False, None, 0, 0, 0)
+            lambda: self._dmLib.pychip_DeviceController_SetIcdRegistrationParameters(False, None)
         ).raise_on_error()
 
     def GetFabricCheckResult(self) -> int:

src/controller/python/chip/icd/PyChipCheckInDelegate.cpp

@@ -18,6 +18,8 @@
 
 #include "PyChipCheckInDelegate.h"
 
+#include <controller/python/chip/native/ChipMainLoopWork.h>
+
 using namespace ::chip;
 using namespace ::chip::app;
 
@@ -29,11 +31,12 @@ void PyChipCheckInDelegate::OnCheckInComplete(const ICDClientInfo & clientInfo)
 
     if (mCallback != nullptr)
     {
-        mCallback(clientInfo.peer_node.GetFabricIndex(), clientInfo.peer_node.GetNodeId());
+        mCallback(clientInfo.peer_node);
     }
 }
 
 extern "C" void pychip_CheckInDelegate_SetOnCheckInCompleteCallback(PyChipCheckInDelegate::OnCheckInCompleteCallback * callback)
 {
-    PyChipCheckInDelegate::GetInstance().SetOnCheckInCompleteCallback(callback);
+    chip::MainLoopWork::ExecuteInMainLoop(
+        [callback]() { PyChipCheckInDelegate::GetInstance().SetOnCheckInCompleteCallback(callback); });
 }

src/controller/python/chip/icd/PyChipCheckInDelegate.h

@@ -23,7 +23,7 @@
 class PyChipCheckInDelegate : public chip::app::DefaultCheckInDelegate
 {
 public:
-    using OnCheckInCompleteCallback = void(uint8_t fabricIndex, uint64_t nodeId);
+    using OnCheckInCompleteCallback = void(chip::ScopedNodeId);
 
     virtual ~PyChipCheckInDelegate() = default;