Skip to content

Commit

Permalink
applications: Improved handling states in Matter Bridge
Browse files Browse the repository at this point in the history
Introduced several improvements for handling different application
states:
* Added using IsReachable attribute to decide about propagating
read/write requests and return failure response in case device
is offline.
* Fixed fault caused by calling connection unref in
BLEConnectivityManager on connection coming from a Matter core
service.
* Disabled single connection mode, as it resulted in stopping
Matter BLE service advertising after any BLE bridged device was
connected.

Signed-off-by: Kamil Kasperczyk <[email protected]>
  • Loading branch information
kkasperczyk-no authored and de-nordic committed Sep 13, 2023
1 parent 46408c1 commit ee68277
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,50 +100,58 @@ CHIP_ERROR HumiditySensorDevice::HandleReadRelativeHumidityMeasurement(Attribute
CHIP_ERROR HumiditySensorDevice::HandleAttributeChange(chip::ClusterId clusterId, chip::AttributeId attributeId,
void *data, size_t dataSize)
{
if (clusterId != Clusters::RelativeHumidityMeasurement::Id || !data) {
CHIP_ERROR err = CHIP_NO_ERROR;
if (!data) {
return CHIP_ERROR_INVALID_ARGUMENT;
}

CHIP_ERROR err;
switch (clusterId) {
case Clusters::BridgedDeviceBasicInformation::Id:
return HandleWriteDeviceBasicInformation(clusterId, attributeId, data, dataSize);
case Clusters::RelativeHumidityMeasurement::Id: {
switch (attributeId) {
case Clusters::RelativeHumidityMeasurement::Attributes::MeasuredValue::Id: {
int16_t value;

switch (attributeId) {
case Clusters::RelativeHumidityMeasurement::Attributes::MeasuredValue::Id: {
int16_t value;
err = CopyAttribute(data, dataSize, &value, sizeof(value));

err = CopyAttribute(data, dataSize, &value, sizeof(value));
if (err != CHIP_NO_ERROR) {
return err;
}

if (err != CHIP_NO_ERROR) {
return err;
SetMeasuredValue(value);

break;
}
case Clusters::RelativeHumidityMeasurement::Attributes::MinMeasuredValue::Id: {
int16_t value;

SetMeasuredValue(value);
err = CopyAttribute(data, dataSize, &value, sizeof(value));

break;
}
case Clusters::RelativeHumidityMeasurement::Attributes::MinMeasuredValue::Id: {
int16_t value;
if (err != CHIP_NO_ERROR) {
return err;
}

err = CopyAttribute(data, dataSize, &value, sizeof(value));
SetMinMeasuredValue(value);

if (err != CHIP_NO_ERROR) {
return err;
break;
}
case Clusters::RelativeHumidityMeasurement::Attributes::MaxMeasuredValue::Id: {
int16_t value;

SetMinMeasuredValue(value);
err = CopyAttribute(data, dataSize, &value, sizeof(value));

break;
}
case Clusters::RelativeHumidityMeasurement::Attributes::MaxMeasuredValue::Id: {
int16_t value;
if (err != CHIP_NO_ERROR) {
return err;
}

err = CopyAttribute(data, dataSize, &value, sizeof(value));
SetMaxMeasuredValue(value);

if (err != CHIP_NO_ERROR) {
return err;
break;
}
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}

SetMaxMeasuredValue(value);

break;
}
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,17 @@ CHIP_ERROR OnOffLightDevice::HandleWrite(ClusterId clusterId, AttributeId attrib
CHIP_ERROR OnOffLightDevice::HandleAttributeChange(chip::ClusterId clusterId, chip::AttributeId attributeId, void *data,
size_t dataSize)
{
CHIP_ERROR err = CHIP_NO_ERROR;
if (!data) {
return CHIP_ERROR_INVALID_ARGUMENT;
}

switch (clusterId) {
case Clusters::BridgedDeviceBasicInformation::Id:
HandleWriteDeviceBasicInformation(clusterId, attributeId, data, dataSize);
return CHIP_NO_ERROR;
return HandleWriteDeviceBasicInformation(clusterId, attributeId, data, dataSize);
case Clusters::OnOff::Id: {
switch (attributeId) {
case Clusters::OnOff::Attributes::OnOff::Id: {
CHIP_ERROR err;

bool value;

err = CopyAttribute(data, dataSize, &value, sizeof(value));
Expand All @@ -139,10 +137,11 @@ CHIP_ERROR OnOffLightDevice::HandleAttributeChange(chip::ClusterId clusterId, ch
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}
break;
}
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}

return CHIP_ERROR_INVALID_ARGUMENT;
return err;
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,50 +98,58 @@ CHIP_ERROR TemperatureSensorDevice::HandleReadTemperatureMeasurement(AttributeId
CHIP_ERROR TemperatureSensorDevice::HandleAttributeChange(chip::ClusterId clusterId, chip::AttributeId attributeId,
void *data, size_t dataSize)
{
if (clusterId != Clusters::TemperatureMeasurement::Id || !data) {
CHIP_ERROR err = CHIP_NO_ERROR;
if (!data) {
return CHIP_ERROR_INVALID_ARGUMENT;
}

CHIP_ERROR err;
switch (clusterId) {
case Clusters::BridgedDeviceBasicInformation::Id:
return HandleWriteDeviceBasicInformation(clusterId, attributeId, data, dataSize);
case Clusters::TemperatureMeasurement::Id: {
switch (attributeId) {
case Clusters::TemperatureMeasurement::Attributes::MeasuredValue::Id: {
int16_t value;

switch (attributeId) {
case Clusters::TemperatureMeasurement::Attributes::MeasuredValue::Id: {
int16_t value;
err = CopyAttribute(data, dataSize, &value, sizeof(value));

err = CopyAttribute(data, dataSize, &value, sizeof(value));
if (err != CHIP_NO_ERROR) {
return err;
}

if (err != CHIP_NO_ERROR) {
return err;
SetMeasuredValue(value);

break;
}
case Clusters::TemperatureMeasurement::Attributes::MinMeasuredValue::Id: {
int16_t value;

SetMeasuredValue(value);
err = CopyAttribute(data, dataSize, &value, sizeof(value));

break;
}
case Clusters::TemperatureMeasurement::Attributes::MinMeasuredValue::Id: {
int16_t value;
if (err != CHIP_NO_ERROR) {
return err;
}

err = CopyAttribute(data, dataSize, &value, sizeof(value));
SetMinMeasuredValue(value);

if (err != CHIP_NO_ERROR) {
return err;
break;
}
case Clusters::TemperatureMeasurement::Attributes::MaxMeasuredValue::Id: {
int16_t value;

SetMinMeasuredValue(value);
err = CopyAttribute(data, dataSize, &value, sizeof(value));

break;
}
case Clusters::TemperatureMeasurement::Attributes::MaxMeasuredValue::Id: {
int16_t value;
if (err != CHIP_NO_ERROR) {
return err;
}

err = CopyAttribute(data, dataSize, &value, sizeof(value));
SetMaxMeasuredValue(value);

if (err != CHIP_NO_ERROR) {
return err;
break;
}
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}

SetMaxMeasuredValue(value);

break;
}
default:
Expand Down
5 changes: 5 additions & 0 deletions applications/matter_bridge/src/chip_project_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,8 @@
#pragma once

#define CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT CONFIG_BRIDGE_MAX_DYNAMIC_ENDPOINTS_NUMBER

/* Disable handling only single connection to avoid stopping Matter service BLE advertising
* while other BLE bridge-related connections are open.
*/
#define CHIP_DEVICE_CONFIG_CHIPOBLE_SINGLE_CONNECTION 0
11 changes: 5 additions & 6 deletions samples/matter/common/src/bridge/ble_connectivity_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ void BLEConnectivityManager::DisconnectionHandler(bt_conn *conn, uint8_t reason)
/* Try to find provider matching the connection */
BLEBridgedDeviceProvider *provider = Instance().FindBLEProvider(*bt_conn_get_dst(conn));

bt_conn_unref(conn);

if (provider) {
bt_conn_unref(provider->GetBLEBridgedDevice().mConn);
provider->SetConnectionObject(nullptr);

/* Verify whether the device should be recovered. */
if (reason == BT_HCI_ERR_CONN_TIMEOUT) {
Instance().mRecovery.NotifyProviderToRecover(provider);
Expand Down Expand Up @@ -411,10 +412,8 @@ CHIP_ERROR BLEConnectivityManager::RemoveBLEProvider(bt_addr_le_t address)
return CHIP_ERROR_INTERNAL;
}

/* Release the connection immediately in case of disconnection failed, as callback will not be called. */
if (bt_conn_disconnect(provider->GetBLEBridgedDevice().mConn, BT_HCI_ERR_REMOTE_USER_TERM_CONN) != 0) {
bt_conn_unref(provider->GetBLEBridgedDevice().mConn);
}
bt_conn_disconnect(provider->GetBLEBridgedDevice().mConn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
bt_conn_unref(provider->GetBLEBridgedDevice().mConn);

return CHIP_NO_ERROR;
}
Expand Down
8 changes: 8 additions & 0 deletions samples/matter/common/src/bridge/bridge_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ CHIP_ERROR BridgeManager::HandleRead(uint16_t index, ClusterId clusterId,
VerifyOrReturnValue(Instance().mDevicesMap.Contains(index), CHIP_ERROR_INTERNAL);

auto *device = Instance().mDevicesMap[index].mDevice;

/* Verify if the device is reachable or we should return prematurely. */
VerifyOrReturnError(device->GetIsReachable(), CHIP_ERROR_INCORRECT_STATE);

return device->HandleRead(clusterId, attributeMetadata->attributeId, buffer, maxReadLength);
}

Expand All @@ -331,6 +335,10 @@ CHIP_ERROR BridgeManager::HandleWrite(uint16_t index, ClusterId clusterId,
VerifyOrReturnValue(Instance().mDevicesMap.Contains(index), CHIP_ERROR_INTERNAL);

auto *device = Instance().mDevicesMap[index].mDevice;

/* Verify if the device is reachable or we should return prematurely. */
VerifyOrReturnError(device->GetIsReachable(), CHIP_ERROR_INCORRECT_STATE);

CHIP_ERROR err = device->HandleWrite(clusterId, attributeMetadata->attributeId, buffer);

/* After updating MatterBridgedDevice state, forward request to the non-Matter device. */
Expand Down
3 changes: 2 additions & 1 deletion samples/matter/common/src/bridge/matter_bridged_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class MatterBridgedDevice {

bool GetIsReachable() const { return mIsReachable; }
const char *GetNodeLabel() const { return mNodeLabel; }
void SetIsReachable(bool isReachable) { mIsReachable = isReachable; }
uint16_t GetBridgedDeviceBasicInformationClusterRevision()
{
return kBridgedDeviceBasicInformationClusterRevision;
Expand All @@ -94,6 +93,8 @@ class MatterBridgedDevice {
size_t mDataVersionSize;

protected:
void SetIsReachable(bool isReachable) { mIsReachable = isReachable; }

chip::EndpointId mEndpointId;

private:
Expand Down

0 comments on commit ee68277

Please sign in to comment.