Skip to content

Commit

Permalink
Return CHIP_ERROR from BlePlatformDelegate APIs (#34387)
Browse files Browse the repository at this point in the history 
* Return CHIP_ERROR from BlePlatformDelegate APIs

* Restyled by clang-format

* Fix issues detected by CI

* Mark BlePlatformDelegate implementations as override

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@arkq @restyled-commits
arkq and restyled-commits authored Jul 19, 2024
1 parent 6ce96ef commit fca7948
Show file tree
Hide file tree
Showing 52 changed files with 713 additions and 787 deletions.
64 changes: 29 additions & 35 deletions src/ble/BLEEndPoint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,8 @@ CHIP_ERROR BLEEndPoint::StartConnect()
// Add reference to message fragment for duration of platform's GATT write attempt. CHIP retains partial
// ownership of message fragment's packet buffer, since this is the same buffer as that of the whole message, just
// with a fragmenter-modified payload offset and data length, by a Retain() on the handle when calling this function.
if (!SendWrite(buf.Retain()))
{
err = BLE_ERROR_GATT_WRITE_FAILED;
ExitNow();
}
err = SendWrite(buf.Retain());
SuccessOrExit(err);

// Free request buffer on write confirmation. Stash a reference to it in mSendQueue, which we don't use anyway
// until the connection has been set up.
Expand Down Expand Up @@ -213,13 +210,13 @@ void BLEEndPoint::HandleSubscribeReceived()
// Add reference to message fragment for duration of platform's GATT indication attempt. CHIP retains partial
// ownership of message fragment's packet buffer, since this is the same buffer as that of the whole message, just
// with a fragmenter-modified payload offset and data length.
if (!SendIndication(mSendQueue.Retain()))
err = SendIndication(mSendQueue.Retain());
if (err != CHIP_NO_ERROR)
{
// Ensure transmit queue is empty and set to NULL.
mSendQueue = nullptr;

ChipLogError(Ble, "cap resp ind failed");
err = BLE_ERROR_GATT_INDICATE_FAILED;
ExitNow();
}

Expand Down Expand Up @@ -389,9 +386,10 @@ void BLEEndPoint::FinalizeClose(uint8_t oldState, uint8_t flags, CHIP_ERROR err)
// Indicate close of chipConnection to peripheral via GATT unsubscribe. Keep end point allocated until
// unsubscribe completes or times out, so platform doesn't close underlying BLE connection before
// we're really sure the unsubscribe request has been sent.
if (!mBle->mPlatformDelegate->UnsubscribeCharacteristic(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_2_UUID))
err = mBle->mPlatformDelegate->UnsubscribeCharacteristic(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_2_UUID);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Ble, "BtpEngine unsub failed");
ChipLogError(Ble, "BtpEngine unsubscribe failed %" CHIP_ERROR_FORMAT, err.Format());

// If unsubscribe fails, release BLE connection and free end point immediately.
Free();
Expand Down Expand Up @@ -568,31 +566,20 @@ CHIP_ERROR BLEEndPoint::SendCharacteristic(PacketBufferHandle && buf)

if (mRole == kBleRole_Central)
{
if (!SendWrite(std::move(buf)))
{
err = BLE_ERROR_GATT_WRITE_FAILED;
}
else
{
// Write succeeded, so shrink remote receive window counter by 1.
mRemoteReceiveWindowSize = static_cast<SequenceNumber_t>(mRemoteReceiveWindowSize - 1);
ChipLogDebugBleEndPoint(Ble, "decremented remote rx window, new size = %u", mRemoteReceiveWindowSize);
}
SuccessOrExit(err = SendWrite(std::move(buf)));
// Write succeeded, so shrink remote receive window counter by 1.
mRemoteReceiveWindowSize = static_cast<SequenceNumber_t>(mRemoteReceiveWindowSize - 1);
ChipLogDebugBleEndPoint(Ble, "decremented remote rx window, new size = %u", mRemoteReceiveWindowSize);
}
else // (mRole == kBleRole_Peripheral), verified on Init
{
if (!SendIndication(std::move(buf)))
{
err = BLE_ERROR_GATT_INDICATE_FAILED;
}
else
{
// Indication succeeded, so shrink remote receive window counter by 1.
mRemoteReceiveWindowSize = static_cast<SequenceNumber_t>(mRemoteReceiveWindowSize - 1);
ChipLogDebugBleEndPoint(Ble, "decremented remote rx window, new size = %u", mRemoteReceiveWindowSize);
}
SuccessOrExit(err = SendIndication(std::move(buf)));
// Indication succeeded, so shrink remote receive window counter by 1.
mRemoteReceiveWindowSize = static_cast<SequenceNumber_t>(mRemoteReceiveWindowSize - 1);
ChipLogDebugBleEndPoint(Ble, "decremented remote rx window, new size = %u", mRemoteReceiveWindowSize);
}

