Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Python] Fix threading model #20234

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/controller/python/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ shared_library("ChipDeviceCtrl") {
"chip/internal/ChipThreadWork.h",
"chip/internal/CommissionerImpl.cpp",
"chip/logging/LoggingRedirect.cpp",
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
"chip/native/StackInit.cpp",
]
} else {
sources += [
Expand Down
90 changes: 56 additions & 34 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@
#include <setup_payload/QRCodeSetupPayloadParser.h>
#include <system/SystemClock.h>

#include <platform/CommissionableDataProvider.h>
#include <platform/PlatformManager.h>
#include <platform/TestOnlyCommissionableDataProvider.h>

using namespace chip;
using namespace chip::Ble;
using namespace chip::Controller;
Expand Down Expand Up @@ -107,6 +111,7 @@ chip::NodeId kRemoteDeviceId = chip::kTestDeviceNodeId;

extern "C" {
ChipError::StorageType pychip_DeviceController_StackInit();
ChipError::StorageType pychip_DeviceController_StackShutdown();

ChipError::StorageType pychip_DeviceController_NewDeviceController(chip::Controller::DeviceCommissioner ** outDevCtrl,
chip::NodeId localDeviceId, bool useTestCommissioner);
Expand Down Expand Up @@ -176,8 +181,6 @@ ChipError::StorageType pychip_DeviceCommissioner_CloseBleConnection(chip::Contro
uint8_t pychip_DeviceController_GetLogFilter();
void pychip_DeviceController_SetLogFilter(uint8_t category);

ChipError::StorageType pychip_Stack_Init();
ChipError::StorageType pychip_Stack_Shutdown();
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);
Expand Down Expand Up @@ -222,6 +225,15 @@ chip::Controller::Python::StorageAdapter * pychip_Storage_GetStorageAdapter()

ChipError::StorageType pychip_DeviceController_StackInit()
{
ReturnErrorOnFailure(chip::Platform::MemoryInit().AsInteger());

auto err = chip::DeviceLayer::PlatformMgr().InitChipStack();
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to initialize CHIP stack: platform init failed: %s", chip::ErrorStr(err));
return err.AsInteger();
}

VerifyOrDie(sStorageAdapter != nullptr);

FactoryInitParams factoryParams;
Expand All @@ -236,6 +248,12 @@ ChipError::StorageType pychip_DeviceController_StackInit()

factoryParams.enableServerInteractions = true;

// Hack needed due to the fact that DnsSd server uses the CommissionableDataProvider even
// when never starting operational advertising. This will not be used but prevents
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
// null pointer dereferences.
static chip::DeviceLayer::TestOnlyCommissionableDataProvider TestOnlyCommissionableDataProvider;
chip::DeviceLayer::SetCommissionableDataProvider(&TestOnlyCommissionableDataProvider);

ReturnErrorOnFailure(DeviceControllerFactory::GetInstance().Init(factoryParams).AsInteger());

//
Expand All @@ -249,6 +267,30 @@ ChipError::StorageType pychip_DeviceController_StackInit()
//
DeviceControllerFactory::GetInstance().RetainSystemState();

//
// Finally, start up the main Matter thread. Any further interactions with the stack
// will now need to happen on the Matter thread, OR protected with the stack lock.
//
ReturnErrorOnFailure(chip::DeviceLayer::PlatformMgr().StartEventLoopTask().AsInteger());

return CHIP_NO_ERROR.AsInteger();
}

ChipError::StorageType pychip_DeviceController_StackShutdown()
{
//
// Let's stop the Matter thread, and wait till the event loop has stopped.
//
ReturnErrorOnFailure(chip::DeviceLayer::PlatformMgr().StopEventLoopTask().AsInteger());

//
// There is the symmetric call to match the Retain called at stack initialization
// time. This will release all resources (if there are no other controllers active).
//
DeviceControllerFactory::GetInstance().ReleaseSystemState();

chip::DeviceLayer::PlatformMgr().Shutdown();

return CHIP_NO_ERROR.AsInteger();
}

Expand Down Expand Up @@ -313,6 +355,18 @@ void pychip_DeviceController_SetLogFilter(uint8_t category)
#endif
}

ChipError::StorageType pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId)
woody-apple marked this conversation as resolved.
Show resolved Hide resolved
{
#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)
{
Expand Down Expand Up @@ -557,38 +611,6 @@ ChipError::StorageType pychip_ScriptDevicePairingDelegate_SetCommissioningStatus
return CHIP_NO_ERROR.AsInteger();
}

andy31415 marked this conversation as resolved.
Show resolved Hide resolved
ChipError::StorageType pychip_Stack_Init()
{
CHIP_ERROR err = CHIP_NO_ERROR;

err = chip::Platform::MemoryInit();
SuccessOrExit(err);

#if !CHIP_SYSTEM_CONFIG_USE_SOCKETS

ExitNow(err = CHIP_ERROR_NOT_IMPLEMENTED);

#else /* CHIP_SYSTEM_CONFIG_USE_SOCKETS */

#endif /* CHIP_SYSTEM_CONFIG_USE_SOCKETS */

exit:
if (err != CHIP_NO_ERROR)
pychip_Stack_Shutdown();

return err.AsInteger();
}

ChipError::StorageType pychip_Stack_Shutdown()
{
//
// There is the symmetric call to match the Retain called at stack initialization
// time.
//
DeviceControllerFactory::GetInstance().ReleaseSystemState();
return CHIP_NO_ERROR.AsInteger();
}

const char * pychip_Stack_ErrorToString(ChipError::StorageType err)
{
return chip::ErrorStr(CHIP_ERROR(err));
Expand Down
26 changes: 13 additions & 13 deletions src/controller/python/chip/ChipStack.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,14 @@ def HandleChipThreadRun(callback):
# set by other modules(BLE) that require service by thread while thread blocks.
self.blockingCB = None

# Initialize the chip library
res = self._ChipStackLib.pychip_Stack_Init()
#
# Storage has to be initialized BEFORE initializing the stack, since the latter
# requires a PersistentStorageDelegate to be provided to DeviceControllerFactory.
#
self._persistentStorage = PersistentStorage(persistentStoragePath)

# Initialize the chip stack.
res = self._ChipStackLib.pychip_DeviceController_StackInit()
if res != 0:
raise self.ErrorToException(res)

Expand All @@ -260,12 +266,6 @@ def HandleChipThreadRun(callback):
if res != 0:
raise self.ErrorToException(res)

self._persistentStorage = PersistentStorage(persistentStoragePath)

res = self._ChipStackLib.pychip_DeviceController_StackInit()
if res != 0:
raise self.ErrorToException(res)

im.InitIMDelegate()
ClusterAttribute.Init()
ClusterCommand.Init()
Expand Down Expand Up @@ -321,7 +321,7 @@ def Shutdown(self):
# Make sure PersistentStorage is destructed before chipStack
# to avoid accessing builtins.chipStack after destruction.
self._persistentStorage = None
self.Call(lambda: self._ChipStackLib.pychip_Stack_Shutdown())
self.Call(lambda: self._ChipStackLib.pychip_DeviceController_StackShutdown())
self.networkLock = None
self.completeEvent = None
self._ChipStackLib = None
Expand Down Expand Up @@ -459,10 +459,10 @@ def _AllDirsToRoot(self, dir):
def _loadLib(self):
if self._ChipStackLib is None:
self._ChipStackLib = CDLL(self.LocateChipDLL())
self._ChipStackLib.pychip_Stack_Init.argtypes = []
self._ChipStackLib.pychip_Stack_Init.restype = c_uint32
self._ChipStackLib.pychip_Stack_Shutdown.argtypes = []
self._ChipStackLib.pychip_Stack_Shutdown.restype = c_uint32
self._ChipStackLib.pychip_DeviceController_StackInit.argtypes = []
self._ChipStackLib.pychip_DeviceController_StackInit.restype = c_uint32
self._ChipStackLib.pychip_DeviceController_StackShutdown.argtypes = []
self._ChipStackLib.pychip_DeviceController_StackShutdown.restype = c_uint32
self._ChipStackLib.pychip_Stack_StatusReportToString.argtypes = [
c_uint32,
c_uint16,
Expand Down
102 changes: 0 additions & 102 deletions src/controller/python/chip/native/StackInit.cpp

This file was deleted.

5 changes: 0 additions & 5 deletions src/controller/python/chip/native/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,6 @@ def GetLibraryHandle() -> ctypes.CDLL:
global _nativeLibraryHandle
if _nativeLibraryHandle is None:
_nativeLibraryHandle = ctypes.CDLL(FindNativeLibraryPath())

setter = NativeLibraryHandleMethodArguments(_nativeLibraryHandle)

setter.Set("pychip_native_init", None, [])

_nativeLibraryHandle.pychip_native_init()

return _nativeLibraryHandle