[RFC PATCH] cli: avoid requiring specific modem if only a single modem is found in the system

[Dan Williams]

On Thu, 2015-04-30 at 17:36 +0200, Aleksander Morgado wrote:
> This change will allow running mmcli operations without needing to specify
> neither the index nor the path of the modem object, but only if a single modem
> is found in the system. E.g.:
> 
> Enabling the modem, when it is the only one in the system, would just be:
>    $ sudo mmcli --enable
>    successfully enabled the modem
> 
> But if multiple modems are found in the system:
>    $ sudo mmcli --enable
>    error: multiple modems found: need specific modem to run the operation
> 
> In particular, the special operation without actions that we had before to print
> the info of a given modem will now also be implicit even if no index or path is
> given:
> 
> So, when a single modem is found in the system:
>    $ sudo mmcli
>    /org/freedesktop/ModemManager1/Modem/1 (device id '51e23d10a4180fcf08a1a55e4c01dce693b17e66')
>      -------------------------
>      Hardware |   manufacturer: 'Sierra Wireless, Incorporated'
>               |          model: 'MC7304'
>               |       revision: 'SWI9X15C_05.05.16.02 r21040 carmd-fwbuild1 2014/03/17 23:49:48'
>               |      supported: 'gsm-umts
>    ...
> 
> But if multiple modems are found:
>    $ sudo mmcli
>    error: multiple modems found: need specific modem to run the operation
> ---
> 
> What does this look like?

I think it looks fine.  Patch looks OK to me too.

> I'm not totally sure about the special operation without explicit action requested, where we
> just showed the modem info. Should we still do that even if no modem index or path is given?

I think that's OK.  I went back and for about it for a few seconds, but
if we're defaulting to 'modem' operations when no group is specified, I
think it's OK to show modem info.

Dan

> ---
>  cli/mmcli-common.c | 43 ++++++++++++++++++++++++++++++++++---------
>  cli/mmcli-modem.c  |  2 +-
>  cli/mmcli.c        |  7 -------
>  3 files changed, 35 insertions(+), 17 deletions(-)
> 
> diff --git a/cli/mmcli-common.c b/cli/mmcli-common.c
> index 2d7f66e..e040a97 100644
> --- a/cli/mmcli-common.c
> +++ b/cli/mmcli-common.c
> @@ -117,18 +117,43 @@ find_modem (MMManager *manager,
>              const gchar *modem_path)
>  {
>      GList *modems;
> -    GList *l;
>      MMObject *found = NULL;
> 
>      modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (manager));
> -    for (l = modems; l; l = g_list_next (l)) {
> -        MMObject *modem = MM_OBJECT (l->data);
> 
> -        if (g_str_equal (mm_object_get_path (modem), modem_path)) {
> -            found = g_object_ref (modem);
> -            break;
> +    /* Single modem expected? */
> +    if (!modem_path) {
> +        guint n_modems;
> +
> +        n_modems = g_list_length (modems);
> +
> +        /* More than one modem found, error out */
> +        if (n_modems > 1) {
> +            g_printerr ("error: multiple modems found: need specific modem to run the operation\n");
> +            exit (EXIT_FAILURE);
> +        }
> +
> +        /* No modems found, error out */
> +        if (!n_modems) {
> +            g_printerr ("error: no modems found: need specific modem to run the operation\n");
> +            exit (EXIT_FAILURE);
> +        }
> +
> +        found = g_object_ref (MM_OBJECT (modems->data));
> +    } else {
> +        GList *l;
> +
> +        /* Iterate list of modems and try to patch one by path */
> +        for (l = modems; l; l = g_list_next (l)) {
> +            MMObject *modem = MM_OBJECT (l->data);
> +
> +            if (g_str_equal (mm_object_get_path (modem), modem_path)) {
> +                found = g_object_ref (modem);
> +                break;
> +            }
>          }
>      }
> +
>      g_list_free_full (modems, (GDestroyNotify) g_object_unref);
> 
>      if (!found) {
> @@ -136,7 +161,7 @@ find_modem (MMManager *manager,
>          exit (EXIT_FAILURE);
>      }
> 
> -    g_debug ("Modem found at '%s'\n", modem_path);
> +    g_debug ("Modem found at '%s'\n", mm_object_get_path (found));
> 
>      return found;
>  }
> @@ -211,8 +236,8 @@ get_modem_path (const gchar *path_or_index)
> 
>      /* We must have a given modem specified */
>      if (!path_or_index) {
> -        g_printerr ("error: no modem was specified\n");
> -        exit (EXIT_FAILURE);
> +        g_debug ("Assuming single modem...");
> +        return NULL;
>      }
> 
>      /* Modem path may come in two ways: full DBus path or just modem index.
> diff --git a/cli/mmcli-modem.c b/cli/mmcli-modem.c
> index f119f6e..a0a7448 100644
> --- a/cli/mmcli-modem.c
> +++ b/cli/mmcli-modem.c
> @@ -174,7 +174,7 @@ mmcli_modem_options_enabled (void)
>                   !!set_preferred_mode_str +
>                   !!set_current_bands_str);
> 
> -    if (n_actions == 0 && mmcli_get_common_modem_string ()) {
> +    if (n_actions == 0) {
>          /* default to info */
>          info_flag = TRUE;
>          n_actions++;
> diff --git a/cli/mmcli.c b/cli/mmcli.c
> index a4e6a4f..c1fc369 100644
> --- a/cli/mmcli.c
> +++ b/cli/mmcli.c
> @@ -247,13 +247,6 @@ main (gint argc, gchar **argv)
> 
>      /* Manager options? */
>      if (mmcli_manager_options_enabled ()) {
> -        /* Ensure options from different groups are not enabled */
> -        if (mmcli_modem_options_enabled ()) {
> -            g_printerr ("error: cannot use manager and modem options "
> -                        "at the same time\n");
> -            exit (EXIT_FAILURE);
> -        }
> -
>          if (async_flag)
>              mmcli_manager_run_asynchronous (connection, cancellable);
>          else
> --
> 2.3.6
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel