Skip to content

Commit

Permalink
Return only operational devices GetConnectedDevice (#11993)
Browse files Browse the repository at this point in the history
* 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 <commits@restyled.io>
  • Loading branch information
2 people authored and pull[bot] committed Jan 11, 2022
1 parent fedc69d commit 3360643
Show file tree
Hide file tree
Showing 17 changed files with 37 additions and 24 deletions.
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/ModelCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<PairingCommand *>(context);
command->OpenCommissioningWindow();
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/reporting/ReportingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReportingCommand *>(context);
VerifyOrReturn(command != nullptr,
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/reporting/ReportingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/tests/TestCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestCommand *>(context);
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/tests/TestCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions examples/ota-requestor-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/app/OperationalDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,12 +676,6 @@ CHIP_ERROR DeviceCommissioner::GetDeviceBeingCommissioned(NodeId deviceId, Commi
CHIP_ERROR DeviceCommissioner::GetConnectedDevice(NodeId deviceId, Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure)
{
if (mDeviceBeingCommissioned != nullptr && mDeviceBeingCommissioned->GetDeviceId() == deviceId &&
mDeviceBeingCommissioned->IsSecureConnected())
{
onConnection->mCall(onConnection->mContext, mDeviceBeingCommissioned);
return CHIP_NO_ERROR;
}
return DeviceController::GetConnectedDevice(deviceId, onConnection, onFailure);
}

Expand Down Expand Up @@ -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<DeviceCommissioner *>(context);
VerifyOrReturn(commissioner != nullptr, ChipLogProgress(Controller, "Device connected callback with null context. Ignoring"));
Expand Down
4 changes: 2 additions & 2 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/controller/java/AndroidCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<GetConnectedDeviceCallback *>(context);
Expand Down
2 changes: 1 addition & 1 deletion src/controller/java/AndroidCallbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<OnDeviceConnected> mOnSuccess;
Expand Down
10 changes: 9 additions & 1 deletion src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<GetDeviceCallbacks *>(context);
self->mCallback(device, CHIP_NO_ERROR.AsInteger());
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/controller/python/chip/ChipDeviceCtrl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class CHIPDeviceConnectionBridge : public chip::ReferenceCounted<CHIPDeviceConne
chip::Callback::Callback<chip::OnDeviceConnected> mOnConnected;
chip::Callback::Callback<chip::OnDeviceConnectionFailure> 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);
};

Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/CHIPDeviceConnectionBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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<CHIPDeviceConnectionBridge *>(context);
CHIPDevice * chipDevice = [[CHIPDevice alloc] initWithDevice:device];
Expand Down

0 comments on commit 3360643

Please sign in to comment.