Making disconnection more robust

[Aleksander Morgado]

Hey,

>
> As previously discussed, the disconnection code currently does no
> checking to verify that the modem has come back into command mode (it
> just drops DTR for a second, and hopes for the best) i.e.
>
> [Drop DTR]
> [end disconnect sequence, assuming modem is back in command mode]
>
>
>
>
> I'd like to change this so that the command flow is:
>
> [Drop DTR]
> [send "AT<CR>"]
> [proceed if the modem replied with "OK"]
>
> ... if "OK" isn't received within the timeout, then:
>
> [report modem still not in command mode]
> ["+++"]
> ["wait for end of escape guard interval"]
> [end disconnect sequence if the modem replied with "OK", otherwise...]
> [send "AT<CR>"]
> [end disconnect sequence if the modem replied with "OK", otherwise...]
>
> ... if "OK" isn't received within the timeout, then:
>
> [report modem still not in command mode]
> [serial "break"]
> [end disconnect sequence if the modem replied with "OK", otherwise...]
> [report failure to disconnect modem?]
>
>
>
> What roughly would a reasonable implementation of the "check AT-OK, if
> not then try next method" process look like?  I've been poking about
> with src/mm-broadband-bearer.c - but don't really have a clear idea how
> to proceed...
>
> I'm assuming that I would use mm_base_modem_at_command_full to send the
> "AT<CR>" commands, but is it ok to do this from a command completion
> handler?
>
> Are there any existing analogous exchanges which I could take a look at
> to base this on?
>

What we usually do here is to setup a Context struct for the
operation, and define a set of STEPs for a small state machine that
defines the operation. See e.g.:
https://cgit.freedesktop.org/ModemManager/ModemManager/tree/src/mm-iface-modem-3gpp.c?id=75036a46580bb38d0957e64cd4a59539df16c6f8#n1400

The DisablingContext contains a step that goes from
DISABLING_STEP_FIRST (usually just a placeholder initial value) to
DISABLING_STEP_LAST (where the async operation is finished if
successful). We then define a e.g. interface_disabling_step() that
runs the current 'step'; which is usually an asynchronous action (e.g.
mm_base_modem_at_command() or even a timeout). On the completion
handler of the asynchronous action, we check if there has been an
error, and if so the global operation is finished with error. If the
step succeeded, we go on to next step (ctx->step++) and we re-run
interface_disabling_step(). That way you're chaining up a set of async
operations and making a global one with all steps.

In your case, though, beware, because the disconnection sequence like
you're suggestion should only be used by default if there is no
secondary port where we can request the disconnection right away.
Maybe it makes sense to have this multi-step operation defined in the
MMPortSerialAt object itself, something like
"mm_port_serial_at_disconnect()/_disconnect_finish()" even, which
would take a port in a connected state and try to disconnect it. Then
the base bearer would just call that async method as one of the steps
during the bearer disconnection logic.

-- 
Aleksander
https://aleksander.es