[Python] Make Commissioning APIs more pythonic and consistent

This commit makes the commissioning APIs more pythonic and consistent
by not returning PyChipError but simply raising ChipStackError
exceptions on errors instead.

The return value instead returns the effectively assigned node ID
as defined by the NOC. If the SDK ends up generating that NOC, it
will use the thing passed to PairDevice, so those will match with
what is provided when calling the commissioning API.

Author: agners

src/controller/python/chip/ChipDeviceCtrl.py

@@ -230,11 +230,14 @@ class CommissionableNode(discovery.CommissionableNode):
     def SetDeviceController(self, devCtrl: 'ChipDeviceController'):
         self._devCtrl = devCtrl
 
-    def Commission(self, nodeId: int, setupPinCode: int) -> PyChipError:
+    def Commission(self, nodeId: int, setupPinCode: int) -> int:
         ''' Commission the device using the device controller discovered this device.
 
         nodeId: The nodeId commissioned to the device
         setupPinCode: The setup pin code of the device
+
+        Returns:
+            - Effective Node ID of the device (as defined by the assigned NOC)
         '''
         return self._devCtrl.CommissionOnNetwork(
             nodeId, setupPinCode, filterType=discovery.FilterType.INSTANCE_NAME, filter=self.instanceName)
@@ -360,7 +363,10 @@ def HandleCommissioningComplete(nodeId: int, err: PyChipError):
                 logging.exception("HandleCommissioningComplete called unexpectedly")
                 return
 
-            self._commissioning_complete_future.set_result(err)
+            if err.is_success:
+                self._commissioning_complete_future.set_result(nodeId)
+            else:
+                self._commissioning_complete_future.set_exception(err.to_exception())
 
         def HandleFabricCheck(nodeId):
             self.fabricCheckNodeId = nodeId
@@ -408,14 +414,17 @@ def HandlePASEEstablishmentComplete(err: PyChipError):
                 # During Commissioning, HandlePASEEstablishmentComplete will also be called.
                 # Only complete the future if PASE session establishment failed.
                 if not err.is_success:
-                    self._commissioning_complete_future.set_result(err)
+                    self._commissioning_complete_future.set_exception(err.to_exception())
                 return
 
             if self._pase_establishment_complete_future is None:
                 logging.exception("HandlePASEEstablishmentComplete called unexpectedly")
                 return
 
-            self._pase_establishment_complete_future.set_result(err)
+            if err.is_success:
+                self._pase_establishment_complete_future.set_result(None)
+            else:
+                self._pase_establishment_complete_future.set_exception(err.to_exception())
 
         self.pairingDelegate = pairingDelegate
         self.devCtrl = devCtrl
@@ -533,7 +542,12 @@ def IsConnected(self):
                 self.devCtrl)
         )
 
-    def ConnectBLE(self, discriminator: int, setupPinCode: int, nodeid: int, isShortDiscriminator: bool = False) -> PyChipError:
+    def ConnectBLE(self, discriminator: int, setupPinCode: int, nodeid: int, isShortDiscriminator: bool = False) -> int:
+        """Connect to a BLE device using the given discriminator and setup pin code.
+
+        Returns:
+            - Effective Node ID of the device (as defined by the assigned NOC)
+        """
         self.CheckIsActive()
 
         self._commissioning_complete_future = concurrent.futures.Future()
@@ -545,11 +559,7 @@ def ConnectBLE(self, discriminator: int, setupPinCode: int, nodeid: int, isShort
                     self.devCtrl, discriminator, isShortDiscriminator, setupPinCode, nodeid)
             ).raise_on_error()
 
-            # TODO: Change return None. Only returning on success is not useful.
-            # but that is what the previous implementation did.
-            res = self._commissioning_complete_future.result()
-            res.raise_on_error()
-            return res
+            return self._commissioning_complete_future.result()
         finally:
             self._commissioning_complete_future = None
 
