Re: [RFC] gatchat: implement timeout setting for commands

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?= <martin@geanix.com>]

[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]

Hi Denis,

On 02/09/2019 23.25, Denis Kenzior wrote:
> Hi Martin,
> 
> On 9/2/19 4:10 PM, Martin Hundebøll wrote:
>> The adds a function to the set a chat-wide timeout for at commands. It
>> allows plugins to handle cases where the modem fails to respond with
>> either an OK or an error.
> 
> Okay, this is sort of a known problem without a really good solution...
> 
>>
>> Without these timeouts, the plugin is never notified about a hanging
>> command, and it has no way to detect it; effectively leaving the device
>> in a broken state, where only a full restart of the ofono service
>> unbreaks it.
> 
> The trouble is, what if this happens on a command handled by a generic 
> atmodem driver atom?  Are you going in and adding timeout handling to 
> all command callbacks?

My initial thought was that calling the command callback with ok=false 
would be handled already. But that might be too optimistic?

> If this timeout only happens in special cases, then those can be taken 
> care of by setting up your own timeout...

Our problem is that this was seen during SIM initialization when doing 
so too early on the quectel modem (worked around in my latest quectel 
sim patches.) So it needs to be chat-wide.

>>
>> If a timeout occurs, the passed callback is invoked with 'ok' set to
>> false, and no final response in the result. This allows callbacks to
>> identify the timeout with this snippet:
>>
>>    if (!ok) {
>>      decode_at_error(&error, g_at_result_final_response(result));
>>      if (error.type == OFONO_ERROR_TYPE_FAILURE) {
>>        /* handle timeout */
>>      }
>>    }
> 
> About the only thing I can suggest is to create a separate timeout 
> callback for the GAtChat object that the modem driver can set.  That way 
> if the timeout happens, the modem driver can just reset the entire 
> setup.  E.g. maybe via ofono_modem_reset.

That could work too.

I have one concern with my current patches though:
The timeout needs to accommodate long-lasting commands like AT+COPS, 
which is can take up to 80 seconds or so. But that is much longer than 
the timeout for powering up the modem.

So if:
   1: user enables modem
   2: gatchat gets stuck with no command
   3: set_powered_timeout() triggers
   4: gatchat timeout fires

What happens is the user tries to enable the modem again after step 3? 
Should the plugin enable function clean up after potential timeouts to 
avoid step 4?

// Martin