exit:
return err;
}

Expand Down Expand Up @@ -750,8 +737,8 @@ CHIP_ERROR BLEEndPoint::HandleHandshakeConfirmationReceived()
{
// Subscribe to characteristic which peripheral will use to send indications. Prompts peripheral to send
// BLE transport capabilities indication.
VerifyOrExit(mBle->mPlatformDelegate->SubscribeCharacteristic(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_2_UUID),
err = BLE_ERROR_GATT_SUBSCRIBE_FAILED);
err = mBle->mPlatformDelegate->SubscribeCharacteristic(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_2_UUID);
SuccessOrExit(err);

// We just sent a GATT subscribe request, so make sure to attempt unsubscribe on close.
mConnStateFlags.Set(ConnectionStateFlag::kDidBeginSubscribe);
Expand Down Expand Up @@ -1309,18 +1296,25 @@ CHIP_ERROR BLEEndPoint::Receive(PacketBufferHandle && data)
return err;
}

bool BLEEndPoint::SendWrite(PacketBufferHandle && buf)
CHIP_ERROR BLEEndPoint::SendWrite(PacketBufferHandle && buf)
{
mConnStateFlags.Set(ConnectionStateFlag::kGattOperationInFlight);

return mBle->mPlatformDelegate->SendWriteRequest(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_1_UUID, std::move(buf));
auto err = mBle->mPlatformDelegate->SendWriteRequest(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_1_UUID, std::move(buf));
VerifyOrReturnError(err == CHIP_NO_ERROR, err,
ChipLogError(Ble, "Send write request failed: %" CHIP_ERROR_FORMAT, err.Format()));

return err;
}

bool BLEEndPoint::SendIndication(PacketBufferHandle && buf)
CHIP_ERROR BLEEndPoint::SendIndication(PacketBufferHandle && buf)
{
mConnStateFlags.Set(ConnectionStateFlag::kGattOperationInFlight);

return mBle->mPlatformDelegate->SendIndication(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_2_UUID, std::move(buf));
auto err = mBle->mPlatformDelegate->SendIndication(mConnObj, &CHIP_BLE_SVC_ID, &CHIP_BLE_CHAR_2_UUID, std::move(buf));
VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(Ble, "Send indication failed: %" CHIP_ERROR_FORMAT, err.Format()));

return err;
}

CHIP_ERROR BLEEndPoint::StartConnectTimer()
Expand Down
4 changes: 2 additions & 2 deletions src/ble/BLEEndPoint.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ class DLL_EXPORT BLEEndPoint
CHIP_ERROR ContinueMessageSend();
CHIP_ERROR DoSendStandAloneAck();
CHIP_ERROR SendCharacteristic(PacketBufferHandle && buf);
bool SendIndication(PacketBufferHandle && buf);
bool SendWrite(PacketBufferHandle && buf);
CHIP_ERROR SendIndication(PacketBufferHandle && buf);
CHIP_ERROR SendWrite(PacketBufferHandle && buf);

// Receive path:
CHIP_ERROR HandleConnectComplete();
Expand Down
23 changes: 10 additions & 13 deletions src/ble/BlePlatformDelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#error "Please include <ble/Ble.h> instead!"
#endif

#include <lib/core/CHIPError.h>
#include <lib/support/DLLUtil.h>
#include <system/SystemPacketBuffer.h>

Expand All @@ -48,30 +49,26 @@ class DLL_EXPORT BlePlatformDelegate
// Following APIs must be implemented by platform:

