Skip to content

Commit 42929d0

Browse files
arkqrestyled-commits
arkq
and
restyled-commits
authored
[Linux] Add indication confirmation support for BlueZ >= 5.80 (project-chip#40147)
* Do not use temporary variable for error message * Use BlueZ provided device when acquiring write/notify * Listen for indication confirmation on the socket * Restyled by clang-format * Address review comments * Apply review suggestions --------- Co-authored-by: Restyled.io <[email protected]>
1 parent ee94936 commit 42929d0

File tree

2 files changed

+71
-53
lines changed

2 files changed

+71
-53
lines changed

‎src/platform/Linux/bluez/BluezConnection.cpp‎

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -171,29 +171,24 @@ const char * BluezConnection::GetPeerAddress() const
171171

172172
gboolean BluezConnection::WriteHandlerCallback(GIOChannel * aChannel, GIOCondition aCond, BluezConnection * apConn)
173173
{
174+
VerifyOrReturnValue(!(aCond & G_IO_HUP), G_SOURCE_REMOVE,
175+
ChipLogError(DeviceLayer, "INFO: socket disconnected in %s", __func__));
176+
VerifyOrReturnValue(aCond == G_IO_IN, G_SOURCE_REMOVE,
177+
ChipLogError(DeviceLayer, "FAIL: socket error in %s: cond=0x%x", __func__, aCond));
178+
174179
uint8_t buf[512 /* characteristic max size per BLE specification */];
175-
bool isSuccess = false;
176-
GVariant * newVal;
177180
ssize_t len;
178181

179-
VerifyOrExit(!(aCond & G_IO_HUP), ChipLogError(DeviceLayer, "INFO: socket disconnected in %s", __func__));
180-
VerifyOrExit(!(aCond & (G_IO_ERR | G_IO_NVAL)), ChipLogError(DeviceLayer, "INFO: socket error in %s", __func__));
181-
VerifyOrExit(aCond == G_IO_IN, ChipLogError(DeviceLayer, "FAIL: error in %s", __func__));
182-
183-
ChipLogDetail(DeviceLayer, "C1 %s MTU: %d", __func__, apConn->GetMTU());
184-
185182
len = read(g_io_channel_unix_get_fd(aChannel), buf, sizeof(buf));
186-
VerifyOrExit(len > 0, ChipLogError(DeviceLayer, "FAIL: short read in %s (%zd)", __func__, len));
183+
VerifyOrReturnValue(len > 0, G_SOURCE_REMOVE, ChipLogError(DeviceLayer, "FAIL: short read in %s: %zd", __func__, len));
187184

185+
ChipLogDetail(DeviceLayer, "C1 %s received %zd bytes", __func__, len);
188186
// Casting len to size_t is safe, since we ensured that it's not negative.
189-
newVal = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, buf, static_cast<size_t>(len), sizeof(uint8_t));
190-
191-
bluez_gatt_characteristic1_set_value(apConn->mC1.get(), newVal);
187+
bluez_gatt_characteristic1_set_value(
188+
apConn->mC1.get(), g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, buf, static_cast<size_t>(len), sizeof(uint8_t)));
192189
BLEManagerImpl::HandleRXCharWrite(apConn, buf, static_cast<size_t>(len));
193-
isSuccess = true;
194190

195-
exit:
196-
return isSuccess ? G_SOURCE_CONTINUE : G_SOURCE_REMOVE;
191+
return G_SOURCE_CONTINUE;
197192
}
198193

199194
void BluezConnection::SetupWriteHandler(int aSocketFd)
@@ -203,7 +198,7 @@ void BluezConnection::SetupWriteHandler(int aSocketFd)
203198
g_io_channel_set_close_on_unref(channel, TRUE);
204199
g_io_channel_set_buffered(channel, FALSE);
205200

206-
auto watchSource = g_io_create_watch(channel, static_cast<GIOCondition>(G_IO_HUP | G_IO_IN | G_IO_ERR | G_IO_NVAL));
201+
auto watchSource = g_io_create_watch(channel, static_cast<GIOCondition>(G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL));
207202
g_source_set_callback(watchSource, G_SOURCE_FUNC(WriteHandlerCallback), this, nullptr);
208203

209204
mC1Channel.mChannel.reset(channel);
@@ -212,9 +207,34 @@ void BluezConnection::SetupWriteHandler(int aSocketFd)
212207
PlatformMgrImpl().GLibMatterContextAttachSource(watchSource);
213208
}
214209

