From 25385939f86a504361a1066b6f02922e78c2fd66 Mon Sep 17 00:00:00 2001 From: C Freeman Date: Mon, 29 Nov 2021 12:36:33 -0500 Subject: [PATCH] Return only operational devices GetConnectedDevice (#11993) * Return only operational devices GetConnectedDevice With the exception of commissioning, all cluster commands should be sent over operational connection. Falling back to the PASE connection is probably not what we actually want here. For commissioning, callers should either use the provided commissioning flow in the SDK or use the provided GetDeviceBeingCommissioned. This also makes it much more obvious in the code which connection is being used. * Fix chip-device-ctrl for BLE connections. chip-device-ctrl BLE commissioning procedure is very manual which means folks are actually sending commands manually over the PASE connection to add and enable the network. This first checks the PASE connection and sends over CASE if we're not currently commissioning the device. * Restyled by autopep8 Co-authored-by: Restyled.io --- examples/chip-tool/commands/clusters/ModelCommand.h | 2 +- .../chip-tool/commands/pairing/PairingCommand.cpp | 2 +- examples/chip-tool/commands/pairing/PairingCommand.h | 2 +- .../chip-tool/commands/reporting/ReportingCommand.cpp | 2 +- .../chip-tool/commands/reporting/ReportingCommand.h | 2 +- examples/chip-tool/commands/tests/TestCommand.cpp | 2 +- examples/chip-tool/commands/tests/TestCommand.h | 2 +- examples/ota-requestor-app/linux/main.cpp | 4 ++-- src/app/OperationalDeviceProxy.h | 2 +- src/controller/CHIPDeviceController.cpp | 8 +------- src/controller/CHIPDeviceController.h | 4 ++-- src/controller/java/AndroidCallbacks.cpp | 2 +- src/controller/java/AndroidCallbacks.h | 2 +- .../python/ChipDeviceController-ScriptBinding.cpp | 10 +++++++++- src/controller/python/chip/ChipDeviceCtrl.py | 11 +++++++++++ .../Framework/CHIP/CHIPDeviceConnectionBridge.h | 2 +- .../Framework/CHIP/CHIPDeviceConnectionBridge.mm | 2 +- 17 files changed, 37 insertions(+), 24 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ModelCommand.h b/examples/chip-tool/commands/clusters/ModelCommand.h index 71f79d4a2b3b08..c99bade1633101 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.h +++ b/examples/chip-tool/commands/clusters/ModelCommand.h @@ -27,7 +27,7 @@ class ModelCommand : public CHIPCommand { public: - using ChipDevice = ::chip::DeviceProxy; + using ChipDevice = ::chip::OperationalDeviceProxy; ModelCommand(const char * commandName) : CHIPCommand(commandName), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index ba8d75bcc28997..55ef8f23e6b8c9 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -96,7 +96,7 @@ CHIP_ERROR PairingCommand::RunInternal(NodeId remoteId) return err; } -void PairingCommand::OnDeviceConnectedFn(void * context, chip::DeviceProxy * device) +void PairingCommand::OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device) { PairingCommand * command = reinterpret_cast(context); command->OpenCommissioningWindow(); diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h index 2dc38bac1c35af..92932efe91246c 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.h +++ b/examples/chip-tool/commands/pairing/PairingCommand.h @@ -214,7 +214,7 @@ class PairingCommand : public CHIPCommand, chip::Controller::NetworkCommissioningCluster mCluster; chip::EndpointId mEndpointId = 0; - static void OnDeviceConnectedFn(void * context, chip::DeviceProxy * device); + static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device); static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); static void OnOpenCommissioningWindowResponse(void * context, NodeId deviceId, CHIP_ERROR status, chip::SetupPayload payload); diff --git a/examples/chip-tool/commands/reporting/ReportingCommand.cpp b/examples/chip-tool/commands/reporting/ReportingCommand.cpp index b0be63f6bf5500..8ff30256a42be8 100644 --- a/examples/chip-tool/commands/reporting/ReportingCommand.cpp +++ b/examples/chip-tool/commands/reporting/ReportingCommand.cpp @@ -35,7 +35,7 @@ CHIP_ERROR ReportingCommand::RunCommand() return err; } -void ReportingCommand::OnDeviceConnectedFn(void * context, chip::DeviceProxy * device) +void ReportingCommand::OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device) { ReportingCommand * command = reinterpret_cast(context); VerifyOrReturn(command != nullptr, diff --git a/examples/chip-tool/commands/reporting/ReportingCommand.h b/examples/chip-tool/commands/reporting/ReportingCommand.h index 4547f256926167..ca5f32409708bb 100644 --- a/examples/chip-tool/commands/reporting/ReportingCommand.h +++ b/examples/chip-tool/commands/reporting/ReportingCommand.h @@ -44,7 +44,7 @@ class ReportingCommand : public CHIPCommand NodeId mNodeId; uint8_t mEndPointId; - static void OnDeviceConnectedFn(void * context, chip::DeviceProxy * device); + static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device); static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); chip::Callback::Callback mOnDeviceConnectedCallback; diff --git a/examples/chip-tool/commands/tests/TestCommand.cpp b/examples/chip-tool/commands/tests/TestCommand.cpp index c6f24989470966..62a86e3e8eea03 100644 --- a/examples/chip-tool/commands/tests/TestCommand.cpp +++ b/examples/chip-tool/commands/tests/TestCommand.cpp @@ -23,7 +23,7 @@ CHIP_ERROR TestCommand::RunCommand() return mController.GetConnectedDevice(mNodeId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback); } -void TestCommand::OnDeviceConnectedFn(void * context, chip::DeviceProxy * device) +void TestCommand::OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device) { ChipLogProgress(chipTool, " **** Test Setup: Device Connected\n"); auto * command = static_cast(context); diff --git a/examples/chip-tool/commands/tests/TestCommand.h b/examples/chip-tool/commands/tests/TestCommand.h index 76206a324267c9..7e4d32923e6e01 100644 --- a/examples/chip-tool/commands/tests/TestCommand.h +++ b/examples/chip-tool/commands/tests/TestCommand.h @@ -58,7 +58,7 @@ class TestCommand : public CHIPCommand ChipDevice * mDevice; chip::NodeId mNodeId; - static void OnDeviceConnectedFn(void * context, chip::DeviceProxy * device); + static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device); static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); static void OnWaitForMsFn(chip::System::Layer * systemLayer, void * context); diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 5d390b5d888f31..08dd38e3d21a64 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -56,7 +56,7 @@ using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands; void OnQueryImageResponse(void * context, const QueryImageResponse::DecodableType & response); void OnQueryImageFailure(void * context, EmberAfStatus status); -void OnConnected(void * context, chip::DeviceProxy * deviceProxy); +void OnConnected(void * context, chip::OperationalDeviceProxy * deviceProxy); void OnConnectionFailure(void * context, NodeId deviceId, CHIP_ERROR error); bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, const char * aName, const char * aValue); @@ -153,7 +153,7 @@ void OnQueryImageFailure(void * context, EmberAfStatus status) ChipLogDetail(SoftwareUpdate, "QueryImage failure response %" PRIu8, status); } -void OnConnected(void * context, chip::DeviceProxy * deviceProxy) +void OnConnected(void * context, chip::OperationalDeviceProxy * deviceProxy) { CHIP_ERROR err = CHIP_NO_ERROR; chip::Controller::OtaSoftwareUpdateProviderCluster cluster; diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index ee2496ec614a4b..5e60c30e9aec32 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -66,7 +66,7 @@ struct DeviceProxyInitParams class OperationalDeviceProxy; -typedef void (*OnDeviceConnected)(void * context, DeviceProxy * device); +typedef void (*OnDeviceConnected)(void * context, OperationalDeviceProxy * device); typedef void (*OnDeviceConnectionFailure)(void * context, NodeId deviceId, CHIP_ERROR error); class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDelegate, public SessionEstablishmentDelegate diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 970a685baa5977..fd35d846fe2ec0 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -676,12 +676,6 @@ CHIP_ERROR DeviceCommissioner::GetDeviceBeingCommissioned(NodeId deviceId, Commi CHIP_ERROR DeviceCommissioner::GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, Callback::Callback * onFailure) { - if (mDeviceBeingCommissioned != nullptr && mDeviceBeingCommissioned->GetDeviceId() == deviceId && - mDeviceBeingCommissioned->IsSecureConnected()) - { - onConnection->mCall(onConnection->mContext, mDeviceBeingCommissioned); - return CHIP_NO_ERROR; - } return DeviceController::GetConnectedDevice(deviceId, onConnection, onFailure); } @@ -1554,7 +1548,7 @@ void DeviceCommissioner::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHI #endif -void DeviceCommissioner::OnDeviceConnectedFn(void * context, DeviceProxy * device) +void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device) { DeviceCommissioner * commissioner = static_cast(context); VerifyOrReturn(commissioner != nullptr, ChipLogProgress(Controller, "Device connected callback with null context. Ignoring")); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 2d549b74d40d28..c3c12300e42b03 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -591,7 +591,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, DevicePairingDelegate * mPairingDelegate; CommissioneeDeviceProxy * mDeviceBeingCommissioned = nullptr; - DeviceProxy * mDeviceOperational = nullptr; + OperationalDeviceProxy * mDeviceOperational = nullptr; Credentials::CertificateType mCertificateTypeBeingRequested = Credentials::CertificateType::kUnknown; @@ -688,7 +688,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, /* Callback called when adding root cert to device results in failure */ static void OnRootCertFailureResponse(void * context, uint8_t status); - static void OnDeviceConnectedFn(void * context, DeviceProxy * device); + static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device); static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error); static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac, diff --git a/src/controller/java/AndroidCallbacks.cpp b/src/controller/java/AndroidCallbacks.cpp index c498c802b187d6..d4b844a6b04a22 100644 --- a/src/controller/java/AndroidCallbacks.cpp +++ b/src/controller/java/AndroidCallbacks.cpp @@ -44,7 +44,7 @@ GetConnectedDeviceCallback::~GetConnectedDeviceCallback() env->DeleteGlobalRef(mJavaCallbackRef); } -void GetConnectedDeviceCallback::OnDeviceConnectedFn(void * context, DeviceProxy * device) +void GetConnectedDeviceCallback::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device) { JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); auto * self = static_cast(context); diff --git a/src/controller/java/AndroidCallbacks.h b/src/controller/java/AndroidCallbacks.h index 123c2fdb10aeb9..9691e77f5fd770 100644 --- a/src/controller/java/AndroidCallbacks.h +++ b/src/controller/java/AndroidCallbacks.h @@ -28,7 +28,7 @@ struct GetConnectedDeviceCallback GetConnectedDeviceCallback(jobject javaCallback); ~GetConnectedDeviceCallback(); - static void OnDeviceConnectedFn(void * context, DeviceProxy * device); + static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device); static void OnDeviceConnectionFailureFn(void * context, NodeId nodeId, CHIP_ERROR error); Callback::Callback mOnSuccess; diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index c7fc136760d8fc..35e85bc3b8a055 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -163,6 +163,8 @@ void pychip_Stack_SetLogFunct(LogMessageFunct logFunct); ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId, DeviceAvailableFunc callback); +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); @@ -534,7 +536,7 @@ struct GetDeviceCallbacks mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnConnectionFailureFn, this), mCallback(callback) {} - static void OnDeviceConnectedFn(void * context, DeviceProxy * device) + static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device) { auto * self = static_cast(context); self->mCallback(device, CHIP_NO_ERROR.AsInteger()); @@ -562,6 +564,12 @@ ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::Devic return devCtrl->GetConnectedDevice(nodeId, &callbacks->mOnSuccess, &callbacks->mOnFailure).AsInteger(); } +ChipError::StorageType pychip_GetDeviceBeingCommissioned(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId, + CommissioneeDeviceProxy ** proxy) +{ + return devCtrl->GetDeviceBeingCommissioned(nodeId, proxy).AsInteger(); +} + ChipError::StorageType pychip_DeviceCommissioner_CloseBleConnection(chip::Controller::DeviceCommissioner * devCtrl) { #if CONFIG_NETWORK_LAYER_BLE diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index edaf1438fdc7b6..e14941a90f277e 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -339,6 +339,13 @@ def DeviceAvailableCallback(device, err): print("Failed in getting the connected device: {}".format(err)) raise self._ChipStack.ErrorToException(err) + res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceBeingCommissioned( + self.devCtrl, nodeid, byref(returnDevice))) + if res == 0: + # TODO: give users more contrtol over whether they want to send this command over a PASE established connection + print('Using PASE connection') + return returnDevice + res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetConnectedDeviceByNodeId( self.devCtrl, nodeid, _DeviceAvailableFunct(DeviceAvailableCallback))) if res != 0: @@ -630,6 +637,10 @@ def _InitLib(self): c_void_p, c_uint64, _DeviceAvailableFunct] self._dmLib.pychip_GetConnectedDeviceByNodeId.restype = c_uint32 + self._dmLib.pychip_GetDeviceBeingCommissioned.argtypes = [ + c_void_p, c_uint64, c_void_p] + self._dmLib.pychip_GetDeviceBeingCommissioned.restype = c_uint32 + self._dmLib.pychip_DeviceCommissioner_CloseBleConnection.argtypes = [ c_void_p] self._dmLib.pychip_DeviceCommissioner_CloseBleConnection.restype = c_uint32 diff --git a/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.h b/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.h index 240dab2523629c..a83cc533ded234 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.h +++ b/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.h @@ -48,7 +48,7 @@ class CHIPDeviceConnectionBridge : public chip::ReferenceCounted mOnConnected; chip::Callback::Callback mOnConnectFailed; - static void OnConnected(void * context, chip::DeviceProxy * device); + static void OnConnected(void * context, chip::OperationalDeviceProxy * device); static void OnConnectionFailure(void * context, chip::NodeId deviceId, CHIP_ERROR error); }; diff --git a/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.mm b/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.mm index 24e49d325908dc..ac0241a7c614f7 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.mm +++ b/src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.mm @@ -19,7 +19,7 @@ #import "CHIPDevice_Internal.h" #import "CHIPError_Internal.h" -void CHIPDeviceConnectionBridge::OnConnected(void * context, chip::DeviceProxy * device) +void CHIPDeviceConnectionBridge::OnConnected(void * context, chip::OperationalDeviceProxy * device) { auto * object = static_cast(context); CHIPDevice * chipDevice = [[CHIPDevice alloc] initWithDevice:device];