[Patch] add qmicli support to create/modify/delete PDP profiles

[Aleksander Morgado]

Hey,

On Thu, Jul 19, 2018 at 8:34 PM, Wolfgang Tolkien <w at tolkien.email> wrote:
> This adds qmicli support to create/modify/delete PDP profiles.
>  * This patch only supports setting APN/auth/etc. using the 'modify'
> command. Creating a new profile is therefore a 2-step process: create and
> then modify.
>  * Syntax is inconsistent: create/delete use old syntax since there are only
> one or two parameters, whereas the modify command uses key/value syntax.
> This mirrors other existing commands, but I'm happy to change it if people
> are unhappy with it.

But "create" allows all parameters that are available in "modify",
right? I think we should add those as well. Would you be up to doing
that?

I'm thinking that we could have --wds-create-profile allow key/value
pairs as well, but also predefined fields at the start of the string,
e.g. "3gpp" or "3gpp2". This could allow us to have for example
actions like this:
  --wds-create-profile="3gpp,apn=internet,pdp-type=ipv4"

Then, --wds-swi-create-profile would allow the same list of key/value
pairs, but also requiring 2 predefined fields at the start of the
string, "3gpp" or "3gpp2" plus the profile index, e.g.:
  --wds-swi-create-profile="3gpp,4,apn=internet,pdp-type=ipv4"

So --wds-modify-profile would follow the same logic of 2 predefined
fields with the exact same format as --wds-swi-create-profile, e.g.:
  --wds-modify-profile="3gpp,4,apn=internet,pdp-type=ipv4"

And the same for --wds-delete-profile:
  --wds-modify-profile="3gpp,4"

This would remove also the inconsistency between "3gpp-profile=#" and "3gpp,#"

>  * Modems seem to allow empty strings for APN/Name/Username/Password (which
> makes sense if one wants to delete the value entirely), however libqmi does
> not allow this. This patch does not address this. Suggested workaround is to
> delete the profile and recreate it.
>

Is this a limitation of the code generator? If so, maybe we should
have a "allow-empty" : "yes" property for this kind of cases?

Other comments:

I see when preparing the SWI Create Profile Indexed command that
you're setting the PDP context number to be equal to the WDS Profile
Index:

 +       qmi_message_wds_swi_create_profile_indexed_input_set_profile_identifier
(input, profile_type, (guint8)profile_index, NULL);
 +       qmi_message_wds_swi_create_profile_indexed_input_set_pdp_context_number
(input, (guint8)profile_index, NULL);

Shouldn't we allow specifying a different PDP context number
associated to the WDS profile index?

In, qmicli_read_pdp_type_from_string() I see:

+    else if (!str[0] || g_ascii_strcasecmp (str, "BOTH") == 0 ||
g_ascii_strcasecmp (str, "IPV4V6") == 0)
+        *out = QMI_WDS_PDP_TYPE_IPV4_OR_IPV6;

I don't think we should match the empty string or "both" to IPv4v6.
The user should set IPv4v6.

Cheers!

-- 
Aleksander
https://aleksander.es