From 70e4ddeacc85747e3ec6406b1437deda45d170b3 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Thu, 24 Feb 2022 08:05:40 -0800 Subject: [PATCH 1/5] fix BLE commissioning on commissionable Matter TVs --- src/transport/raw/BLE.cpp | 13 +++++++++-- src/transport/raw/BLE.h | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/transport/raw/BLE.cpp b/src/transport/raw/BLE.cpp index f41a01b07e236a..d1581b4d97db26 100644 --- a/src/transport/raw/BLE.cpp +++ b/src/transport/raw/BLE.cpp @@ -47,6 +47,7 @@ void BLEBase::ClearState() { mBleLayer->CancelBleIncompleteConnection(); mBleLayer->OnChipBleConnectReceived = nullptr; + mBleLayer->mBleTransport = nullptr; mBleLayer = nullptr; } @@ -66,8 +67,16 @@ CHIP_ERROR BLEBase::Init(const BleListenParameters & param) VerifyOrReturnError(mState == State::kNotReady, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(bleLayer != nullptr, CHIP_ERROR_INCORRECT_STATE); - mBleLayer = bleLayer; - mBleLayer->mBleTransport = this; + mBleLayer = bleLayer; + if (mBleLayer->mBleTransport == nullptr || !param.PreserveExistingBleLayerTransport()) + { + mBleLayer->mBleTransport = this; + ChipLogDetail(Inet, "BLEBase::Init - setting/overriding transport"); + } + else + { + ChipLogDetail(Inet, "BLEBase::Init - not overriding transport"); + } mBleLayer->OnChipBleConnectReceived = nullptr; mState = State::kInitialized; diff --git a/src/transport/raw/BLE.h b/src/transport/raw/BLE.h index d54ecc2454d586..548af414b2fffe 100644 --- a/src/transport/raw/BLE.h +++ b/src/transport/raw/BLE.h @@ -48,8 +48,39 @@ class BleListenParameters Ble::BleLayer * GetBleLayer() const { return mLayer; } + /** + * PreserveExistingBleLayerTransport controls whether the BleBase transport + * initialized with these parameters should update the global BleLayer transport + * if it is already set. + * + * This is relevant when there is more than one TransportMgr, + * for example, when a device is both a commissioner (has a CHIPDeviceController) + * and a commissionee (has a Server) since each TransportMgr will have a BleBase + * which can override the global BleLayer transport to point to itself. + * + * The default value is true - don't override the global BleLayer transport if it is + * already set. In other words, the first BleBase to initialize (eg. Server) will be + * the active BleBase transport, and the second BleBase to initialize (eg. CHIPDeviceController) + * will need to call BleBase.SetBleLayerTransportToSelf if it needs to commission using BLE. + * + * Call SetPreserveExistingBleLayerTransport(false) to set the global + * BleLayer transport to the BleBase created with these parameters, even if it is already + * set to another BleBase. + * + * Use the BleBase.IsBleLayerTransportSetToSelf() and BleBase.SetBleLayerTransportToSelf + * methods to toggle between BleBase transports when there is more than one. + */ + bool PreserveExistingBleLayerTransport() const { return mPreserveExistingBleLayerTransport; } + BleListenParameters & SetPreserveExistingBleLayerTransport(bool preserveExistingBleLayerTransport) + { + mPreserveExistingBleLayerTransport = preserveExistingBleLayerTransport; + + return *this; + } + private: Ble::BleLayer * mLayer; + bool mPreserveExistingBleLayerTransport = true; }; /** Implements a transport using BLE. @@ -91,6 +122,20 @@ class DLL_EXPORT BLEBase : public Base, public Ble::BleLayerDelegate CHIP_ERROR SetEndPoint(Ble::BLEEndPoint * endPoint) override; + /** + * Change BLE transport to this + * + * This is relevant when there is more than one TransportMgr, + * for example, when a device is both a commissioner (has a CHIPDeviceController) + * and a commissionee (has a Server) since each TransportMgr will set + * the global BleLayer transport to point to itself. + * + * In this scenario, the device will need the ability to toggle between a + * BleLayer transport for commissioner functionality and one for commissionee functionality. + */ + void SetBleLayerTransportToSelf() { mBleLayer->mBleTransport = this; } + bool IsBleLayerTransportSetToSelf() { return mBleLayer->mBleTransport == this; } + private: void ClearState(); From 659f2616eca3733419fd27d3bea50ecc72f35357 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Thu, 24 Feb 2022 17:23:48 -0800 Subject: [PATCH 2/5] make sure BLE transport is setup for commissioning --- src/controller/CHIPDeviceController.cpp | 14 ++++++++++++++ src/controller/CHIPDeviceController.h | 9 +++++++++ src/controller/SetUpCodePairer.cpp | 4 ++++ 3 files changed, 27 insertions(+) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index c7db1e3a4cd035..bc6409c8884e81 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -773,6 +773,9 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re params.GetPeerAddress().GetTransportType() == Transport::Type::kUndefined) { #if CONFIG_NETWORK_LAYER_BLE +#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE + PrepareBleTransport(); +#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE if (!params.HasBleLayer()) { params.SetPeerAddress(Transport::PeerAddress::BLE()); @@ -1327,6 +1330,17 @@ CHIP_ERROR DeviceCommissioner::OnOperationalCredentialsProvisioningCompletion(Co } #if CONFIG_NETWORK_LAYER_BLE +#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE +void DeviceCommissioner::PrepareBleTransport() +{ + Transport::BLEBase transport = mSystemState->TransportMgr()->GetTransport().template GetImplAtIndex<2>(); + if (!transport.IsBleLayerTransportSetToSelf()) + { + transport.SetBleLayerTransportToSelf(); + } +} +#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE + CHIP_ERROR DeviceCommissioner::CloseBleConnection() { // It is fine since we can only commission one device at the same time. diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 63a04bf9635348..35fdde9d07af5d 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -596,6 +596,15 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, CommissioningDelegate::CommissioningReport report = CommissioningDelegate::CommissioningReport()); #if CONFIG_NETWORK_LAYER_BLE +#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE + /** + * @brief + * Prior to commissioning, the Controller should make sure the BleLayer transport + * is set to the Commissioner transport and not the Server transport. + */ + void PrepareBleTransport(); +#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE + /** * @brief * Once we have finished all commissioning work, the Controller should close the BLE diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp index 6817d2181bf801..05f13b9dcec89c 100644 --- a/src/controller/SetUpCodePairer.cpp +++ b/src/controller/SetUpCodePairer.cpp @@ -85,6 +85,10 @@ CHIP_ERROR SetUpCodePairer::Connect(SetupPayload & payload) CHIP_ERROR SetUpCodePairer::StartDiscoverOverBle(SetupPayload & payload) { #if CONFIG_NETWORK_LAYER_BLE +#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE + VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); + mCommissioner->PrepareBleTransport(); +#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE VerifyOrReturnError(mBleLayer != nullptr, CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); return mBleLayer->NewBleConnectionByDiscriminator(payload.discriminator, this, OnDiscoveredDeviceOverBleSuccess, OnDiscoveredDeviceOverBleError); From 8317cf82525f9f0584e634a826a6d8073471f295 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Fri, 25 Feb 2022 15:21:25 -0800 Subject: [PATCH 3/5] address comments, attempt to fix Linux build --- src/controller/CHIPDeviceController.cpp | 10 +++++++--- src/controller/CHIPDeviceController.h | 2 +- src/controller/SetUpCodePairer.cpp | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index bc6409c8884e81..6e552d9c367051 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -774,7 +774,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re { #if CONFIG_NETWORK_LAYER_BLE #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE - PrepareBleTransport(); + ConnectBleTransportToSelf(); #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE if (!params.HasBleLayer()) { @@ -1331,9 +1331,13 @@ CHIP_ERROR DeviceCommissioner::OnOperationalCredentialsProvisioningCompletion(Co #if CONFIG_NETWORK_LAYER_BLE #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE -void DeviceCommissioner::PrepareBleTransport() +void DeviceCommissioner::ConnectBleTransportToSelf() { - Transport::BLEBase transport = mSystemState->TransportMgr()->GetTransport().template GetImplAtIndex<2>(); + // The following returns error on Linux builds: + // /usr/include/c++/9/tuple:1303:25: error: static assertion failed: tuple index is in range + // static_assert(__i < tuple_size>::value, + // Transport::BLEBase transport = mSystemState->TransportMgr()->GetTransport().template GetImplAtIndex<2>(); + Transport::BLEBase transport = mSystemState->TransportMgr()->GetTransport().GetImplAtIndex<2>(); if (!transport.IsBleLayerTransportSetToSelf()) { transport.SetBleLayerTransportToSelf(); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 35fdde9d07af5d..c58680b19e4082 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -602,7 +602,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, * Prior to commissioning, the Controller should make sure the BleLayer transport * is set to the Commissioner transport and not the Server transport. */ - void PrepareBleTransport(); + void ConnectBleTransportToSelf(); #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE /** diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp index 05f13b9dcec89c..0892f3957e58a4 100644 --- a/src/controller/SetUpCodePairer.cpp +++ b/src/controller/SetUpCodePairer.cpp @@ -87,7 +87,7 @@ CHIP_ERROR SetUpCodePairer::StartDiscoverOverBle(SetupPayload & payload) #if CONFIG_NETWORK_LAYER_BLE #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE); - mCommissioner->PrepareBleTransport(); + mCommissioner->ConnectBleTransportToSelf(); #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE VerifyOrReturnError(mBleLayer != nullptr, CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); return mBleLayer->NewBleConnectionByDiscriminator(payload.discriminator, this, OnDiscoveredDeviceOverBleSuccess, From 9c1eddc4c37b18015a580dc9c5267172a2aeea75 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Tue, 1 Mar 2022 16:35:26 -0800 Subject: [PATCH 4/5] fix CI --- src/controller/CHIPDeviceController.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 6e552d9c367051..a5b15b9d3242d3 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1333,11 +1333,13 @@ CHIP_ERROR DeviceCommissioner::OnOperationalCredentialsProvisioningCompletion(Co #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE void DeviceCommissioner::ConnectBleTransportToSelf() { - // The following returns error on Linux builds: - // /usr/include/c++/9/tuple:1303:25: error: static assertion failed: tuple index is in range - // static_assert(__i < tuple_size>::value, - // Transport::BLEBase transport = mSystemState->TransportMgr()->GetTransport().template GetImplAtIndex<2>(); - Transport::BLEBase transport = mSystemState->TransportMgr()->GetTransport().GetImplAtIndex<2>(); +#if INET_CONFIG_ENABLE_IPV4 + static const size_t kBleTupleIndex = 2; +#else // INET_CONFIG_ENABLE_IPV4 + static const size_t kBleTupleIndex = 1; +#endif // INET_CONFIG_ENABLE_IPV4 + + Transport::BLEBase & transport = mSystemState->TransportMgr()->GetTransport().GetImplAtIndex(); if (!transport.IsBleLayerTransportSetToSelf()) { transport.SetBleLayerTransportToSelf(); From d9f72114288151a60ffb9e7c3f9efa8e62df1552 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Wed, 2 Mar 2022 08:43:00 -0800 Subject: [PATCH 5/5] address comments --- src/controller/CHIPDeviceController.cpp | 8 +------- src/transport/raw/Tuple.h | 2 ++ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index a5b15b9d3242d3..f9b1cc3c770ef5 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1333,13 +1333,7 @@ CHIP_ERROR DeviceCommissioner::OnOperationalCredentialsProvisioningCompletion(Co #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE void DeviceCommissioner::ConnectBleTransportToSelf() { -#if INET_CONFIG_ENABLE_IPV4 - static const size_t kBleTupleIndex = 2; -#else // INET_CONFIG_ENABLE_IPV4 - static const size_t kBleTupleIndex = 1; -#endif // INET_CONFIG_ENABLE_IPV4 - - Transport::BLEBase & transport = mSystemState->TransportMgr()->GetTransport().GetImplAtIndex(); + Transport::BLEBase & transport = std::get>(mSystemState->TransportMgr()->GetTransport().GetTransports()); if (!transport.IsBleLayerTransportSetToSelf()) { transport.SetBleLayerTransportToSelf(); diff --git a/src/transport/raw/Tuple.h b/src/transport/raw/Tuple.h index 3cd3b387909bf5..743b9b9b0e09aa 100644 --- a/src/transport/raw/Tuple.h +++ b/src/transport/raw/Tuple.h @@ -281,6 +281,8 @@ class Tuple : public Base { return std::get(mTransports); } + + std::tuple & GetTransports() { return mTransports; } }; } // namespace Transport