@@ -595,7 +605,7 @@ def CloseSession(self, nodeid):
                 self.devCtrl, nodeid)
         ).raise_on_error()
 
-    def EstablishPASESessionBLE(self, setupPinCode: int, discriminator: int, nodeid: int):
+    def EstablishPASESessionBLE(self, setupPinCode: int, discriminator: int, nodeid: int) -> None:
         self.CheckIsActive()
 
         self._pase_establishment_complete_future = concurrent.futures.Future()
@@ -606,16 +616,11 @@ def EstablishPASESessionBLE(self, setupPinCode: int, discriminator: int, nodeid:
                     self.devCtrl, setupPinCode, discriminator, nodeid)
             ).raise_on_error()
 
-            # TODO: This is a bit funky, but what the API returned with the previous
-            # implementation. We should revisit this.
-            err = self._pase_establishment_complete_future.result()
-            if not err.is_success:
-                return err.to_exception()
-            return None
+            self._pase_establishment_complete_future.result()
         finally:
             self._pase_establishment_complete_future = None
 
-    def EstablishPASESessionIP(self, ipaddr: str, setupPinCode: int, nodeid: int, port: int = 0):
+    def EstablishPASESessionIP(self, ipaddr: str, setupPinCode: int, nodeid: int, port: int = 0) -> None:
         self.CheckIsActive()
 
         self._pase_establishment_complete_future = concurrent.futures.Future()
@@ -626,16 +631,11 @@ def EstablishPASESessionIP(self, ipaddr: str, setupPinCode: int, nodeid: int, po
                     self.devCtrl, ipaddr.encode("utf-8"), setupPinCode, nodeid, port)
             ).raise_on_error()
 
-            # TODO: This is a bit funky, but what the API returned with the previous
-            # implementation. We should revisit this.
-            err = self._pase_establishment_complete_future.result()
-            if not err.is_success:
-                return err.to_exception()
-            return None
+            self._pase_establishment_complete_future.result()
         finally:
             self._pase_establishment_complete_future = None
 
-    def EstablishPASESession(self, setUpCode: str, nodeid: int):
+    def EstablishPASESession(self, setUpCode: str, nodeid: int) -> None:
         self.CheckIsActive()
 
         self._pase_establishment_complete_future = concurrent.futures.Future()
@@ -646,12 +646,7 @@ def EstablishPASESession(self, setUpCode: str, nodeid: int):
                     self.devCtrl, setUpCode.encode("utf-8"), nodeid)
             ).raise_on_error()
 
-            # TODO: This is a bit funky, but what the API returned with the previous
-            # implementation. We should revisit this.
-            err = self._pase_establishment_complete_future.result()
-            if not err.is_success:
-                return err.to_exception()
-            return None
+            self._pase_establishment_complete_future.result()
         finally:
             self._pase_establishment_complete_future = None
 
@@ -1853,17 +1848,19 @@ def caIndex(self) -> int:
     def fabricAdmin(self) -> FabricAdmin:
         return self._fabricAdmin
 
-    def Commission(self, nodeid) -> PyChipError:
+    def Commission(self, nodeid) -> int:
         '''
         Start the auto-commissioning process on a node after establishing a PASE connection.
         This function is intended to be used in conjunction with `EstablishPASESessionBLE` or
         `EstablishPASESessionIP`. It can be called either before or after the DevicePairingDelegate
         receives the OnPairingComplete call. Commissioners that want to perform simple
-        auto-commissioning should use the supplied "PairDevice" functions above, which will
+        auto-commissioning should use the supplied "CommissionWithCode" function, which will
         establish the PASE connection and commission automatically.
 
-        Return:
-          bool: True if successful, False otherwise.
+        Raises a ChipStackError on failure.
+
+        Returns:
+            - Effective Node ID of the device (as defined by the assigned NOC)
         '''
         self.CheckIsActive()
 
@@ -1879,13 +1876,13 @@ def Commission(self, nodeid) -> PyChipError:
         finally:
             self._commissioning_complete_future = None
 