215-
gboolean BluezConnection::NotifyHandlerCallback(GIOChannel *, GIOCondition, BluezConnection *)
210+
gboolean BluezConnection::NotifyHandlerCallback(GIOChannel * aChannel, GIOCondition aCond, BluezConnection * apConn)
216211
{
217-
return G_SOURCE_REMOVE;
212+
VerifyOrReturnValue(!(aCond & G_IO_HUP), G_SOURCE_REMOVE,
213+
ChipLogError(DeviceLayer, "INFO: socket disconnected in %s", __func__));
214+
VerifyOrReturnValue(aCond == G_IO_IN, G_SOURCE_REMOVE,
215+
ChipLogError(DeviceLayer, "FAIL: socket error in %s: cond=0x%x", __func__, aCond));
216+
217+
uint8_t value = 0;
218+
ssize_t len;
219+
220+
// NOTE: For BlueZ version <= 5.73 the confirmation was delivered via the D-Bus "Confirm"
221+
// method call. However, for BlueZ >= 5.80 the confirmation is sent via the socket
222+
// opened in the "AcquireNotify" method. For versions in between, the confirmation
223+
// might not work correctly at all!
224+
len = read(g_io_channel_unix_get_fd(aChannel), &value, sizeof(value));
225+
VerifyOrReturnValue(len > 0, G_SOURCE_REMOVE, ChipLogError(DeviceLayer, "FAIL: short read in %s: %zd", __func__, len));
226+
227+
if (value != 1)
228+
{
229+
ChipLogError(DeviceLayer, "FAIL: Invalid indication confirmation: value=%u", value);
230+
}
231+
else
232+
{
233+
ChipLogDetail(DeviceLayer, "Indication confirmation: conn=%p", apConn);
234+
BLEManagerImpl::HandleTXComplete(apConn);
235+
}
236+
237+
return G_SOURCE_CONTINUE;
218238
}
219239

220240
void BluezConnection::SetupNotifyHandler(int aSocketFd, bool aAdditionalAdvertising)
@@ -224,7 +244,7 @@ void BluezConnection::SetupNotifyHandler(int aSocketFd, bool aAdditionalAdvertis
224244
g_io_channel_set_close_on_unref(channel, TRUE);
225245
g_io_channel_set_buffered(channel, FALSE);
226246

227-
auto watchSource = g_io_create_watch(channel, static_cast<GIOCondition>(G_IO_HUP | G_IO_ERR | G_IO_NVAL));
247+
auto watchSource = g_io_create_watch(channel, static_cast<GIOCondition>(G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL));
228248
g_source_set_callback(watchSource, G_SOURCE_FUNC(NotifyHandlerCallback), this, nullptr);
229249

230250
#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING

‎src/platform/Linux/bluez/BluezEndpoint.cpp‎

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -97,31 +97,29 @@ gboolean BluezEndpoint::BluezCharacteristicReadValue(BluezGattCharacteristic1 *
9797
gboolean BluezEndpoint::BluezCharacteristicAcquireWrite(BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInvocation,
9898
GUnixFDList * aFDList, GVariant * aOptions)
9999
{
100-
int fds[2] = { -1, -1 };
101-
#if CHIP_ERROR_LOGGING
102-
char * errStr;
103-
#endif // CHIP_ERROR_LOGGING
100+
int fds[2] = { -1, -1 };
104101
BluezConnection * conn = nullptr;
102+
const char * deviceObjectPath;
105103
uint16_t mtu;
106104

107-
conn = GetBluezConnectionViaDevice();
108105
VerifyOrReturnValue(
109-
conn != nullptr, FALSE,
110-
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "No CHIPoBLE connection"));
111-
112-
ChipLogDetail(DeviceLayer, "BluezCharacteristicAcquireWrite is called, conn: %p", conn);
113-
106+
g_variant_lookup(aOptions, "device", "&o", &deviceObjectPath), FALSE,
107+
ChipLogError(DeviceLayer, "FAIL: No device in options in %s", __func__);
108+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "No device object path"));
114109
VerifyOrReturnValue(
115110
g_variant_lookup(aOptions, "mtu", "q", &mtu), FALSE, ChipLogError(DeviceLayer, "FAIL: No MTU in options in %s", __func__);
116-
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "MTU negotiation failed"));
111+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "No MTU value"));
112+
113+
conn = GetBluezConnection(deviceObjectPath);
114+
VerifyOrReturnValue(
115+
conn != nullptr, FALSE,
116+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "No CHIPoBLE connection for the device"));
117+
117118
conn->SetMTU(mtu);
118119

