I submitted some patches separately.
I didn't submit patch "device: Handle error from discover services request after pair"
because there was some merge conflict, but I renamed it as you suggested.
I reworked patch "device: Fix pairing and discovery with dual mode devices"
so that both discoveries could run in parallel. Because of that I moved
browse and timer to bearer struct.
Simon Mikuda (5):
device: Refactor device_discover_services function
device: Rename start_discovery function
device: Fix returning discovery error for Device.Pair
device: Fix pairing with dual mode devices
device: Fix reverse service discovery handling for dual mode devices
profiles/input/server.c | 2 +-
src/adapter.c | 2 +-
src/device.c | 167 +++++++++++++++++++++++++---------------
src/device.h | 3 +-
4 files changed, 108 insertions(+), 66 deletions(-)
--
2.34.1
We'll prefer to pair 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.
---
src/device.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/device.c b/src/device.c
index 5a39a6f83..367e2f046 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3017,6 +3017,11 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
bdaddr_type = device->bdaddr_type;
else if (device->le_state.bonded)
bdaddr_type = BDADDR_BREDR;
+ else if (device->bredr_state.connected && !device->le_state.connected)
+ bdaddr_type = BDADDR_BREDR;
+ else if (!device->bredr_state.connected && device->le_state.connected)
+ bdaddr_type = device->bdaddr_type == BDADDR_BREDR
+ ? BDADDR_LE_PUBLIC : device->bdaddr_type;
else
bdaddr_type = select_conn_bearer(device);
--
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 2679d4302..b77d90c7b 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
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
There can be situation that after connection to LE bearer we will try
to start reverse discovery on BR/EDR bearer even when it is not connected.
This change separates SDP and GATT reverse services discoveries to their
respective bearers. SDP to BR/EDR and GATT to LE.
---
src/device.c | 129 +++++++++++++++++++++++++++++++++------------------
1 file changed, 83 insertions(+), 46 deletions(-)
diff --git a/src/device.c b/src/device.c
index 367e2f046..90d6a7615 100644
--- a/src/device.c
+++ b/src/device.c
@@ -158,6 +158,10 @@ struct bearer_state {
bool initiator;
bool connectable;
time_t last_seen;
+ /* reverse service discovery timer */
+ unsigned int discov_timer;
+ /* service discover request (SDP, GATT) */
+ struct browse_req *browse;
};
struct ltk_info {
@@ -236,9 +240,7 @@ struct btd_device {
bool temporary;
bool connectable;
unsigned int disconn_timer;
- unsigned int discov_timer;
unsigned int temporary_timer; /* Temporary/disappear timer */
- struct browse_req *browse; /* service discover request */
struct bonding_req *bonding;
struct authentication_req *authr; /* authentication request */
GSList *disconnects; /* disconnects message */
@@ -684,8 +686,10 @@ static void browse_request_free(struct browse_req *req)
{
struct btd_device *device = req->device;
- if (device->browse == req)
- device->browse = NULL;
+ if (device->bredr_state.browse == req)
+ device->bredr_state.browse = NULL;
+ if (device->le_state.browse == req)
+ device->le_state.browse = NULL;
if (req->listener_id)
g_dbus_remove_watch(dbus_conn, req->listener_id);
@@ -833,8 +837,10 @@ static void device_free(gpointer user_data)
if (device->disconn_timer)
timeout_remove(device->disconn_timer);
- if (device->discov_timer)
- timeout_remove(device->discov_timer);
+ if (device->bredr_state.discov_timer)
+ timeout_remove(device->bredr_state.discov_timer);
+ if (device->le_state.discov_timer)
+ timeout_remove(device->le_state.discov_timer);
if (device->temporary_timer)
timeout_remove(device->temporary_timer);
@@ -1848,8 +1854,10 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
if (device->bonding)
bonding_request_cancel(device->bonding);
- if (device->browse)
- browse_request_cancel(device->browse);
+ if (device->bredr_state.browse)
+ browse_request_cancel(device->bredr_state.browse);
+ if (device->le_state.browse)
+ browse_request_cancel(device->le_state.browse);
if (device->att_io) {
g_io_channel_shutdown(device->att_io, FALSE, NULL);
@@ -2304,7 +2312,7 @@ void btd_device_update_allowed_services(struct btd_device *dev)
/* If service discovery is ongoing, let the service discovery complete
* callback call this function.
*/
- if (dev->browse) {
+ if (dev->bredr_state.browse) {
ba2str(&dev->bdaddr, addr);
DBG("service discovery of %s is ongoing. Skip updating allowed "
"services", addr);
@@ -2370,7 +2378,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services)
{
GSList *l;
- if (dev->pending || dev->connect || dev->browse)
+ if (dev->pending || dev->connect || dev->bredr_state.browse)
return -EBUSY;
if (!btd_adapter_get_powered(dev->adapter))
@@ -2401,7 +2409,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
DBG("%s %s, client %s", dev->path, uuid ? uuid : "(all)",
dbus_message_get_sender(msg));
- if (dev->pending || dev->connect || dev->browse)
+ if (dev->pending || dev->connect || dev->bredr_state.browse)
return btd_error_in_progress_str(msg, ERR_BREDR_CONN_BUSY);
if (!btd_adapter_get_powered(dev->adapter)) {
@@ -2784,7 +2792,7 @@ static void browse_request_complete(struct browse_req *req, uint8_t type,
/* if successfully resolved services we need to free browsing request
* before passing message back to connect functions, otherwise
- * device->browse is set and "InProgress" error is returned instead
+ * device->state.browse is set and "InProgress" error is returned instead
* of actually connecting services
*/
msg = dbus_message_ref(req->msg);
@@ -2829,7 +2837,7 @@ static void device_svc_resolved(struct btd_device *dev, uint8_t browse_type,
uint8_t bdaddr_type, int err)
{
struct bearer_state *state = get_state(dev, bdaddr_type);
- struct browse_req *req = dev->browse;
+ struct browse_req *req = state->browse;
DBG("%s err %d", dev->path, err);
@@ -3060,7 +3068,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,
@@ -4657,8 +4665,10 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
device_cancel_bonding(device, status);
}
- if (device->browse)
- browse_request_cancel(device->browse);
+ if (device->bredr_state.browse)
+ browse_request_cancel(device->bredr_state.browse);
+ if (device->le_state.browse)
+ browse_request_cancel(device->le_state.browse);
while (device->services != NULL) {
struct btd_service *service = device->services->data;
@@ -5317,7 +5327,7 @@ static void att_disconnected_cb(int err, void *user_data)
DBG("");
- if (device->browse)
+ if (device->le_state.browse)
goto done;
DBG("%s (%d)", strerror(err), err);
@@ -5345,7 +5355,7 @@ done:
static void register_gatt_services(struct btd_device *device)
{
- struct browse_req *req = device->browse;
+ struct browse_req *req = device->le_state.browse;
GSList *services = NULL;
if (!bt_gatt_client_is_ready(device->client))
@@ -5636,8 +5646,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
adapter_connect_list_add(device->adapter, device);
}
- if (device->browse)
- browse_request_complete(device->browse,
+ if (device->le_state.browse)
+ browse_request_complete(device->le_state.browse,
BROWSE_GATT,
device->bdaddr_type,
-ECONNABORTED);
@@ -5751,15 +5761,24 @@ static struct browse_req *browse_request_new(struct btd_device *device,
DBusMessage *msg)
{
struct browse_req *req;
+ struct bearer_state *state;
- if (device->browse)
+ switch (type) {
+ case BROWSE_SDP:
+ state = get_state(device, BDADDR_BREDR);
+ break;
+ case BROWSE_GATT:
+ state = get_state(device, BDADDR_LE_PUBLIC);
+ break;
+ }
+ if (state->browse)
return NULL;
req = g_new0(struct browse_req, 1);
req->device = device;
req->type = type;
- device->browse = req;
+ state->browse = req;
if (!msg)
return req;
@@ -5879,15 +5898,17 @@ int device_discover_services(struct btd_device *device,
uint8_t bdaddr_type, DBusMessage *msg)
{
int err;
+ struct bearer_state *state;
if (bdaddr_type == BDADDR_BREDR)
err = device_browse_sdp(device, msg);
else
err = device_browse_gatt(device, msg);
- if (err == 0 && device->discov_timer) {
- timeout_remove(device->discov_timer);
- device->discov_timer = 0;
+ state = get_state(device, bdaddr_type);
+ if (err == 0 && state->discov_timer) {
+ timeout_remove(state->discov_timer);
+ state->discov_timer = 0;
}
return err;
@@ -6208,16 +6229,22 @@ bool device_is_connectable(struct btd_device *device)
return state->connectable;
}
-static bool start_discovery_cb(gpointer user_data)
+static bool start_sdp_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->bredr_state.discov_timer = 0;
+ device_browse_sdp(device, NULL);
- device->discov_timer = 0;
+ return FALSE;
+}
+
+static bool start_gatt_discovery_cb(gpointer user_data)
+{
+ struct btd_device *device = user_data;
+
+ device->le_state.discov_timer = 0;
+ device_browse_gatt(device, NULL);
return FALSE;
}
@@ -6368,17 +6395,27 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
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 &&
- btd_opts.reverse_discovery) {
- /* If we are not initiators and there is no currently
- * active discovery or discovery timer, set discovery
- * timer */
- DBG("setting timer for reverse service discovery");
- device->discov_timer = timeout_add_seconds(
- DISCOVERY_TIMER,
- start_discovery_cb,
- device, NULL);
+
+ } else if (!state->svc_resolved
+ && !state->browse
+ && !state->discov_timer
+ && btd_opts.reverse_discovery) {
+
+ /* If we are not initiators and there is no currently
+ * active discovery or discovery timer, set discovery
+ * timer */
+ if (bdaddr_type == BDADDR_BREDR) {
+ DBG("setting timer for reverse SDP service discovery");
+ state->discov_timer = timeout_add_seconds(
+ DISCOVERY_TIMER,
+ start_sdp_discovery_cb,
+ device, NULL);
+ } else {
+ DBG("setting timer for reverse GATT service discovery");
+ state->discov_timer = timeout_add_seconds(
+ DISCOVERY_TIMER,
+ start_gatt_discovery_cb,
+ device, NULL);
}
}
}
@@ -6416,11 +6453,11 @@ unsigned int device_wait_for_svc_complete(struct btd_device *dev,
if (state->svc_resolved || !btd_opts.reverse_discovery)
cb->idle_id = g_idle_add(svc_idle_cb, cb);
- else if (dev->discov_timer > 0) {
- timeout_remove(dev->discov_timer);
- dev->discov_timer = timeout_add_seconds(
+ else if (dev->bredr_state.discov_timer > 0) {
+ timeout_remove(dev->bredr_state.discov_timer);
+ dev->bredr_state.discov_timer = timeout_add_seconds(
0,
- start_discovery_cb,
+ start_sdp_discovery_cb,
dev, NULL);
}
--
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 6f28e261e..5a39a6f83 100644
--- a/src/device.c
+++ b/src/device.c
@@ -6302,6 +6302,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);
@@ -6351,8 +6352,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 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=770402
---Test result---
Test Summary:
CheckPatch FAIL 2.88 seconds
GitLint PASS 1.55 seconds
BuildEll PASS 33.07 seconds
BluezMake PASS 1196.56 seconds
MakeCheck PASS 12.58 seconds
MakeDistcheck PASS 192.41 seconds
CheckValgrind PASS 304.17 seconds
CheckSmatch PASS 422.49 seconds
bluezmakeextell PASS 127.59 seconds
IncrementalBuild PASS 5054.50 seconds
ScanBuild PASS 1352.14 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/5] 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/13331214.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/13331214.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,3/5] device: Fix returning discovery error for Device.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:6355:
+ err = device_discover_services(device, bdaddr_type, bonding->msg);
WARNING:LONG_LINE: line length of 85 exceeds 80 columns
#107: FILE: src/device.c:6358:
+ g_dbus_emit_property_changed(dbus_conn, device->path,
WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#112: FILE: src/device.c:6363:
+ g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
/github/workspace/src/src/13331216.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/13331216.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,4/5] device: Fix pairing with dual mode devices
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#82:
even when we have connection to LE bearer only. In these situation we should
/github/workspace/src/src/13331217.patch total: 0 errors, 1 warnings, 11 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/13331217.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/5] device: Fix reverse service discovery handling for dual mode devices
WARNING:LONG_LINE_COMMENT: line length of 81 exceeds 80 columns
#185: FILE: src/device.c:2795:
+ * device->state.browse is set and "InProgress" error is returned instead
WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#349: FILE: src/device.c:6406:
+ * timer */
/github/workspace/src/src/13331218.patch total: 0 errors, 2 warnings, 268 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/13331218.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.
---
Regards,
Linux Bluetooth
Hi Luiz.
Do you plan to review and merge also this commit series?
I already updated it according to your message:
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.
Thank you.
BR,
Simon
On 28. 7. 2023 7:31, Simon Mikuda wrote:
> There can be situation that after connection to LE bearer we will try
> to start reverse discovery on BR/EDR bearer even when it is not connected.
>
> This change separates SDP and GATT reverse services discoveries to their
> respective bearers. SDP to BR/EDR and GATT to LE.
> ---
> src/device.c | 129 +++++++++++++++++++++++++++++++++------------------
> 1 file changed, 83 insertions(+), 46 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 367e2f046..90d6a7615 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -158,6 +158,10 @@ struct bearer_state {
> bool initiator;
> bool connectable;
> time_t last_seen;
> + /* reverse service discovery timer */
> + unsigned int discov_timer;
> + /* service discover request (SDP, GATT) */
> + struct browse_req *browse;
> };
>
> struct ltk_info {
> @@ -236,9 +240,7 @@ struct btd_device {
> bool temporary;
> bool connectable;
> unsigned int disconn_timer;
> - unsigned int discov_timer;
> unsigned int temporary_timer; /* Temporary/disappear timer */
> - struct browse_req *browse; /* service discover request */
> struct bonding_req *bonding;
> struct authentication_req *authr; /* authentication request */
> GSList *disconnects; /* disconnects message */
> @@ -684,8 +686,10 @@ static void browse_request_free(struct browse_req *req)
> {
> struct btd_device *device = req->device;
>
> - if (device->browse == req)
> - device->browse = NULL;
> + if (device->bredr_state.browse == req)
> + device->bredr_state.browse = NULL;
> + if (device->le_state.browse == req)
> + device->le_state.browse = NULL;
>
> if (req->listener_id)
> g_dbus_remove_watch(dbus_conn, req->listener_id);
> @@ -833,8 +837,10 @@ static void device_free(gpointer user_data)
> if (device->disconn_timer)
> timeout_remove(device->disconn_timer);
>
> - if (device->discov_timer)
> - timeout_remove(device->discov_timer);
> + if (device->bredr_state.discov_timer)
> + timeout_remove(device->bredr_state.discov_timer);
> + if (device->le_state.discov_timer)
> + timeout_remove(device->le_state.discov_timer);
>
> if (device->temporary_timer)
> timeout_remove(device->temporary_timer);
> @@ -1848,8 +1854,10 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
> if (device->bonding)
> bonding_request_cancel(device->bonding);
>
> - if (device->browse)
> - browse_request_cancel(device->browse);
> + if (device->bredr_state.browse)
> + browse_request_cancel(device->bredr_state.browse);
> + if (device->le_state.browse)
> + browse_request_cancel(device->le_state.browse);
>
> if (device->att_io) {
> g_io_channel_shutdown(device->att_io, FALSE, NULL);
> @@ -2304,7 +2312,7 @@ void btd_device_update_allowed_services(struct btd_device *dev)
> /* If service discovery is ongoing, let the service discovery complete
> * callback call this function.
> */
> - if (dev->browse) {
> + if (dev->bredr_state.browse) {
> ba2str(&dev->bdaddr, addr);
> DBG("service discovery of %s is ongoing. Skip updating allowed "
> "services", addr);
> @@ -2370,7 +2378,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services)
> {
> GSList *l;
>
> - if (dev->pending || dev->connect || dev->browse)
> + if (dev->pending || dev->connect || dev->bredr_state.browse)
> return -EBUSY;
>
> if (!btd_adapter_get_powered(dev->adapter))
> @@ -2401,7 +2409,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
> DBG("%s %s, client %s", dev->path, uuid ? uuid : "(all)",
> dbus_message_get_sender(msg));
>
> - if (dev->pending || dev->connect || dev->browse)
> + if (dev->pending || dev->connect || dev->bredr_state.browse)
> return btd_error_in_progress_str(msg, ERR_BREDR_CONN_BUSY);
>
> if (!btd_adapter_get_powered(dev->adapter)) {
> @@ -2784,7 +2792,7 @@ static void browse_request_complete(struct browse_req *req, uint8_t type,
>
> /* if successfully resolved services we need to free browsing request
> * before passing message back to connect functions, otherwise
> - * device->browse is set and "InProgress" error is returned instead
> + * device->state.browse is set and "InProgress" error is returned instead
> * of actually connecting services
> */
> msg = dbus_message_ref(req->msg);
> @@ -2829,7 +2837,7 @@ static void device_svc_resolved(struct btd_device *dev, uint8_t browse_type,
> uint8_t bdaddr_type, int err)
> {
> struct bearer_state *state = get_state(dev, bdaddr_type);
> - struct browse_req *req = dev->browse;
> + struct browse_req *req = state->browse;
>
> DBG("%s err %d", dev->path, err);
>
> @@ -3060,7 +3068,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,
> @@ -4657,8 +4665,10 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
> device_cancel_bonding(device, status);
> }
>
> - if (device->browse)
> - browse_request_cancel(device->browse);
> + if (device->bredr_state.browse)
> + browse_request_cancel(device->bredr_state.browse);
> + if (device->le_state.browse)
> + browse_request_cancel(device->le_state.browse);
>
> while (device->services != NULL) {
> struct btd_service *service = device->services->data;
> @@ -5317,7 +5327,7 @@ static void att_disconnected_cb(int err, void *user_data)
>
> DBG("");
>
> - if (device->browse)
> + if (device->le_state.browse)
> goto done;
>
> DBG("%s (%d)", strerror(err), err);
> @@ -5345,7 +5355,7 @@ done:
>
> static void register_gatt_services(struct btd_device *device)
> {
> - struct browse_req *req = device->browse;
> + struct browse_req *req = device->le_state.browse;
> GSList *services = NULL;
>
> if (!bt_gatt_client_is_ready(device->client))
> @@ -5636,8 +5646,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
> adapter_connect_list_add(device->adapter, device);
> }
>
> - if (device->browse)
> - browse_request_complete(device->browse,
> + if (device->le_state.browse)
> + browse_request_complete(device->le_state.browse,
> BROWSE_GATT,
> device->bdaddr_type,
> -ECONNABORTED);
> @@ -5751,15 +5761,24 @@ static struct browse_req *browse_request_new(struct btd_device *device,
> DBusMessage *msg)
> {
> struct browse_req *req;
> + struct bearer_state *state;
>
> - if (device->browse)
> + switch (type) {
> + case BROWSE_SDP:
> + state = get_state(device, BDADDR_BREDR);
> + break;
> + case BROWSE_GATT:
> + state = get_state(device, BDADDR_LE_PUBLIC);
> + break;
> + }
> + if (state->browse)
> return NULL;
>
> req = g_new0(struct browse_req, 1);
> req->device = device;
> req->type = type;
>
> - device->browse = req;
> + state->browse = req;
>
> if (!msg)
> return req;
> @@ -5879,15 +5898,17 @@ int device_discover_services(struct btd_device *device,
> uint8_t bdaddr_type, DBusMessage *msg)
> {
> int err;
> + struct bearer_state *state;
>
> if (bdaddr_type == BDADDR_BREDR)
> err = device_browse_sdp(device, msg);
> else
> err = device_browse_gatt(device, msg);
>
> - if (err == 0 && device->discov_timer) {
> - timeout_remove(device->discov_timer);
> - device->discov_timer = 0;
> + state = get_state(device, bdaddr_type);
> + if (err == 0 && state->discov_timer) {
> + timeout_remove(state->discov_timer);
> + state->discov_timer = 0;
> }
>
> return err;
> @@ -6208,16 +6229,22 @@ bool device_is_connectable(struct btd_device *device)
> return state->connectable;
> }
>
> -static bool start_discovery_cb(gpointer user_data)
> +static bool start_sdp_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->bredr_state.discov_timer = 0;
> + device_browse_sdp(device, NULL);
>
> - device->discov_timer = 0;
> + return FALSE;
> +}
> +
> +static bool start_gatt_discovery_cb(gpointer user_data)
> +{
> + struct btd_device *device = user_data;
> +
> + device->le_state.discov_timer = 0;
> + device_browse_gatt(device, NULL);
>
> return FALSE;
> }
> @@ -6368,17 +6395,27 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
> 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 &&
> - btd_opts.reverse_discovery) {
> - /* If we are not initiators and there is no currently
> - * active discovery or discovery timer, set discovery
> - * timer */
> - DBG("setting timer for reverse service discovery");
> - device->discov_timer = timeout_add_seconds(
> - DISCOVERY_TIMER,
> - start_discovery_cb,
> - device, NULL);
> +
> + } else if (!state->svc_resolved
> + && !state->browse
> + && !state->discov_timer
> + && btd_opts.reverse_discovery) {
> +
> + /* If we are not initiators and there is no currently
> + * active discovery or discovery timer, set discovery
> + * timer */
> + if (bdaddr_type == BDADDR_BREDR) {
> + DBG("setting timer for reverse SDP service discovery");
> + state->discov_timer = timeout_add_seconds(
> + DISCOVERY_TIMER,
> + start_sdp_discovery_cb,
> + device, NULL);
> + } else {
> + DBG("setting timer for reverse GATT service discovery");
> + state->discov_timer = timeout_add_seconds(
> + DISCOVERY_TIMER,
> + start_gatt_discovery_cb,
> + device, NULL);
> }
> }
> }
> @@ -6416,11 +6453,11 @@ unsigned int device_wait_for_svc_complete(struct btd_device *dev,
>
> if (state->svc_resolved || !btd_opts.reverse_discovery)
> cb->idle_id = g_idle_add(svc_idle_cb, cb);
> - else if (dev->discov_timer > 0) {
> - timeout_remove(dev->discov_timer);
> - dev->discov_timer = timeout_add_seconds(
> + else if (dev->bredr_state.discov_timer > 0) {
> + timeout_remove(dev->bredr_state.discov_timer);
> + dev->bredr_state.discov_timer = timeout_add_seconds(
> 0,
> - start_discovery_cb,
> + start_sdp_discovery_cb,
> dev, NULL);
> }
>