-    def CommissionThread(self, discriminator, setupPinCode, nodeId, threadOperationalDataset: bytes, isShortDiscriminator: bool = False) -> PyChipError:
+    def CommissionThread(self, discriminator, setupPinCode, nodeId, threadOperationalDataset: bytes, isShortDiscriminator: bool = False) -> int:
         ''' Commissions a Thread device over BLE
         '''
         self.SetThreadOperationalDataset(threadOperationalDataset)
         return self.ConnectBLE(discriminator, setupPinCode, nodeId, isShortDiscriminator)
 
-    def CommissionWiFi(self, discriminator, setupPinCode, nodeId, ssid: str, credentials: str, isShortDiscriminator: bool = False) -> PyChipError:
+    def CommissionWiFi(self, discriminator, setupPinCode, nodeId, ssid: str, credentials: str, isShortDiscriminator: bool = False) -> int:
         ''' Commissions a Wi-Fi device over BLE.
         '''
         self.SetWiFiCredentials(ssid, credentials)
@@ -1988,7 +1985,7 @@ def GetFabricCheckResult(self) -> int:
         return self.fabricCheckNodeId
 
     def CommissionOnNetwork(self, nodeId: int, setupPinCode: int,
-                            filterType: DiscoveryFilterType = DiscoveryFilterType.NONE, filter: typing.Any = None, discoveryTimeoutMsec: int = 30000) -> PyChipError:
+                            filterType: DiscoveryFilterType = DiscoveryFilterType.NONE, filter: typing.Any = None, discoveryTimeoutMsec: int = 30000) -> int:
         '''
         Does the routine for OnNetworkCommissioning, with a filter for mDNS discovery.
         Supported filters are:
@@ -2004,6 +2001,11 @@ def CommissionOnNetwork(self, nodeId: int, setupPinCode: int,
             DiscoveryFilterType.COMPRESSED_FABRIC_ID
 
         The filter can be an integer, a string or None depending on the actual type of selected filter.
+
+        Raises a ChipStackError on failure.
+
+        Returns:
+            - Effective Node ID of the device (as defined by the assigned NOC)
         '''
         self.CheckIsActive()
 
@@ -2023,9 +2025,14 @@ def CommissionOnNetwork(self, nodeId: int, setupPinCode: int,
         finally:
             self._commissioning_complete_future = None
 
-    def CommissionWithCode(self, setupPayload: str, nodeid: int, discoveryType: DiscoveryType = DiscoveryType.DISCOVERY_ALL) -> PyChipError:
+    def CommissionWithCode(self, setupPayload: str, nodeid: int, discoveryType: DiscoveryType = DiscoveryType.DISCOVERY_ALL) -> int:
         ''' Commission with the given nodeid from the setupPayload.
             setupPayload may be a QR or manual code.
+
+            Raises a ChipStackError on failure.
+
+        Returns:
+            - Effective Node ID of the device (as defined by the assigned NOC)
         '''
         self.CheckIsActive()
 
@@ -2042,8 +2049,14 @@ def CommissionWithCode(self, setupPayload: str, nodeid: int, discoveryType: Disc
         finally:
             self._commissioning_complete_future = None
 
-    def CommissionIP(self, ipaddr: str, setupPinCode: int, nodeid: int) -> PyChipError:
-        """ DEPRECATED, DO NOT USE! Use `CommissionOnNetwork` or `CommissionWithCode` """
+    def CommissionIP(self, ipaddr: str, setupPinCode: int, nodeid: int) -> int:
+        """ DEPRECATED, DO NOT USE! Use `CommissionOnNetwork` or `CommissionWithCode`
+
+        Raises a ChipStackError on failure.
+
+        Returns:
+            - Effective Node ID of the device (as defined by the assigned NOC)
+        """
         self.CheckIsActive()
 
         self._commissioning_complete_future = concurrent.futures.Future()