SIM power patch

[Kasper Holtze]

Hi Aleksander,


On 12/07/2016 07:34 PM, Aleksander Morgado wrote:
> Hey Kasper,
>
>> I created a patch for libqmi/qmicli, for powering up/down the SIM card:
>>
>> --uim-sim-power=[(ON|OFF),(slot number)]
>> Power on/off SIM card
>>
>> Slot number is 1-based index of the SIM slot in the modem - usually 1 for
>> modems with 1 SIM interface.
>>
>> We are using this function to switch SIM card without resetting the modem.
>>
>> I have not commited any patches for libqmi before, so I'm not sure how to do
>> it..? Also, as this is my first patch for this project, please let me know
>> if anything needs to be changed.
>>
>> Tested with Sierra MC7304.
>>
>> The patch is attached to this email.
>>
> Superb, thanks!
>
> See some comments inline below. Patch looks mostly good, but there are
> several coding style issues to fix, plus some memleaks (we try to keep
> the memleak report clean even in qmicli, so that valgrind reports of
> qmicli help us find real leaks in libqmi-glib).
Thank you very much - I appreciate your help! Sorry about all the tabs - 
I changed my editor to use space, but obviously, it didn't comply all 
the way.
A few comments below.
>> +     "type"    : "Message",
>> +     "service" : "UIM",
>> +     "id"      : "0x0031",
>> +     "version" : "1.0",
>> +     "input"   : [ { "name"      : "Slot",
>> +                     "id"        : "0x01",
>> +                     "mandatory" : "yes",
>> +                     "type"      : "TLV",
>> +                     "format"    : "sequence",
>> +                     "contents"  : [ { "name"          : "Slot",
>> +                                       "format"        : "guint8" } ] }
>> +                  ],
>> +     "output"  : [ { "common-ref" : "Operation Result" } ] }
>>
>>   ]
>> diff --git a/src/qmicli/qmicli-uim.c b/src/qmicli/qmicli-uim.c
>> index 453b46d..7df1556 100644
>> --- a/src/qmicli/qmicli-uim.c
>> +++ b/src/qmicli/qmicli-uim.c
>> @@ -48,6 +48,7 @@ static gchar *verify_pin_str;
>>   static gchar *unblock_pin_str;
>>   static gchar *change_pin_str;
>>   static gchar *get_file_attributes_str;
>> +static gchar *sim_power_str;
>>   static gboolean get_card_status_flag;
>>   static gboolean get_supported_messages_flag;
>>   static gboolean reset_flag;
>> @@ -86,6 +87,10 @@ static GOptionEntry entries[] = {
>>         "Get supported messages",
>>         NULL
>>       },
>> + { "uim-sim-power", 0, 0, G_OPTION_ARG_STRING, &sim_power_str,
>> + "Power on/off SIM card",
>> + "[(ON|OFF),(slot number)]"
> (on|off)
See below
> +static QmiMessageUimPowerOnSimInput *
> +power_on_sim_input_create (guint slot)
> "guint8 slot" directly?
Yes - I need to create a helper function to parse the input as guint8, 
but I can do that?
>> +
>> +        if (g_str_equal (power, "ON")) {
> Instead of g_str_equal() use !g_ascii_strcasecmp() and then you'll
> allow both ON and on.
>
> Declare "on" input here, and don't use camelCasedNames:
> QmiMessageUimPowerOnSimInput *input;
> Instead of g_str_equal() use !g_ascii_strcasecmp() and then you'll
> allow both OFF and off.
The reason I used upper case ON/OFF, was to follow the convention in the 
PIN functions (--dms-uim-verify-pin=PIN,0000 for instance), but I can 
make it lower case and accept both.

Kasper