// Subscribe to updates and indications on the specfied characteristic
virtual bool SubscribeCharacteristic(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId) = 0;
virtual CHIP_ERROR SubscribeCharacteristic(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId,
const ChipBleUUID * charId) = 0;

// Unsubscribe from updates and indications on the specified characteristic
virtual bool UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId,
const ChipBleUUID * charId) = 0;
virtual CHIP_ERROR UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId,
const ChipBleUUID * charId) = 0;

// Close the underlying BLE connection.
virtual bool CloseConnection(BLE_CONNECTION_OBJECT connObj) = 0;
virtual CHIP_ERROR CloseConnection(BLE_CONNECTION_OBJECT connObj) = 0;

// Get MTU size negotiated for specified BLE connection. Return value of 0 means MTU size could not be determined.
virtual uint16_t GetMTU(BLE_CONNECTION_OBJECT connObj) const = 0;

// Data path calling convention:
// A 'true' return value from a Send* function indicates that the characteristic was written or updated
// successfully. A 'false' value indicates failure, and is used to report this failure to the user via the return
// value of chipConnection::SendMessage.

// Send GATT characteristic indication request
virtual bool SendIndication(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf) = 0;
virtual CHIP_ERROR SendIndication(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf) = 0;

// Send GATT characteristic write request
virtual bool SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf) = 0;
virtual CHIP_ERROR SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf) = 0;
};

} /* namespace Ble */
Expand Down
24 changes: 14 additions & 10 deletions src/ble/tests/TestBleLayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,23 @@ class TestBleLayer : public BleLayer,
///
// Implementation of BlePlatformDelegate

bool SubscribeCharacteristic(BLE_CONNECTION_OBJECT, const ChipBleUUID *, const ChipBleUUID *) override { return true; }
bool UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT, const ChipBleUUID *, const ChipBleUUID *) override { return true; }
bool CloseConnection(BLE_CONNECTION_OBJECT connObj) override { return true; }
uint16_t GetMTU(BLE_CONNECTION_OBJECT connObj) const override { return 0; }
bool SendIndication(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf) override
CHIP_ERROR SubscribeCharacteristic(BLE_CONNECTION_OBJECT, const ChipBleUUID *, const ChipBleUUID *) override
{
return true;
return CHIP_NO_ERROR;
}
bool SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf) override
CHIP_ERROR UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT, const ChipBleUUID *, const ChipBleUUID *) override
{
return true;
return CHIP_NO_ERROR;
}
CHIP_ERROR CloseConnection(BLE_CONNECTION_OBJECT) override { return CHIP_NO_ERROR; }
uint16_t GetMTU(BLE_CONNECTION_OBJECT) const override { return 0; }
CHIP_ERROR SendIndication(BLE_CONNECTION_OBJECT, const ChipBleUUID *, const ChipBleUUID *, PacketBufferHandle) override
{
return CHIP_NO_ERROR;
}
CHIP_ERROR SendWriteRequest(BLE_CONNECTION_OBJECT, const ChipBleUUID *, const ChipBleUUID *, PacketBufferHandle) override
{
return CHIP_NO_ERROR;
}

private:
Expand Down
31 changes: 14 additions & 17 deletions src/platform/ASR/BLEManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,25 +284,27 @@ void BLEManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
}
}

bool BLEManagerImpl::SubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId)
CHIP_ERROR BLEManagerImpl::SubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId,
const ChipBleUUID * charId)
{
log_i("%s:%s:%d\r\n", "BLEManagerImpl", __func__, __LINE__);
ChipLogProgress(DeviceLayer, "BLEManagerImpl::SubscribeCharacteristic() not supported");
return false;
return CHIP_ERROR_NOT_IMPLEMENTED;
}

bool BLEManagerImpl::UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId)
CHIP_ERROR BLEManagerImpl::UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId,
const ChipBleUUID * charId)
{
log_i("%s:%s:%d\r\n", "BLEManagerImpl", __func__, __LINE__);
ChipLogProgress(DeviceLayer, "BLEManagerImpl::UnsubscribeCharacteristic() not supported");
return false;
return CHIP_ERROR_NOT_IMPLEMENTED;
}

