From c9b6e0c38d92926cff99b8b9879ebd5d28015192 Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Thu, 7 Jul 2022 07:04:31 -0700 Subject: [PATCH] Review feedback --- src/controller/python/BUILD.gn | 2 + .../ChipDeviceController-ScriptBinding.cpp | 52 ++++++-------- src/controller/python/CommonStackInit.cpp | 67 +++++++++++++++++++ src/controller/python/CommonStackInit.h | 47 +++++++++++++ src/controller/python/chip/ChipStack.py | 15 +---- 5 files changed, 140 insertions(+), 43 deletions(-) create mode 100644 src/controller/python/CommonStackInit.cpp create mode 100644 src/controller/python/CommonStackInit.h diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index 6016d58957469c..d9cc0dd4fd3d6a 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -44,6 +44,8 @@ shared_library("ChipDeviceCtrl") { "chip/setup_payload/Parser.cpp", ] + sources += [ "CommonStackInit.cpp" ] + if (chip_controller) { sources += [ "ChipCommissionableNodeController-ScriptBinding.cpp", diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index a9016ec0e5b6a4..2d54e6fcf7cd09 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -40,12 +40,6 @@ #include #include -#include "ChipDeviceController-ScriptDevicePairingDelegate.h" -#include "ChipDeviceController-StorageDelegate.h" - -#include "chip/interaction_model/Delegate.h" - -#include #include #include #include @@ -55,6 +49,12 @@ #include #include #include + +#include +#include +#include +#include + #include #include #include @@ -110,7 +110,7 @@ chip::NodeId kDefaultLocalDeviceId = chip::kTestControllerNodeId; chip::NodeId kRemoteDeviceId = chip::kTestDeviceNodeId; extern "C" { -ChipError::StorageType pychip_DeviceController_StackInit(); +ChipError::StorageType pychip_DeviceController_StackInit(uint32_t bluetoothAdapterId); ChipError::StorageType pychip_DeviceController_StackShutdown(); ChipError::StorageType pychip_DeviceController_NewDeviceController(chip::Controller::DeviceCommissioner ** outDevCtrl, @@ -190,8 +190,6 @@ ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::Devic ChipError::StorageType pychip_GetDeviceBeingCommissioned(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId, CommissioneeDeviceProxy ** proxy); uint64_t pychip_GetCommandSenderHandle(chip::DeviceProxy * device); -// CHIP Stack objects -ChipError::StorageType pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId); chip::ChipError::StorageType pychip_InteractionModel_ShutdownSubscription(SubscriptionId subscriptionId); @@ -223,19 +221,19 @@ chip::Controller::Python::StorageAdapter * pychip_Storage_GetStorageAdapter() return sStorageAdapter; } -ChipError::StorageType pychip_DeviceController_StackInit() +ChipError::StorageType pychip_DeviceController_StackInit(uint32_t bluetoothAdapterId) { - ReturnErrorOnFailure(chip::Platform::MemoryInit().AsInteger()); - - auto err = chip::DeviceLayer::PlatformMgr().InitChipStack(); - if (err != CHIP_NO_ERROR) - { - ChipLogError(DeviceLayer, "Failed to initialize CHIP stack: platform init failed: %s", chip::ErrorStr(err)); - return err.AsInteger(); - } + ReturnErrorOnFailure(chip::Controller::Python::CommonStackInit().AsInteger()); VerifyOrDie(sStorageAdapter != nullptr); +#if CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE + // By default, Linux device is configured as a BLE peripheral while the controller needs a BLE central. + CHIP_ERROR err = + chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(/* BLE adapter ID */ bluetoothAdapterId, /* BLE central */ true); + VerifyOrReturnError(err == CHIP_NO_ERROR, err.AsInteger()); +#endif + FactoryInitParams factoryParams; factoryParams.fabricIndependentStorage = sStorageAdapter; @@ -278,6 +276,8 @@ ChipError::StorageType pychip_DeviceController_StackInit() ChipError::StorageType pychip_DeviceController_StackShutdown() { + ChipLogError(Controller, "Shutting down the stack..."); + // // Let's stop the Matter thread, and wait till the event loop has stopped. // @@ -289,7 +289,9 @@ ChipError::StorageType pychip_DeviceController_StackShutdown() // DeviceControllerFactory::GetInstance().ReleaseSystemState(); - chip::DeviceLayer::PlatformMgr().Shutdown(); + DeviceControllerFactory::GetInstance().Shutdown(); + + chip::Controller::Python::CommonStackShutdown(); return CHIP_NO_ERROR.AsInteger(); } @@ -355,18 +357,6 @@ void pychip_DeviceController_SetLogFilter(uint8_t category) #endif } -ChipError::StorageType pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId) -{ -#if CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE - // By default, Linux device is configured as a BLE peripheral while the controller needs a BLE central. - CHIP_ERROR err = - chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(/* BLE adapter ID */ bluetoothAdapterId, /* BLE central */ true); - VerifyOrReturnError(err == CHIP_NO_ERROR, err.AsInteger()); -#endif - - return CHIP_NO_ERROR.AsInteger(); -} - ChipError::StorageType pychip_DeviceController_ConnectBLE(chip::Controller::DeviceCommissioner * devCtrl, uint16_t discriminator, uint32_t setupPINCode, chip::NodeId nodeid) { diff --git a/src/controller/python/CommonStackInit.cpp b/src/controller/python/CommonStackInit.cpp new file mode 100644 index 00000000000000..7bfdb860e04531 --- /dev/null +++ b/src/controller/python/CommonStackInit.cpp @@ -0,0 +1,67 @@ +/* + * + * Copyright (c) 2020-2022 Project CHIP Authors + * Copyright (c) 2019-2020 Google LLC. + * Copyright (c) 2013-2018 Nest Labs, Inc. + * All rights reserved. + * + * 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. + */ + +/** + * @file + * Implementation of the native methods expected by the Python + * version of Chip Device Manager. + * + */ + +#include + +#include +#include + +#include +#include + +#include +#include +#include +#include +#include + +namespace chip { +namespace Controller { +namespace Python { + +CHIP_ERROR CommonStackInit() +{ + ReturnErrorOnFailure(chip::Platform::MemoryInit()); + ReturnErrorOnFailure(chip::DeviceLayer::PlatformMgr().InitChipStack()); + return CHIP_NO_ERROR; +} + +void CommonStackShutdown() +{ + chip::DeviceLayer::PlatformMgr().Shutdown(); + +#if 0 // + // We cannot actually call this because the destructor for the MdnsContexts singleton on Darwin only gets called + // on termination of the program, and that unfortunately makes a bunch of Platform::MemoryFree calls. + // + chip::Platform::MemoryShutdown(); +#endif +} + +} // namespace Python +} // namespace Controller +} // namespace chip diff --git a/src/controller/python/CommonStackInit.h b/src/controller/python/CommonStackInit.h new file mode 100644 index 00000000000000..a28f4163ee5e0f --- /dev/null +++ b/src/controller/python/CommonStackInit.h @@ -0,0 +1,47 @@ +/* + * + * Copyright (c) 2020-2022 Project CHIP Authors + * Copyright (c) 2019-2020 Google LLC. + * Copyright (c) 2013-2018 Nest Labs, Inc. + * All rights reserved. + * + * 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. + */ + +/** + * @file + * Implementation of the native methods expected by the Python + * version of Chip Device Manager. + * + */ +#include + +namespace chip { +namespace Controller { +namespace Python { + +// +// This encapsulates stack initialization logic that is common to both +// controller and server initialization sequences. +// +CHIP_ERROR CommonStackInit(); + +// +// This encapsulates stack shutdown logic that is common to both +// controller and server shutdown sequences. +// +void CommonStackShutdown(); + +} // namespace Python +} // namespace Controller +} // namespace chip diff --git a/src/controller/python/chip/ChipStack.py b/src/controller/python/chip/ChipStack.py index d43bd923fcb4f4..f49f28d85bfe4e 100644 --- a/src/controller/python/chip/ChipStack.py +++ b/src/controller/python/chip/ChipStack.py @@ -253,16 +253,11 @@ def HandleChipThreadRun(callback): # self._persistentStorage = PersistentStorage(persistentStoragePath) - # Initialize the chip stack. - res = self._ChipStackLib.pychip_DeviceController_StackInit() - if res != 0: - raise self.ErrorToException(res) - if (bluetoothAdapter is None): bluetoothAdapter = 0 - res = self._ChipStackLib.pychip_BLEMgrImpl_ConfigureBle( - bluetoothAdapter) + # Initialize the chip stack. + res = self._ChipStackLib.pychip_DeviceController_StackInit(bluetoothAdapter) if res != 0: raise self.ErrorToException(res) @@ -459,7 +454,7 @@ def _AllDirsToRoot(self, dir): def _loadLib(self): if self._ChipStackLib is None: self._ChipStackLib = CDLL(self.LocateChipDLL()) - self._ChipStackLib.pychip_DeviceController_StackInit.argtypes = [] + self._ChipStackLib.pychip_DeviceController_StackInit.argtypes = [c_uint32] self._ChipStackLib.pychip_DeviceController_StackInit.restype = c_uint32 self._ChipStackLib.pychip_DeviceController_StackShutdown.argtypes = [] self._ChipStackLib.pychip_DeviceController_StackShutdown.restype = c_uint32 @@ -474,10 +469,6 @@ def _loadLib(self): _LogMessageFunct] self._ChipStackLib.pychip_Stack_SetLogFunct.restype = c_uint32 - self._ChipStackLib.pychip_BLEMgrImpl_ConfigureBle.argtypes = [ - c_uint32] - self._ChipStackLib.pychip_BLEMgrImpl_ConfigureBle.restype = c_uint32 - self._ChipStackLib.pychip_DeviceController_PostTaskOnChipThread.argtypes = [ _ChipThreadTaskRunnerFunct, py_object] self._ChipStackLib.pychip_DeviceController_PostTaskOnChipThread.restype = c_uint32