Bug 96855

Hey Aliaksandr,

I've pushed your patch, plus some additional follow up commits fixing some issues afterwards, to the "pdc" branch in the upstream git repository. Could you validate that the additional fixes don't break the logic you're using?

Created attachment 127257 [details] [review]
Fix situation if no PDC config loaded

The changed version works good, and I have few small additions to the code.

1. I found situation where original code haven't worked as expected:
If modem have no uploaded configs (e.g. just after boot), it doesn't send indication with config list, so code hung up.

2. I've returned printable_id function. The return format of the function is the same to expected format for pdc-activate-config/pdc-deactivate-config/pdc-delete-config, so user can just copy/paste the config id.

Attached patch fixes both things.

Created attachment 127262 [details] [review]
Fix situation if no PDC config loaded

Removed debug from patch

Regarding your last patch:

The timeout logic looked mostly good; I've pushed to the "pdc" branch a custom version of the logic, where the timeout is removed once we get the indication (so that the timeout doesn't break the logic if we're listing the configurations one by one), and also using g_timeout_add_seconds() instead of g_timeout_add().

I don't agree about getting back printable_id(). I think we should use qmicli_get_raw_data_printable() because we already have it. Note that I've pushed a new commit to fix how the ID was being printed, please retry with this latest fix installed.

Your patch was also doing this logic change; which I haven't included in the branch because I don't think it makes sense. The number of configs loaded should be equal to the number of configs available, the extra check for ctx->configs_loaded doesn't make much sense I think, or does it? Please elaborate on why that was needed.
@@ -276,7 +298,7 @@ static void
 check_list_config_completed (void)
 {
     if (ctx->configs_loaded == ctx->config_list->len &&
-        ctx->ids_loaded) {
+        (ctx->configs_loaded == 0 || ctx->ids_loaded)) {
         print_configs (ctx->config_list);
         operation_shutdown (TRUE);
     }

Created attachment 127544 [details] [review]
Minor fixes

I've changed logic a bit to support situation when there is no config activated.
In  this case indication generated with error NOT_PROVISIONED. I've added handling for the case.

Also I've reduced verbosity for debug messages and made small fix in config printing.

Created attachment 127545 [details] [review]
Minor fixes

Included your latest changes and merged to git master already, thanks!

I recently updated my Gnome 3.22 jhbuild checkout and ModemManager fails to build:

In file included from /home/spasche/jhbuild_322/checkout/ModemManager/src/mm-modem-helpers-qmi.h:22:0,
                 from /home/spasche/jhbuild_322/checkout/ModemManager/src/mm-modem-helpers-qmi.c:16:
/home/spasche/jhbuild_322/install/include/libqmi-glib/libqmi-glib.h:58:27: fatal error: qmi-enums-pdc.h: No such file or directory
 #include "qmi-enums-pdc.h"
                         

It seems that this PDC change is not installing all headers?