bool BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId)
CHIP_ERROR BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId)
{
log_i("%s:%s:%d\r\n", "BLEManagerImpl", __func__, __LINE__);
matter_close_connection(conId);
return true;
return CHIP_NO_ERROR;
}

uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const
Expand All @@ -324,8 +326,8 @@ uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const
}
}

bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle data)
CHIP_ERROR BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle data)
{
CHIP_ERROR err = CHIP_NO_ERROR;
CHIPoBLEConState * conState = GetConnectionState(conId);
Expand All @@ -335,20 +337,15 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU
matter_tx_char_send_indication(conId, data->DataLength(), data->Start());

exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err));
return false;
}
return true;
return err;
}

bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle data)
CHIP_ERROR BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle data)
{
log_i("%s:%s:%d\r\n", "BLEManagerImpl", __func__, __LINE__);
ChipLogError(DeviceLayer, "BLEManagerImpl::SendWriteRequest() not supported");
return false;
return CHIP_ERROR_NOT_IMPLEMENTED;
}

void BLEManagerImpl::NotifyChipConnectionClosed(BLE_CONNECTION_OBJECT conId) {}
Expand Down
18 changes: 9 additions & 9 deletions src/platform/ASR/BLEManagerImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ class BLEManagerImpl final : public BLEManager,

// ===== Members that implement virtual methods on BlePlatformDelegate.

bool SubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId,
const Ble::ChipBleUUID * charId) override;
bool UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId,
const Ble::ChipBleUUID * charId) override;
bool CloseConnection(BLE_CONNECTION_OBJECT conId) override;
CHIP_ERROR SubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId,
const Ble::ChipBleUUID * charId) override;
CHIP_ERROR UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId,
const Ble::ChipBleUUID * charId) override;
CHIP_ERROR CloseConnection(BLE_CONNECTION_OBJECT conId) override;
uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const override;
bool SendIndication(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId,
System::PacketBufferHandle data) override;
bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId,
System::PacketBufferHandle data) override;
CHIP_ERROR SendIndication(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId,
System::PacketBufferHandle data) override;
CHIP_ERROR SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId,
System::PacketBufferHandle data) override;

// ===== Members that implement virtual methods on BleApplicationDelegate.

Expand Down
31 changes: 14 additions & 17 deletions src/platform/Ameba/BLEManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,19 +511,21 @@ void BLEManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
}
}

bool BLEManagerImpl::SubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId)
CHIP_ERROR BLEManagerImpl::SubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId,
const ChipBleUUID * charId)
{
ChipLogProgress(DeviceLayer, "BLEManagerImpl::SubscribeCharacteristic() not supported");
return false;
return CHIP_ERROR_NOT_IMPLEMENTED;
}

bool BLEManagerImpl::UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId)
CHIP_ERROR BLEManagerImpl::UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId,
const ChipBleUUID * charId)
{
ChipLogProgress(DeviceLayer, "BLEManagerImpl::UnsubscribeCharacteristic() not supported");
return false;
return CHIP_ERROR_NOT_IMPLEMENTED;
}

bool BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId)
CHIP_ERROR BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId)
{
CHIP_ERROR err;
ChipLogProgress(DeviceLayer, "Closing BLE GATT connection (con %u)", conId);
Expand All @@ -543,7 +545,7 @@ bool BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId)
mFlags.Clear(Flags::kAdvertisingConfigured);
PlatformMgr().ScheduleWork(DriveBLEState, 0);

return (err == CHIP_NO_ERROR);
return err;
}

uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const
Expand All @@ -557,20 +559,20 @@ uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const
return mtu;
}

bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf)
CHIP_ERROR BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf)
{
ChipLogError(DeviceLayer, "BLEManagerImpl::SendWriteRequest() not supported");
return false;
return CHIP_ERROR_NOT_IMPLEMENTED;
}

void BLEManagerImpl::NotifyChipConnectionClosed(BLE_CONNECTION_OBJECT conId)
{
// Nothing to do
}

bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle data)
CHIP_ERROR BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle data)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -583,12 +585,7 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU
#endif

exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err));
return false;
}
return true;
return err;
}

/*******************************************************************************
Expand Down
Loading

0 comments on commit fca7948

Please sign in to comment.