119120
if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0, fds) < 0)
120121
{
121-
#if CHIP_ERROR_LOGGING
122-
errStr = strerror(errno);
123-
#endif // CHIP_ERROR_LOGGING
124-
ChipLogError(DeviceLayer, "FAIL: socketpair: %s in %s", StringOrNullMarker(errStr), __func__);
122+
ChipLogError(DeviceLayer, "FAIL: socketpair: %s in %s", StringOrNullMarker(strerror(errno)), __func__);
125123
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "FD creation failed");
126124
return FALSE;
127125
}
@@ -148,40 +146,37 @@ static gboolean BluezCharacteristicAcquireWriteError(BluezGattCharacteristic1 *
148146
gboolean BluezEndpoint::BluezCharacteristicAcquireNotify(BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInvocation,
149147
GUnixFDList * aFDList, GVariant * aOptions)
150148
{
151-
int fds[2] = { -1, -1 };
152-
#if CHIP_ERROR_LOGGING
153-
char * errStr;
154-
#endif // CHIP_ERROR_LOGGING
149+
int fds[2] = { -1, -1 };
155150
BluezConnection * conn = nullptr;
156151
bool isAdditionalAdvertising = false;
152+
const char * deviceObjectPath;
157153
uint16_t mtu;
158154

159155
#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING
160156
isAdditionalAdvertising = (aChar == mC3.get());
161157
#endif
162158

163-
if (bluez_gatt_characteristic1_get_notifying(aChar))
164-
{
165-
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.NotPermitted", "Already notifying");
166-
return FALSE;
167-
}
159+
VerifyOrReturnValue(
160+
!bluez_gatt_characteristic1_get_notifying(aChar), FALSE,
161+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.NotPermitted", "Already notifying"));
162+
VerifyOrReturnValue(
163+
g_variant_lookup(aOptions, "device", "&o", &deviceObjectPath), FALSE,
164+
ChipLogError(DeviceLayer, "FAIL: No device in options in %s", __func__);
165+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "No device object path"));
166+
VerifyOrReturnValue(
167+
g_variant_lookup(aOptions, "mtu", "q", &mtu), FALSE, ChipLogError(DeviceLayer, "FAIL: No MTU in options in %s", __func__);
168+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "No MTU value"));
168169

169-
conn = GetBluezConnectionViaDevice();
170+
conn = GetBluezConnection(deviceObjectPath);
170171
VerifyOrReturnValue(
171172
conn != nullptr, FALSE,
172-
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "No CHIPoBLE connection"));
173+
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "No CHIPoBLE connection for the device"));
173174

174-
VerifyOrReturnValue(
175-
g_variant_lookup(aOptions, "mtu", "q", &mtu), FALSE, ChipLogError(DeviceLayer, "FAIL: No MTU in options in %s", __func__);
176-
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "MTU negotiation failed"););
177175
conn->SetMTU(mtu);
178176

179177
if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0, fds) < 0)
180178
{
181-
#if CHIP_ERROR_LOGGING
182-
errStr = strerror(errno);
183-
#endif // CHIP_ERROR_LOGGING
184-
ChipLogError(DeviceLayer, "FAIL: socketpair: %s in %s", StringOrNullMarker(errStr), __func__);
179+
ChipLogError(DeviceLayer, "FAIL: socketpair: %s in %s", StringOrNullMarker(strerror(errno)), __func__);
185180
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "FD creation failed");
186181
return FALSE;
187182
}
@@ -208,10 +203,13 @@ static gboolean BluezCharacteristicAcquireNotifyError(BluezGattCharacteristic1 *
208203
return TRUE;
209204
}
210205

206+
// NOTE: When using with BlueZ >= 5.80, this method can be removed. Also, the GetBluezConnectionViaDevice
207+
// method will no longer be needed, since every callback will have the device object path available
208+
// in the options parameter.
211209
gboolean BluezEndpoint::BluezCharacteristicConfirm(BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInvocation)
212210
{
213211
BluezConnection * conn = GetBluezConnectionViaDevice();
214-
ChipLogDetail(Ble, "Indication confirmation, %p", conn);
212+
ChipLogDetail(DeviceLayer, "Indication confirmation: conn=%p", conn);
215213
bluez_gatt_characteristic1_complete_confirm(aChar, aInvocation);
216214
BLEManagerImpl::HandleTXComplete(conn);
217215
return TRUE;

0 commit comments

Comments
 (0)