Fixes for pairing and discovery for dual mode devices
Fix for EIR_BREDR_UNSUP flag
Add handle numbers on dbus for client
Minor refactor, fixes
Simon Mikuda (8):
advertising: Fix setting "BR/EDR not supported" flag
adapter: Be able to use ConnectDevice for discovered devices
device: Refactor device_discover_services function
device: Rename start_discovery function
device: Fix pairing and discovery with dual mode devices
device: Handle error from discover services request after pair
gatt-client: Add read-only handles to dbus
adapter: Ensure that file exists on IRK write
doc/gatt-api.txt | 3 +++
profiles/input/server.c | 2 +-
src/adapter.c | 7 +++--
src/advertising.c | 8 +++---
src/device.c | 60 ++++++++++++++++++++++-------------------
src/device.h | 3 ++-
src/gatt-client.c | 36 +++++++++++++++++++++++++
7 files changed, 83 insertions(+), 36 deletions(-)
--
2.34.1
After refactoring we can reuse function once more in function
void device_bonding_complete(...)
---
profiles/input/server.c | 2 +-
src/adapter.c | 2 +-
src/device.c | 24 +++++++-----------------
src/device.h | 3 ++-
4 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/profiles/input/server.c b/profiles/input/server.c
index 79cf08a66..12912cae6 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -102,7 +102,7 @@ static void sixaxis_browse_sdp(const bdaddr_t *src, const bdaddr_t *dst,
data->psm = psm;
if (psm == L2CAP_PSM_HIDP_CTRL)
- device_discover_services(device);
+ device_discover_services(device, BDADDR_BREDR, NULL);
device_wait_for_svc_complete(device, sixaxis_sdp_cb, data);
}
diff --git a/src/adapter.c b/src/adapter.c
index 17f4a637d..4c3bb091d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3637,7 +3637,7 @@ static void device_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
device_attach_att(device, io);
}
- device_discover_services(device);
+ device_discover_services(device, data->dst_type, NULL);
device_wait_for_svc_complete(device, device_browse_cb, NULL);
g_io_channel_unref(io);
diff --git a/src/device.c b/src/device.c
index b43ced8b5..82740216d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2446,10 +2446,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
resolve_services:
DBG("Resolving services for %s", dev->path);
- if (bdaddr_type == BDADDR_BREDR)
- err = device_browse_sdp(dev, msg);
- else
- err = device_browse_gatt(dev, msg);
+ err = device_discover_services(dev, bdaddr_type, msg);
if (err < 0) {
return btd_error_failed(msg, bdaddr_type == BDADDR_BREDR ?
ERR_BREDR_CONN_SDP_SEARCH : ERR_LE_CONN_GATT_BROWSE);
@@ -5873,14 +5870,15 @@ static int device_browse_sdp(struct btd_device *device, DBusMessage *msg)
return err;
}
-int device_discover_services(struct btd_device *device)
+int device_discover_services(struct btd_device *device,
+ uint8_t bdaddr_type, DBusMessage *msg)
{
int err;
- if (device->bredr)
- err = device_browse_sdp(device, NULL);
+ if (bdaddr_type == BDADDR_BREDR)
+ err = device_browse_sdp(device, msg);
else
- err = device_browse_gatt(device, NULL);
+ err = device_browse_gatt(device, msg);
if (err == 0 && device->discov_timer) {
timeout_remove(device->discov_timer);
@@ -6353,15 +6351,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
DBG("Proceeding with service discovery");
/* If we are initiators remove any discovery timer and just
* start discovering services directly */
- if (device->discov_timer) {
- timeout_remove(device->discov_timer);
- device->discov_timer = 0;
- }
-
- if (bdaddr_type == BDADDR_BREDR)
- device_browse_sdp(device, bonding->msg);
- else
- device_browse_gatt(device, bonding->msg);
+ device_discover_services(device, bdaddr_type, bonding->msg);
bonding_request_free(bonding);
} else if (!state->svc_resolved) {
diff --git a/src/device.h b/src/device.h
index 3252e14ef..0393478df 100644
--- a/src/device.h
+++ b/src/device.h
@@ -176,7 +176,8 @@ bool device_remove_svc_complete_callback(struct btd_device *dev,
struct btd_service *btd_device_get_service(struct btd_device *dev,
const char *remote_uuid);
-int device_discover_services(struct btd_device *device);
+int device_discover_services(struct btd_device *device,
+ uint8_t bdaddr_type, DBusMessage *msg);
int btd_device_connect_services(struct btd_device *dev, GSList *services);
uint32_t btd_device_get_current_flags(struct btd_device *dev);
--
2.34.1
This can be useful when you want to specify trasport type for already
paired device (e.g. use LE transport for dual mode device).
---
src/adapter.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 2679d4302..17f4a637d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3763,9 +3763,6 @@ static DBusMessage *connect_device(DBusConnection *conn,
if (!bacmp(&addr, BDADDR_ANY))
return btd_error_invalid_args(msg);
- if (btd_adapter_find_device(adapter, &addr, addr_type))
- return btd_error_already_exists(msg);
-
device_connect(adapter, &addr, addr_type, msg);
return NULL;
}
--
2.34.1
Rename it to start_discovery_cb to indicate that it is callback function
from timer.
---
src/device.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/device.c b/src/device.c
index 82740216d..6f28e261e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -6203,7 +6203,7 @@ bool device_is_connectable(struct btd_device *device)
return state->connectable;
}
-static bool start_discovery(gpointer user_data)
+static bool start_discovery_cb(gpointer user_data)
{
struct btd_device *device = user_data;
@@ -6363,7 +6363,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
DBG("setting timer for reverse service discovery");
device->discov_timer = timeout_add_seconds(
DISCOVERY_TIMER,
- start_discovery,
+ start_discovery_cb,
device, NULL);
}
}
@@ -6406,7 +6406,7 @@ unsigned int device_wait_for_svc_complete(struct btd_device *dev,
timeout_remove(dev->discov_timer);
dev->discov_timer = timeout_add_seconds(
0,
- start_discovery,
+ start_discovery_cb,
dev, NULL);
}
--
2.34.1
We'll prefer to pair and discover services on connected bearer first.
There was a problem with pairing, that select_conn_bearer returned BR/EDR
even when we have connection to LE bearer only. In these situation we should
pair over connected bearer, since connection to another bearer can fail.
Similar problem with discovery that after connection on LE bearer discovery
was requested on BR/EDR bearer which can cause connection error (e.g. Page
Timeout).
---
src/device.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/device.c b/src/device.c
index 6f28e261e..446e978ee 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2504,6 +2504,16 @@ static uint8_t select_conn_bearer(struct btd_device *dev)
return dev->bdaddr_type;
}
+static uint8_t select_active_bearer(struct btd_device *dev)
+{
+ if (dev->bredr_state.connected)
+ return BDADDR_BREDR;
+ else if (dev->le_state.connected)
+ return dev->bdaddr_type == BDADDR_BREDR
+ ? BDADDR_LE_PUBLIC : dev->bdaddr_type;
+ return select_conn_bearer(dev);
+}
+
static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
void *user_data)
{
@@ -3018,7 +3028,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
else if (device->le_state.bonded)
bdaddr_type = BDADDR_BREDR;
else
- bdaddr_type = select_conn_bearer(device);
+ bdaddr_type = select_active_bearer(device);
state = get_state(device, bdaddr_type);
@@ -3055,7 +3065,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
err = device_connect_le(device);
else
err = adapter_create_bonding(adapter, &device->bdaddr,
- device->bdaddr_type,
+ bdaddr_type,
io_cap);
} else {
err = adapter_create_bonding(adapter, &device->bdaddr,
@@ -6207,12 +6217,9 @@ static bool start_discovery_cb(gpointer user_data)
{
struct btd_device *device = user_data;
- if (device->bredr)
- device_browse_sdp(device, NULL);
- else
- device_browse_gatt(device, NULL);
-
device->discov_timer = 0;
+ device_discover_services(device, select_active_bearer(device),
+ NULL);
return FALSE;
}
--
2.34.1
If discovery was requesed from pair request we will report successfull
pairing even if there was an error during discovery.
---
src/device.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/device.c b/src/device.c
index 446e978ee..35c46e451 100644
--- a/src/device.c
+++ b/src/device.c
@@ -6309,6 +6309,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
struct bonding_req *bonding = device->bonding;
struct authentication_req *auth = device->authr;
struct bearer_state *state = get_state(device, bdaddr_type);
+ int err;
DBG("bonding %p status 0x%02x", bonding, status);
@@ -6358,8 +6359,16 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
DBG("Proceeding with service discovery");
/* If we are initiators remove any discovery timer and just
* start discovering services directly */
- device_discover_services(device, bdaddr_type, bonding->msg);
-
+ err = device_discover_services(device, bdaddr_type, bonding->msg);
+ if (err) {
+ if (device->pending_paired) {
+ g_dbus_emit_property_changed(dbus_conn, device->path,
+ DEVICE_INTERFACE, "Paired");
+ device->pending_paired = false;
+ }
+ /* Disregard browse errors in case of Pair */
+ g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
+ }
bonding_request_free(bonding);
} else if (!state->svc_resolved) {
if (!device->browse && !device->discov_timer &&
--
2.34.1
This can be usefull when mapping names for services and characteristics
to their handle numbers.
---
doc/gatt-api.txt | 3 +++
src/gatt-client.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 5042c5470..f29308aec 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -51,6 +51,7 @@ Properties string UUID [read-only]
services of this service.
uint16 Handle [read-write, optional] (Server Only)
+ [read-only] (Client Only)
Service handle. When available in the server it
would attempt to use to allocate into the database
@@ -291,6 +292,7 @@ Properties string UUID [read-only]
"authorize"
uint16 Handle [read-write, optional] (Server Only)
+ [read-only] (Client Only)
Characteristic handle. When available in the server it
would attempt to use to allocate into the database
@@ -380,6 +382,7 @@ Properties string UUID [read-only]
"authorize"
uint16 Handle [read-write, optional] (Server Only)
+ [read-only] (Client Only)
Characteristic handle. When available in the server it
would attempt to use to allocate into the database
diff --git a/src/gatt-client.c b/src/gatt-client.c
index a54d65e30..60a21e3f6 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -137,6 +137,17 @@ static bool uuid_cmp(const bt_uuid_t *uuid, uint16_t u16)
return bt_uuid_cmp(uuid, &uuid16) == 0;
}
+static gboolean descriptor_get_handle(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct service *desc = data;
+ uint16_t handle = desc->start_handle;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &handle);
+
+ return TRUE;
+}
+
static gboolean descriptor_get_uuid(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
@@ -635,6 +646,7 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
}
static const GDBusPropertyTable descriptor_properties[] = {
+ { "Handle", "q", descriptor_get_handle },
{ "UUID", "s", descriptor_get_uuid },
{ "Characteristic", "o", descriptor_get_characteristic, },
{ "Value", "ay", descriptor_get_value, NULL, descriptor_value_exists },
@@ -713,6 +725,17 @@ static void unregister_descriptor(void *data)
GATT_DESCRIPTOR_IFACE);
}
+static gboolean characteristic_get_handle(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct characteristic *chrc = data;
+ uint16_t handle = chrc->handle;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &handle);
+
+ return TRUE;
+}
+
static gboolean characteristic_get_uuid(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
@@ -1660,6 +1683,7 @@ static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
}
static const GDBusPropertyTable characteristic_properties[] = {
+ { "Handle", "q", characteristic_get_handle },
{ "UUID", "s", characteristic_get_uuid, NULL, NULL },
{ "Service", "o", characteristic_get_service, NULL, NULL },
{ "Value", "ay", characteristic_get_value, NULL,
@@ -1821,6 +1845,17 @@ static void unregister_characteristic(void *data)
GATT_CHARACTERISTIC_IFACE);
}
+static gboolean service_get_handle(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct service *service = data;
+ uint16_t handle = service->start_handle;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &handle);
+
+ return TRUE;
+}
+
static gboolean service_get_uuid(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
@@ -1884,6 +1919,7 @@ static gboolean service_get_includes(const GDBusPropertyTable *property,
}
static const GDBusPropertyTable service_properties[] = {
+ { "Handle", "q", service_get_handle },
{ "UUID", "s", service_get_uuid },
{ "Device", "o", service_get_device },
{ "Primary", "b", service_get_primary },
--
2.34.1
---
src/adapter.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index 4c3bb091d..fe8ae7604 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -8823,6 +8823,8 @@ static void store_irk(struct btd_adapter *adapter, const bdaddr_t *peer,
g_key_file_set_string(key_file, "IdentityResolvingKey", "Key", str);
+ create_file(filename, 0600);
+
store_data = g_key_file_to_data(key_file, &length, NULL);
if (!g_file_set_contents(filename, store_data, length, &gerr)) {
error("Unable set contents for %s: (%s)", filename,
--
2.34.1
We need to check if adapter is connectable since remote device can connect
to our device even when we are not discoverable according to advertised
MAC address.
---
src/advertising.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/advertising.c b/src/advertising.c
index d959bf38f..b50900029 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -95,6 +95,8 @@ struct dbus_obj_match {
const char *path;
};
+static int get_adv_flags(struct btd_adv_client *client);
+
static bool match_client(const void *a, const void *b)
{
const struct btd_adv_client *client = a;
@@ -736,11 +738,11 @@ static bool set_flags(struct btd_adv_client *client, uint8_t flags)
if (!btd_adapter_get_bredr(client->manager->adapter))
flags |= BT_AD_FLAG_NO_BREDR;
- /* Set BR/EDR Not Supported if adapter is not discoverable but the
+ /* Set BR/EDR Not Supported if adapter is not connectable but the
* instance is.
*/
- if ((flags & (BT_AD_FLAG_GENERAL | BT_AD_FLAG_LIMITED)) &&
- !btd_adapter_get_discoverable(client->manager->adapter))
+ if ((get_adv_flags(client) & MGMT_ADV_FLAG_CONNECTABLE) &&
+ !btd_adapter_get_connectable(client->manager->adapter))
flags |= BT_AD_FLAG_NO_BREDR;
if (!bt_ad_add_flags(client->data, &flags, 1))
--
2.34.1
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=769189
---Test result---
Test Summary:
CheckPatch FAIL 3.40 seconds
GitLint PASS 1.85 seconds
BuildEll PASS 32.84 seconds
BluezMake PASS 1109.64 seconds
MakeCheck PASS 12.64 seconds
MakeDistcheck PASS 181.70 seconds
CheckValgrind FAIL 302.59 seconds
CheckSmatch PASS 410.32 seconds
bluezmakeextell PASS 120.02 seconds
IncrementalBuild PASS 7615.67 seconds
ScanBuild PASS 1314.79 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,3/8] device: Refactor device_discover_services function
WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#136: FILE: src/device.c:5874:
+ uint8_t bdaddr_type, DBusMessage *msg)
WARNING:LONG_LINE: line length of 87 exceeds 80 columns
#177: FILE: src/device.h:180:
+ uint8_t bdaddr_type, DBusMessage *msg);
/github/workspace/src/src/13326120.patch total: 0 errors, 2 warnings, 71 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13326120.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[BlueZ,5/8] device: Fix pairing and discovery with dual mode devices
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#83:
even when we have connection to LE bearer only. In these situation we should
/github/workspace/src/src/13326123.patch total: 0 errors, 1 warnings, 46 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13326123.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[BlueZ,6/8] device: Handle error from discover services request after pair
WARNING:TYPO_SPELLING: 'successfull' may be misspelled - perhaps 'successful'?
#80:
If discovery was requesed from pair request we will report successfull
^^^^^^^^^^^
WARNING:LONG_LINE: line length of 82 exceeds 80 columns
#104: FILE: src/device.c:6362:
+ err = device_discover_services(device, bdaddr_type, bonding->msg);
WARNING:LONG_LINE: line length of 85 exceeds 80 columns
#107: FILE: src/device.c:6365:
+ g_dbus_emit_property_changed(dbus_conn, device->path,
WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#112: FILE: src/device.c:6370:
+ g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
/github/workspace/src/src/13326124.patch total: 0 errors, 4 warnings, 25 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13326124.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[BlueZ,7/8] gatt-client: Add read-only handles to dbus
WARNING:TYPO_SPELLING: 'usefull' may be misspelled - perhaps 'useful'?
#79:
This can be usefull when mapping names for services and characteristics
^^^^^^^
/github/workspace/src/src/13326125.patch total: 0 errors, 1 warnings, 93 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13326125.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12763:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12763 | int main(int argc, char *argv[])
| ^~~~
./test-driver: line 107: 74956 Aborted (core dumped) "$@" > $log_file 2>&1
make[3]: *** [Makefile:11350: test-suite.log] Error 1
make[2]: *** [Makefile:11458: check-TESTS] Error 2
make[1]: *** [Makefile:11873: check-am] Error 2
make: *** [Makefile:11875: check] Error 2
---
Regards,
Linux Bluetooth
Hi Simon,
On Tue, Jul 25, 2023 at 1:56 AM Simon Mikuda
<[email protected]> wrote:
>
> We'll prefer to pair and discover services on connected bearer first.
>
> There was a problem with pairing, that select_conn_bearer returned BR/EDR
> even when we have connection to LE bearer only. In these situation we should
> pair over connected bearer, since connection to another bearer can fail.
>
> Similar problem with discovery that after connection on LE bearer discovery
> was requested on BR/EDR bearer which can cause connection error (e.g. Page
> Timeout).
> ---
> src/device.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 6f28e261e..446e978ee 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2504,6 +2504,16 @@ static uint8_t select_conn_bearer(struct btd_device *dev)
> return dev->bdaddr_type;
> }
>
> +static uint8_t select_active_bearer(struct btd_device *dev)
> +{
> + if (dev->bredr_state.connected)
> + return BDADDR_BREDR;
> + else if (dev->le_state.connected)
> + return dev->bdaddr_type == BDADDR_BREDR
> + ? BDADDR_LE_PUBLIC : dev->bdaddr_type;
The code above assumes BR/EDR has the priority in case both are
connected, perhaps we should have a clear policy in case both are
connected.
> + return select_conn_bearer(dev);
> +}
> +
> static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
> void *user_data)
> {
> @@ -3018,7 +3028,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
> else if (device->le_state.bonded)
> bdaddr_type = BDADDR_BREDR;
> else
> - bdaddr_type = select_conn_bearer(device);
> + bdaddr_type = select_active_bearer(device);
>
> state = get_state(device, bdaddr_type);
>
> @@ -3055,7 +3065,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
> err = device_connect_le(device);
> else
> err = adapter_create_bonding(adapter, &device->bdaddr,
> - device->bdaddr_type,
> + bdaddr_type,
> io_cap);
> } else {
> err = adapter_create_bonding(adapter, &device->bdaddr,
> @@ -6207,12 +6217,9 @@ static bool start_discovery_cb(gpointer user_data)
> {
> struct btd_device *device = user_data;
>
> - if (device->bredr)
> - device_browse_sdp(device, NULL);
> - else
> - device_browse_gatt(device, NULL);
> -
> device->discov_timer = 0;
> + device_discover_services(device, select_active_bearer(device),
> + NULL);
Perhaps in case of discovery we could do both in parallel, although if
the remote side supports GATT services as part of SDP we may end up
with redundant discovery but bt_gatt_client/gatt_db shall be able to
handle that.
> return FALSE;
> }
> --
> 2.34.1
>
--
Luiz Augusto von Dentz
Hi Simon,
On Tue, Jul 25, 2023 at 1:56 AM Simon Mikuda
<[email protected]> wrote:
>
> ---
> src/adapter.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 4c3bb091d..fe8ae7604 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -8823,6 +8823,8 @@ static void store_irk(struct btd_adapter *adapter, const bdaddr_t *peer,
>
> g_key_file_set_string(key_file, "IdentityResolvingKey", "Key", str);
>
> + create_file(filename, 0600);
> +
> store_data = g_key_file_to_data(key_file, &length, NULL);
> if (!g_file_set_contents(filename, store_data, length, &gerr)) {
> error("Unable set contents for %s: (%s)", filename,
> --
> 2.34.1
This looks like a fix as well so reword it and send it separately,
also please add the runtime error in the description.
--
Luiz Augusto von Dentz
Hi Simon,
On Tue, Jul 25, 2023 at 1:56 AM Simon Mikuda
<[email protected]> wrote:
>
> This can be useful when you want to specify trasport type for already
> paired device (e.g. use LE transport for dual mode device).
> ---
> src/adapter.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 2679d4302..17f4a637d 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -3763,9 +3763,6 @@ static DBusMessage *connect_device(DBusConnection *conn,
> if (!bacmp(&addr, BDADDR_ANY))
> return btd_error_invalid_args(msg);
>
> - if (btd_adapter_find_device(adapter, &addr, addr_type))
> - return btd_error_already_exists(msg);
> -
> device_connect(adapter, &addr, addr_type, msg);
> return NULL;
> }
> --
> 2.34.1
While this is probably a good idea we need to document it on
adapter-api first, also not that Device.Connect could be used a second
time to connect both transports but this seems to be a better
alternative if one wants to bypass the daemon policy of connecting the
last seen bearer first.
--
Luiz Augusto von Dentz
Hi Simon,
On Tue, Jul 25, 2023 at 1:56 AM Simon Mikuda
<[email protected]> wrote:
>
> If discovery was requesed from pair request we will report successfull
> pairing even if there was an error during discovery.
> ---
> src/device.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 446e978ee..35c46e451 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -6309,6 +6309,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
> struct bonding_req *bonding = device->bonding;
> struct authentication_req *auth = device->authr;
> struct bearer_state *state = get_state(device, bdaddr_type);
> + int err;
>
> DBG("bonding %p status 0x%02x", bonding, status);
>
> @@ -6358,8 +6359,16 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
> DBG("Proceeding with service discovery");
> /* If we are initiators remove any discovery timer and just
> * start discovering services directly */
> - device_discover_services(device, bdaddr_type, bonding->msg);
> -
> + err = device_discover_services(device, bdaddr_type, bonding->msg);
> + if (err) {
> + if (device->pending_paired) {
> + g_dbus_emit_property_changed(dbus_conn, device->path,
> + DEVICE_INTERFACE, "Paired");
> + device->pending_paired = false;
> + }
> + /* Disregard browse errors in case of Pair */
> + g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
> + }
> bonding_request_free(bonding);
> } else if (!state->svc_resolved) {
> if (!device->browse && !device->discov_timer &&
> --
> 2.34.1
This looks like a fix so I'd recommend sending it separately rewording
the commit to something like: device: Fix returning discovery error
for Device.Pair...
--
Luiz Augusto von Dentz
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 25 Jul 2023 10:44:23 +0200 you wrote:
> Fixes for pairing and discovery for dual mode devices
> Fix for EIR_BREDR_UNSUP flag
> Add handle numbers on dbus for client
> Minor refactor, fixes
>
> Simon Mikuda (8):
> advertising: Fix setting "BR/EDR not supported" flag
> adapter: Be able to use ConnectDevice for discovered devices
> device: Refactor device_discover_services function
> device: Rename start_discovery function
> device: Fix pairing and discovery with dual mode devices
> device: Handle error from discover services request after pair
> gatt-client: Add read-only handles to dbus
> adapter: Ensure that file exists on IRK write
>
> [...]
Here is the summary with links:
- [BlueZ,1/8] advertising: Fix setting "BR/EDR not supported" flag
(no matching commit)
- [BlueZ,2/8] adapter: Be able to use ConnectDevice for discovered devices
(no matching commit)
- [BlueZ,3/8] device: Refactor device_discover_services function
(no matching commit)
- [BlueZ,4/8] device: Rename start_discovery function
(no matching commit)
- [BlueZ,5/8] device: Fix pairing and discovery with dual mode devices
(no matching commit)
- [BlueZ,6/8] device: Handle error from discover services request after pair
(no matching commit)
- [BlueZ,7/8] gatt-client: Add read-only handles to dbus
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=cda5549f2475
- [BlueZ,8/8] adapter: Ensure that file exists on IRK write
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 25 Jul 2023 10:44:23 +0200 you wrote:
> Fixes for pairing and discovery for dual mode devices
> Fix for EIR_BREDR_UNSUP flag
> Add handle numbers on dbus for client
> Minor refactor, fixes
>
> Simon Mikuda (8):
> advertising: Fix setting "BR/EDR not supported" flag
> adapter: Be able to use ConnectDevice for discovered devices
> device: Refactor device_discover_services function
> device: Rename start_discovery function
> device: Fix pairing and discovery with dual mode devices
> device: Handle error from discover services request after pair
> gatt-client: Add read-only handles to dbus
> adapter: Ensure that file exists on IRK write
>
> [...]
Here is the summary with links:
- [BlueZ,1/8] advertising: Fix setting "BR/EDR not supported" flag
(no matching commit)
- [BlueZ,2/8] adapter: Be able to use ConnectDevice for discovered devices
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=caf7db313e9d
- [BlueZ,3/8] device: Refactor device_discover_services function
(no matching commit)
- [BlueZ,4/8] device: Rename start_discovery function
(no matching commit)
- [BlueZ,5/8] device: Fix pairing and discovery with dual mode devices
(no matching commit)
- [BlueZ,6/8] device: Handle error from discover services request after pair
(no matching commit)
- [BlueZ,7/8] gatt-client: Add read-only handles to dbus
(no matching commit)
- [BlueZ,8/8] adapter: Ensure that file exists on IRK write
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html