Re: ModemManager port grabbing rework

[Aleksander Morgado <aleksander lanedo com>]

Hey Dan,

I really do like the new approach, and I would even include it for
0.5.x. Some comments below.

> The core problem was that some devices need to specify a PPP port that's
> *not* the primary port.  In these cases the primary port always stays
> open for command and status, while some other port is used for PPP.
> This is different than the current model where the primary port is used
> for command and status until PPP is started, whereupon command and
> status switches to a secondary port.  The internal MM model was too
> inflexible here.
> 
> In the 'ports' branch, the core change here is to make MM_PORT_TYPE_xxx
> only about types like AT, QCDM, etc.  A new flags enum is created for AT
> ports specifically (since these are currently the only ones we care
> about for primary/secondary) called MM_AT_PORT_FLAG_xxx which is what
> plugins now use to give hints to the core about
> primary/secondary/data/etc.
> 
> 'ports' now uses the correct port layout on the Huawei E220, and even
> makes newer Sierra devices more functional by using the limited APP
> ports for PPP instead of command and status when connected.
> 
> Any review appreciated; it's a pretty big patch and it changes how
> plugins tag ports and interact with the core.  If it looks good we'll
> merge to git master for 0.6, and maybe merge it later for 0.5.x.
> 

One of the main behaviour changes is that now all modem initialization
(e.g. PIN lock checking) is done only after all ports were probed and
organized. I do like that, as we shouldn't run the initialization on a
secondary port even if it was grabbed first. From the current point of
view it shouldn't be a big deal, as we were anyway exporting the modem
only when all ports were probed, but I was hoping to improve the
situation by exporting the modem as soon as we got the first AT port and
the initialization sequence run, so that we could unlock PIN even while
other ports were still being probed. With this new behaviour, that will
not be possible, but no big deal as it could have been anyway an issue
if the primary port was not the one being used for that. I still have
some other ideas to mitigate the problem of spending too much time
probing ports that end up being not AT, which shouldn't be affected by
this new behaviour.

MMPortType seems redundant now, as we already have
MM_IS_AT_SERIAL_PORT() and MM_IS_QCDM_SERIAL_PORT(). But we can also
keep it and add a MM_PORT_TYPE_NET to be set when creating a MMPort for
net devices; that would simplify organize_ports() as you don't need to
look for net devices based on comparing the subsystem of the port with
"net". I would just add the new MM_PORT_TYPE_NET here, and then
MMPortType would be useful again.

In mm-manager.c, the call to mm_modem_organize_ports() could have been
done in check_export_modem(), after checking if there are support tasks
pending. Also, I would just allow multiple calls to
mm_modem_organize_ports(), and just return TRUE if they were already
organized.

In mm_modem_base_organize_ports(), I would just iterate the hashtable
instead of creating a GSList from the hash table and then iterating the
list; and also remove mm_modem_base_get_ports() which seems nowhere else
used.

In 06-api, and now that we postpone all initialization until we organize
all ports, port_grabbed() and ports_organized() will possibly not be
needed. A first "setup ports" step in the initialization sequence of the
MMBroadbandModem may be enough, which should take care of setting up all
expected unsolicited message handlers (either in primary or in both),
before any command is sent.

Cheers!

-- 
Aleksander