Remove use of GetDevice() from Python controller (project-chip#10463)

* Remove use of GetDevice() from Python controller

* fix ZCLSubscribeAttribute

* address review comments

src/controller/CHIPDeviceController.cpp

@@ -531,6 +531,15 @@ void DeviceController::PersistNextKeyId()
     }
 }
 
+CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port)
+{
+    VerifyOrReturnError(GetCompressedFabricId() == peerId.GetCompressedFabricId(), CHIP_ERROR_INVALID_ARGUMENT);
+    uint16_t index = FindDeviceIndex(peerId.GetNodeId());
+    VerifyOrReturnError(index < kNumMaxActiveDevices, CHIP_ERROR_NOT_CONNECTED);
+    VerifyOrReturnError(mActiveDevices[index].GetAddress(addr, port), CHIP_ERROR_NOT_CONNECTED);
+    return CHIP_NO_ERROR;
+}
+
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 void DeviceController::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData)
 {

src/controller/CHIPDeviceController.h

@@ -216,6 +216,8 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate,
      */
     CHIP_ERROR GetDevice(NodeId deviceId, Device ** device);
 
+    CHIP_ERROR GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port);
+
     /**
      *   This function returns true if the device corresponding to `deviceId` has previously been commissioned
      *   on the fabric.

src/controller/python/ChipDeviceController-ScriptBinding.cpp

@@ -160,8 +160,6 @@ const char * pychip_Stack_ErrorToString(ChipError::StorageType err);
 const char * pychip_Stack_StatusReportToString(uint32_t profileId, uint16_t statusCode);
 void pychip_Stack_SetLogFunct(LogMessageFunct logFunct);
 
-ChipError::StorageType pychip_GetDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
-                                                chip::Controller::Device ** device);
 ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
                                                          DeviceAvailableFunc callback);
 uint64_t pychip_GetCommandSenderHandle(chip::Controller::Device * device);
@@ -246,12 +244,12 @@ ChipError::StorageType pychip_DeviceController_GetAddressAndPort(chip::Controlle
                                                                  chip::NodeId nodeId, char * outAddress, uint64_t maxAddressLen,
                                                                  uint16_t * outPort)
 {
-    Device * device;
-    CHIP_ERROR err = devCtrl->GetDevice(nodeId, &device);
-    VerifyOrReturnError(err == CHIP_NO_ERROR, err.AsInteger());
-
     Inet::IPAddress address;
-    VerifyOrReturnError(device->GetAddress(address, *outPort), CHIP_ERROR_INCORRECT_STATE.AsInteger());
+    ReturnErrorOnFailure(
+        devCtrl
+            ->GetPeerAddressAndPort(PeerId().SetCompressedFabricId(devCtrl->GetCompressedFabricId()).SetNodeId(nodeId), address,
+                                    *outPort)
+            .AsInteger());
     VerifyOrReturnError(address.ToString(outAddress, maxAddressLen), CHIP_ERROR_BUFFER_TOO_SMALL.AsInteger());
 
     return CHIP_NO_ERROR.AsInteger();
@@ -323,13 +321,21 @@ ChipError::StorageType pychip_DeviceController_ConnectIP(chip::Controller::Devic
     return devCtrl->PairDevice(nodeid, params).AsInteger();
 }
 
-ChipError::StorageType pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
+void CloseSessionCallback(Device * device, ChipError::StorageType err)
 {
-    Device * device;
-    CHIP_ERROR err = devCtrl->GetDevice(nodeid, &device);
-    VerifyOrReturnError(err == CHIP_NO_ERROR, err.AsInteger());
+    if (device != nullptr)
+    {
+        device->CloseSession();
+    }
+    if (!ChipError::IsSuccess(err))
+    {
+        ChipLogError(Controller, "Close session callback was called with an error:  %d", err);
+    }
+}
 
-    return device->CloseSession().AsInteger();
+ChipError::StorageType pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
+{
+    return pychip_GetConnectedDeviceByNodeId(devCtrl, nodeid, CloseSessionCallback);
 }
 
 ChipError::StorageType pychip_DeviceController_DiscoverAllCommissionableNodes(chip::Controller::DeviceCommissioner * devCtrl)
@@ -514,13 +520,6 @@ const char * pychip_Stack_StatusReportToString(uint32_t profileId, uint16_t stat
     return NULL;
 }
 
-ChipError::StorageType pychip_GetDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
-                                                chip::Controller::Device ** device)
-{
-    VerifyOrReturnError(devCtrl != nullptr, CHIP_ERROR_INVALID_ARGUMENT.AsInteger());
-    return devCtrl->GetDevice(nodeId, device).AsInteger();
-}
-
 namespace {
 struct GetDeviceCallbacks
 {

src/controller/python/chip/ChipDeviceCtrl.py

@@ -33,6 +33,7 @@
 from .interaction_model import delegate as im
 from .exceptions import *
 import enum
+import threading
 
 
 __all__ = ["ChipDeviceController"]
@@ -304,14 +305,38 @@ def GetFabricId(self):
     def GetClusterHandler(self):
         return self._Cluster
 
-    def ZCLSend(self, cluster, command, nodeid, endpoint, groupid, args, blocking=False):
-        device = c_void_p(None)
-        # We should really use pychip_GetConnectedDeviceByNodeId and do the
-        # command off its callback....
-        res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceByNodeId(
-            self.devCtrl, nodeid, pointer(device)))
+    def GetConnectedDeviceSync(self, nodeid):
+        returnDevice = c_void_p(None)
+        deviceAvailableCV = threading.Condition()
+
+        def DeviceAvailableCallback(device, err):
+            nonlocal returnDevice
+            nonlocal deviceAvailableCV
+            with deviceAvailableCV:
+                returnDevice = device
+                deviceAvailableCV.notify_all()
+            if err != 0:
+                print("Failed in getting the connected device: {}".format(err))
+                raise self._ChipStack.ErrorToException(err)
+
+        res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetConnectedDeviceByNodeId(
+            self.devCtrl, nodeid, _DeviceAvailableFunct(DeviceAvailableCallback)))
         if res != 0:
             raise self._ChipStack.ErrorToException(res)
+
+        # The callback might have been received synchronously (during self._ChipStack.Call()).
+        # Check if the device is already set before waiting for the callback.
+        if returnDevice == c_void_p(None):
+            with deviceAvailableCV:
+                deviceAvailableCV.wait()
+
+        if returnDevice == c_void_p(None):
+            raise self._ChipStack.ErrorToException(CHIP_ERROR_INTERNAL)
+        return returnDevice
+
+    def ZCLSend(self, cluster, command, nodeid, endpoint, groupid, args, blocking=False):
+        device = self.GetConnectedDeviceSync(nodeid)
+
         im.ClearCommandStatus(im.PLACEHOLDER_COMMAND_HANDLE)
         self._Cluster.SendCommand(
             device, cluster, command, endpoint, groupid, args, True)
@@ -321,13 +346,7 @@ def ZCLSend(self, cluster, command, nodeid, endpoint, groupid, args, blocking=Fa
         return (0, None)
 
     def ZCLReadAttribute(self, cluster, attribute, nodeid, endpoint, groupid, blocking=True):
-        device = c_void_p(None)
-        # We should really use pychip_GetConnectedDeviceByNodeId and do the
-        # read off its callback....
-        res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceByNodeId(
-            self.devCtrl, nodeid, pointer(device)))
-        if res != 0:
-            raise self._ChipStack.ErrorToException(res)
+        device = self.GetConnectedDeviceSync(nodeid)
 
         # We are not using IM for Attributes.
         res = self._Cluster.ReadAttribute(
@@ -336,13 +355,7 @@ def ZCLReadAttribute(self, cluster, attribute, nodeid, endpoint, groupid, blocki
             return im.GetAttributeReadResponse(im.DEFAULT_ATTRIBUTEREAD_APPID)
 
     def ZCLWriteAttribute(self, cluster, attribute, nodeid, endpoint, groupid, value, blocking=True):
-        device = c_void_p(None)
-        # We should really use pychip_GetConnectedDeviceByNodeId and do the
-        # write off its callback....
-        res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceByNodeId(
-            self.devCtrl, nodeid, pointer(device)))
-        if res != 0:
-            raise self._ChipStack.ErrorToException(res)
+        device = self.GetConnectedDeviceSync(nodeid)
 
         # We are not using IM for Attributes.
         res = self._Cluster.WriteAttribute(
@@ -351,13 +364,7 @@ def ZCLWriteAttribute(self, cluster, attribute, nodeid, endpoint, groupid, value
             return im.GetAttributeWriteResponse(im.DEFAULT_ATTRIBUTEWRITE_APPID)
 
     def ZCLSubscribeAttribute(self, cluster, attribute, nodeid, endpoint, minInterval, maxInterval, blocking=True):
-        device = c_void_p(None)
-        # We should really use pychip_GetConnectedDeviceByNodeId and do the
-        # SubscribeAttribute off its callback....
-        res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceByNodeId(
-            self.devCtrl, nodeid, pointer(device)))
-        if res != 0:
-            raise self._ChipStack.ErrorToException(res)
+        device = self.GetConnectedDeviceSync(nodeid)
 
         commandSenderHandle = self._dmLib.pychip_GetCommandSenderHandle(device)
         im.ClearCommandStatus(commandSenderHandle)
@@ -473,13 +480,9 @@ def _InitLib(self):
                 c_uint64, c_uint64]
             self._dmLib.pychip_Resolver_ResolveNode.restype = c_uint32
 
-            self._dmLib.pychip_GetDeviceByNodeId.argtypes = [
-                c_void_p, c_uint64, POINTER(c_void_p)]
-            self._dmLib.pychip_GetDeviceByNodeId.restype = c_uint32
-
             self._dmLib.pychip_GetConnectedDeviceByNodeId.argtypes = [
                 c_void_p, c_uint64, _DeviceAvailableFunct]
-            self._dmLib.pychip_GetDeviceByNodeId.restype = c_uint32
+            self._dmLib.pychip_GetConnectedDeviceByNodeId.restype = c_uint32
 
             self._dmLib.pychip_DeviceCommissioner_CloseBleConnection.argtypes = [
                 c_void_p]