Skip to content

Commit

Permalink
fix BLE commissioning on commissionable Matter TVs (#15525)
Browse files Browse the repository at this point in the history
* fix BLE commissioning on commissionable Matter TVs

* make sure BLE transport is setup for commissioning

* address comments, attempt to fix Linux build

* fix CI

* address comments
  • Loading branch information
chrisdecenzo authored and pull[bot] committed Nov 30, 2023
1 parent e5c1d24 commit 3190409
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 2 deletions.
14 changes: 14 additions & 0 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
ConnectBleTransportToSelf();
#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
if (!params.HasBleLayer())
{
params.SetPeerAddress(Transport::PeerAddress::BLE());
Expand Down Expand Up @@ -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::ConnectBleTransportToSelf()
{
Transport::BLEBase & transport = std::get<Transport::BLE<1>>(mSystemState->TransportMgr()->GetTransport().GetTransports());
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.
Expand Down
9 changes: 9 additions & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,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 ConnectBleTransportToSelf();
#endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE

/**
* @brief
* Once we have finished all commissioning work, the Controller should close the BLE
Expand Down
4 changes: 4 additions & 0 deletions src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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->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,
OnDiscoveredDeviceOverBleError);
Expand Down
13 changes: 11 additions & 2 deletions src/transport/raw/BLE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ void BLEBase::ClearState()
{
mBleLayer->CancelBleIncompleteConnection();
mBleLayer->OnChipBleConnectReceived = nullptr;
mBleLayer->mBleTransport = nullptr;
mBleLayer = nullptr;
}

Expand All @@ -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;
Expand Down
45 changes: 45 additions & 0 deletions src/transport/raw/BLE.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions src/transport/raw/Tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ class Tuple : public Base
{
return std::get<i>(mTransports);
}

std::tuple<TransportTypes...> & GetTransports() { return mTransports; }
};

} // namespace Transport
Expand Down

0 comments on commit 3190409

Please sign in to comment.