Parallel enables occurring (when SIM PIN enabled)

[Aleksander Morgado]

On Wed, Jul 19, 2017 at 2:22 PM, Colin Helliwell
<colin.helliwell at ln-systems.com> wrote:
>
>> On 19 July 2017 at 09:19 Aleksander Morgado <aleksander at aleksander.es> wrote:
>>
>> On Wed, Jul 19, 2017 at 8:42 AM, Colin Helliwell
>>
>> <colin.helliwell at ln-systems.com> wrote:
>>
>> > But I wondered whether anything had been re-structured elsewhere which ought to address the problem anyway?
>>
>> Don't think anything has been done in that regard, no.
>>
>
> Ok I think I've modified my patch for the GTask mods, but would appreciate it if someone could take a look and see if I've got all the flow and tidy-up correct.
>
> Patch attached, but patched source maybe easier to digest:
>
> static void
> enabling_wait_for_final_state_ready2 (MMIfaceModem *self,
>                                      GAsyncResult *res,
>                                      GTask *task)
> {
>     EnablingContext *ctx;
>     GError *error = NULL;
>     MMModemState state = MM_MODEM_STATE_UNKNOWN;
>
>     ctx = g_task_get_task_data (task);
>
>     ctx->previous_state = mm_iface_modem_wait_for_final_state_finish (self, res, &error);
>     if (error) {
>         g_task_return_error (task, error);
>         g_object_unref (task);
>         return;
>     }
>
>     g_object_get (self,
>                   MM_IFACE_MODEM_STATE, &state,
>                   NULL);
>
>     if (state < MM_MODEM_STATE_ENABLED) {
>         g_task_return_error (task,
>                              g_error_new_literal (MM_CORE_ERROR,
>                                                   MM_CORE_ERROR_FAILED,
>                                                   "waited longer, still not enabled"));
>         g_object_unref (task);
>         return;
>     }
>
>     /* We're enabled now, go on */
>     g_task_return_boolean (task, TRUE);
>     g_object_unref (task);
>     return;
> }
>
> static void
> enabling_wait_for_final_state_ready (MMIfaceModem *self,
>                                      GAsyncResult *res,
>                                      GTask *task)
> {
>     EnablingContext *ctx;
>     GError *error = NULL;
>     MMModemState state = MM_MODEM_STATE_UNKNOWN;
>
>     ctx = g_task_get_task_data (task);
>
>     ctx->previous_state = mm_iface_modem_wait_for_final_state_finish (self, res, &error);
>     if (error) {
>         g_task_return_error (task, error);
>         g_object_unref (task);
>         return;
>     }
>
>     g_object_get (self,
>                   MM_IFACE_MODEM_STATE, &state,
>                   NULL);
>     if (state == MM_MODEM_STATE_ENABLING) {
>         /*g_message ("######## parallel enables!");*/
>
>         mm_iface_modem_wait_for_final_state (MM_IFACE_MODEM (ctx->self),
>                                              MM_MODEM_STATE_UNKNOWN, /* just any */
>                                              (GAsyncReadyCallback)enabling_wait_for_final_state_ready2,
>                                              task);
>         return;
>     }
>
>     /* We're in a final state now, go on */
>
>     mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self),
>                                  MM_MODEM_STATE_ENABLING,
>                                  MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED);
>
>     ctx->step++;
>     enabling_step (task);
> }

I believed this is a bit too complex... you shouldn't need to run the
first "wait_for_final_state" to look for "any" final state, and then
retry if "enabling" already. Does that ever happen actually?

How about if, instead of waiting for "any" final state, we wait for
"enabled" if we're already in enabling state? E.g.

case ENABLING_STEP_WAIT_FOR_FINAL_STATE:
    g_object_get (self, MM_IFACE_MODEM_STATE, &state, NULL);
    mm_iface_modem_wait_for_final_state (MM_IFACE_MODEM (ctx->self),
                                              state ==
MM_MODEM_STATE_ENABLING ? MM_MODEM_STATE_ENABLED :
MM_MODEM_STATE_UNKNOWN,

(GAsyncReadyCallback)enabling_wait_for_final_state_ready,
                                              task);

(or something along that idea)

-- 
Aleksander