From 615466a50251997716f70f5558822f4785478ad7 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Mon, 30 Nov 2020 11:41:28 +0800 Subject: [PATCH 01/15] Add IP rendezvous to ChipDeviceController --- src/controller/CHIPDeviceController.cpp | 46 +++++--- src/controller/python/BUILD.gn | 1 + .../ChipDeviceController-ScriptBinding.cpp | 29 ++++- .../ChipDeviceController-StorageDelegate.h | 30 +++++ src/controller/python/chip-device-ctrl.py | 15 ++- src/controller/python/chip/ChipDeviceCtrl.py | 105 ++++++++++++------ src/transport/RendezvousSession.cpp | 15 ++- src/transport/RendezvousSession.h | 1 + src/transport/SecurePairingSession.cpp | 6 +- 9 files changed, 190 insertions(+), 58 deletions(-) create mode 100644 src/controller/python/ChipDeviceController-StorageDelegate.h diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 008ac0fb4cdefb..92f93056c31f84 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -42,6 +42,10 @@ #include #include #include +#include +#include +#include +#include #include #include #include @@ -49,11 +53,6 @@ #include #include #include - -#include -#include -#include -#include #include using namespace chip::Inet; @@ -67,14 +66,15 @@ using namespace chip::Encoding; constexpr const char kPairedDeviceListKeyPrefix[] = "ListPairedDevices"; constexpr const char kPairedDeviceKeyPrefix[] = "PairedDevice"; -// This macro generates a key using node ID an key prefix, and performs the given action -// on that key. +// This macro generates a key using node ID an key prefix, and performs the +// given action on that key. #define PERSISTENT_KEY_OP(node, keyPrefix, key, action) \ do \ { \ constexpr size_t len = std::extent::value; \ nlSTATIC_ASSERT_PRINT(len > 0, "keyPrefix length must be known at compile time"); \ - /* 2 * sizeof(NodeId) to accomodate 2 character for each byte in Node Id */ \ + /* 2 * sizeof(NodeId) to accomodate 2 character for each byte in Node Id \ + */ \ char key[len + 2 * sizeof(NodeId) + 1]; \ nlSTATIC_ASSERT_PRINT(sizeof(node) <= sizeof(uint64_t), "Node ID size is greater than expected"); \ snprintf(key, sizeof(key), "%s%" PRIx64, keyPrefix, node); \ @@ -310,7 +310,8 @@ CHIP_ERROR DeviceController::ServiceEventSignal() DeviceLayer::SystemLayer.WakeSelect(); #else err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; -#endif // CONFIG_DEVICE_LAYER && (CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK) +#endif // CONFIG_DEVICE_LAYER && (CHIP_SYSTEM_CONFIG_USE_SOCKETS || + // CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK) exit: return err; @@ -468,12 +469,15 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE); VerifyOrExit(mDeviceBeingPaired == kNumMaxActiveDevices, err = CHIP_ERROR_INCORRECT_STATE); -#if CONFIG_DEVICE_LAYER && CONFIG_NETWORK_LAYER_BLE - if (!params.HasBleLayer()) + if (params.GetPeerAddress().GetTransportType() == Transport::Type::kBle) { - params.SetBleLayer(DeviceLayer::ConnectivityMgr().GetBleLayer()); - } +#if CONFIG_DEVICE_LAYER && CONFIG_NETWORK_LAYER_BLE + if (!params.HasBleLayer()) + { + params.SetBleLayer(DeviceLayer::ConnectivityMgr().GetBleLayer()); + } #endif // CONFIG_DEVICE_LAYER && CONFIG_NETWORK_LAYER_BLE + } mDeviceBeingPaired = GetInactiveDeviceIndex(); VerifyOrExit(mDeviceBeingPaired < kNumMaxActiveDevices, err = CHIP_ERROR_NO_MEMORY); @@ -486,6 +490,12 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam device->Init(mTransportMgr, mSessionManager, mInetLayer, remoteDeviceId, remotePort, interfaceId); + if (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle) + { + // IP Rendezvous + mRendezvousSession->OnRendezvousConnectionOpened(); + } + exit: if (err != CHIP_NO_ERROR) { @@ -630,11 +640,11 @@ void DeviceCommissioner::OnRendezvousComplete() mPairedDevices.Insert(device->GetDeviceId()); mPairedDevicesUpdated = true; - // mStorageDelegate would not be null for a typical pairing scenario, as Pair() - // requires a valid storage delegate. However, test pairing usecase, that's used - // mainly by test applications, do not require a storage delegate. This is to - // reduce overheads on these tests. - // Let's make sure the delegate object is available before calling into it. + // mStorageDelegate would not be null for a typical pairing scenario, as + // Pair() requires a valid storage delegate. However, test pairing usecase, + // that's used mainly by test applications, do not require a storage delegate. + // This is to reduce overheads on these tests. Let's make sure the delegate + // object is available before calling into it. if (mStorageDelegate != nullptr) { SerializedDevice serialized; diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index 84b22a14e87614..acda664c7cc277 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -40,6 +40,7 @@ shared_library("ChipDeviceCtrl") { "ChipDeviceController-ScriptBinding.cpp", "ChipDeviceController-ScriptDevicePairingDelegate.cpp", "ChipDeviceController-ScriptDevicePairingDelegate.h", + "ChipDeviceController-StorageDelegate.h", ] public_deps = [ diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 3518f94600ab92..93d5297d1dcc9f 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -44,6 +44,7 @@ #endif /* CONFIG_NETWORK_LAYER_BLE */ #include "ChipDeviceController-ScriptDevicePairingDelegate.h" +#include "ChipDeviceController-StorageDelegate.h" #include #include @@ -125,6 +126,7 @@ class BleDisconnectEvent : public BleEventBase static chip::System::Layer sSystemLayer; static chip::Inet::InetLayer sInetLayer; +static chip::Controller::PythonPersistentStorageDelegate sStorageDelegate; // NOTE: Remote device ID is in sync with the echo server device id // At some point, we may want to add an option to connect to a device without @@ -170,6 +172,9 @@ CHIP_ERROR nl_Chip_DeviceController_DeleteDeviceController(chip::DeviceControlle CHIP_ERROR nl_Chip_DeviceController_Connect(chip::DeviceController::ChipDeviceController * devCtrl, BLE_CONNECTION_OBJECT connObj, uint32_t setupPinCode, OnConnectFunct onConnect, OnMessageFunct onMessage, OnErrorFunct onError); +CHIP_ERROR nl_Chip_DeviceController_ConnectIP(chip::DeviceController::ChipDeviceController * devCtrl, const char * peerAddrStr, + uint32_t setupPINCode, OnConnectFunct onConnect, OnMessageFunct onMessage, + OnErrorFunct onError); // Network Provisioning CHIP_ERROR @@ -204,7 +209,7 @@ CHIP_ERROR nl_Chip_DeviceController_NewDeviceController(chip::DeviceController:: *outDevCtrl = new chip::DeviceController::ChipDeviceController(); VerifyOrExit(*outDevCtrl != NULL, err = CHIP_ERROR_NO_MEMORY); - err = (*outDevCtrl)->Init(kLocalDeviceId, &sSystemLayer, &sInetLayer); + err = (*outDevCtrl)->Init(kLocalDeviceId, &sSystemLayer, &sInetLayer, nullptr, &sStorageDelegate); SuccessOrExit(err); exit: @@ -541,6 +546,28 @@ CHIP_ERROR nl_Chip_DeviceController_Connect(chip::DeviceController::ChipDeviceCo return err; } +CHIP_ERROR nl_Chip_DeviceController_ConnectIP(chip::DeviceController::ChipDeviceController * devCtrl, const char * peerAddrStr, + uint32_t setupPINCode, OnConnectFunct onConnect, OnMessageFunct onMessage, + OnErrorFunct onError) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::Inet::IPAddress peerAddr; + chip::Transport::PeerAddress addr; + chip::RendezvousParameters params = chip::RendezvousParameters().SetSetupPINCode(setupPINCode); + + VerifyOrExit(chip::Inet::IPAddress::FromString(peerAddrStr, peerAddr), err = CHIP_ERROR_INVALID_ARGUMENT); + addr.SetIPAddress(peerAddr); + // TODO: IP rendezvous should use TCP connection. + addr.SetTransportType(chip::Transport::Type::kUdp); + params.SetPeerAddress(addr); + params.SetDiscriminator(0); + err = devCtrl->ConnectDevice(kRemoteDeviceId, params, (void *) devCtrl, onConnect, onMessage, onError); + SuccessOrExit(err); + +exit: + return err; +} + CHIP_ERROR nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate(chip::DeviceController::ScriptDevicePairingDelegate ** pairingDelegate) { diff --git a/src/controller/python/ChipDeviceController-StorageDelegate.h b/src/controller/python/ChipDeviceController-StorageDelegate.h new file mode 100644 index 00000000000000..cb958477a86031 --- /dev/null +++ b/src/controller/python/ChipDeviceController-StorageDelegate.h @@ -0,0 +1,30 @@ +#pragma once + +#include + +class PythonPersistentStorageDelegate; + +extern "C" { +typedef void (*GetKeyValueFunct)(const uint8_t * key, uint8_t * value, uint16_t * size); +typedef void (*SetKeyValueFunct)(const uint8_t * key, const uint8_t * value); +typedef void (*DeleteKeyValueFunct)(const uint8_t * key); +} + +namespace chip { +namespace Controller { + +// TODO: Implement this. +class PythonPersistentStorageDelegate : public PersistentStorageDelegate +{ +public: + PythonPersistentStorageDelegate() {} + ~PythonPersistentStorageDelegate() {} + void SetDelegate(PersistentStorageResultDelegate * delegate) override{}; + void GetKeyValue(const char * key) override{}; + CHIP_ERROR GetKeyValue(const char * key, char * value, uint16_t & size) override { return CHIP_NO_ERROR; }; + void SetKeyValue(const char * key, const char * value) override{}; + void DeleteKeyValue(const char * key) override{}; +}; + +} // namespace Controller +} // namespace chip diff --git a/src/controller/python/chip-device-ctrl.py b/src/controller/python/chip-device-ctrl.py index 6b07db4c43c10d..1aa9b49217c4a1 100755 --- a/src/controller/python/chip-device-ctrl.py +++ b/src/controller/python/chip-device-ctrl.py @@ -365,7 +365,8 @@ def do_btpconnect(self, line): def do_connect(self, line): """ - connect (via BLE) + connect -ip + connect -ble connect command is used for establishing a rendezvous session to the device. currently, only connect using setupPinCode is supported. @@ -376,14 +377,18 @@ def do_connect(self, line): try: args = shlex.split(line) - if not args: + if len(args) <= 1: print("Usage:") self.do_help("connect SetupPinCode") return - if len(args) > 1: - print("Unexpected argument: " + args[1]) + if args[0] == "-ip" and len(args) == 3: + self.devCtrl.ConnectIP(args[1].encode("utf-8"), int(args[2])) + elif args[0] == "-ble" and len(args) == 2: + self.devCtrl.Connect(FAKE_CONN_OBJ_VALUE, int(args[0])) + else: + print("Usage:") + self.do_help("connect SetupPinCode") return - self.devCtrl.Connect(FAKE_CONN_OBJ_VALUE, int(args[0])) except ChipStack.ChipStackException as ex: print(str(ex)) return diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 0e7a84b4e93c7a..af3495132b8cc7 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -38,7 +38,8 @@ _CompleteFunct = CFUNCTYPE(None, c_void_p, c_void_p) -_ErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_ulong, POINTER(DeviceStatusStruct)) +_ErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, + c_ulong, POINTER(DeviceStatusStruct)) _GetBleEventFunct = CFUNCTYPE(c_void_p) _WriteBleCharacteristicFunct = CFUNCTYPE( c_bool, c_void_p, c_void_p, c_void_p, c_void_p, c_uint16 @@ -55,11 +56,14 @@ # typedef void (*OnMessageFunct)(Chip::DeviceController::ChipDeviceController * dc, void * appReqState, PacketBuffer * buffer); _OnConnectFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_void_p) -_OnRendezvousErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_uint32, c_void_p) +_OnRendezvousErrorFunct = CFUNCTYPE( + None, c_void_p, c_void_p, c_uint32, c_void_p) _OnMessageFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_void_p) # This is a fix for WEAV-429. Jay Logue recommends revisiting this at a later # date to allow for truely multiple instances so this is temporary. + + def _singleton(cls): instance = [None] @@ -78,6 +82,7 @@ class DCState(enum.IntEnum): RENDEZVOUS_ONGOING = 3 RENDEZVOUS_CONNECTED = 4 + @_singleton class ChipDeviceController(object): def __init__(self, startNetworkThread=True): @@ -91,16 +96,19 @@ def __init__(self, startNetworkThread=True): self._InitLib() devCtrl = c_void_p(None) - res = self._dmLib.nl_Chip_DeviceController_NewDeviceController(pointer(devCtrl)) + res = self._dmLib.nl_Chip_DeviceController_NewDeviceController( + pointer(devCtrl)) if res != 0: raise self._ChipStack.ErrorToException(res) - + pairingDelegate = c_void_p(None) - res = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate(pointer(pairingDelegate)) + res = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate( + pointer(pairingDelegate)) if res != 0: raise self._ChipStack.ErrorToException(res) - - res = self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate(devCtrl, pairingDelegate) + + res = self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate( + devCtrl, pairingDelegate) if res != 0: raise self._ChipStack.ErrorToException(res) @@ -108,9 +116,11 @@ def __init__(self, startNetworkThread=True): self.pairingDelegate = pairingDelegate self._ChipStack.devCtrl = devCtrl - self.blockingCB = None # set by other modules(BLE) that require service by thread while thread blocks. + # set by other modules(BLE) that require service by thread while thread blocks. + self.blockingCB = None self.cbHandleBleEvent = ( - None # set by other modules (BLE) that provide event callback to Chip. + # set by other modules (BLE) that provide event callback to Chip. + None ) self.cbHandleBleWriteChar = None self.cbHandleBleSubscribeChar = None @@ -118,9 +128,9 @@ def __init__(self, startNetworkThread=True): def DeviceCtrlHandleMessage(appReqState, buffer): pass - + self.cbHandleMessage = _OnMessageFunct(DeviceCtrlHandleMessage) - + def HandleRendezvousError(appState, reqState, err, devStatusPtr): if self.state == DCState.RENDEZVOUS_ONGOING: print("Failed to connect to device: {}".format(err)) @@ -128,8 +138,9 @@ def HandleRendezvousError(appState, reqState, err, devStatusPtr): self._ChipStack.completeEvent.set() elif self.state == DCState.RENDEZVOUS_CONNECTED: print("Disconnected from device") - - self.cbHandleRendezvousError = _OnRendezvousErrorFunct(HandleRendezvousError) + + self.cbHandleRendezvousError = _OnRendezvousErrorFunct( + HandleRendezvousError) if startNetworkThread: self.StartNetworkThread() @@ -137,7 +148,8 @@ def HandleRendezvousError(appState, reqState, err, devStatusPtr): def __del__(self): if self.devCtrl != None: - self._dmLib.nl_Chip_DeviceController_DeleteDeviceManager(self.devCtrl) + self._dmLib.nl_Chip_DeviceController_DeleteDeviceManager( + self.devCtrl) self.devCtrl = None def DriveBleIO(self): @@ -149,11 +161,13 @@ def DriveBleIO(self): def SetBleEventCB(self, bleEventCB): if self.devCtrl != None: self.cbHandleBleEvent = _GetBleEventFunct(bleEventCB) - self._dmLib.nl_Chip_DeviceController_SetBleEventCB(self.cbHandleBleEvent) + self._dmLib.nl_Chip_DeviceController_SetBleEventCB( + self.cbHandleBleEvent) def SetBleWriteCharCB(self, bleWriteCharCB): if self.devCtrl != None: - self.cbHandleBleWriteChar = _WriteBleCharacteristicFunct(bleWriteCharCB) + self.cbHandleBleWriteChar = _WriteBleCharacteristicFunct( + bleWriteCharCB) self._dmLib.nl_Chip_DeviceController_SetBleWriteCharacteristic( self.cbHandleBleWriteChar ) @@ -170,7 +184,8 @@ def SetBleSubscribeCharCB(self, bleSubscribeCharCB): def SetBleCloseCB(self, bleCloseCB): if self.devCtrl != None: self.cbHandleBleClose = _CloseBleFunct(bleCloseCB) - self._dmLib.nl_Chip_DeviceController_SetBleClose(self.cbHandleBleClose) + self._dmLib.nl_Chip_DeviceController_SetBleClose( + self.cbHandleBleClose) def StartNetworkThread(self): if self.networkThread != None: @@ -183,14 +198,16 @@ def RunNetworkThread(): self._ChipStack.networkLock.release() time.sleep(0.005) - self.networkThread = Thread(target=RunNetworkThread, name="ChipNetworkThread") + self.networkThread = Thread( + target=RunNetworkThread, name="ChipNetworkThread") self.networkThread.daemon = True self.networkThreadRunable = True self.networkThread.start() def IsConnected(self): return self._ChipStack.Call( - lambda: self._dmLib.nl_Chip_DeviceController_IsConnected(self.devCtrl) + lambda: self._dmLib.nl_Chip_DeviceController_IsConnected( + self.devCtrl) ) def ConnectBle(self, bleConnection): @@ -202,19 +219,33 @@ def ConnectBle(self, bleConnection): self._ChipStack.cbHandleError, ) ) - + def Connect(self, connObj, setupPinCode): def HandleComplete(dc, connState, appState): print("Rendezvoud Complete") self.state = DCState.RENDEZVOUS_CONNECTED self._ChipStack.callbackRes = True self._ChipStack.completeEvent.set() - onConnectFunct = _OnConnectFunct(HandleComplete) self.state = DCState.RENDEZVOUS_ONGOING return self._ChipStack.CallAsync( - lambda: self._dmLib.nl_Chip_DeviceController_Connect(self.devCtrl, connObj, setupPinCode, onConnectFunct, self.cbHandleMessage, self.cbHandleRendezvousError) + lambda: self._dmLib.nl_Chip_DeviceController_Connect( + self.devCtrl, connObj, setupPinCode, onConnectFunct, self.cbHandleMessage, self.cbHandleRendezvousError) + ) + + def ConnectIP(self, ipaddr, setupPinCode): + def HandleComplete(dc, connState, appState): + print("Rendezvoud Complete") + self.state = DCState.RENDEZVOUS_CONNECTED + self._ChipStack.callbackRes = True + self._ChipStack.completeEvent.set() + onConnectFunct = _OnConnectFunct(HandleComplete) + + self.state = DCState.RENDEZVOUS_ONGOING + return self._ChipStack.CallAsync( + lambda: self._dmLib.nl_Chip_DeviceController_ConnectIP( + self.devCtrl, ipaddr, setupPinCode, onConnectFunct, self.cbHandleMessage, self.cbHandleRendezvousError) ) def Close(self): @@ -237,9 +268,10 @@ def GetLogFilter(self): def SetBlockingCB(self, blockingCB): self._ChipStack.blockingCB = blockingCB - + def SetWifiCredential(self, ssid, password): - ret = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential(self.pairingDelegate, ssid.encode("utf-8"), password.encode("utf-8")) + ret = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential( + self.pairingDelegate, ssid.encode("utf-8"), password.encode("utf-8")) if ret != 0: raise self._ChipStack.ErrorToException(res) @@ -288,10 +320,12 @@ def _InitLib(self): c_uint32 ) - self._dmLib.nl_Chip_DeviceController_SetBleClose.argtypes = [_CloseBleFunct] + self._dmLib.nl_Chip_DeviceController_SetBleClose.argtypes = [ + _CloseBleFunct] self._dmLib.nl_Chip_DeviceController_SetBleClose.restype = c_uint32 - self._dmLib.nl_Chip_DeviceController_IsConnected.argtypes = [c_void_p] + self._dmLib.nl_Chip_DeviceController_IsConnected.argtypes = [ + c_void_p] self._dmLib.nl_Chip_DeviceController_IsConnected.restype = c_bool self._dmLib.nl_Chip_DeviceController_ValidateBTP.argtypes = [ @@ -305,17 +339,26 @@ def _InitLib(self): self._dmLib.nl_Chip_DeviceController_GetLogFilter.argtypes = [] self._dmLib.nl_Chip_DeviceController_GetLogFilter.restype = c_uint8 - self._dmLib.nl_Chip_DeviceController_SetLogFilter.argtypes = [c_uint8] + self._dmLib.nl_Chip_DeviceController_SetLogFilter.argtypes = [ + c_uint8] self._dmLib.nl_Chip_DeviceController_SetLogFilter.restype = None - self._dmLib.nl_Chip_DeviceController_Connect.argtypes = [c_void_p, c_void_p, c_uint32, _OnConnectFunct, _OnMessageFunct, _OnRendezvousErrorFunct] + self._dmLib.nl_Chip_DeviceController_Connect.argtypes = [ + c_void_p, c_void_p, c_uint32, _OnConnectFunct, _OnMessageFunct, _OnRendezvousErrorFunct] self._dmLib.nl_Chip_DeviceController_Connect.restype = c_uint32 - self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate.argtypes = [POINTER(c_void_p)] + self._dmLib.nl_Chip_DeviceController_ConnectIP.argtypes = [ + c_void_p, c_char_p, c_uint32, _OnConnectFunct, _OnMessageFunct, _OnRendezvousErrorFunct] + self._dmLib.nl_Chip_DeviceController_ConnectIP.restype = c_uint32 + + self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate.argtypes = [ + POINTER(c_void_p)] self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate.restype = c_uint32 - self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential.argtypes = [c_void_p, c_char_p, c_char_p] + self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential.argtypes = [ + c_void_p, c_char_p, c_char_p] self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential.restype = c_uint32 - self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate.argtypes = [c_void_p, c_void_p] + self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate.argtypes = [ + c_void_p, c_void_p] self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate.restype = c_uint32 diff --git a/src/transport/RendezvousSession.cpp b/src/transport/RendezvousSession.cpp index 7e1265ffea5dae..2a88b245a1c6cb 100644 --- a/src/transport/RendezvousSession.cpp +++ b/src/transport/RendezvousSession.cpp @@ -181,7 +181,19 @@ void RendezvousSession::OnPairingComplete() return; } - UpdateState(State::kNetworkProvisioning); + if (mParams.GetPeerAddress().GetTransportType() != Transport::Type::kBle || // For rendezvous initializer + mPeerAddress.GetTransportType() != Transport::Type::kBle) // For rendezvous target + { + UpdateState(State::kRendezvousComplete); + if (!mParams.IsController()) + { + OnRendezvousConnectionClosed(); + } + } + else + { + UpdateState(State::kNetworkProvisioning); + } } void RendezvousSession::OnNetworkProvisioningError(CHIP_ERROR err) @@ -290,6 +302,7 @@ void RendezvousSession::OnRendezvousMessageReceived(const PacketHeader & packetH PacketBufferHandle msgBuf) { CHIP_ERROR err = CHIP_NO_ERROR; + mPeerAddress = peerAddress; // TODO: RendezvousSession should handle SecurePairing messages only switch (mCurrentState) diff --git a/src/transport/RendezvousSession.h b/src/transport/RendezvousSession.h index 9fa4f7759f63e0..ae9d82b791c89b 100644 --- a/src/transport/RendezvousSession.h +++ b/src/transport/RendezvousSession.h @@ -152,6 +152,7 @@ class RendezvousSession : public SecurePairingSessionDelegate, SecurePairingSession mPairingSession; NetworkProvisioning mNetworkProvision; SecureSession mSecureSession; + Transport::PeerAddress mPeerAddress; // Current peer address we are doing rendezvous with. TransportMgrBase * mTransportMgr; uint32_t mSecureMessageIndex = 0; uint16_t mNextKeyId = 0; diff --git a/src/transport/SecurePairingSession.cpp b/src/transport/SecurePairingSession.cpp index 59c114d4ba6df4..a5334da2dd986c 100644 --- a/src/transport/SecurePairingSession.cpp +++ b/src/transport/SecurePairingSession.cpp @@ -405,6 +405,10 @@ CHIP_ERROR SecurePairingSession::HandleCompute_cA(const PacketHeader & header, c CHIP_ERROR err = CHIP_NO_ERROR; const uint8_t * hash = msg->Start(); + // We will set NextExpectedMsg to kSpake2pMsgTypeMax in all cases + // However, when we are using IP rendezvous, we might set it to kSpake2pCompute_pA. + mNextExpectedMsg = Spake2pMsgType::kSpake2pMsgTypeMax; + VerifyOrExit(hash != nullptr, err = CHIP_ERROR_MESSAGE_INCOMPLETE); VerifyOrExit(msg->TotalLength() == kMAX_Hash_Length, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH); @@ -423,8 +427,6 @@ CHIP_ERROR SecurePairingSession::HandleCompute_cA(const PacketHeader & header, c mDelegate->OnPairingComplete(); exit: - - mNextExpectedMsg = Spake2pMsgType::kSpake2pMsgTypeMax; return err; } From d1561400785cc7da515a6ced477b55a2869ed3d8 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Mon, 30 Nov 2020 14:45:59 +0800 Subject: [PATCH 02/15] Fix NodeId issue --- src/transport/RendezvousSession.cpp | 11 +++++++++++ src/transport/RendezvousSession.h | 1 + 2 files changed, 12 insertions(+) diff --git a/src/transport/RendezvousSession.cpp b/src/transport/RendezvousSession.cpp index 2a88b245a1c6cb..669104a5d7bbd5 100644 --- a/src/transport/RendezvousSession.cpp +++ b/src/transport/RendezvousSession.cpp @@ -184,6 +184,11 @@ void RendezvousSession::OnPairingComplete() if (mParams.GetPeerAddress().GetTransportType() != Transport::Type::kBle || // For rendezvous initializer mPeerAddress.GetTransportType() != Transport::Type::kBle) // For rendezvous target { + if (mRendezvousRemoteNodeId.HasValue() && !mParams.HasRemoteNodeId()) + { + ChipLogProgress(Ble, "Completed rendezvous with %llu", mRendezvousRemoteNodeId.Value()); + mParams.SetRemoteNodeId(mRendezvousRemoteNodeId.Value()); + } UpdateState(State::kRendezvousComplete); if (!mParams.IsController()) { @@ -228,6 +233,7 @@ void RendezvousSession::OnRendezvousConnectionClosed() } mSecureSession.Reset(); + mRendezvousRemoteNodeId.ClearValue(); CHIP_ERROR err = WaitForPairing(mParams.GetLocalNodeId(), mParams.GetSetupPINCode()); if (err != CHIP_NO_ERROR) @@ -308,6 +314,11 @@ void RendezvousSession::OnRendezvousMessageReceived(const PacketHeader & packetH switch (mCurrentState) { case State::kSecurePairing: + if (packetHeader.GetSourceNodeId().HasValue()) + { + ChipLogProgress(Ble, "Received rendezvous message from %llu", packetHeader.GetSourceNodeId().Value()); + mRendezvousRemoteNodeId.SetValue(packetHeader.GetSourceNodeId().Value()); + } err = HandlePairingMessage(packetHeader, peerAddress, std::move(msgBuf)); break; diff --git a/src/transport/RendezvousSession.h b/src/transport/RendezvousSession.h index ae9d82b791c89b..7a80affc68fc6b 100644 --- a/src/transport/RendezvousSession.h +++ b/src/transport/RendezvousSession.h @@ -153,6 +153,7 @@ class RendezvousSession : public SecurePairingSessionDelegate, NetworkProvisioning mNetworkProvision; SecureSession mSecureSession; Transport::PeerAddress mPeerAddress; // Current peer address we are doing rendezvous with. + Optional mRendezvousRemoteNodeId; TransportMgrBase * mTransportMgr; uint32_t mSecureMessageIndex = 0; uint16_t mNextKeyId = 0; From 8cdb2ac310afefbb7ad02d0f95f7b217d0de7fb5 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 1 Dec 2020 09:34:07 +0800 Subject: [PATCH 03/15] Disable network provisioning for IP rendezvous --- src/controller/CHIPDeviceController.cpp | 4 +++- src/controller/CHIPDeviceController.h | 4 ++++ src/controller/CHIPDeviceController_deprecated.cpp | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 92f93056c31f84..91eee9841e7118 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -483,6 +483,7 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam VerifyOrExit(mDeviceBeingPaired < kNumMaxActiveDevices, err = CHIP_ERROR_NO_MEMORY); device = &mActiveDevices[mDeviceBeingPaired]; + mIsIPRendezvous = (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle); mRendezvousSession = chip::Platform::New(this); VerifyOrExit(mRendezvousSession != nullptr, err = CHIP_ERROR_NO_MEMORY); err = mRendezvousSession->Init(params.SetLocalNodeId(mLocalDeviceId).SetRemoteNodeId(remoteDeviceId), mTransportMgr); @@ -493,6 +494,7 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam if (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle) { // IP Rendezvous + device->SetAddress(params.GetPeerAddress().GetIPAddress()); mRendezvousSession->OnRendezvousConnectionOpened(); } @@ -679,7 +681,7 @@ void DeviceCommissioner::OnRendezvousStatusUpdate(RendezvousSessionDelegate::Sta ChipLogDetail(Controller, "Remote device completed SPAKE2+ handshake\n"); mRendezvousSession->GetPairingSession().ToSerializable(device->GetPairing()); - if (mPairingDelegate != nullptr) + if (!mIsIPRendezvous && mPairingDelegate != nullptr) { mPairingDelegate->OnNetworkCredentialsRequested(mRendezvousSession); } diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 3c7050a97de52b..525abe08281b2a 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -297,6 +297,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, public Rendezvous If no device is currently being paired, this value will be kNumMaxPairedDevices. */ uint16_t mDeviceBeingPaired; + /* This is a temporary flag, when using IP rendezvous, we need to disable network provisioning. + In the future, network provisioning will no longer be a part of rendezvous procedure. */ + bool mIsIPRendezvous; + /* This field is true when device pairing information changes, e.g. a new device is paired, or the pairing for a device is removed. The DeviceCommissioner uses this to decide when to persist the device list */ diff --git a/src/controller/CHIPDeviceController_deprecated.cpp b/src/controller/CHIPDeviceController_deprecated.cpp index 4c69c931c0f723..9a5d94f88846e5 100644 --- a/src/controller/CHIPDeviceController_deprecated.cpp +++ b/src/controller/CHIPDeviceController_deprecated.cpp @@ -112,6 +112,9 @@ CHIP_ERROR ChipDeviceController::ConnectDevice(NodeId remoteDeviceId, Rendezvous mOnComplete.Response = onMessageReceived; mOnError = onError; + // TODO: Should call mOnNewConnected when rendezvous completed + mOnNewConnection(this, nullptr, mAppReqState); + exit: return err; } From b5a9394f9af6478d1d1d61f12bec19d413b3ccc8 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 2 Dec 2020 10:18:26 +0800 Subject: [PATCH 04/15] Update cirque tests --- .../linux-cirque/test-on-off-cluster.py | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/test_driver/linux-cirque/test-on-off-cluster.py b/src/test_driver/linux-cirque/test-on-off-cluster.py index 09e38fdd77cb68..bad65257de2ce3 100644 --- a/src/test_driver/linux-cirque/test-on-off-cluster.py +++ b/src/test_driver/linux-cirque/test-on-off-cluster.py @@ -46,6 +46,8 @@ } } +SETUPPINCODE = 12345678 +DISCRIMINATOR = 1 # Randomw number, not used CHIP_PORT = 11097 CIRQUE_URL = "http://localhost:5000" @@ -80,22 +82,30 @@ def run_data_model_test(self): command = "chip-tool onoff {} 1" for ip in server_ip_address: - ret = self.execute_device_cmd(tool_device_id, "chip-tool pairing bypass {} {}".format(ip, CHIP_PORT)) - self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format("pairing bypass", ret['output'])) + ret = self.execute_device_cmd( + tool_device_id, "chip-tool pairing softap ssid_not_used passwd_not_used {} {} {} {}".format(SETUPPINCODE, DISCRIMINATOR, ip, CHIP_PORT)) + self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format( + "pairing bypass", ret['output'])) ret = self.execute_device_cmd(tool_device_id, command.format("on")) - self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format("on", ret['output'])) + self.assertEqual( + ret['return_code'], '0', "{} command failure: {}".format("on", ret['output'])) - ret = self.execute_device_cmd(tool_device_id, command.format("off")) - self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format("off", ret['output'])) + ret = self.execute_device_cmd( + tool_device_id, command.format("off")) + self.assertEqual( + ret['return_code'], '0', "{} command failure: {}".format("off", ret['output'])) - ret = self.execute_device_cmd(tool_device_id, "chip-tool pairing unpair") - self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format("pairing unpair", ret['output'])) + ret = self.execute_device_cmd( + tool_device_id, "chip-tool pairing unpair") + self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format( + "pairing unpair", ret['output'])) time.sleep(1) for device_id in server_ids: - self.logger.info("checking device log for {}".format(self.get_device_pretty_id(device_id))) + self.logger.info("checking device log for {}".format( + self.get_device_pretty_id(device_id))) self.assertTrue(self.sequenceMatch(self.get_device_log(device_id).decode('utf-8'), ["LightingManager::InitiateAction(ON_ACTION)", "LightingManager::InitiateAction(OFF_ACTION)"]), "Datamodel test failed: cannot find matching string from device {}".format(device_id)) From d6e141880aa686ce7e7261d47bf03007ddcd1757 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 2 Dec 2020 10:41:20 +0800 Subject: [PATCH 05/15] Remove bypass_rendezvous flag for cirque tests --- src/test_driver/linux-cirque/OnOffClusterTest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test_driver/linux-cirque/OnOffClusterTest.sh b/src/test_driver/linux-cirque/OnOffClusterTest.sh index 1be3d8612b84b4..2175893d40e1d4 100755 --- a/src/test_driver/linux-cirque/OnOffClusterTest.sh +++ b/src/test_driver/linux-cirque/OnOffClusterTest.sh @@ -39,7 +39,7 @@ function build_chip_lighting() { source "$REPO_DIR/scripts/activate.sh" >/dev/null set -x cd "$chip_light_dir" - gn gen --check --fail-on-unused-args out/debug --args='chip_bypass_rendezvous=true' + gn gen --check --fail-on-unused-args out/debug run_ninja -C out/debug docker build -t chip_server -f Dockerfile . 2>&1 set +x From b91fe57bc3e216ef0137afa4e30ac1e1112f0654 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 2 Dec 2020 11:29:06 +0800 Subject: [PATCH 06/15] Fix BLE rendezvous --- .../python/ChipDeviceController-ScriptBinding.cpp | 10 +++++++--- src/controller/python/chip-device-ctrl.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 93d5297d1dcc9f..b6d9669000136e 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -52,6 +52,7 @@ #include #include +using namespace chip; using namespace chip::Ble; using namespace chip::DeviceController; @@ -536,9 +537,12 @@ CHIP_ERROR nl_Chip_DeviceController_Connect(chip::DeviceController::ChipDeviceCo uint32_t setupPINCode, OnConnectFunct onConnect, OnMessageFunct onMessage, OnErrorFunct onError) { - CHIP_ERROR err = CHIP_NO_ERROR; - chip::RendezvousParameters params = - chip::RendezvousParameters().SetSetupPINCode(setupPINCode).SetConnectionObject(connObj).SetBleLayer(&sBle); + CHIP_ERROR err = CHIP_NO_ERROR; + chip::RendezvousParameters params = chip::RendezvousParameters() + .SetPeerAddress(Transport::PeerAddress(Transport::Type::kBle)) + .SetSetupPINCode(setupPINCode) + .SetConnectionObject(connObj) + .SetBleLayer(&sBle); err = devCtrl->ConnectDevice(kRemoteDeviceId, params, (void *) devCtrl, onConnect, onMessage, onError); SuccessOrExit(err); diff --git a/src/controller/python/chip-device-ctrl.py b/src/controller/python/chip-device-ctrl.py index 1aa9b49217c4a1..6921cc0a45f21f 100755 --- a/src/controller/python/chip-device-ctrl.py +++ b/src/controller/python/chip-device-ctrl.py @@ -384,7 +384,7 @@ def do_connect(self, line): if args[0] == "-ip" and len(args) == 3: self.devCtrl.ConnectIP(args[1].encode("utf-8"), int(args[2])) elif args[0] == "-ble" and len(args) == 2: - self.devCtrl.Connect(FAKE_CONN_OBJ_VALUE, int(args[0])) + self.devCtrl.Connect(FAKE_CONN_OBJ_VALUE, int(args[1])) else: print("Usage:") self.do_help("connect SetupPinCode") From 010cbc618c31673d50049fdce57e2d985104859b Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 2 Dec 2020 11:44:16 +0800 Subject: [PATCH 07/15] Fix format issue --- src/controller/CHIPDeviceController.cpp | 29 ++++++++++++------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 91eee9841e7118..68cb118fafe55d 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -42,10 +42,6 @@ #include #include #include -#include -#include -#include -#include #include #include #include @@ -53,6 +49,11 @@ #include #include #include + +#include +#include +#include +#include #include using namespace chip::Inet; @@ -66,15 +67,14 @@ using namespace chip::Encoding; constexpr const char kPairedDeviceListKeyPrefix[] = "ListPairedDevices"; constexpr const char kPairedDeviceKeyPrefix[] = "PairedDevice"; -// This macro generates a key using node ID an key prefix, and performs the -// given action on that key. +// This macro generates a key using node ID an key prefix, and performs the given action +// on that key. #define PERSISTENT_KEY_OP(node, keyPrefix, key, action) \ do \ { \ constexpr size_t len = std::extent::value; \ nlSTATIC_ASSERT_PRINT(len > 0, "keyPrefix length must be known at compile time"); \ - /* 2 * sizeof(NodeId) to accomodate 2 character for each byte in Node Id \ - */ \ + /* 2 * sizeof(NodeId) to accomodate 2 character for each byte in Node Id */ \ char key[len + 2 * sizeof(NodeId) + 1]; \ nlSTATIC_ASSERT_PRINT(sizeof(node) <= sizeof(uint64_t), "Node ID size is greater than expected"); \ snprintf(key, sizeof(key), "%s%" PRIx64, keyPrefix, node); \ @@ -310,8 +310,7 @@ CHIP_ERROR DeviceController::ServiceEventSignal() DeviceLayer::SystemLayer.WakeSelect(); #else err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; -#endif // CONFIG_DEVICE_LAYER && (CHIP_SYSTEM_CONFIG_USE_SOCKETS || - // CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK) +#endif // CONFIG_DEVICE_LAYER && (CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK) exit: return err; @@ -642,11 +641,11 @@ void DeviceCommissioner::OnRendezvousComplete() mPairedDevices.Insert(device->GetDeviceId()); mPairedDevicesUpdated = true; - // mStorageDelegate would not be null for a typical pairing scenario, as - // Pair() requires a valid storage delegate. However, test pairing usecase, - // that's used mainly by test applications, do not require a storage delegate. - // This is to reduce overheads on these tests. Let's make sure the delegate - // object is available before calling into it. + // mStorageDelegate would not be null for a typical pairing scenario, as Pair() + // requires a valid storage delegate. However, test pairing usecase, that's used + // mainly by test applications, do not require a storage delegate. This is to + // reduce overheads on these tests. + // Let's make sure the delegate object is available before calling into it. if (mStorageDelegate != nullptr) { SerializedDevice serialized; From 1cb6badf879d0b7d14487bd907b241ea3a80657b Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 2 Dec 2020 14:01:52 +0800 Subject: [PATCH 08/15] Rollback most style changes in python scripts --- src/controller/python/chip/ChipDeviceCtrl.py | 60 +++++++------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index af3495132b8cc7..de3b073fd976c7 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -38,8 +38,7 @@ _CompleteFunct = CFUNCTYPE(None, c_void_p, c_void_p) -_ErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, - c_ulong, POINTER(DeviceStatusStruct)) +_ErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_ulong, POINTER(DeviceStatusStruct)) _GetBleEventFunct = CFUNCTYPE(c_void_p) _WriteBleCharacteristicFunct = CFUNCTYPE( c_bool, c_void_p, c_void_p, c_void_p, c_void_p, c_uint16 @@ -56,14 +55,11 @@ # typedef void (*OnMessageFunct)(Chip::DeviceController::ChipDeviceController * dc, void * appReqState, PacketBuffer * buffer); _OnConnectFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_void_p) -_OnRendezvousErrorFunct = CFUNCTYPE( - None, c_void_p, c_void_p, c_uint32, c_void_p) +_OnRendezvousErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_uint32, c_void_p) _OnMessageFunct = CFUNCTYPE(None, c_void_p, c_void_p, c_void_p) # This is a fix for WEAV-429. Jay Logue recommends revisiting this at a later # date to allow for truely multiple instances so this is temporary. - - def _singleton(cls): instance = [None] @@ -82,7 +78,6 @@ class DCState(enum.IntEnum): RENDEZVOUS_ONGOING = 3 RENDEZVOUS_CONNECTED = 4 - @_singleton class ChipDeviceController(object): def __init__(self, startNetworkThread=True): @@ -96,19 +91,16 @@ def __init__(self, startNetworkThread=True): self._InitLib() devCtrl = c_void_p(None) - res = self._dmLib.nl_Chip_DeviceController_NewDeviceController( - pointer(devCtrl)) + res = self._dmLib.nl_Chip_DeviceController_NewDeviceController(pointer(devCtrl)) if res != 0: raise self._ChipStack.ErrorToException(res) pairingDelegate = c_void_p(None) - res = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate( - pointer(pairingDelegate)) + res = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate(pointer(pairingDelegate)) if res != 0: raise self._ChipStack.ErrorToException(res) - res = self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate( - devCtrl, pairingDelegate) + res = self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate(devCtrl, pairingDelegate) if res != 0: raise self._ChipStack.ErrorToException(res) @@ -139,8 +131,7 @@ def HandleRendezvousError(appState, reqState, err, devStatusPtr): elif self.state == DCState.RENDEZVOUS_CONNECTED: print("Disconnected from device") - self.cbHandleRendezvousError = _OnRendezvousErrorFunct( - HandleRendezvousError) + self.cbHandleRendezvousError = _OnRendezvousErrorFunct(HandleRendezvousError) if startNetworkThread: self.StartNetworkThread() @@ -148,8 +139,7 @@ def HandleRendezvousError(appState, reqState, err, devStatusPtr): def __del__(self): if self.devCtrl != None: - self._dmLib.nl_Chip_DeviceController_DeleteDeviceManager( - self.devCtrl) + self._dmLib.nl_Chip_DeviceController_DeleteDeviceManager(self.devCtrl) self.devCtrl = None def DriveBleIO(self): @@ -161,13 +151,11 @@ def DriveBleIO(self): def SetBleEventCB(self, bleEventCB): if self.devCtrl != None: self.cbHandleBleEvent = _GetBleEventFunct(bleEventCB) - self._dmLib.nl_Chip_DeviceController_SetBleEventCB( - self.cbHandleBleEvent) + self._dmLib.nl_Chip_DeviceController_SetBleEventCB(self.cbHandleBleEvent) def SetBleWriteCharCB(self, bleWriteCharCB): if self.devCtrl != None: - self.cbHandleBleWriteChar = _WriteBleCharacteristicFunct( - bleWriteCharCB) + self.cbHandleBleWriteChar = _WriteBleCharacteristicFunct(bleWriteCharCB) self._dmLib.nl_Chip_DeviceController_SetBleWriteCharacteristic( self.cbHandleBleWriteChar ) @@ -184,8 +172,7 @@ def SetBleSubscribeCharCB(self, bleSubscribeCharCB): def SetBleCloseCB(self, bleCloseCB): if self.devCtrl != None: self.cbHandleBleClose = _CloseBleFunct(bleCloseCB) - self._dmLib.nl_Chip_DeviceController_SetBleClose( - self.cbHandleBleClose) + self._dmLib.nl_Chip_DeviceController_SetBleClose(self.cbHandleBleClose) def StartNetworkThread(self): if self.networkThread != None: @@ -198,16 +185,14 @@ def RunNetworkThread(): self._ChipStack.networkLock.release() time.sleep(0.005) - self.networkThread = Thread( - target=RunNetworkThread, name="ChipNetworkThread") + self.networkThread = Thread(target=RunNetworkThread, name="ChipNetworkThread") self.networkThread.daemon = True self.networkThreadRunable = True self.networkThread.start() def IsConnected(self): return self._ChipStack.Call( - lambda: self._dmLib.nl_Chip_DeviceController_IsConnected( - self.devCtrl) + lambda: self._dmLib.nl_Chip_DeviceController_IsConnected(self.devCtrl) ) def ConnectBle(self, bleConnection): @@ -270,8 +255,7 @@ def SetBlockingCB(self, blockingCB): self._ChipStack.blockingCB = blockingCB def SetWifiCredential(self, ssid, password): - ret = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential( - self.pairingDelegate, ssid.encode("utf-8"), password.encode("utf-8")) + ret = self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential(self.pairingDelegate, ssid.encode("utf-8"), password.encode("utf-8")) if ret != 0: raise self._ChipStack.ErrorToException(res) @@ -320,12 +304,10 @@ def _InitLib(self): c_uint32 ) - self._dmLib.nl_Chip_DeviceController_SetBleClose.argtypes = [ - _CloseBleFunct] + self._dmLib.nl_Chip_DeviceController_SetBleClose.argtypes = [_CloseBleFunct] self._dmLib.nl_Chip_DeviceController_SetBleClose.restype = c_uint32 - self._dmLib.nl_Chip_DeviceController_IsConnected.argtypes = [ - c_void_p] + self._dmLib.nl_Chip_DeviceController_IsConnected.argtypes = [c_void_p] self._dmLib.nl_Chip_DeviceController_IsConnected.restype = c_bool self._dmLib.nl_Chip_DeviceController_ValidateBTP.argtypes = [ @@ -339,8 +321,7 @@ def _InitLib(self): self._dmLib.nl_Chip_DeviceController_GetLogFilter.argtypes = [] self._dmLib.nl_Chip_DeviceController_GetLogFilter.restype = c_uint8 - self._dmLib.nl_Chip_DeviceController_SetLogFilter.argtypes = [ - c_uint8] + self._dmLib.nl_Chip_DeviceController_SetLogFilter.argtypes = [c_uint8] self._dmLib.nl_Chip_DeviceController_SetLogFilter.restype = None self._dmLib.nl_Chip_DeviceController_Connect.argtypes = [ @@ -351,14 +332,11 @@ def _InitLib(self): c_void_p, c_char_p, c_uint32, _OnConnectFunct, _OnMessageFunct, _OnRendezvousErrorFunct] self._dmLib.nl_Chip_DeviceController_ConnectIP.restype = c_uint32 - self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate.argtypes = [ - POINTER(c_void_p)] + self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate.argtypes = [POINTER(c_void_p)] self._dmLib.nl_Chip_ScriptDevicePairingDelegate_NewPairingDelegate.restype = c_uint32 - self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential.argtypes = [ - c_void_p, c_char_p, c_char_p] + self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential.argtypes = [c_void_p, c_char_p, c_char_p] self._dmLib.nl_Chip_ScriptDevicePairingDelegate_SetWifiCredential.restype = c_uint32 - self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate.argtypes = [ - c_void_p, c_void_p] + self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate.argtypes = [c_void_p, c_void_p] self._dmLib.nl_Chip_DeviceController_SetDevicePairingDelegate.restype = c_uint32 From da51b99fa0093d18d706d7dc79e5aea2cac38c5f Mon Sep 17 00:00:00 2001 From: Song Guo Date: Thu, 3 Dec 2020 11:51:02 +0800 Subject: [PATCH 09/15] Address comments --- src/controller/CHIPDeviceController.cpp | 3 +- .../ChipDeviceController-ScriptBinding.cpp | 33 +++++++------------ .../ChipDeviceController-StorageDelegate.h | 5 +-- src/transport/RendezvousSession.cpp | 2 ++ 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 68cb118fafe55d..faa875b7ab1eae 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -490,9 +490,10 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam device->Init(mTransportMgr, mSessionManager, mInetLayer, remoteDeviceId, remotePort, interfaceId); + // TODO: BLE rendezvous and IP rendezvous should have same logic in the future after BLE becomes a transport and network + // provisiong cluster is ready. if (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle) { - // IP Rendezvous device->SetAddress(params.GetPeerAddress().GetIPAddress()); mRendezvousSession->OnRendezvousConnectionOpened(); } diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index b6d9669000136e..5f605732aeee47 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include using namespace chip; @@ -537,17 +538,13 @@ CHIP_ERROR nl_Chip_DeviceController_Connect(chip::DeviceController::ChipDeviceCo uint32_t setupPINCode, OnConnectFunct onConnect, OnMessageFunct onMessage, OnErrorFunct onError) { - CHIP_ERROR err = CHIP_NO_ERROR; - chip::RendezvousParameters params = chip::RendezvousParameters() - .SetPeerAddress(Transport::PeerAddress(Transport::Type::kBle)) - .SetSetupPINCode(setupPINCode) - .SetConnectionObject(connObj) - .SetBleLayer(&sBle); - err = devCtrl->ConnectDevice(kRemoteDeviceId, params, (void *) devCtrl, onConnect, onMessage, onError); - SuccessOrExit(err); - -exit: - return err; + return devCtrl->ConnectDevice(kRemoteDeviceId, + chip::RendezvousParameters() + .SetPeerAddress(Transport::PeerAddress(Transport::Type::kBle)) + .SetSetupPINCode(setupPINCode) + .SetConnectionObject(connObj) + .SetBleLayer(&sBle), + (void *) devCtrl, onConnect, onMessage, onError); } CHIP_ERROR nl_Chip_DeviceController_ConnectIP(chip::DeviceController::ChipDeviceController * devCtrl, const char * peerAddrStr, @@ -559,17 +556,11 @@ CHIP_ERROR nl_Chip_DeviceController_ConnectIP(chip::DeviceController::ChipDevice chip::Transport::PeerAddress addr; chip::RendezvousParameters params = chip::RendezvousParameters().SetSetupPINCode(setupPINCode); - VerifyOrExit(chip::Inet::IPAddress::FromString(peerAddrStr, peerAddr), err = CHIP_ERROR_INVALID_ARGUMENT); - addr.SetIPAddress(peerAddr); + VerifyOrReturnError(chip::Inet::IPAddress::FromString(peerAddrStr, peerAddr), err = CHIP_ERROR_INVALID_ARGUMENT); // TODO: IP rendezvous should use TCP connection. - addr.SetTransportType(chip::Transport::Type::kUdp); - params.SetPeerAddress(addr); - params.SetDiscriminator(0); - err = devCtrl->ConnectDevice(kRemoteDeviceId, params, (void *) devCtrl, onConnect, onMessage, onError); - SuccessOrExit(err); - -exit: - return err; + addr.SetTransportType(chip::Transport::Type::kUdp).SetIPAddress(peerAddr); + params.SetPeerAddress(addr).SetDiscriminator(0); + return devCtrl->ConnectDevice(kRemoteDeviceId, params, (void *) devCtrl, onConnect, onMessage, onError); } CHIP_ERROR diff --git a/src/controller/python/ChipDeviceController-StorageDelegate.h b/src/controller/python/ChipDeviceController-StorageDelegate.h index cb958477a86031..606fddda6e3e97 100644 --- a/src/controller/python/ChipDeviceController-StorageDelegate.h +++ b/src/controller/python/ChipDeviceController-StorageDelegate.h @@ -4,11 +4,9 @@ class PythonPersistentStorageDelegate; -extern "C" { typedef void (*GetKeyValueFunct)(const uint8_t * key, uint8_t * value, uint16_t * size); typedef void (*SetKeyValueFunct)(const uint8_t * key, const uint8_t * value); typedef void (*DeleteKeyValueFunct)(const uint8_t * key); -} namespace chip { namespace Controller { @@ -18,10 +16,9 @@ class PythonPersistentStorageDelegate : public PersistentStorageDelegate { public: PythonPersistentStorageDelegate() {} - ~PythonPersistentStorageDelegate() {} void SetDelegate(PersistentStorageResultDelegate * delegate) override{}; void GetKeyValue(const char * key) override{}; - CHIP_ERROR GetKeyValue(const char * key, char * value, uint16_t & size) override { return CHIP_NO_ERROR; }; + CHIP_ERROR GetKeyValue(const char * key, char * value, uint16_t & size) override { return CHIP_ERROR_NOT_IMPLEMENTED; }; void SetKeyValue(const char * key, const char * value) override{}; void DeleteKeyValue(const char * key) override{}; }; diff --git a/src/transport/RendezvousSession.cpp b/src/transport/RendezvousSession.cpp index 669104a5d7bbd5..b696434f37ba73 100644 --- a/src/transport/RendezvousSession.cpp +++ b/src/transport/RendezvousSession.cpp @@ -181,6 +181,8 @@ void RendezvousSession::OnPairingComplete() return; } + // TODO: This check of BLE transport should be removed in the future, after we have network provisioning cluster and ble becomes + // a transport. if (mParams.GetPeerAddress().GetTransportType() != Transport::Type::kBle || // For rendezvous initializer mPeerAddress.GetTransportType() != Transport::Type::kBle) // For rendezvous target { From fe7185dd58dbd2fb4b4165e2c77b615e65f7e0dc Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 8 Dec 2020 09:40:48 +0800 Subject: [PATCH 10/15] Address comments --- .../python/ChipDeviceController-StorageDelegate.h | 10 +++++----- src/controller/python/chip/ChipDeviceCtrl.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/controller/python/ChipDeviceController-StorageDelegate.h b/src/controller/python/ChipDeviceController-StorageDelegate.h index 606fddda6e3e97..065350388741d9 100644 --- a/src/controller/python/ChipDeviceController-StorageDelegate.h +++ b/src/controller/python/ChipDeviceController-StorageDelegate.h @@ -16,11 +16,11 @@ class PythonPersistentStorageDelegate : public PersistentStorageDelegate { public: PythonPersistentStorageDelegate() {} - void SetDelegate(PersistentStorageResultDelegate * delegate) override{}; - void GetKeyValue(const char * key) override{}; - CHIP_ERROR GetKeyValue(const char * key, char * value, uint16_t & size) override { return CHIP_ERROR_NOT_IMPLEMENTED; }; - void SetKeyValue(const char * key, const char * value) override{}; - void DeleteKeyValue(const char * key) override{}; + void SetDelegate(PersistentStorageResultDelegate * delegate) override{} + void GetKeyValue(const char * key) override{} + CHIP_ERROR GetKeyValue(const char * key, char * value, uint16_t & size) override { return CHIP_ERROR_NOT_IMPLEMENTED; } + void SetKeyValue(const char * key, const char * value) override{} + void DeleteKeyValue(const char * key) override{} }; } // namespace Controller diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index de3b073fd976c7..ab484958616abe 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -207,7 +207,7 @@ def ConnectBle(self, bleConnection): def Connect(self, connObj, setupPinCode): def HandleComplete(dc, connState, appState): - print("Rendezvoud Complete") + print("Rendezvous Complete") self.state = DCState.RENDEZVOUS_CONNECTED self._ChipStack.callbackRes = True self._ChipStack.completeEvent.set() @@ -221,7 +221,7 @@ def HandleComplete(dc, connState, appState): def ConnectIP(self, ipaddr, setupPinCode): def HandleComplete(dc, connState, appState): - print("Rendezvoud Complete") + print("Rendezvous Complete") self.state = DCState.RENDEZVOUS_CONNECTED self._ChipStack.callbackRes = True self._ChipStack.completeEvent.set() From dfbe58a20a3df5f368b35ea65ed2bd757b84a954 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 8 Dec 2020 01:41:15 +0000 Subject: [PATCH 11/15] Restyled by clang-format --- .../python/ChipDeviceController-StorageDelegate.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/controller/python/ChipDeviceController-StorageDelegate.h b/src/controller/python/ChipDeviceController-StorageDelegate.h index 065350388741d9..c7d046eed261dd 100644 --- a/src/controller/python/ChipDeviceController-StorageDelegate.h +++ b/src/controller/python/ChipDeviceController-StorageDelegate.h @@ -16,11 +16,11 @@ class PythonPersistentStorageDelegate : public PersistentStorageDelegate { public: PythonPersistentStorageDelegate() {} - void SetDelegate(PersistentStorageResultDelegate * delegate) override{} - void GetKeyValue(const char * key) override{} + void SetDelegate(PersistentStorageResultDelegate * delegate) override {} + void GetKeyValue(const char * key) override {} CHIP_ERROR GetKeyValue(const char * key, char * value, uint16_t & size) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - void SetKeyValue(const char * key, const char * value) override{} - void DeleteKeyValue(const char * key) override{} + void SetKeyValue(const char * key, const char * value) override {} + void DeleteKeyValue(const char * key) override {} }; } // namespace Controller From 3905f1ba868ccc3ff2c6e36e784aa5f64df4783e Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 8 Dec 2020 10:01:38 +0800 Subject: [PATCH 12/15] Add copyright header --- .../ChipDeviceController-StorageDelegate.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/controller/python/ChipDeviceController-StorageDelegate.h b/src/controller/python/ChipDeviceController-StorageDelegate.h index c7d046eed261dd..29de2e62b178dc 100644 --- a/src/controller/python/ChipDeviceController-StorageDelegate.h +++ b/src/controller/python/ChipDeviceController-StorageDelegate.h @@ -1,3 +1,21 @@ +/* + * + * Copyright (c) 2020 Project CHIP Authors + * 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. + */ + #pragma once #include From 2c1538b386cddf17fcf1321886e449fa41e03d46 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 8 Dec 2020 13:09:53 +0800 Subject: [PATCH 13/15] Fix log in cirque test --- src/test_driver/linux-cirque/test-on-off-cluster.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test_driver/linux-cirque/test-on-off-cluster.py b/src/test_driver/linux-cirque/test-on-off-cluster.py index bad65257de2ce3..94d3f5e32aae5c 100644 --- a/src/test_driver/linux-cirque/test-on-off-cluster.py +++ b/src/test_driver/linux-cirque/test-on-off-cluster.py @@ -85,7 +85,7 @@ def run_data_model_test(self): ret = self.execute_device_cmd( tool_device_id, "chip-tool pairing softap ssid_not_used passwd_not_used {} {} {} {}".format(SETUPPINCODE, DISCRIMINATOR, ip, CHIP_PORT)) self.assertEqual(ret['return_code'], '0', "{} command failure: {}".format( - "pairing bypass", ret['output'])) + "pairing softap", ret['output'])) ret = self.execute_device_cmd(tool_device_id, command.format("on")) self.assertEqual( From 5459a4f2d59e622baa1929281378a3ad991d6bb5 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Tue, 8 Dec 2020 15:00:45 +0800 Subject: [PATCH 14/15] Fix cirque test --- examples/common/chip-app-server/RendezvousServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/common/chip-app-server/RendezvousServer.cpp b/examples/common/chip-app-server/RendezvousServer.cpp index 1bf3a980285764..ead0cf85d46088 100644 --- a/examples/common/chip-app-server/RendezvousServer.cpp +++ b/examples/common/chip-app-server/RendezvousServer.cpp @@ -62,7 +62,7 @@ void RendezvousServer::OnRendezvousMessageReceived(const PacketHeader & packetHe void RendezvousServer::OnRendezvousComplete() { ChipLogProgress(AppServer, "Device completed Rendezvous process"); - if (mRendezvousSession.GetLocalNodeId().HasValue() && mRendezvousSession.GetRemoteNodeId().HasValue()) + if (mRendezvousSession.GetRemoteNodeId().HasValue()) { SessionManager().NewPairing(Optional{}, mRendezvousSession.GetRemoteNodeId().Value(), &mRendezvousSession.GetPairingSession()); From c87550f81a4097685c7d3403bd4278a833408987 Mon Sep 17 00:00:00 2001 From: Song Guo Date: Wed, 9 Dec 2020 09:45:18 +0800 Subject: [PATCH 15/15] Add todo for mIsIPRendezvous flag --- src/controller/CHIPDeviceController.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 525abe08281b2a..e69c4f621e2679 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -297,8 +297,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, public Rendezvous If no device is currently being paired, this value will be kNumMaxPairedDevices. */ uint16_t mDeviceBeingPaired; - /* This is a temporary flag, when using IP rendezvous, we need to disable network provisioning. - In the future, network provisioning will no longer be a part of rendezvous procedure. */ + /* TODO: BLE rendezvous and IP rendezvous should share the same procedure, so this is just a + workaround-like flag and should be removed in the future. + When using IP rendezvous, we need to disable network provisioning. In the future, network + provisioning will no longer be a part of rendezvous procedure. */ bool mIsIPRendezvous; /* This field is true when device pairing information changes, e.g. a new device is paired, or