From a90ae1a417d08d6ba29078fb44677bb79a526c8b Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 17 Jun 2024 15:54:13 +0200 Subject: [PATCH] [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. --- src/controller/python/chip/ChipDeviceCtrl.py | 95 +++++++++++--------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 6e2e1336b5c837..da952f790acebe 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/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()