Issues with modem reset

[Piotr Figiel]

// sorry, re-sending to the group

Hi Aleksander,

2017-06-28 17:20 GMT+02:00 Aleksander Morgado <aleksander at aleksander.es>:
> The key point is the kind of error to consider the action fatal and
> abort right away. If the device is removed, we should definitely take
> it as a fatal error and abort any other action; for other kind of
> errors we may want to continue.

I limited the change to only MM_SERIAL_ERROR_SEND_FAILED which occurs
when the modem is gone. It seemed to be relevant for cases where the
AT port was functional but the PDP context deactivation failed with
error because it was deactivated for other reason than the
deactivation command (i.e. modem simply said ERROR to CGACT). It seems
without this constraint nothing would reopen the data tty after
hangup.

> This issue is probably solved best by the idea of having "device wide
> cancellations", were we would g_cancellable_cancel() a GCancellable
> object shared among all AT/QMI/MBIM command actions.

Yes though I think it would be needed in this case not only at AT
command level but also at bearer disconnect operation, as it looks
that the AT commands in this case failed correctly, yet bearer
disconnection logic tried to fix (reopen) the port after hangup while
it wasn't correct any more since the modem was gone in the mean time.
In any case I don't think my MM kung fu is strong enough to introduce
such wide change in reasonable time frame.
I attach two patches which I think we will intend to use as original
issue doesn't appear there and we didn't notice obvious regressions.
Maybe you could consider including them if you don't see risk of
regression on other supported modems.

Best regards, Piotr.

2017-06-28 17:20 GMT+02:00 Aleksander Morgado <aleksander at aleksander.es>:
> On Wed, Jun 28, 2017 at 4:42 PM, Piotr Figiel <figiel at gmail.com> wrote:
>>
>> diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c
>> index aeff2ef..db38970 100644
>> --- a/src/mm-broadband-bearer.c
>> +++ b/src/mm-broadband-bearer.c
>> @@ -1586,8 +1586,10 @@ cgact_ready (MMBaseModem *modem,
>>      if (!error)
>>          ctx->cgact_sent = TRUE;
>>      else {
>> -        mm_dbg ("PDP context deactivation failed (not fatal): %s", error->message);
>> -        g_error_free (error);
>> +        mm_dbg ("PDP context deactivation failed: %s", error->message);
>> +        g_simple_async_result_take_error (ctx->result, error);
>> +        detailed_disconnect_context_complete_and_free (ctx);
>> +        return;
>>      }
>>
>>      data_reopen_3gpp (ctx);
>
>
> The key point is the kind of error to consider the action fatal and
> abort right away. If the device is removed, we should definitely take
> it as a fatal error and abort any other action; for other kind of
> errors we may want to continue.
>
> This issue is probably solved best by the idea of having "device wide
> cancellations", were we would g_cancellable_cancel() a GCancellable
> object shared among all AT/QMI/MBIM command actions.
>
> --
> Aleksander
> https://aleksander.es
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-telit-increase-timeout-for-AT-REBOOT.patch
Type: text/x-patch
Size: 1076 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170629/ec035108/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-bearer-don-t-reopen-AT-commands-port-on-serial-failu.patch
Type: text/x-patch
Size: 1681 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170629/ec035108/attachment-0001.bin>