2020-01-13 12:28:08

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH RESEND BlueZ 0/1] Select mesh-io from command line

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


2020-01-13 12:28:08

by Michał Lowas-Rzechonek

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

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

2020-01-14 19:39:27

by Stotland, Inga

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

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



2020-01-14 20:10:32

by Michał Lowas-Rzechonek

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

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 <[email protected]>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

2020-01-14 22:26:23

by Stotland, Inga

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

Hi Michal,

On Tue, 2020-01-14 at 21:09 +0100, [email protected]
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