Re: [PATCH BlueZ v2 2/2] mesh: Changed --index option to --io

["Gix, Brian" <brian.gix@intel.com>]

Hi Michał,

This patch may need further discussion.  I understand what you are trying to do, and do not
disagree with it per se, but we are attempting to keep with precidence of bluetoothd and
the various tools and tests.  -i currently is used to specify the controller, so we may want a
different startup option to force the daemon to choose based on the supported controller
capabilities, but only if we are going through the MGMT list of controllers.

I think -i should basically be left the same (although the controller itself may need to be
queried before deciding what "controller type" it is. However, if MGMT is doing all the Cap
evaluation, it can do that by itself.  


On Wed, 2019-06-19 at 13:25 +0200, 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.
> ---
>  mesh/main.c | 73 ++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 56 insertions(+), 17 deletions(-)
> 
> diff --git a/mesh/main.c b/mesh/main.c
> index 262e3da48..233d95f30 100644
> --- a/mesh/main.c
> +++ b/mesh/main.c
> @@ -38,7 +38,7 @@
>  #include "mesh/mesh-io.h"
>  
>  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' },
> @@ -49,16 +49,22 @@ 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--io=<io>         Use specified io\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\n");
>  }
>  
>  static void do_debug(const char *str, void *user_data)
> @@ -107,6 +113,37 @@ 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;
> @@ -114,7 +151,8 @@ int main(int argc, char *argv[])
>  	bool dbus_debug = false;
>  	struct l_dbus *dbus = NULL;
>  	const char *config_dir = NULL;
> -	int index = MGMT_INDEX_NONE;
> +	enum mesh_io_type io_type = MESH_IO_TYPE_NONE;
> +	void *io_opts = NULL;
>  
>  	if (!l_main_init())
>  		return -1;
> @@ -123,7 +161,6 @@ int main(int argc, char *argv[])
>  
>  	for (;;) {
>  		int opt;
> -		const char *str;
>  
>  		opt = getopt_long(argc, argv, "i:c:ndbh", main_options, NULL);
>  		if (opt < 0)
> @@ -131,18 +168,11 @@ 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");
> +			if (!parse_io(optarg, &io_type, &io_opts)) {
> +				l_error("Invalid io: %s", optarg);
>  				status = EXIT_FAILURE;
>  				goto done;
>  			}
> -
> -			index = atoi(str);
> -
>  			break;
>  		case 'n':
>  			detached = false;
> @@ -167,8 +197,14 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	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 (!mesh_init(config_dir, MESH_IO_TYPE_GENERIC, &index)) {
> +	if (!mesh_init(config_dir, io_type, io_opts)) {
>  		l_error("Failed to initialize mesh");
>  		status = EXIT_FAILURE;
>  		goto done;
> @@ -198,6 +234,9 @@ int main(int argc, char *argv[])
>  	status = l_main_run_with_signal(signal_handler, NULL);
>  
>  done:
> +	if (io_opts)
> +		l_free(io_opts);
> +
>  	mesh_cleanup();
>  	l_dbus_destroy(dbus);
>  	l_main_exit();