Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] tools/btmgmt: Introduce stop-find command
@ 2015-03-12  2:11 Jakub Pawlowski
  2015-03-12  3:04 ` Arman Uguray
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Pawlowski @ 2015-03-12  2:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jakub Pawlowski

This patch adds stop-find command that stops discovery started by
find, or find-service command.
---
 tools/btmgmt.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/tools/btmgmt.c b/tools/btmgmt.c
index 8eff56b..89c2f43 100644
--- a/tools/btmgmt.c
+++ b/tools/btmgmt.c
@@ -1897,6 +1897,84 @@ static void cmd_find(struct mgmt *mgmt, uint16_t index, int argc, char **argv)
 	}
 }
 
+static void stop_find_rsp(uint8_t status, uint16_t len, const void *param,
+							void *user_data)
+{
+	if (status != 0) {
+		fprintf(stderr,
+			"Unable to stop discovery. status 0x%02x (%s)\n",
+						status, mgmt_errstr(status));
+		mainloop_quit();
+		return;
+	}
+
+	printf("Discovery stopped\n");
+	discovery = false;
+
+	noninteractive_quit(EXIT_SUCCESS);
+}
+
+static void stop_find_usage(void)
+{
+	printf("Usage: btmgmt stop-find [-l|-b]>\n");
+}
+
+static struct option stop_find_options[] = {
+	{ "help",	0, 0, 'h' },
+	{ "le-only",	1, 0, 'l' },
+	{ "bredr-only",	1, 0, 'b' },
+	{ 0, 0, 0, 0 }
+};
+
+static void cmd_stop_find(struct mgmt *mgmt, uint16_t index, int argc,
+			  char **argv)
+{
+	struct mgmt_cp_stop_discovery cp;
+	uint8_t type;
+	int opt;
+
+	if (index == MGMT_INDEX_NONE)
+		index = 0;
+
+	type = 0;
+	type |= (1 << BDADDR_BREDR);
+	type |= (1 << BDADDR_LE_PUBLIC);
+	type |= (1 << BDADDR_LE_RANDOM);
+
+	while ((opt = getopt_long(argc, argv, "+lbh", stop_find_options,
+								NULL)) != -1) {
+		switch (opt) {
+		case 'l':
+			type &= ~(1 << BDADDR_BREDR);
+			type |= (1 << BDADDR_LE_PUBLIC);
+			type |= (1 << BDADDR_LE_RANDOM);
+			break;
+		case 'b':
+			type |= (1 << BDADDR_BREDR);
+			type &= ~(1 << BDADDR_LE_PUBLIC);
+			type &= ~(1 << BDADDR_LE_RANDOM);
+			break;
+		case 'h':
+		default:
+			stop_find_usage();
+			exit(EXIT_SUCCESS);
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+	optind = 0;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.type = type;
+
+	if (mgmt_send(mgmt, MGMT_OP_STOP_DISCOVERY, index, sizeof(cp), &cp,
+					     stop_find_rsp, NULL, NULL) == 0) {
+		fprintf(stderr, "Unable to send stop_discovery cmd\n");
+		exit(EXIT_FAILURE);
+	}
+}
+
 static void name_rsp(uint8_t status, uint16_t len, const void *param,
 							void *user_data)
 {
@@ -3268,6 +3346,7 @@ static struct cmd_info all_cmd[] = {
 	{ "con",	cmd_con,	"List connections"		},
 	{ "find",	cmd_find,	"Discover nearby devices"	},
 	{ "find-service", cmd_find_service, "Discover nearby service"	},
+	{ "stop-find",	cmd_stop_find,	"Stop discovery"		},
 	{ "name",	cmd_name,	"Set local name"		},
 	{ "pair",	cmd_pair,	"Pair with a remote device"	},
 	{ "cancelpair",	cmd_cancel_pair,"Cancel pairing"		},
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools/btmgmt: Introduce stop-find command
  2015-03-12  2:11 [PATCH] tools/btmgmt: Introduce stop-find command Jakub Pawlowski
@ 2015-03-12  3:04 ` Arman Uguray
  2015-03-12  3:25   ` Jakub Pawlowski
  0 siblings, 1 reply; 3+ messages in thread
From: Arman Uguray @ 2015-03-12  3:04 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: BlueZ development

Hi Jakub,

> On Wed, Mar 11, 2015 at 7:11 PM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> This patch adds stop-find command that stops discovery started by
> find, or find-service command.
> ---
>  tools/btmgmt.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>
> diff --git a/tools/btmgmt.c b/tools/btmgmt.c
> index 8eff56b..89c2f43 100644
> --- a/tools/btmgmt.c
> +++ b/tools/btmgmt.c
> @@ -1897,6 +1897,84 @@ static void cmd_find(struct mgmt *mgmt, uint16_t index, int argc, char **argv)
>         }
>  }
>
> +static void stop_find_rsp(uint8_t status, uint16_t len, const void *param,
> +                                                       void *user_data)
> +{
> +       if (status != 0) {
> +               fprintf(stderr,
> +                       "Unable to stop discovery. status 0x%02x (%s)\n",
> +                                               status, mgmt_errstr(status));

You should stay consistent with other error messages, I would format
this as "Stop Discovery failed: status 0x%02x (%s)\n" (note the ':'
after "Stop Discovery failed").

> +               mainloop_quit();
> +               return;
> +       }
> +
> +       printf("Discovery stopped\n");
> +       discovery = false;
> +
> +       noninteractive_quit(EXIT_SUCCESS);
> +}
> +
> +static void stop_find_usage(void)
> +{
> +       printf("Usage: btmgmt stop-find [-l|-b]>\n");
> +}
> +
> +static struct option stop_find_options[] = {
> +       { "help",       0, 0, 'h' },
> +       { "le-only",    1, 0, 'l' },
> +       { "bredr-only", 1, 0, 'b' },
> +       { 0, 0, 0, 0 }
> +};
> +
> +static void cmd_stop_find(struct mgmt *mgmt, uint16_t index, int argc,
> +                         char **argv)
> +{
> +       struct mgmt_cp_stop_discovery cp;
> +       uint8_t type;
> +       int opt;
> +
> +       if (index == MGMT_INDEX_NONE)
> +               index = 0;
> +
> +       type = 0;
> +       type |= (1 << BDADDR_BREDR);
> +       type |= (1 << BDADDR_LE_PUBLIC);
> +       type |= (1 << BDADDR_LE_RANDOM);
> +
> +       while ((opt = getopt_long(argc, argv, "+lbh", stop_find_options,
> +                                                               NULL)) != -1) {

What happens if I pass "-b -l" or "-l -b"? The behavior is not very
well defined.

> +               switch (opt) {
> +               case 'l':
> +                       type &= ~(1 << BDADDR_BREDR);
> +                       type |= (1 << BDADDR_LE_PUBLIC);
> +                       type |= (1 << BDADDR_LE_RANDOM);

You've already or'd LE_PUBLIC and LE_RANDOM before entering the loop,
why do this again?

> +                       break;
> +               case 'b':
> +                       type |= (1 << BDADDR_BREDR);

Same, you've already or'd BREDR before the loop, so why is this line
needed? Also, if these shifted values are frequently used, can we just
define constants for them? I see that you mostly stayed consistent
with cmd_find and cmd_find_service, but perhaps the address type can
be calculated in a helper since this code is repeated.

> +                       type &= ~(1 << BDADDR_LE_PUBLIC);
> +                       type &= ~(1 << BDADDR_LE_RANDOM);
> +                       break;
> +               case 'h':
> +               default:
> +                       stop_find_usage();
> +                       exit(EXIT_SUCCESS);
> +               }
> +       }
> +
> +       argc -= optind;
> +       argv += optind;
> +       optind = 0;
> +
> +       memset(&cp, 0, sizeof(cp));
> +       cp.type = type;
> +
> +       if (mgmt_send(mgmt, MGMT_OP_STOP_DISCOVERY, index, sizeof(cp), &cp,
> +                                            stop_find_rsp, NULL, NULL) == 0) {
> +               fprintf(stderr, "Unable to send stop_discovery cmd\n");
> +               exit(EXIT_FAILURE);
> +       }
> +}
> +
>  static void name_rsp(uint8_t status, uint16_t len, const void *param,
>                                                         void *user_data)
>  {
> @@ -3268,6 +3346,7 @@ static struct cmd_info all_cmd[] = {
>         { "con",        cmd_con,        "List connections"              },
>         { "find",       cmd_find,       "Discover nearby devices"       },
>         { "find-service", cmd_find_service, "Discover nearby service"   },
> +       { "stop-find",  cmd_stop_find,  "Stop discovery"                },
>         { "name",       cmd_name,       "Set local name"                },
>         { "pair",       cmd_pair,       "Pair with a remote device"     },
>         { "cancelpair", cmd_cancel_pair,"Cancel pairing"                },
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,
Arman

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools/btmgmt: Introduce stop-find command
  2015-03-12  3:04 ` Arman Uguray
