Hi Luiz,
On Friday, 5 June 2020 19:22:56 CEST Luiz Augusto von Dentz 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.
This is about doing limited discovery as scanner, not advertising as limited
discoverable :-)
>
> > > ---
> > >
> > > 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
--
pozdrawiam
Szymon Janc
