Hi Tedd, On Fri, Jun 5, 2020 at 10:22 AM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Tedd, > > On Fri, Jun 5, 2020 at 2:09 AM Szymon Janc <szymon.janc@xxxxxxxxxxx> wrote: > > > > Hi Ted, > > > > On Friday, 5 June 2020 08:10:03 CEST Tedd Ho-Jeong An wrote: > > > From: Tedd Ho-Jeong An <tedd.an@xxxxxxxxx> > > > > > > There are a few test cases for limited discovery in GAP qualification > > > test suite. But the d-bus API doesn't support it and the only way to > > > start the limited discovery is using the management API. > > > > > > This patch adds support for limited discovery by using management API. > > > > Isn't it enough by spec to check if limited discoverable flag is set? > > We provide this info in AdvertisingFlags so we should be conforming. > > > > Also, if we want to be able to explicitly do Limited Discovery this should be > > added to SetDiscoveryFilter so that btpclient can stay D-Bus only. > > We should probably use DiscoverableTimeout to indicate a limited > discoverable flag instead of general discoverable, I will send a patch > for that. In fact it is already supported: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/advertising.c#n1067 Though there is a small bug related to it that it doesn't set NO_BREDR in case the adapter global discoverable is not set, but Ive send a fix for that. > > > --- > > > tools/btpclient.c | 194 ++++++++++++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 187 insertions(+), 7 deletions(-) > > > > > > diff --git a/tools/btpclient.c b/tools/btpclient.c > > > index f9c693056..46f866117 100644 > > > --- a/tools/btpclient.c > > > +++ b/tools/btpclient.c > > > @@ -36,6 +36,10 @@ > > > #include "lib/bluetooth.h" > > > #include "src/shared/btp.h" > > > > > > +/* For BT_MGMT API */ > > > +#include "lib/mgmt.h" > > > +#include "src/shared/mgmt.h" > > > + > > > #define AD_PATH "/org/bluez/advertising" > > > #define AG_PATH "/org/bluez/agent" > > > #define AD_IFACE "org.bluez.LEAdvertisement1" > > > @@ -77,6 +81,14 @@ static struct btp *btp; > > > > > > static bool gap_service_registered; > > > > > > +/* For BT_MGMT API */ > > > +static struct mgmt *mgmt; > > > +static uint16_t mgmt_index; > > > + > > > +static uint32_t mgmt_flags; > > > + > > > +#define MGMT_OPS_DISCOVERY 0x01 > > > + > > > struct ad_data { > > > uint8_t data[25]; > > > uint8_t len; > > > @@ -1403,6 +1415,88 @@ static void set_discovery_filter_reply(struct > > > l_dbus_proxy *proxy, start_discovery_reply, NULL, NULL); > > > } > > > > > > +#define SCAN_TYPE_BREDR (1 << BDADDR_BREDR) > > > +#define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM)) > > > +#define SCAN_TYPE_DUAL (SCAN_TYPE_BREDR | SCAN_TYPE_LE) > > > + > > > +struct discovery_flags { > > > + uint8_t flags; > > > +}; > > > + > > > +static int btp_mgmt_start_limited_discovery(uint8_t flags) > > > +{ > > > + struct mgmt_cp_start_discovery cp; > > > + uint8_t type = SCAN_TYPE_DUAL; > > > + > > > + memset(&cp, 0, sizeof(cp)); > > > + > > > + if (flags & (BTP_GAP_DISCOVERY_FLAG_LE | > > > + BTP_GAP_DISCOVERY_FLAG_BREDR)) { > > > + type = SCAN_TYPE_DUAL; > > > + } else if (flags & BTP_GAP_DISCOVERY_FLAG_LE) { > > > + type &= ~SCAN_TYPE_BREDR; > > > + type |= SCAN_TYPE_LE; > > > + } else if (flags & BTP_GAP_DISCOVERY_FLAG_BREDR) { > > > + type |= SCAN_TYPE_BREDR; > > > + type &= ~SCAN_TYPE_LE; > > > + } > > > + cp.type = type; > > > + > > > + return mgmt_send(mgmt, MGMT_OP_START_LIMITED_DISCOVERY, mgmt_index, > > > + sizeof(cp), &cp, NULL, NULL, NULL); > > > +} > > > + > > > +static void btp_mgmt_discovering_destroy(void *user_data) > > > +{ > > > + l_free(user_data); > > > +} > > > + > > > +static void btp_mgmt_discovering_cb(uint16_t index, uint16_t len, > > > + const void *param, void > > *user_data) > > > +{ > > > + const struct mgmt_ev_discovering *ev = param; > > > + struct discovery_flags *df = user_data; > > > + > > > + if (len < sizeof(*ev)) { > > > + l_error("Too short (%u bytes) discovering event", len); > > > + return; > > > + } > > > + > > > + l_info("MGMT: discovering %s", ev->discovering ? "on" : "off"); > > > + > > > + /* Start new discovery */ > > > + if (ev->discovering == 0 && (mgmt_flags & MGMT_OPS_DISCOVERY)) > > > + btp_mgmt_start_limited_discovery(df->flags); > > > +} > > > + > > > +static void btp_mgmt_setup_limited_discovery(uint8_t index, uint8_t flags) > > > +{ > > > + int ret; > > > + struct discovery_flags *df; > > > + > > > + /* Saves the flags so it can be used to start new discovery */ > > > + df = l_new(struct discovery_flags, 1); > > > + df->flags = flags; > > > + > > > + /* Register event for discovering */ > > > + mgmt_register(mgmt, MGMT_EV_DISCOVERING, mgmt_index, > > > + btp_mgmt_discovering_cb, df, > > > + btp_mgmt_discovering_destroy); > > > + > > > + ret = btp_mgmt_start_limited_discovery(flags); > > > + if (ret == 0) { > > > + l_error("Unable to send start_discovery cmd"); > > > + btp_send_error(btp, BTP_GAP_SERVICE, index, > > BTP_ERROR_FAIL); > > > + return; > > > + } > > > + > > > + /* Set flag that mgmt interface is used for scanning */ > > > + mgmt_flags |= MGMT_OPS_DISCOVERY; > > > + > > > + btp_send(btp, BTP_GAP_SERVICE, BTP_OP_GAP_START_DISCOVERY, > > > + index, 0, NULL); > > > +} > > > + > > > static void btp_gap_start_discovery(uint8_t index, const void *param, > > > uint16_t length, void > > *user_data) > > > { > > > @@ -1423,10 +1517,17 @@ static void btp_gap_start_discovery(uint8_t index, > > > const void *param, return; > > > } > > > > > > - l_dbus_proxy_method_call(adapter->proxy, "SetDiscoveryFilter", > > > - > > set_discovery_filter_setup, > > > - > > set_discovery_filter_reply, > > > - L_UINT_TO_PTR(cp- > > >flags), NULL); > > > + /* Use BT MGMT interface to start limited discovery procedure since > > > + * it is not supported by D-BUS API > > > + */ > > > + if (cp->flags & BTP_GAP_DISCOVERY_FLAG_LIMITED) > > > + btp_mgmt_setup_limited_discovery(index, cp->flags); > > > + else { > > > + l_dbus_proxy_method_call(adapter->proxy, > > "SetDiscoveryFilter", > > > + set_discovery_filter_setup, > > > + set_discovery_filter_reply, > > > + L_UINT_TO_PTR(cp->flags), > > NULL); > > > + } > > > } > > > > > > static void clear_discovery_filter_setup(struct l_dbus_message *message, > > > @@ -1501,6 +1602,29 @@ static void stop_discovery_reply(struct l_dbus_proxy > > > *proxy, NULL, NULL); > > > } > > > > > > +static void btp_mgmt_stop_discovery(uint8_t index) > > > +{ > > > + struct mgmt_cp_stop_discovery cp; > > > + int ret; > > > + > > > + memset(&cp, 0, sizeof(cp)); > > > + cp.type = SCAN_TYPE_DUAL; > > > + > > > + ret = mgmt_send(mgmt, MGMT_OP_STOP_DISCOVERY, mgmt_index, > > sizeof(cp), > > > + &cp, NULL, NULL, NULL); > > > + if (ret == 0) { > > > + l_error("Unable to send stop_discovery cmd"); > > > + btp_send_error(btp, BTP_GAP_SERVICE, index, > > BTP_ERROR_FAIL); > > > + return; > > > + } > > > + > > > + /* Clear flag that mgmt interface is used for scanning */ > > > + mgmt_flags &= ~MGMT_OPS_DISCOVERY; > > > + > > > + btp_send(btp, BTP_GAP_SERVICE, BTP_OP_GAP_STOP_DISCOVERY, > > > + index, 0, NULL); > > > +} > > > + > > > static void btp_gap_stop_discovery(uint8_t index, const void *param, > > > uint16_t length, void > > *user_data) > > > { > > > @@ -1520,8 +1644,14 @@ static void btp_gap_stop_discovery(uint8_t index, > > > const void *param, return; > > > } > > > > > > - l_dbus_proxy_method_call(adapter->proxy, "StopDiscovery", NULL, > > > - stop_discovery_reply, NULL, > > NULL); > > > + /* If the discovery procedure is started by BT MGMT API for limited > > > + * discovering, it should be stopped by mgmt API. > > > + */ > > > + if (mgmt_flags & MGMT_OPS_DISCOVERY) > > > + btp_mgmt_stop_discovery(index); > > > + else > > > + l_dbus_proxy_method_call(adapter->proxy, "StopDiscovery", > > NULL, > > > + stop_discovery_reply, NULL, > > NULL); > > > } > > > > > > static void connect_reply(struct l_dbus_proxy *proxy, > > > @@ -3140,6 +3270,7 @@ static void usage(void) > > > l_info("\tbtpclient [options]"); > > > l_info("options:\n" > > > "\t-s, --socket <socket> Socket to use for BTP\n" > > > + "\t-i, --index <id> Specify the adapter index\n" > > > "\t-q, --quiet Don't emit any logs\n" > > > "\t-v, --version Show version\n" > > > "\t-h, --help Show help options"); > > > @@ -3147,12 +3278,52 @@ static void usage(void) > > > > > > static const struct option options[] = { > > > { "socket", 1, 0, 's' }, > > > + { "index", 1, 0, 'i' }, > > > { "quiet", 0, 0, 'q' }, > > > { "version", 0, 0, 'v' }, > > > { "help", 0, 0, 'h' }, > > > { 0, 0, 0, 0 } > > > }; > > > > > > +static void set_index(const char *arg) > > > +{ > > > + /* Use default index 0 */ > > > + if (!arg || !strcmp(arg, "none") || !strcmp(arg, "any") || > > > + !strcmp(arg, > > "all")) > > > + mgmt_index = MGMT_INDEX_NONE; > > > + else if (strlen(arg) > 3 && !strncasecmp(arg, "hci", 3)) > > > + mgmt_index = atoi(&arg[3]); > > > + else > > > + mgmt_index = atoi(arg); > > > +} > > > + > > > +static int btp_mgmt_init(const char *index_opt) > > > +{ > > > + mgmt = mgmt_new_default(); > > > + if (!mgmt) { > > > + l_info("Unable to open mgmt_socket\n"); > > > + return EXIT_FAILURE; > > > + } > > > + > > > + if (index_opt) > > > + set_index(index_opt); > > > + > > > + if (mgmt_index == MGMT_INDEX_NONE) > > > + mgmt_index = 0; > > > + > > > + return EXIT_SUCCESS; > > > +} > > > + > > > +static void btp_mgmt_release(void) > > > +{ > > > + l_error("MGMT: Release all management resources"); > > > + mgmt_cancel_all(mgmt); > > > + mgmt_unregister_all(mgmt); > > > + mgmt_unref(mgmt); > > > +} > > > + > > > +static const char *index_opt; > > > + > > > int main(int argc, char *argv[]) > > > { > > > struct l_dbus_client *client; > > > @@ -3160,7 +3331,8 @@ int main(int argc, char *argv[]) > > > > > > l_log_set_stderr(); > > > > > > - while ((opt = getopt_long(argc, argv, "+hs:vq", options, NULL)) != > > -1) { > > > + while ((opt = getopt_long(argc, argv, "+hs:vqi:", options, > > > + > > NULL)) != -1) { > > > switch (opt) { > > > case 's': > > > socket_path = l_strdup(optarg); > > > @@ -3173,6 +3345,9 @@ int main(int argc, char *argv[]) > > > case 'v': > > > l_info("%s", VERSION); > > > return EXIT_SUCCESS; > > > + case 'i': > > > + index_opt = l_strdup(optarg); > > > + break; > > > case 'h': > > > default: > > > usage(); > > > @@ -3189,6 +3364,10 @@ int main(int argc, char *argv[]) > > > if (!l_main_init()) > > > return EXIT_FAILURE; > > > > > > + if (btp_mgmt_init(index_opt)) { > > > + l_error("Unable to initialize the management interface"); > > > + return EXIT_FAILURE; > > > + } > > > > > > adapters = l_queue_new(); > > > > > > @@ -3210,6 +3389,7 @@ int main(int argc, char *argv[]) > > > l_dbus_client_destroy(client); > > > l_dbus_destroy(dbus); > > > btp_cleanup(btp); > > > + btp_mgmt_release(); > > > > > > l_queue_destroy(adapters, (l_queue_destroy_func_t)btp_adapter_free); > > > > > > -- > > pozdrawiam > > Szymon Janc > > > > > > > -- > Luiz Augusto von Dentz -- Luiz Augusto von Dentz