@ 2015-03-12  3:25   ` Jakub Pawlowski
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Pawlowski @ 2015-03-12  3:25 UTC (permalink / raw)
  To: Arman Uguray; +Cc: BlueZ development

On Wed, Mar 11, 2015 at 8:04 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Jakub,
>
>> On Wed, Mar 11, 2015 at 7:11 PM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>> This patch adds stop-find command that stops discovery started by
>> find, or find-service command.
>> ---
>>  tools/btmgmt.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>>
>> diff --git a/tools/btmgmt.c b/tools/btmgmt.c
>> index 8eff56b..89c2f43 100644
>> --- a/tools/btmgmt.c
>> +++ b/tools/btmgmt.c
>> @@ -1897,6 +1897,84 @@ static void cmd_find(struct mgmt *mgmt, uint16_t index, int argc, char **argv)
>>         }
>>  }
>>
>> +static void stop_find_rsp(uint8_t status, uint16_t len, const void *param,
>> +                                                       void *user_data)
>> +{
>> +       if (status != 0) {
>> +               fprintf(stderr,
>> +                       "Unable to stop discovery. status 0x%02x (%s)\n",
>> +                                               status, mgmt_errstr(status));
>
> You should stay consistent with other error messages, I would format
> this as "Stop Discovery failed: status 0x%02x (%s)\n" (note the ':'
> after "Stop Discovery failed").
>
ok, I'll fix that
>> +               mainloop_quit();
>> +               return;
>> +       }
>> +
>> +       printf("Discovery stopped\n");
>> +       discovery = false;
>> +
>> +       noninteractive_quit(EXIT_SUCCESS);
>> +}
>> +
>> +static void stop_find_usage(void)
>> +{
>> +       printf("Usage: btmgmt stop-find [-l|-b]>\n");
>> +}
>> +
>> +static struct option stop_find_options[] = {
>> +       { "help",       0, 0, 'h' },
>> +       { "le-only",    1, 0, 'l' },
>> +       { "bredr-only", 1, 0, 'b' },
>> +       { 0, 0, 0, 0 }
>> +};
>> +
>> +static void cmd_stop_find(struct mgmt *mgmt, uint16_t index, int argc,
>> +                         char **argv)
>> +{
>> +       struct mgmt_cp_stop_discovery cp;
>> +       uint8_t type;
>> +       int opt;
>> +
>> +       if (index == MGMT_INDEX_NONE)
>> +               index = 0;
>> +
>> +       type = 0;
>> +       type |= (1 << BDADDR_BREDR);
>> +       type |= (1 << BDADDR_LE_PUBLIC);
>> +       type |= (1 << BDADDR_LE_RANDOM);
>> +
>> +       while ((opt = getopt_long(argc, argv, "+lbh", stop_find_options,
>> +                                                               NULL)) != -1) {
>
> What happens if I pass "-b -l" or "-l -b"? The behavior is not very
> well defined.
>
>> +               switch (opt) {
>> +               case 'l':
>> +                       type &= ~(1 << BDADDR_BREDR);
>> +                       type |= (1 << BDADDR_LE_PUBLIC);
>> +                       type |= (1 << BDADDR_LE_RANDOM);
>
> You've already or'd LE_PUBLIC and LE_RANDOM before entering the loop,
> why do this again?
>
>> +                       break;
>> +               case 'b':
>> +                       type |= (1 << BDADDR_BREDR);
>
> Same, you've already or'd BREDR before the loop, so why is this line
> needed? Also, if these shifted values are frequently used, can we just
> define constants for them? I see that you mostly stayed consistent
> with cmd_find and cmd_find_service, but perhaps the address type can
> be calculated in a helper since this code is repeated.

Can I just stay consistent with cmd_find and cmd_find_service, and
I'll refactor that in next patch ?

>
>> +                       type &= ~(1 << BDADDR_LE_PUBLIC);
>> +                       type &= ~(1 << BDADDR_LE_RANDOM);
>> +                       break;
>> +               case 'h':
>> +               default:
>> +                       stop_find_usage();
>> +                       exit(EXIT_SUCCESS);
>> +               }
>> +       }
>> +
>> +       argc -= optind;
>> +       argv += optind;
>> +       optind = 0;
>> +
>> +       memset(&cp, 0, sizeof(cp));
>> +       cp.type = type;
>> +
>> +       if (mgmt_send(mgmt, MGMT_OP_STOP_DISCOVERY, index, sizeof(cp), &cp,
>> +                                            stop_find_rsp, NULL, NULL) == 0) {
>> +               fprintf(stderr, "Unable to send stop_discovery cmd\n");
>> +               exit(EXIT_FAILURE);
>> +       }
>> +}
>> +
>>  static void name_rsp(uint8_t status, uint16_t len, const void *param,
>>                                                         void *user_data)
>>  {
>> @@ -3268,6 +3346,7 @@ static struct cmd_info all_cmd[] = {
>>         { "con",        cmd_con,        "List connections"              },
>>         { "find",       cmd_find,       "Discover nearby devices"       },
>>         { "find-service", cmd_find_service, "Discover nearby service"   },
>> +       { "stop-find",  cmd_stop_find,  "Stop discovery"                },
>>         { "name",       cmd_name,       "Set local name"                },
>>         { "pair",       cmd_pair,       "Pair with a remote device"     },
>>         { "cancelpair", cmd_cancel_pair,"Cancel pairing"                },
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> Thanks,
> Arman

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-03-12  3:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-12  2:11 [PATCH] tools/btmgmt: Introduce stop-find command Jakub Pawlowski
2015-03-12  3:04 ` Arman Uguray
2015-03-12  3:25   ` Jakub Pawlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox