[PATCH RESEND BlueZ 0/1] Select mesh-io from command line

[PATCH RESEND BlueZ 0/1] Select mesh-io from command line

[Michał Lowas-Rzechonek]

Continuing changes started in 12b984d1d4d47a8fd5bc8455d586bb208b804ebf,
this patch allows us to select mesh-io type using command line
option.

By default, daemon still uses mesh-io-generic, and currently there are
no other implementations available, but this patch enables us to
register more io variants.

Michał Lowas-Rzechonek (1):
  mesh: Add --io option

 mesh/main.c | 99 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 81 insertions(+), 18 deletions(-)

-- 
2.20.1

[PATCH RESEND BlueZ 1/1] mesh: Add --io option

[Michał Lowas-Rzechonek]

This allows specifying io type and options when invoking the daemon.

By default, meshd runs with MESH_IO_TYPE_GENERIC and tries to attach to
the first available HCI interface.

Option "--index=<n>" is now just a shortcut for "--io=generic:<n>"
---
 mesh/main.c | 99 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 81 insertions(+), 18 deletions(-)

diff --git a/mesh/main.c b/mesh/main.c
index 010edcf85..243e756f9 100644
--- a/mesh/main.c
+++ b/mesh/main.c
@@ -40,10 +40,12 @@
 
 static const char *config_dir;
 static const char *mesh_conf_fname;
