Patch to set authentication data

[A. Valentin]

Hey Aleksander,

thanks for your advice. I'll change the code.
I also thought about using comma as separator, but I think the chance of
having a comma in a password is higher than a pipe symbol. What's your
opinion ?
To switch authentication method should be no problem. I hope there is
the possibility to set both like:
(pap|chap|both)

Thanks for your help!

André
Am 09.01.2013 10:22, schrieb Aleksander Morgado:
> Hey!
>
> Review below.
>
>> diff --git a/cli/qmicli-wds.c b/cli/qmicli-wds.c
>> index 775824b..2079338 100644
>> --- a/cli/qmicli-wds.c
>> +++ b/cli/qmicli-wds.c
>> @@ -57,8 +57,8 @@ static gboolean noop_flag;
>>  
>>  static GOptionEntry entries[] = {
>>      { "wds-start-network", 0, 0, G_OPTION_ARG_STRING, &start_network_str,
>> -      "Start network",
>> -      "[APN]"
>> +      "Start network (Username and Password are optional)",
>> +      "[APN|Username|Password]"
>
> Other commands with multiple parameters passed in the same string would
> use plain commas to separate the items, so better use commas, e.g.:
>   --dms-set-user-lock-code=[(old lock code),(new lock code)]
>
> The pipe/OR sign "|" (however is called) is used in the documentation of
> the commands to show a closed set of options to use; e.g.:
>   --dms-uim-verify-pin=[(PIN|PIN2),(current PIN)]
>
> Which means that you can pass either "PIN" or "PIN2"; e.g. "PIN|1234" or
> "PIN2,1234" for example.
>
>
>>      },
>>      { "wds-follow-network", 0, 0, G_OPTION_ARG_NONE, &follow_network_flag,
>>        "Follow the network status until disconnected. Use with `--wds-start-network'",
>> @@ -593,17 +593,32 @@ qmicli_wds_run (QmiDevice *device,
>>      /* Request to start network? */
>>      if (start_network_str) {
>>          QmiMessageWdsStartNetworkInput *input = NULL;
>> +    	gchar **split;
> Please don't use tabs; indent and align with spaces always.
>
>> +        QmiWdsAuthentication qmiwdsauth;
>>  
>>          /* Use the input string as APN */
>>          if (start_network_str[0]) {
>> +	    split = g_strsplit (start_network_str, "|", 0);
>>              input = qmi_message_wds_start_network_input_new ();
>> -            qmi_message_wds_start_network_input_set_apn (input, start_network_str, NULL);
>> +            qmi_message_wds_start_network_input_set_apn (input, split[0], NULL);
>> +	    if (split[1] && split[2]) {
>> +	            qmi_message_wds_start_network_input_set_username (input, split[1], NULL);
>> +		    qmiwdsauth = QMI_WDS_AUTHENTICATION_PAP;
>> +	            qmi_message_wds_start_network_input_set_authentication_preference (input, qmiwdsauth, NULL);
>> +	            qmi_message_wds_start_network_input_set_password (input, split[2], NULL);
>> +	    } else if (split[1]) {
> It would probably be cleaner to have:
>   if (split[1]) {
>        /* add split[1] */
>        if (split[2]) {
>            /* add split[2] */
>        }
>   }
>
>> +	            qmi_message_wds_start_network_input_set_username (input, split[1], NULL);
>> +		    qmiwdsauth = QMI_WDS_AUTHENTICATION_PAP;
>> +	            qmi_message_wds_start_network_input_set_authentication_preference (input, qmiwdsauth, NULL);
>> +	            qmi_message_wds_start_network_input_set_password (input, "", NULL);
>> +	    }
>> +	    g_strfreev (split);
>
> I'm missing the possibility to use CHAP authentication. Could you update
> the command so that we can also pass "chap" or "pap"? Something like:
> --wds-start-network=[(APN),(chap|pap),(user),(pass)]
>
>
>
>>          }
>>  
>>          g_debug ("Asynchronously starting network...");
>>          qmi_client_wds_start_network (ctx->client,
>>                                        input,
>> -                                      10,
>> +                                      45,
> Could you give the timeout update in a separate patch?
>
>>                                        ctx->cancellable,
>>                                        (GAsyncReadyCallback)start_network_ready,
>>                                        NULL);
>
> Cheers!
>