[PATCH 2/2] qmicli: report which new images the firmware expects to download

[Aleksander Morgado]

On Mon, Aug 29, 2016 at 12:18 PM, Bjørn Mork <bjorn at mork.no> wrote:
> Aleksander Morgado <aleksander at aleksander.es> writes:
>
>> On 19/08/16 16:29, Bjørn Mork wrote:
>>> Signed-off-by: Bjørn Mork <bjorn at mork.no>
>>> ---
>>>  src/qmicli/qmicli-dms.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/src/qmicli/qmicli-dms.c b/src/qmicli/qmicli-dms.c
>>> index 485b73400af1..5ff7dddccc9c 100644
>>> --- a/src/qmicli/qmicli-dms.c
>>> +++ b/src/qmicli/qmicli-dms.c
>>> @@ -2799,6 +2799,7 @@ select_stored_image_ready (QmiClientDms *client,
>>>  {
>>>      QmiMessageDmsSetFirmwarePreferenceOutput *output;
>>>      GError *error = NULL;
>>> +    GArray *array;
>>>
>>>      output = qmi_client_dms_set_firmware_preference_finish (client, res, &error);
>>>      if (!output) {
>>> @@ -2829,6 +2830,25 @@ select_stored_image_ready (QmiClientDms *client,
>>>               "\n",
>>>               qmi_device_get_path_display (ctx->device));
>>>
>>> +    /* do we need to download a new modem and/or pri image? */
>>> +    if (qmi_message_dms_set_firmware_preference_output_get_image_download_list (output, &array, &error)) {
>>> +        guint i;
>>> +        GString *images;
>>> +        QmiDmsFirmwareImageType *type;
>>> +
>>> +        images = g_string_new ("");
>>> +        for (i = 0; i < array->len; i++) {
>>> +            type = &g_array_index (array, QmiDmsFirmwareImageType, i);
>>> +            g_string_append (images, qmi_dms_firmware_image_type_get_string (*type));
>>> +            if (i < array->len -1)
>>> +                g_string_append (images, ", ");
>>> +        }
>>
>> Really minor thing, but being the GArray an array of
>> QmiDmsFirmwareImageType enum values, I would instead do:
>>
>> type = g_array_index (array, QmiDmsFirmwareImageType, i);
>> g_string_append (images, qmi_dms_firmware_image_type_get_string (type));
>>
>> (i.e., use a QmiDmsFirmwareImageType directly instead of a pointer to a
>> QmiDmsFirmwareImageType).
>
> Agreed. Much better to avoid the pointer indirection here.  You can tell
> that I don't know how to write g_ prefixed code, and just copy and paste
> until it "works" :)
>

Pushed your patch and a follow up fixing this.

Cheers!

-- 
Aleksander
https://aleksander.es