-static int ctlr_index = MGMT_INDEX_NONE;
+static enum mesh_io_type io_type;
+static void *io_opts;
 
 static const struct option main_options[] = {
 	{ "index",	required_argument,	NULL, 'i' },
+	{ "io",		required_argument,	NULL, 'I' },
 	{ "config",	optional_argument,	NULL, 'c' },
 	{ "nodetach",	no_argument,		NULL, 'n' },
 	{ "debug",	no_argument,		NULL, 'd' },
@@ -54,16 +56,23 @@ static const struct option main_options[] = {
 
 static void usage(void)
 {
-	l_info("");
-	l_info("Usage:\n"
+	fprintf(stderr,
+		"Usage:\n"
 	       "\tbluetooth-meshd [options]\n");
-	l_info("Options:\n"
-	       "\t--index <hcinum>  Use specified controller\n"
+	fprintf(stderr,
+		"Options:\n"
+	       "\t--index <hcinum>  Equivalent of `--io=generic:<hcinum>`\n"
+	       "\t--io <io>         Use specified io (default: generic)\n"
 	       "\t--config          Configuration directory\n"
 	       "\t--nodetach        Run in foreground\n"
 	       "\t--debug           Enable debug output\n"
 	       "\t--dbus-debug      Enable D-Bus debugging\n"
 	       "\t--help            Show %s information\n", __func__);
+	fprintf(stderr,
+	       "io:\n"
+	       "\tgeneric[:<index>]\n"
+	       "\t\tUse generic HCI io on interface hci<index>, or the first\n"
+	       "\t\tavailable one\n");
 }
 
 static void do_debug(const char *str, void *user_data)
@@ -100,8 +109,8 @@ static void request_name_callback(struct l_dbus *dbus, bool success,
 		return;
 	}
 
-	if (!mesh_init(config_dir, mesh_conf_fname, MESH_IO_TYPE_GENERIC,
-				&ctlr_index, mesh_ready_callback, dbus)) {
+	if (!mesh_init(config_dir, mesh_conf_fname, io_type, io_opts,
+					mesh_ready_callback, dbus)) {
 		l_error("Failed to initialize mesh");
 		l_main_quit();
 	}
@@ -133,12 +142,44 @@ static void signal_handler(uint32_t signo, void *user_data)
 	terminated = true;
 }
 
+static bool parse_io(const char *optarg, enum mesh_io_type *type, void **opts)
+{
+	if (strstr(optarg, "generic") == optarg) {
+		int *index = l_new(int, 1);
+
+		*type = MESH_IO_TYPE_GENERIC;
+		*opts = index;
+
+		optarg += strlen("generic");
+		if (!*optarg) {
+			*index = MGMT_INDEX_NONE;
+			return true;
+		}
+
+		if (*optarg != ':')
+			return false;
+
+		optarg++;
+
+		if (sscanf(optarg, "hci%d", index) == 1)
+			return true;
+
+		if (sscanf(optarg, "%d", index) == 1)
+			return true;
+
+		return false;
+	}
+
+	return false;
+}
+
 int main(int argc, char *argv[])
 {
 	int status;
 	bool detached = true;
 	bool dbus_debug = false;
 	struct l_dbus *dbus = NULL;
+	char *io = NULL;
 
 	if (!l_main_init())
 		return -1;
@@ -153,7 +194,6 @@ int main(int argc, char *argv[])
 
 	for (;;) {
 		int opt;
-		const char *str;
 
 		opt = getopt_long(argc, argv, "i:c:f:ndbh", main_options, NULL);
 		if (opt < 0)
@@ -161,18 +201,20 @@ int main(int argc, char *argv[])
 
 		switch (opt) {
 		case 'i':
-			if (strlen(optarg) > 3 && !strncmp(optarg, "hci", 3))
-				str = optarg + 3;
-			else
-				str = optarg;
-			if (!isdigit(*str)) {
-				l_error("Invalid controller index value");
-				status = EXIT_FAILURE;
+			if (io) {
+				l_error("Use either --index or --io, not both");
+				status = EXIT_SUCCESS;
 				goto done;
 			}
-
-			ctlr_index = atoi(str);
-
+			io = l_strdup_printf("generic:%s", optarg);
+			break;
+		case 'I':
+			if (io) {
+				l_error("Use either --index or --io, not both");
+				status = EXIT_SUCCESS;
+				goto done;
+			}
+			io = l_strdup(optarg);
 			break;
 		case 'n':
 			detached = false;
@@ -200,6 +242,21 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (!io)
+		io = l_strdup_printf("generic");
+
+	if (!parse_io(io, &io_type, &io_opts)) {
+		l_error("Invalid io: %s", io);
+		status = EXIT_FAILURE;
+		goto done;
+	}
+
+	if ((io_type == MESH_IO_TYPE_NONE) && !io_opts) {
+		int *index = l_new(int, 1);
+		*index = MGMT_INDEX_NONE;
+		io_type = MESH_IO_TYPE_GENERIC;
+		io_opts = index;
+	}
 
 	if (!detached)
 		umask(0077);
@@ -225,6 +282,12 @@ int main(int argc, char *argv[])
 	status = l_main_run_with_signal(signal_handler, NULL);
 
 done:
+	if (io)
+		l_free(io);
+
+	if (io_opts)
+		l_free(io_opts);
+
 	mesh_cleanup();
 	l_dbus_destroy(dbus);
 	l_main_exit();
-- 
2.20.1

Re: [PATCH RESEND BlueZ 1/1] mesh: Add --io option

[Stotland, Inga]

Hi Michal,

On Mon, 2020-01-13 at 13:27 +0100, Michał Lowas-Rzechonek wrote:
> This allows specifying io type and options when invoking the daemon.
> 
> By default, meshd runs with MESH_IO_TYPE_GENERIC and tries to attach to
> the first available HCI interface.
> 
> Option "--index=<n>" is now just a shortcut for "--io=generic:<n>"
> ---
>  mesh/main.c | 99 +++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 81 insertions(+), 18 deletions(-)
> 
> diff --git a/mesh/main.c b/mesh/main.c
> index 010edcf85..243e756f9 100644
> --- a/mesh/main.c
> +++ b/mesh/main.c
> @@ -40,10 +40,12 @@
>  
>  static const char *config_dir;
>  static const char *mesh_conf_fname;
> -static int ctlr_index = MGMT_INDEX_NONE;
> +static enum mesh_io_type io_type;
> +static void *io_opts;
>  
>  static const struct option main_options[] = {
>  	{ "index",	required_argument,	NULL, 'i' },
> +	{ "io",		required_argument,	NULL, 'I' },
>  	{ "config",	optional_argument,	NULL, 'c' },
>  	{ "nodetach",	no_argument,		NULL, 'n' },
>  	{ "debug",	no_argument,		NULL, 'd' },
> @@ -54,16 +56,23 @@ static const struct option main_options[] = {
>  
>  static void usage(void)
>  {
> -	l_info("");
> -	l_info("Usage:\n"
> +	fprintf(stderr,
> +		"Usage:\n"
>  	       "\tbluetooth-meshd [options]\n");
> -	l_info("Options:\n"
> -	       "\t--index <hcinum>  Use specified controller\n"
> +	fprintf(stderr,
> +		"Options:\n"
> +	       "\t--index <hcinum>  Equivalent of `--io=generic:<hcinum>`\n"
> +	       "\t--io <io>         Use specified io (default: generic)\n"
>  	       "\t--config          Configuration directory\n"
>  	       "\t--nodetach        Run in foreground\n"
>  	       "\t--debug           Enable debug output\n"
>  	       "\t--dbus-debug      Enable D-Bus debugging\n"
>  	       "\t--help            Show %s information\n", __func__);
> +	fprintf(stderr,
> +	       "io:\n"
> +	       "\tgeneric[:<index>]\n"
> +	       "\t\tUse generic HCI io on interface hci<index>, or the first\n"
> +	       "\t\tavailable one\n");
>  }
>  
>  static void do_debug(const char *str, void *user_data)
> @@ -100,8 +109,8 @@ static void request_name_callback(struct l_dbus *dbus, bool success,
>  		return;
>  	}
>  
> -	if (!mesh_init(config_dir, mesh_conf_fname, MESH_IO_TYPE_GENERIC,
> -				&ctlr_index, mesh_ready_callback, dbus)) {
> +	if (!mesh_init(config_dir, mesh_conf_fname, io_type, io_opts,
> +					mesh_ready_callback, dbus)) {
>  		l_error("Failed to initialize mesh");
>  		l_main_quit();
>  	}
> @@ -133,12 +142,44 @@ static void signal_handler(uint32_t signo, void *user_data)
>  	terminated = true;
>  }
>  
> +static bool parse_io(const char *optarg, enum mesh_io_type *type, void **opts)
> +{
> +	if (strstr(optarg, "generic") == optarg) {
> +		int *index = l_new(int, 1);
> +
> +		*type = MESH_IO_TYPE_GENERIC;
> +		*opts = index;
> +
> +		optarg += strlen("generic");
> +		if (!*optarg) {
> +			*index = MGMT_INDEX_NONE;
> +			return true;
> +		}
> +
> +		if (*optarg != ':')
> +			return false;
> +
> +		optarg++;
> +
> +		if (sscanf(optarg, "hci%d", index) == 1)
> +			return true;
> +
> +		if (sscanf(optarg, "%d", index) == 1)
> +			return true;
> +
> +		return false;
> +	}
> +
> +	return false;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	int status;
>  	bool detached = true;
>  	bool dbus_debug = false;
>  	struct l_dbus *dbus = NULL;
> +	char *io = NULL;
>  
>  	if (!l_main_init())
>  		return -1;
> @@ -153,7 +194,6 @@ int main(int argc, char *argv[])
>  
>  	for (;;) {
>  		int opt;
> -		const char *str;
>  
>  		opt = getopt_long(argc, argv, "i:c:f:ndbh", main_options, NULL);
>  		if (opt < 0)
> @@ -161,18 +201,20 @@ int main(int argc, char *argv[])
>  
>  		switch (opt) {
>  		case 'i':
> -			if (strlen(optarg) > 3 && !strncmp(optarg, "hci", 3))
> -				str = optarg + 3;
> -			else
> -				str = optarg;
> -			if (!isdigit(*str)) {
> -				l_error("Invalid controller index value");
> -				status = EXIT_FAILURE;
> +			if (io) {
> +				l_error("Use either --index or --io, not both");
> +				status = EXIT_SUCCESS;
>  				goto done;
>  			}
> -
> -			ctlr_index = atoi(str);
> -
> +			io = l_strdup_printf("generic:%s", optarg);
> +			break;
> +		case 'I':
> +			if (io) {
> +				l_error("Use either --index or --io, not both");
> +				status = EXIT_SUCCESS;
> +				goto done;
> +			}
> +			io = l_strdup(optarg);
>  			break;
>  		case 'n':
>  			detached = false;
> @@ -200,6 +242,21 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	if (!io)
> +		io = l_strdup_printf("generic");
> +
> +	if (!parse_io(io, &io_type, &io_opts)) {
> +		l_error("Invalid io: %s", io);
> +		status = EXIT_FAILURE;
> +		goto done;
> +	}
> +
> +	if ((io_type == MESH_IO_TYPE_NONE) && !io_opts) {
> +		int *index = l_new(int, 1);
> +		*index = MGMT_INDEX_NONE;
> +		io_type = MESH_IO_TYPE_GENERIC;
> +		io_opts = index;
> +	}
>  
>  	if (!detached)
>  		umask(0077);
> @@ -225,6 +282,12 @@ int main(int argc, char *argv[])
>  	status = l_main_run_with_signal(signal_handler, NULL);
>  
>  done:
> +	if (io)
> +		l_free(io);
> +
> +	if (io_opts)
> +		l_free(io_opts);
> +
>  	mesh_cleanup();
>  	l_dbus_destroy(dbus);
>  	l_main_exit();


I wonder if it would be better to re-use "-i" option by changing it's
meaning form "index" to "i/o".

So that " -i hci<#>" will map to generic i/o on a specified controller
and no "-i" option means any controller.
 
Yes, we will loose some uniformity across all of the bluez in a sense
that "-i <#>" won't work, but imo it's preferable to having two options
with inter-dependecies.

Best regards,

Inga


 

Re: [PATCH RESEND BlueZ 1/1] mesh: Add --io option

[michal.lowas-rzechonek]

Hi Inga,

On 01/14, Stotland, Inga wrote:
> I wonder if it would be better to re-use "-i" option by changing it's
> meaning form "index" to "i/o".
> 
> So that " -i hci<#>" will map to generic i/o on a specified controller
> and no "-i" option means any controller.
>  
> Yes, we will loose some uniformity across all of the bluez in a sense
> that "-i <#>" won't work, but imo it's preferable to having two options
> with inter-dependecies.

Hm, might be... The reason I added the "--io=<type>:<options>" was the
"<options>" part.

For example, we have a non-HCI radio adapter that uses
"--io=uart:/dev/tty<n>" syntax, or remote radio using
"--io=tcp:<host>:<port>" syntax.

So maybe I could parse  "-i <n>" and  "-i hci<n>" (where <n> is a
number) as a shortcut for "-i generic:hci<n>"?

That would allow us to merge both options and rename "--index" to
"--io".

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [PATCH RESEND BlueZ 1/1] mesh: Add --io option

[Stotland, Inga]

Hi Michal,

On Tue, 2020-01-14 at 21:09 +0100, michal.lowas-rzechonek@silvair.com
wrote:
> Hi Inga,
> 
> On 01/14, Stotland, Inga wrote:
> > I wonder if it would be better to re-use "-i" option by changing it's
> > meaning form "index" to "i/o".
> > 
> > So that " -i hci<#>" will map to generic i/o on a specified controller
> > and no "-i" option means any controller.
> >  
> > Yes, we will loose some uniformity across all of the bluez in a sense
> > that "-i <#>" won't work, but imo it's preferable to having two options
> > with inter-dependecies.
> 
> Hm, might be... The reason I added the "--io=<type>:<options>" was the
> "<options>" part.
> 
> For example, we have a non-HCI radio adapter that uses
> "--io=uart:/dev/tty<n>" syntax, or remote radio using
> "--io=tcp:<host>:<port>" syntax.
> 
> So maybe I could parse  "-i <n>" and  "-i hci<n>" (where <n> is a
> number) as a shortcut for "-i generic:hci<n>"?
> 
> That would allow us to merge both options and rename "--index" to
> "--io".
> 

Yes, I'd be fine with this kind of change. Depending on the type,
we would know how to parse the options. With the fallback to legacy
cases:
	* no option given: generic, any controller
	* "-i <hci#>" or -i <#> : generic with the specified controller

Thank you,

Inga