Fwd: Cinterion plugin patch
matthew stanger
stangerm2 at gmail.com
Mon Oct 24 05:13:13 UTC 2016
Back from the grave,
Here is finally the update... to the above code reviewed patch. A big thank
you Dan and Aleksander for the detail in the review it was really a great
learning experience for me. Please don't hold back with this one either :).
It's a pretty big rewrite so I'm sure there will still be some minor things
to tidy up still but I think it should be more inline now. There is one
thing in here for sure that needs fixing still but I wanted your options so
please see the note at line 1192 and let me know what you think. I promise
I will be much faster on getting the next tidy up done (>_<) and after this
I still have a few more patches.
Cheers,
Matt
On Thu, Sep 15, 2016 at 5:30 AM, Aleksander Morgado <
aleksander at aleksander.es> wrote:
> Hey Matthew,
>
> I reviewed the first patch of the series only for now; let's try to get
> this right first and then we go on with the other ones. See comments
> inline below. Some of the comments are coding style only things, but
> others try to simplify or change the flow of the logic, let me know what
> you think.
>
> On 06/09/16 03:47, matthew stanger wrote:
> > Here is try two. First off if you are uncomfortable with the attachment
> let
> > me know and I'll send the patch in the body. It's just 4k lines and
> Chrome
> > almost crashes when I paste it in. I won't take offence if that is needed
> > though. I can also squash it down to a single commit if that makes
> viewing
> > it easier.
> >
>
> You're not supposed to "paste" the patch in the email body or "merge"
> the patches in a single file and attach them to the email :) The target
> is that we can take the patches and quickly apply them with "git am"; if
> you paste the patch in the body it's likely that your email application
> may decide to modify the body before sending it; if you merge patches
> together in a single file, "git am" may end up choking when processing it.
>
> Sending patches can be done in 3 different ways:
> a) Use git-send-email command line tool, this will generate emails
> that can then be saved and applied with "git am".
> b) Use git format-patch and generate 1 file per patch; then attach the
> patches to the email. Again, we can use "git am" to apply them.
> c) Prepare a custom branch somewhere; e.g. bitbucket or github, and
> email the link to the branch so that we can pull from it.
>
> For reviewing, I usually prefer a), and if too complex to setup, just do
> b). For large developments c) is preferred, though.
>
> > The attached patch consist of 5 commits since I branched from '
> > 59f57befa4be81b562f9adfbac39fbe19d88c111'. The first commit being the
> > original I sent and the next 4 address suggestions made by Dan. I tried
> to
> > get the GPS separated but there is some hardship in doing that with our
> > commit history.
>
> Reviewing logic is very very hard with commits including different things
> :/
>
> I was able to make all of the changes except, I don't think
> > I can move PDP_CID to modem initialization as it is tied to the net
> > interface being used by the bearer. Leading in to answer your last
> > question, it's not hard coded technically but our tec rep's at Cinterion
> > told me to map these contexts to these interface to avoid issues, though
> it
> > does seem to work with any context to any interface. I could not
> > get report_connection_status() to trigger off a manual connect
> cancellation
> > either so I left that logic in. As far as using the MM core for CGDCONT
> and
> > PDP ctx I thought that would only work if I was using
> > the MM_BROADBAND_MODEM class instead of the MM_BROADBAND_MODEM_CINTERION
> > class when creating a bearer. I may be super wrong on that though. I just
> > say that as I wasn't able to trace how it was called inside
> > of find_cid_ready in mm-bb-bearer.c. I though because of that class I had
> > to handle our own CGD & PDP ctx but if that's not the case then please
> let
> > me know how I can do that. I made all the other changes & to be honest it
> > was quite a mess so thanks for the feedback.
> >
> > I also see you are using an old AT command ref for this modem. Cinterions
> > newest is 3.017. I can't give it to you due to NDA but if you want it I'd
> > be happy to lobby for you to Cinterion so they will give you a new copy.
> I
> > have already been lobbying MM to them just FYI, hoping they might take
> some
> > of the programming burden but they seemed really only focused on windows
> > atm. I'm not sure if you guys have PLS8 modems either but if you want
> ones
> > that's another thing I'm sure I could help you get. Having you test
> > anything I push even for 5 mins is well worth it so if you need anything
> > like that please let me know.
>
> Cinterion did send me a test-only PLS8 to play with the QMI integration
> they had, but not sure if they're still targeting to use QMI or not.
>
> See comments for the first patch below.
>
> > From 3b45e2f8afdd9f3aa284f3938e618b4b5437c572 Mon Sep 17 00:00:00 2001
> > From: Matthew Stanger <matthew_stanger at trimble.com>
> > Date: Fri, 26 Aug 2016 16:35:49 -0600
> > Subject: [PATCH 1/5] Features: Added SWWAN support in Cinterion plugin.
> > Updated GPS support for Cinterion modems that use new SGPSC commands.
> Added
> > LTE status for Cinterion plugin. Made general Cinterion plugin
> improvements
> > to better support PLS8-X & PLS8-E modems.
> >
> > Known issues:
> > SWWAN connection not tied into mmcli status updates.
> > Unknown logic flow for APN's which require User & Password.
> > No support for IP version.
> > Does not pull IP address from SWWAN into mmcli.
> > Does not auto preform DHCP for SWWAN connection.
> > Dual pdp context connections not yet supported.
> > Simple connect/disconnect does not clean up bearer.
> > ---
> > plugins/Makefile.am | 2 +
> > plugins/cinterion/77-mm-cinterion-port-types.rules | 2 +-
> > plugins/cinterion/mm-broadband-bearer-cinterion.c | 1126
> ++++++++++++++++++++
> > plugins/cinterion/mm-broadband-bearer-cinterion.h | 56 +
> > plugins/cinterion/mm-broadband-modem-cinterion.c | 279 ++++-
> > plugins/cinterion/mm-broadband-modem-cinterion.h | 3 +
> > plugins/cinterion/mm-common-cinterion.c | 402 ++++++-
> > plugins/cinterion/mm-modem-helpers-cinterion.c | 188 ++++
> > plugins/cinterion/mm-modem-helpers-cinterion.h | 17 +
> > plugins/cinterion/mm-plugin-cinterion.c | 141 ++-
> > src/mm-base-modem.c | 20 +
> > src/mm-base-modem.h | 1 +
> > 12 files changed, 2182 insertions(+), 55 deletions(-)
> > create mode 100644 plugins/cinterion/mm-broadband-bearer-cinterion.c
> > create mode 100644 plugins/cinterion/mm-broadband-bearer-cinterion.h
> >
> > diff --git a/plugins/Makefile.am b/plugins/Makefile.am
> > index 018b696..744fd18 100644
> > --- a/plugins/Makefile.am
> > +++ b/plugins/Makefile.am
> > @@ -565,6 +565,8 @@ libmm_plugin_cinterion_la_SOURCES = \
> > cinterion/mm-common-cinterion.h \
> > cinterion/mm-broadband-modem-cinterion.c \
> > cinterion/mm-broadband-modem-cinterion.h \
> > + cinterion/mm-broadband-bearer-cinterion.c \
> > + cinterion/mm-broadband-bearer-cinterion.h \
> > $(NULL)
> > if WITH_QMI
> > libmm_plugin_cinterion_la_SOURCES += \
> > diff --git a/plugins/cinterion/77-mm-cinterion-port-types.rules
> b/plugins/cinterion/77-mm-cinterion-port-types.rules
> > index 09de742..ee386f0 100644
> > --- a/plugins/cinterion/77-mm-cinterion-port-types.rules
> > +++ b/plugins/cinterion/77-mm-cinterion-port-types.rules
> > @@ -8,4 +8,4 @@ SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
> ENV{.MM_USBIFNUM}="$attr{bInte
> >
> > ATTRS{idVendor}=="1e2d", ATTRS{idProduct}=="0053",
> ENV{.MM_USBIFNUM}=="01", ENV{ID_MM_CINTERION_PORT_TYPE_GPS}="1"
> >
> > -LABEL="mm_cinterion_port_types_end"
> > +LABEL="mm_cinterion_port_types_end"
> > \ No newline at end of file
>
> This previous change may be skipped.
>
> > diff --git a/plugins/cinterion/mm-broadband-bearer-cinterion.c
> b/plugins/cinterion/mm-broadband-bearer-cinterion.c
> > new file mode 100644
> > index 0000000..eb73cf5
> > --- /dev/null
> > +++ b/plugins/cinterion/mm-broadband-bearer-cinterion.c
> > @@ -0,0 +1,1126 @@
> > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4
> -*- */
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details:
> > + *
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > + * Author: Matthew Stanger <matthew_stanger at trimble.com>
> > + */
> > +
> > +#include <config.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <ctype.h>
> > +#include <arpa/inet.h>
> > +#include <ModemManager.h>
> > +#include "mm-base-modem-at.h"
> > +#include "mm-broadband-bearer-cinterion.h"
> > +#include "mm-log.h"
> > +#include "mm-modem-helpers.h"
> > +#include "mm-modem-helpers-cinterion.h"
> > +#include "mm-daemon-enums-types.h"
> > +
> > +G_DEFINE_TYPE (MMBroadbandBearerCinterion,
> mm_broadband_bearer_cinterion, MM_TYPE_BROADBAND_BEARER);
>
> No need for the last ";"
>
> > +
> > +/**********************************************************
> *******************/
> > +/* Common enums and structs */
> > +
> > +typedef enum {
> > + MM_BEARER_CINTERION_AUTH_UNKNOWN = -1,
> > + MM_BEARER_CINTERION_AUTH_NONE = 0,
> > + MM_BEARER_CINTERION_AUTH_PAP = 1,
> > + MM_BEARER_CINTERION_AUTH_CHAP = 2,
> > + MM_BEARER_CINTERION_AUTH_MSCHAPV2 = 3,
> > +} MMBearerCinterionAuthPref;
> > +
>
> No need for the MM prefix in the enum if it's not public.
>
> > +typedef enum {
> > + CONNECTION_UNKNOWN = 0,
> > + CONNECTION_ACTIVE,
> > + CONNECTION_INACTIVE,
> > +} SWWAN_USB_CONNECTION_STATE;
>
> Enum types should be CamelCaseLikeThis.
>
> > +
> > +typedef enum {
> > + CONNECT_3GPP_CONTEXT_STEP_INIT = 0,
> > + CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN,
> > + CONNECT_3GPP_CONTEXT_STEP_BEARER_PROPERTIES,
> > + CONNECT_3GPP_CONTEXT_STEP_SET_SWWAN,
> > + CONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS,
> > + CONNECT_3GPP_CONTEXT_STEP_DONE,
> > +} Connect3gppContextStep;
> > +
> > +typedef enum {
> > + DISCONNECT_3GPP_CONTEXT_STEP_INIT = 0,
> > + DISCONNECT_3GPP_CONTEXT_STEP_SWWAN_DETACH,
> > + DISCONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS,
> > + DISCONNECT_3GPP_CONTEXT_STEP_DONE
> > +} Disconnect3gppContextStep;
> > +
> > +typedef struct {
> > + MMBroadbandBearerCinterion *self;
> > + MMBaseModem *modem;
> > + MMPortSerialAt *primary;
> > + MMPort *data;
> > + Connect3gppContextStep connect;
> > + Disconnect3gppContextStep disconnect;
> > + SWWAN_USB_CONNECTION_STATE usb0_state;
> > + SWWAN_USB_CONNECTION_STATE usb1_state;
> > + MMBearerIpConfig *ipv4_config;
> > + GCancellable *cancellable;
> > + GSimpleAsyncResult *result;
> > +} Control3gppContext;
>
> I think it should be better to have separate Connect/Disconnect contexts.
>
> > +
> > +struct _MMBroadbandBearerCinterionPrivate {
> > + gpointer connect_pending;
> > + gpointer disconnect_pending;
> > + guint network_disconnect_pending_id;/* Tag for the post task for
> network-initiated disconnect */
> > + const gchar *bearer_interface;
> > + const gchar *pdp_cid;
>
> This should be a uint.
>
> > + guint retry_count;
>
> Retry count looks like part of the connection/disconnect logic;
> shouldn't it go in the connect/disconnect context?
>
> > + guint swwan_read_write;
> > +};
> > +
> > +/**********************************************************
> *******************/
> > +/* Common 3GPP Function Declarations */
> > +static void connect_3gpp_context_step (Control3gppContext *ctx);
> > +static void disconnect_3gpp_context_step (Control3gppContext *ctx);
> > +static void connect_3gpp_context_complete_and_free (Control3gppContext
> *ctx);
> > +static void disconnect_3gpp_context_complete_and_free
> (Control3gppContext *ctx);
>
> The connect/disconnect logic should have different contexts, and then
> the complete_and_free() names could match the context name.
>
> E.g. Connect3gppContext with connect_3gpp_context_step() and
> connect_3gpp_context_complete_and_free ().
>
> > +
> > +/**********************************************************
> *******************/
> > +/* Common 3GPP */
> > +
> > +static MMPortSerialAt *
> > +get_dial_port (MMBroadbandModemCinterion *modem,
> > + MMPort *data,
> > + MMPortSerialAt *primary)
> > +{
> > + //Use only the primary port for sending AT commands.
>
> Coding style thing; we don't use "//" ever, not even for single-comment
> lines. Use /* this instead */. Applies to multiple places in the code.
>
>
> > + return g_object_ref (primary);
>
> What is the purpose of this method if it's just ref-ing the passed
> primary port? Why not just "g_object_ref (primary)" in the caller?
>
> > +}
> > +
> > +static void
> > +cinterion_3gpp_state_machine_logic (MMBroadbandBearerCinterion *self)
> > +{
> > + Control3gppContext *ctx;
> > +
> > + //For connecting flows.
> > + if (self->priv->connect_pending != NULL)
> > + {
>
> Coding style thing only; the open-braces at the end of the if() line; e.g.
>
> if (1) {
> /* something */
> }
>
> > + ctx = self->priv->connect_pending;
> > +
> > + switch (ctx->connect) {
> > +
> > + //Always go from INIT to VERIFY_SWWAN.
> > + case CONNECT_3GPP_CONTEXT_STEP_INIT: {
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN;
> > + break;
>
> This step just wants to get to the next step. So just do ctx->connect++;
> and fall down to the next case without a break; E.g.
>
> case CONNECT_3GPP_CONTEXT_STEP_INIT:
> ctx->connect++;
> /* fall down */
> case CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN:
> ....
>
>
> > + }
> > +
> > + //Always go from VERIFY_SWWAN to BEARER_PROPERTIES.
> > + case CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN: {
>
> The {} for the case is only really needed if you're going to define
> variables within the case clause; I would remove those otherwise.
>
> > + //Only try 5 times to check the connection then give up.
> > + if (self->priv->retry_count > 5)
> > + {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_TOO_MANY,
> > + "Unknown bearer state
> during connection attempt.");
> > +
> > + connect_3gpp_context_complete_and_free (ctx);
> > + return;
> > + }
> > + self->priv->retry_count++;
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_BEARER_PROPERTIES;
> > + break;
>
> Same here for the step and fall-down.
>
> > + }
> > +
> > + //When in this state we can be setting up a brand new
> connection or,
> > + //have just torn down a connection to setup a new one.
> > + case CONNECT_3GPP_CONTEXT_STEP_BEARER_PROPERTIES: {
> > + //We were connected (flags still set) & just issued SWWAN
> disconnect.
> > + //Now refresh our usb connection state & retry.
> > + //TODO: Rework for dual sim connections.
> > + if (ctx->usb0_state == CONNECTION_ACTIVE || ctx->usb1_state
> == CONNECTION_ACTIVE)
> > + {
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN;
>
> Just set the new step to run and call connect_3gpp_context_step (ctx);
> directly. E.g.:
>
> if (...) {
> ctx->connect = NEXT_STEP;
> connect_3gpp_context_step (ctx);
> return;
> }
>
> if (some other thing) {
> ctx->connect = SOME_OTHER_NEXT_STEP;
> connect_3gpp_context_step (ctx);
> return;
> }
>
> ...
>
> > + }
> > + //There is no active connection and we set the context
> correctly so continue to active SWWAN.
> > + else if (ctx->usb0_state == CONNECTION_INACTIVE ||
> ctx->usb1_state == CONNECTION_INACTIVE)
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_SET_SWWAN;
> > + else
> > + {
> > + //Error, should be impossible to get here. State
> machine broken.
>
> If this is impossible to get here, just g_assert_not_reached(); don't
> return an error to the caller if this is a programmer error.
>
> > + ctx->self->priv->connect_pending = NULL;
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > +
> MM_CORE_ERROR_WRONG_STATE,
> > + "Unknown bearer state
> during connection attempt.");
> > +
> > + connect_3gpp_context_complete_and_free (ctx);
> > + return;
> > + }
> > +
> > + break;
> > + }
> > +
> > + //Always verify that the SWWAN connection is active after
> trying to set it up.
> > + //Same step as CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN but
> makes it easy to detect
> > + //if we just came from a new or old connection.
> > + case CONNECT_3GPP_CONTEXT_STEP_SET_SWWAN: {
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS;
> > + break;
>
> Again, fall-down without break;.
>
> > + }
> > +
> > + //If SWWAN was just set, we expect it should be on.
> > + case CONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS: {
> > + if ((g_ascii_strncasecmp(ctx->self->priv->bearer_interface,
> "usb0", 4)==0 && ctx->usb0_state == CONNECTION_ACTIVE) ||
> > + (g_ascii_strncasecmp(ctx->self->priv->bearer_interface,
> "usb1", 4)==0 && ctx->usb1_state == CONNECTION_ACTIVE))
>
> No, this is not right, usb0 and usb1 are network interface names that
> the kernel decided to use; what if you have 2 Cinterion modems at the
> same time?
>
> Instead of relying on the interface name, you may want to rely on the
> USB interface number; e.g.
>
> g_udev_device_get_property_as_int (
> udev_device,
> "ID_USB_INTERFACE_NUM")
>
> NOTE: you'll need a GUdevDevice for the WWAN; which you could get using
> a new GUdevClient. This will be much easier to handle once the work I'm
> doing to allow skipping udev is integrated, as then you can just get a
> MMKernelDevice from the MMPort and query the property directly, like:
>
> mm_kernel_device_get_property_as_int (
> mm_port_peek_kernel_device (port),
> "ID_USB_INTERFACE_NUM");
>
> See e.g. this similar case in the Huawei plugin:
> https://lists.freedesktop.org/archives/modemmanager-devel/
> 2016-August/003279.html
>
>
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_DONE;
> > + else
> > + {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > +
> MM_CORE_ERROR_WRONG_STATE,
> > + "Unknown bearer state
> during connection attempt.");
> > +
> > + connect_3gpp_context_complete_and_free (ctx);
> > + return;
> > + }
>
> You could have a single if() to catch the error case, and then do the
> ctx->connect++; and /* fall-down */ thing to get to the next case.
>
> > +
> > + break;
> > + }
> > +
> > + case CONNECT_3GPP_CONTEXT_STEP_DONE: {
> > + //Place holder, nothing should call this. Fall into defualt
> error.
> > + }
> > +
> > + default: {
>
> I personally don't like a default case in a state machine where the step
> is a known enumeration.
>
> > + mm_err ("Unexpected SWWAN connect state. Unable to
> advance.");
> > + ctx->self->priv->connect_pending = NULL;
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_WRONG_STATE,
> > + "Unexpected SWWAN connect
> state. Unable to advance.");
> > +
> > + connect_3gpp_context_complete_and_free (ctx);
> > + return;
> > + }
> > +
> > +
> > + }
> > +
> > + connect_3gpp_context_step (ctx);
>
> This connect_3gpp_context_step() wouldn't be needed as each case above
> should call it itself when needed and directly return.
>
> > + }
> > + //Assume disconnecting flow, assert protects assumption.
>
> Separate logic methods for connection and disconnection, please.
>
> > + else
> > + {
> > + g_assert (self->priv->disconnect_pending != NULL);
> > + ctx = self->priv->disconnect_pending;
> > +
> > + switch (ctx->disconnect) {
> > +
> > + case DISCONNECT_3GPP_CONTEXT_STEP_INIT: {
> > + ctx->disconnect = DISCONNECT_3GPP_CONTEXT_STEP_
> SWWAN_DETACH;
> > + break;
>
> Same as above, when just going to the next case, ctx->disconnect++; and
> /* fall down */
>
> > + }
> > +
> > + case DISCONNECT_3GPP_CONTEXT_STEP_SWWAN_DETACH: {
> > + ctx->disconnect = DISCONNECT_3GPP_CONTEXT_STEP_
> CONNECTION_STATUS;
> > + break;
>
> Another fall-down? Why do we need this step then?
>
> > + }
> > +
> > + case DISCONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS: {
> > + //We expect the bearer to be disconnected now. If it's not
> try again.
> > + //TODO: Rework for dual sim connections.
> > + if ((g_ascii_strncasecmp(ctx->self->priv->bearer_interface,
> "usb0", 4)==0 && ctx->usb0_state == CONNECTION_INACTIVE) ||
> > + (g_ascii_strncasecmp(ctx->self->priv->bearer_interface,
> "usb1", 4)==0 && ctx->usb1_state == CONNECTION_INACTIVE))
>
> As above, never rely on interface names.
>
> > + ctx->disconnect = DISCONNECT_3GPP_CONTEXT_STEP_DONE;
> > + else
> > + {
> > + ctx->disconnect = DISCONNECT_3GPP_CONTEXT_STEP_
> SWWAN_DETACH;
> > + sleep (1); //Wait a second on disconnect retry's.
>
> Never never ever just sleep() :) If you need to reschedule the same step
> in 1s, you should:
>
> g_timeout_add_seconds (
> 1,
> (GSourceFunc) cinterion_3gpp_state_machine_logic_reschedule_cb,
> ctx);
> return;
>
> And then implement the callback:
> static gboolean
> cinterion_3gpp_state_machine_logic_cb (Control3gppContext *ctx)
> {
> cinterion_3gpp_state_machine_logic (ctx);
> return G_SOURCE_REMOVE;
> }
>
>
> > + }
> > + break;
> > + }
> > +
> > + case DISCONNECT_3GPP_CONTEXT_STEP_DONE: {
> > + //Place holder, nothing should call this. Fall into defualt
> error.
>
> Nothing should call this, yet there is an explicit step selection for
> this one some lines above... :)
> ctx->disconnect = DISCONNECT_3GPP_CONTEXT_STEP_DONE;
>
>
> > + }
> > +
> > + default: {
> > + mm_err ("Unexpected SWWAN disconnect state. Unable to
> advance.");
> > + ctx->self->priv->connect_pending = NULL;
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_WRONG_STATE,
> > + "Unexpected SWWAN
> disconnect state. Unable to advance.");
> > +
> > + connect_3gpp_context_complete_and_free (ctx);
> > +
> > + return;
> > + }
> > +
> > + }
> > +
> > + disconnect_3gpp_context_step (ctx);
>
> As for the connect logic, I think it's better to just call the
> context_step() method directly after setting the next step, not doing a
> break; and a single context_step() here. This is because each step will
> be auto-contained within the case(), I think it helps readability of the
> state machine.
>
> > + }
> > +}
> > +
> > +static gboolean
> > +proccess_swwan_response(MMBroadbandBearerCinterion *self, GList
> *result)
> > +{
> > + Control3gppContext *ctx;
> > +
> > + if (self->priv->connect_pending != NULL)
> > + ctx = self->priv->connect_pending;
> > + else
> > + {
> > + g_assert (self->priv->disconnect_pending != NULL);
> > + ctx = self->priv->disconnect_pending;
> > + }
> > +
>
> I see this logic mixing connect/disconnect again; I think we shouldn't
> do this. If the SWWAN response is parsed in the same way for both,
> better write a parser method and let the different connect/disconnect
> flows call it.
>
> > +
> > + if (g_list_length(result) != 0) {
> > + int first_result = GPOINTER_TO_INT(result->data);
> > +
> > + //mm_serial_parser_v1_parse will catch CME Error. Parent
> function will then send
> > + //that error to dbus out.
> > + if(first_result == -1)
> > + return FALSE;
> > + //Recived an 'OK' response from a write command, place holder
> so we don't error below.
>
> *Received
>
> > + if (first_result == 0 && ctx->self->priv->swwan_read_write == 1)
> > + NULL;
> > + //Recived an 'OK'(0) response from an swwwan read command.
>
> *Received, *swwan
>
> > + else if (first_result == 0 && ctx->self->priv->swwan_read_write
> == 0)
> > + {
> > + ctx->usb0_state = CONNECTION_INACTIVE;
> > + ctx->usb1_state = CONNECTION_INACTIVE;
> > + }
> > + //1 || 3 result is the CID, given when that context is
> activated.
> > + //TODO: Rework for dual sim connections.
> > + else if (first_result == 1 && ctx->self->priv->swwan_read_write
> == 0)
> > + ctx->usb1_state = CONNECTION_ACTIVE;
> > + else if (first_result == 3 && ctx->self->priv->swwan_read_write
> == 0)
> > + ctx->usb0_state = CONNECTION_ACTIVE;
> > + else
> > + {
> > + for (; result; result = g_list_next (result))
> > + mm_err ("Unknown SWWAN response data:%i",
> GPOINTER_TO_INT(result->data));
> > +
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_FAILED,
> > + "Internal error while
> processing SWWAN response.");
> > +
> > + return FALSE;
> > + }
> > + }
> > + else {
> > + mm_err ("Unable to parse zero length SWWAN response.");
> > + return FALSE;
> > + }
> > +
> > + return TRUE;
> > +}
> > +
> > +static void
> > +get_swwan_response (MMBaseModem *modem,
> > + GAsyncResult *res,
> > + MMBroadbandBearerCinterion *self)
> > +{
> > + Control3gppContext *ctx;
> > + const gchar *response;
> > + GError *error = NULL;
> > + GList *response_parsed = NULL;
> > +
> > + if (self->priv->connect_pending != NULL)
> > + ctx = self->priv->connect_pending;
> > + else
> > + {
> > + g_assert (self->priv->disconnect_pending != NULL);
> > + ctx = self->priv->disconnect_pending;
> > + }
> > +
> > + // Balance refcount
> > + g_object_unref (self);
>
> Too complex, too complex :). If you had one logic for connect and
> another logic for disconnect, you could pass around the actual Context
> struct in the async calls user_data, which holds already a reference to
> the bearer object. Instead, you're storing the contexts in private info
> and passing the bearer object.
>
> It is good to store the connect and disconnect contexts in the private
> info ONLY if there is no other place to manage them. If the logic relies
> on queries to the modem to check e.g. SWWAN status, you can pass around
> the contexts in the async calls and not ever store them in private info.
> One of the good reasons to store the context in private info is when you
> rely on unsolicited messages from the modem. For your use case, I think
> you could totally avoid storing the contexts in private info, and the
> logic would be much much simpler. What do you think?
>
> > +
> > + //Parse the swwan response.
> > + response = mm_base_modem_at_command_finish (modem, res, &error);
>
> If !response, just return the error right away.
>
> > +
> > + //1st elem of response_parsed will be:
> > + //1 or 3 for an active swwan, -1 for a valid error, 0 for assumed
> 'OK' (no conection)
> > + if (error == NULL && !mm_cinterion_parse_swwan_response (response,
> &response_parsed, &error))
> > + NULL;
>
> Hum?
>
> > +
> > +
> > + if (error != NULL || !proccess_swwan_response(ctx->self,
> response_parsed)) {
> > + ctx->self->priv->connect_pending = NULL;
> > + ctx->self->priv->disconnect_pending = NULL;
> > +
> > + g_simple_async_result_take_error (ctx->result, error);
>
> But proccess_swwan_response() doesn't set a GError to return here if it
> fails.
>
> > +
> > + if (ctx->connect)
> > + connect_3gpp_context_complete_and_free (ctx);
> > + else
> > + disconnect_3gpp_context_complete_and_free (ctx);
> > +
> > + return;
> > +
> > + }
> > +
> > + mm_dbg ("usb0-state:%i usb1-state:%i", ctx->usb0_state,
> ctx->usb1_state);
> > +
> > + g_list_free(response_parsed);
> > + g_clear_error (&error);
>
> You would never need a g_clear_error() here if errors are propagated
> earlier whenever they happen.
>
> > +
> > + cinterion_3gpp_state_machine_logic(ctx->self);
> > +
>
> Will this run the same state machine step again? I think it is usually
> more clear to update the step to run, and then call the state machine
> logic, something like:
>
> ctx->step++;
> context_step (ctx);
>
>
> > + return;
>
> Just remove this return; :)
>
> > +}
> > +
> > +/**********************************************************
> *******************/
> > +/* Cinterion AT Command Wrappers */
> > +
> > +static void
> > +send_swwan_read_command(Control3gppContext *ctx)
> > +{
> > + ctx->self->priv->swwan_read_write = 0;
> > +
>
> I'm sure this swwan_read_write flag could be avoided if different
> GAsyncReadyCallback are used for read/write operations :)
>
> > + //Check for swwan connection. The next state will be preformed by
> the callback.
>
> *performed
>
> > + mm_base_modem_at_command_full (ctx->modem,
> > + ctx->primary,
> > + "^SWWAN?",
> > + 5,
> > + FALSE,
> > + FALSE,
> > + NULL,
> > + (GAsyncReadyCallback)get_
> swwan_response,
> > + g_object_ref (ctx->self));
> > +}
> > +
> > +static void
> > +send_swwan_connect_command(Control3gppContext *ctx)
> > +{
> > + //USB0(1st wwan adapt) -> 3rd context, USB1(2nd wwan adapt) -> 1st
> context
> > + gchar *command;
>
> Remove wrong alignment whitespaces.
>
> > + command = g_strdup_printf ("^SWWAN=%s,%s,%s",
> > + "1",
> > + ctx->self->priv->pdp_cid, //Expect 1 or 3
> > + g_ascii_strncasecmp(ctx->self->priv->pdp_cid,
> "3", 1)==0 ? "1" : "2");
>
> Could you comment here on why "1" or "2" are given here based on CID
> number?
>
> > +
> > + ctx->self->priv->swwan_read_write = 1;
> > +
> > + //Start the swwan connection. The next state will be preformed by
> the callback.
>
> *performed
>
> > + mm_base_modem_at_command_full (ctx->modem,
> > + ctx->primary,
> > + command,
> > + 10,/*Seen it take 5 seconds :0 */
> > + FALSE,
> > + FALSE,
> > + NULL,
> > + (GAsyncReadyCallback)get_
> swwan_response,
> > + g_object_ref (ctx->self));
>
> As said earlier, passing around the context is much simpler.
>
> > +
> > + g_free (command);
> > +}
> > +
> > +static void
> > +send_swwan_disconnect_command(Control3gppContext *ctx)
> > +{
> > + gchar *command;
>
> Remove wrong alignment whitespaces.
>
> > + command = g_strdup_printf ("^SWWAN=%s,%s,%s",
> > + "0",
> > + ctx->self->priv->pdp_cid,
> > + g_ascii_strncasecmp(ctx->self->priv->pdp_cid,
> "3", 1)==0 ? "1" : "2");
> > +
> > + ctx->self->priv->swwan_read_write = 1;
> > +
> > + //Check for swwan connection. The next state will be preformed by
> the callback.
> > + mm_base_modem_at_command_full (ctx->modem,
> > + ctx->primary,
> > + command,
> > + 10,
> > + FALSE,
> > + FALSE,
> > + NULL,
> > + (GAsyncReadyCallback)get_
> swwan_response,
> > + g_object_ref (ctx->self));
>
> As said earlier, passing around the context is much simpler.
>
> > +
> > + g_free (command);
> > +}
> > +
>
> Too many whitelines here :) Easy change!
>
> > +
> > +
> > +
> > +/**********************************************************
> *******************/
> > +/* Connect 3GPP */
> > +
> > +static gint
> > +cinterion_parse_auth_type (MMBearerAllowedAuth mm_auth)
> > +{
> > + switch (mm_auth) {
> > + case MM_BEARER_ALLOWED_AUTH_NONE:
> > + return MM_BEARER_CINTERION_AUTH_NONE;
> > + case MM_BEARER_ALLOWED_AUTH_PAP:
> > + return MM_BEARER_CINTERION_AUTH_PAP;
> > + case MM_BEARER_ALLOWED_AUTH_CHAP:
> > + return MM_BEARER_CINTERION_AUTH_CHAP;
> > + case MM_BEARER_ALLOWED_AUTH_MSCHAPV2:
> > + return MM_BEARER_CINTERION_AUTH_MSCHAPV2;
> > + default:
> > + return MM_BEARER_CINTERION_AUTH_UNKNOWN;
> > + }
> > +}
> > +
> > +static void
> > +connect_3gpp_context_complete_and_free (Control3gppContext *ctx)
> > +{
> > + g_simple_async_result_complete_in_idle (ctx->result);
> > + g_object_unref (ctx->cancellable);
> > + g_object_unref (ctx->result);
> > + g_object_unref (ctx->modem);
> > + g_object_unref (ctx->self);
> > + g_clear_object (&ctx->ipv4_config);
> > + g_clear_object (&ctx->data);
> > + g_clear_object (&ctx->primary);
> > +
> > + g_slice_free (Control3gppContext, ctx);
> > +}
> > +
> > +static MMBearerConnectResult *
> > +connect_3gpp_finish (MMBroadbandBearer *self,
> > + GAsyncResult *res,
> > + GError **error)
> > +{
> > + if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
> (res), error))
> > + return NULL;
> > +
> > + return mm_bearer_connect_result_ref (g_simple_async_result_get_op_res_gpointer
> (G_SIMPLE_ASYNC_RESULT (res)));
> > +}
> > +
> > +static gchar *
> > +build_cinterion_pdp_context_string(Control3gppContext *ctx)
> > +{
> > + const gchar *apn = NULL;
> > + const gchar *user = NULL;
> > + const gchar *passwd = NULL;
> > + MMBearerAllowedAuth auth;
> > + gint encoded_auth = MM_BEARER_CINTERION_AUTH_
> UNKNOWN;
> > + gchar *command;
> > +
> > + apn = mm_bearer_properties_get_apn (mm_base_bearer_peek_config
> (MM_BASE_BEARER (ctx->self)));
> > + user = mm_bearer_properties_get_user (mm_base_bearer_peek_config
> (MM_BASE_BEARER (ctx->self)));
> > + passwd = mm_bearer_properties_get_password
> (mm_base_bearer_peek_config (MM_BASE_BEARER (ctx->self)));
> > + auth = mm_bearer_properties_get_allowed_auth
> (mm_base_bearer_peek_config (MM_BASE_BEARER (ctx->self)));
> > + encoded_auth = cinterion_parse_auth_type (auth);
> > +
> > + /* Default to no authentication if not specified */
> > + if (encoded_auth == MM_BEARER_CINTERION_AUTH_UNKNOWN) {
> > + encoded_auth = MM_BEARER_CINTERION_AUTH_NONE;
> > + mm_dbg ("Unable to detect authentication type. Defaulting
> to:%i", encoded_auth);
> > + }
> > +
> > + //TODO: Get IP type if specific protocol was specified. Hardcoded
> to IPV4 for now.
> > +
> > + if (!user && !passwd)
> > + command = g_strdup_printf ("+CGDCONT=%s,\"IP\",\"%s\",\"
> 0.0.0.0\",0,0",
> > + ctx->self->priv->pdp_cid,
> > + apn == NULL ? "" : apn);
> > +
> > + /*TODO: Can't test this as we can't get a hold of a SIM w/ this
> feature atm. This is a place
> > + * holder. When we can test this then it should be refactored &
> split into another state so we
> > + * can tell independently if the SGAUTH fails.
> > + * Write Command
> > + * AT^SGAUTH=<cid>[, <auth_type>[, <passwd>, <user>]]
> > + * Response(s)
> > + * OK
> > + * ERROR
> > + * +CME ERROR: <err>
> > + */
> > + else
> > + command = g_strdup_printf ("^SGAUTH=%s,%i,%s,%s;
> +CGDCONT=%s,\"IP\",\"%s\",\"0.0.0.0\",0,0 ",
> > + ctx->self->priv->pdp_cid,
> > + encoded_auth,
> > + passwd == NULL ? "" : passwd,
> > + user == NULL ? "" : user,
> > + ctx->self->priv->pdp_cid,
> > + apn == NULL ? "" : apn);
>
> I wouldn't chain up these 2 AT commands.
>
> > + return command;
> > +}
> > +
> > +static gboolean
> > +set_pdp_cid(Control3gppContext *ctx)
> > +{
> > + GUdevClient *client;
> > + GUdevDevice *data_device;
> > +
> > + //Need the data port to figure out what context to activate.
> > + client = g_udev_client_new (NULL);
> > + data_device = (g_udev_client_query_by_subsystem_and_name (
> > + client,
> > + "net",
> > + mm_port_get_device (ctx->data)));
> > + ctx->self->priv->bearer_interface = g_udev_device_get_name(data_
> device);
> > +
> > + //Map PDP context from the current Bearer. USB0 -> 3rd context,
> USB1 -> 1st context
> > + if (g_ascii_strncasecmp(ctx->self->priv->bearer_interface, "usb0",
> 4) == 0)
> > + ctx->self->priv-> pdp_cid = "3";
> > + else if (g_ascii_strncasecmp(ctx->self->priv->bearer_interface,
> "usb1", 4) == 0)
> > + ctx->self->priv->pdp_cid = "1";
> > + else
> > + {
> > + mm_err ("Unable to map usb interface:(%s) to context",
> ctx->self->priv->bearer_interface);
> > +
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_FAILED,
> > + "Internal error while mapping
> USB network interface.");
> > +
> > + ctx->self->priv->connect_pending = NULL;
> > + ctx->self->priv->disconnect_pending = NULL;
> > +
> > + if (ctx->connect)
> > + connect_3gpp_context_complete_and_free (ctx);
> > + else
> > + disconnect_3gpp_context_complete_and_free (ctx);
> > +
> > + return FALSE;
> > + }
> > +
> > + return TRUE;
> > +}
> > +
> > +static void
> > +handle_cancel_connect(Control3gppContext *ctx)
> > +{
> > + gchar *command;
> > +
> > + // Clear context
> > + ctx->self->priv->connect_pending = NULL;
> > +
> > + command = g_strdup_printf ("^SWWAN=%s,%s,%s",
> > + "0",
> > + ctx->self->priv->pdp_cid, //Expect 1 or
> 3,
> > + g_ascii_strncasecmp(ctx->self->priv->pdp_cid,
> "3", 1)==0 ? "1" : "2");
> > +
> > + //Disconnect, may not succeed. Will not check response on cancel.
> > + mm_base_modem_at_command_full (ctx->modem,
> > + ctx->primary,
> > + command,
> > + 3,
> > + FALSE,
> > + FALSE,
> > + NULL,
> > + NULL, // Do not care the AT response
> > + NULL);
> > +
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_CANCELLED,
> > + "Cinterion connection operation
> has been cancelled");
> > + connect_3gpp_context_complete_and_free (ctx);
> > +
> > +}
> > +
> > +static void
> > +context_ready (MMBaseModem *modem,
> > + GAsyncResult *res,
> > + MMBroadbandBearerCinterion *self)
> > +{
> > + Control3gppContext *ctx;
> > + GError *error = NULL;
> > + const gchar *response;
> > +
> > + ctx = self->priv->connect_pending;
> > + g_assert (ctx != NULL);
> > +
> > + /* Balance refcount */
> > + g_object_unref (self);
> > +
> > + //Get the cgdcont write response. We expect: 'OK'
> > + //Responses: OK, ERROR
> > + response = mm_base_modem_at_command_finish (modem, res, &error);
> > +
> > + if (!response || error != NULL) {
> > + mm_err ("CGDCONT read- Error:%d response:%s", error->code,
> response);
> > +
> > + self->priv->connect_pending = NULL;
> > + g_simple_async_result_take_error (ctx->result, error);
> > + connect_3gpp_context_complete_and_free (ctx);
> > +
> > + return;
> > + }
> > +
> > + /* Go to next step */
> > + //ctx->connect = CONNECT_3GPP_CONTEXT_STEP_SET_SWWAN;
> > + cinterion_3gpp_state_machine_logic(ctx->self);
> > +}
> > +
> > +static void
> > +connect_3gpp_context_step (Control3gppContext *ctx)
> > +{
> > + /* Check for cancellation */
> > + if (g_cancellable_is_cancelled (ctx->cancellable)) {
> > + //Init the current context if not done already.
> > + if (g_ascii_strncasecmp(ctx->self->priv->pdp_cid, "0", 1)==0)
> > + if(set_pdp_cid(ctx)) //Don't care about failure case for
> cancel.
> > +
> > + handle_cancel_connect(ctx);
> > + return;
> > + }
> > +
> > + // Network-initiated disconnect should not be outstanding at this
> point,
> > + // because it interferes with the connect attempt.
> > + g_assert (ctx->self->priv->network_disconnect_pending_id == 0);
> > + g_assert (ctx->self->priv->disconnect_pending == NULL);
> > +
> > +
> > + switch (ctx->connect) {
> > + case CONNECT_3GPP_CONTEXT_STEP_INIT: {
> > + MMBearerIpFamily ip_family;
> > +
> > + //Initialize variables, reminder!
> > + ctx->usb0_state = CONNECTION_UNKNOWN;
> > + ctx->usb1_state = CONNECTION_UNKNOWN;
>
> You could do this just after allocating the context.
>
> > + ctx->self->priv->pdp_cid = "0"; //Cinterion doesn't have cid ==
> 0
>
> I think I said it earlier, please make pdp_cid a uint.
>
> > + ctx->self->priv->swwan_read_write = -1;
> > + ctx->self->priv->retry_count = 0;
> > +
>
> And you shouldn't really need those 2 things above in private info...
>
> > + if(!set_pdp_cid(ctx))
> > + return;
> > +
> > + ip_family = mm_bearer_properties_get_ip_type
> (mm_base_bearer_peek_config (MM_BASE_BEARER (ctx->self)));
> > +
> > + //TODO: Fix so more than IPV4 can be used.
> > + if (ip_family == MM_BEARER_IP_FAMILY_NONE ||
> > + ip_family == MM_BEARER_IP_FAMILY_ANY) {
> > + gchar *ip_family_str;
> > +
> > + ip_family = mm_base_bearer_get_default_ip_family
> (MM_BASE_BEARER (ctx->self));
> > + ip_family_str = mm_bearer_ip_family_build_string_from_mask
> (ip_family);
> > + mm_dbg ("No specific IP family requested, defaulting to %s",
> > + ip_family_str);
> > + g_free (ip_family_str);
> > + }
> > +
> > + /* Default to automatic/DHCP addressing */
>
> Oh, weird, you're creating the MMBearerIpConfig before even knowing that
> the modem is connected... ok...
>
> > + ctx->ipv4_config = mm_bearer_ip_config_new ();
> > + mm_bearer_ip_config_set_method (ctx->ipv4_config,
> MM_BEARER_IP_METHOD_DHCP);
> > +
> > + /* Store the context */
> > + ctx->self->priv->connect_pending = ctx;
> > +
> > + cinterion_3gpp_state_machine_logic(ctx->self);
> > + return;
> > + }
> > + case CONNECT_3GPP_CONTEXT_STEP_VERIFY_SWWAN: {
> > + send_swwan_read_command(ctx);
> > + return;
> > + }
> > +
> > + case CONNECT_3GPP_CONTEXT_STEP_BEARER_PROPERTIES: {
> > + gchar *command = NULL;
> > +
> > + //TODO: Rework for dual sim connections, don't disconnect the
> wrong one.
> > + //If there is already an active SWWAN connection disconnect
> before trying to set it's context.
> > + if ((ctx->usb0_state == CONNECTION_ACTIVE &&
> g_ascii_strncasecmp(ctx->self->priv->pdp_cid, "3", 1)) || ctx->usb0_state
> == CONNECTION_UNKNOWN ||
> > + (ctx->usb1_state == CONNECTION_ACTIVE &&
> g_ascii_strncasecmp(ctx->self->priv->pdp_cid, "1", 1)) || ctx->usb1_state
> == CONNECTION_UNKNOWN)
> > + {
>
> If "3" and "1" CIDs are special, we should have some enumeration or
> something flagging them with a name, now that I think of it.
>
> > + send_swwan_disconnect_command(ctx);
> > + g_free (command);
> > + return;
> > + }
> > +
> > + command = build_cinterion_pdp_context_string(ctx);
> > +
> > + //Set the PDP context with cgdcont.
>
> Isn't there already a generic way in MMBroadbandBearer doing this?
>
> > + mm_base_modem_at_command_full (ctx->modem,
> > + ctx->primary,
> > + command,
> > + 3,
> > + FALSE,
> > + FALSE,
> > + NULL,
> > + (GAsyncReadyCallback)context_
> ready,
> > + g_object_ref (ctx->self));
> > +
> > + g_free (command);
> > + return;
> > + }
> > + case CONNECT_3GPP_CONTEXT_STEP_SET_SWWAN: {
> > + send_swwan_connect_command(ctx);
> > + return;
> > + }
> > + case CONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS: {
> > + send_swwan_read_command(ctx);
> > + return;
> > + }
> > +
> > + case CONNECT_3GPP_CONTEXT_STEP_DONE: {
> > + /* Clear context */
> > + ctx->self->priv->connect_pending = NULL;
> > + ctx->self->priv->bearer_interface = NULL;
> > + ctx->self->priv->pdp_cid = NULL;
> > + ctx->self->priv->swwan_read_write = -1;
> > +
> > + /* Setup result */
> > + {
> > + if (ctx->ipv4_config) {
> > + g_simple_async_result_set_op_res_gpointer (
> > + ctx->result,
> > + mm_bearer_connect_result_new (ctx->data,
> ctx->ipv4_config, NULL),
> > + (GDestroyNotify)mm_bearer_connect_result_unref);
> > + }
> > + else {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > +
> MM_CORE_ERROR_WRONG_STATE,
> > + "Cinterion connection
> failed to set IP protocol");
> > + }
> > + }
> > +
> > + connect_3gpp_context_complete_and_free (ctx);
> > + return;
> > + }
> > + }
> > +}
> > +
> > +static void
> > +connect_3gpp (MMBroadbandBearer *self,
> > + MMBroadbandModem *modem,
> > + MMPortSerialAt *primary,
> > + MMPortSerialAt *secondary,
> > + GCancellable *cancellable,
> > + GAsyncReadyCallback callback,
> > + gpointer user_data)
> > +{
> > + Control3gppContext *ctx;
> > + MMPort *port;
> > +
> > + g_assert (primary != NULL);
> > +
> > + // We need a net port
> > + port = mm_base_modem_peek_best_data_port (MM_BASE_MODEM (modem),
> MM_PORT_TYPE_NET);
> > + if (!port) {
> > + g_simple_async_report_error_in_idle (G_OBJECT (self),
> > + callback,
> > + user_data,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_NOT_FOUND,
> > + "No valid data port found
> to launch connection");
> > + return;
> > + }
> > +
> > +
> > + /* Setup connection context */
> > + ctx = g_slice_new0 (Control3gppContext);
> > + ctx->self = g_object_ref (self);
> > + ctx->modem = g_object_ref (modem);
> > + ctx->data = g_object_ref (port);
> > + ctx->result = g_simple_async_result_new (G_OBJECT (self),
> > + callback,
> > + user_data,
> > + connect_3gpp);
> > + ctx->cancellable = g_object_ref (cancellable);
> > + ctx->connect = CONNECT_3GPP_CONTEXT_STEP_INIT;
> > +
> > + g_assert (ctx->self->priv->connect_pending == NULL);
> > + g_assert (ctx->self->priv->disconnect_pending == NULL);
> > +
> > + ctx->primary = get_dial_port (MM_BROADBAND_MODEM_CINTERION
> (ctx->modem), ctx->data, primary);
>
> What if primary is connected? Shouldn't we be checking that?
>
> > +
> > + /* Run! */
> > + connect_3gpp_context_step (ctx);
> > +}
> > +
> > +/**********************************************************
> *******************/
> > +/* Disconnect 3GPP */
> > +
> > +static void
> > +disconnect_3gpp_context_complete_and_free (Control3gppContext *ctx)
> > +{
> > + g_simple_async_result_complete_in_idle (ctx->result);
> > + g_object_unref (ctx->result);
> > + g_object_unref (ctx->primary);
> > + g_object_unref (ctx->self);
> > + g_object_unref (ctx->modem);
> > + g_slice_free (Control3gppContext, ctx);
> > +}
>
> Yeah, nasty nasty to have 2 different complete_and_free() for the same
> Context struct based on different logics; just use 2 different contexts :)
>
> > +
> > +static gboolean
> > +disconnect_3gpp_finish (MMBroadbandBearer *self,
> > + GAsyncResult *res,
> > + GError **error)
> > +{
> > + return !g_simple_async_result_propagate_error
> (G_SIMPLE_ASYNC_RESULT (res), error);
> > +}
> > +
> > +static void
> > +disconnect_3gpp_context_step (Control3gppContext *ctx)
> > +{
> > + //There is no cancel handling b/c the only thing we could do would
> > + //be disconnect, which is already happening and bad things can
> happen
> > + //if we abandon the modem in an unknown state.
> > +
> > + //Don't allow disconnect while connect in progress.
> > + g_assert (ctx->self->priv->connect_pending == NULL);
> > +
> > + switch (ctx->disconnect) {
> > + case DISCONNECT_3GPP_CONTEXT_STEP_INIT:
> > + /* Store the context */
> > + ctx->self->priv->disconnect_pending = ctx;
> > +
> > + //Initialize variables, reminder!
> > + ctx->self->priv->pdp_cid = "0";
> > + ctx->self->priv->retry_count = 0;
> > + ctx->self->priv->swwan_read_write = -1;
> > +
> > + // We ignore any pending network-initiated disconnection in
> order to prevent it
> > + // from interfering with the client-initiated disconnection, as
> we would like to
> > + // proceed with the latter anyway.
> > + if (ctx->self->priv->network_disconnect_pending_id != 0) {
> > + g_source_remove (ctx->self->priv->network_
> disconnect_pending_id);
> > + ctx->self->priv->network_disconnect_pending_id = 0;
> > + }
> > +
> > + if(!set_pdp_cid(ctx))
> > + return;
> > +
> > + cinterion_3gpp_state_machine_logic(ctx->self);
> > + return;
> > +
> > + case DISCONNECT_3GPP_CONTEXT_STEP_SWWAN_DETACH:
> > + // If too many retries (1s of wait between the retries), failed
> > + if ( ctx->self->priv->retry_count > 5) {
> > + // Clear context
> > + ctx->self->priv->disconnect_pending = NULL;
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_TOO_MANY,
> > + "Disconnection attempt
> timed out");
> > +
> > + disconnect_3gpp_context_complete_and_free (ctx);
> > + return;
> > + }
> > +
> > + //Has call back to next state.
> > + send_swwan_disconnect_command(ctx);
> > +
> > + //Only try to detach so many times.
> > + ctx->self->priv->retry_count++;
> > +
> > + return;
> > + case DISCONNECT_3GPP_CONTEXT_STEP_CONNECTION_STATUS:
> > + //Has call back to next state.
> > + send_swwan_read_command(ctx);
> > + return;
> > +
> > + case DISCONNECT_3GPP_CONTEXT_STEP_DONE:
> > + // Clear context
> > + ctx->self->priv->disconnect_pending = NULL;
> > + ctx->self->priv->pdp_cid = NULL;
> > + ctx->self->priv->swwan_read_write = -1;
> > + // Set data port as result
> > + g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> > + disconnect_3gpp_context_complete_and_free (ctx);
> > +
> > + return;
> > + }
> > +}
> > +
> > +static void
> > +disconnect_3gpp (MMBroadbandBearer *self,
> > + MMBroadbandModem *modem,
> > + MMPortSerialAt *primary,
> > + MMPortSerialAt *secondary,
> > + MMPort *data,
> > + guint cid,
> > + GAsyncReadyCallback callback,
> > + gpointer user_data)
> > +{
> > + Control3gppContext *ctx;
> > + MMPort *port;
> > +
> > + g_assert (primary != NULL);
> > +
> > + ctx = g_slice_new0 (Control3gppContext);
> > + ctx->self = g_object_ref (self);
> > + ctx->modem = MM_BASE_MODEM (g_object_ref (modem));
> > + ctx->result = g_simple_async_result_new (G_OBJECT (self),
> > + callback,
> > + user_data,
> > + disconnect_3gpp);
> > + ctx->disconnect = DISCONNECT_3GPP_CONTEXT_STEP_INIT;
> > +
> > + g_assert (ctx->self->priv->connect_pending == NULL);
> > + g_assert (ctx->self->priv->disconnect_pending == NULL);
> > +
> > + //TODO: Not sure how else to get active data port? Can this be done
> without adding this
> > + //function to mm-base-modem.c?
> > + //TODO: Dual SIM how do we know which interface to grab/disconnect
> if two are active?
> > + port = mm_base_modem_peek_current_data_port (MM_BASE_MODEM
> (modem), MM_PORT_TYPE_NET);
> > + if (!port) {
> > + g_simple_async_report_error_in_idle (G_OBJECT (self),
> > + callback,
> > + user_data,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_NOT_FOUND,
> > + "No valid data port found
> to tear down.");
> > + return;
> > + }
> > +
> > + ctx->data = g_object_ref (port);
> > +
> > + ctx->primary = get_dial_port (MM_BROADBAND_MODEM_CINTERION
> (ctx->modem), data, primary);
> > +
> > + /* Start! */
> > + disconnect_3gpp_context_step (ctx);
> > +}
> > +
> > +/**********************************************************
> *******************/
> > +
> > +static gboolean
> > +network_disconnect_3gpp_delayed (MMBroadbandBearerCinterion *self)
> > +{
> > + mm_dbg ("Disconnect bearer '%s' on network request.",
> > + mm_base_bearer_get_path (MM_BASE_BEARER (self)));
> > +
> > + self->priv->network_disconnect_pending_id = 0;
> > + mm_base_bearer_report_connection_status (MM_BASE_BEARER (self),
> > +
> MM_BEARER_CONNECTION_STATUS_DISCONNECTED);
> > + return G_SOURCE_REMOVE;
> > +}
> > +
> > +//TODO: Test this.
> > +static void
> > +report_connection_status (MMBaseBearer *bearer,
> > + MMBearerConnectionStatus status)
> > +{
>
> report_connection_status() will be called by the MODEM object when a
> specific unsolicited message is detected in the TTY port. If you don't
> setup an unsolicited message handler to catch disconnections, you don't
> need this.
>
> > + MMBroadbandBearerCinterion *self = MM_BROADBAND_BEARER_CINTERION
> (bearer);
> > +
> > + g_assert (status == MM_BEARER_CONNECTION_STATUS_CONNECTED ||
> > + status == MM_BEARER_CONNECTION_STATUS_DISCONNECTING ||
> > + status == MM_BEARER_CONNECTION_STATUS_DISCONNECTED);
> > +
> > + /* When a pending connection / disconnection attempt is in
> progress, we use
> > + * ^SWWAN? to check the connection status and thus temporarily
> ignore
> > + * unsolicited messages */
> > + if (self->priv->connect_pending || self->priv->disconnect_pending)
> > + return;
> > +
> > + /* Ignore 'CONNECTED' */
> > + if (status == MM_BEARER_CONNECTION_STATUS_CONNECTED)
> > + return;
> > +
> > + /* We already use ^SWWAN? to poll the connection status, so only
> > + * handle network-initiated disconnection here. */
> > + if (status == MM_BEARER_CONNECTION_STATUS_DISCONNECTING) {
> > + /* MM_BEARER_CONNECTION_STATUS_DISCONNECTING is used to
> indicate that the
> > + * reporting of disconnection should be delayed. See
> MMBroadbandModemCinterion's
> > + * bearer_report_connection_status for details. */
> > + if (mm_base_bearer_get_status (bearer) ==
> MM_BEARER_STATUS_CONNECTED &&
> > + self->priv->network_disconnect_pending_id == 0) {
> > + mm_dbg ("Delay network-initiated disconnection of bearer
> '%s'",
> > + mm_base_bearer_get_path (MM_BASE_BEARER (self)));
> > + self->priv->network_disconnect_pending_id =
> (g_timeout_add_seconds (
> > + 4,
> > +
> (GSourceFunc) network_disconnect_3gpp_delayed,
> > + self));
> > + }
> > + return;
> > + }
> > +
> > + /* Report disconnected right away */
> > + MM_BASE_BEARER_CLASS (mm_broadband_bearer_cinterion_parent_class)->report_connection_status
> (
> > + bearer,
> > + MM_BEARER_CONNECTION_STATUS_DISCONNECTED);
> > +}
> > +
> > +/**********************************************************
> *******************/
> > +
> > +MMBaseBearer *
> > +mm_broadband_bearer_cinterion_new_finish (GAsyncResult *res,
> > + GError **error)
> > +{
> > + GObject *bearer;
> > + GObject *source;
> > +
> > + source = g_async_result_get_source_object (res);
> > + bearer = g_async_initable_new_finish (G_ASYNC_INITABLE (source),
> res, error);
> > + g_object_unref (source);
> > +
> > + if (!bearer)
> > + return NULL;
> > +
> > + /* Only export valid bearers */
> > + mm_base_bearer_export (MM_BASE_BEARER (bearer));
> > +
> > + return MM_BASE_BEARER (bearer);
> > +}
> > +
> > +static void
> > +dispose (GObject *object)
> > +{
> > + MMBroadbandBearerCinterion *self = MM_BROADBAND_BEARER_CINTERION
> (object);
> > +
> > + if (self->priv->network_disconnect_pending_id != 0) {
> > + g_source_remove (self->priv->network_disconnect_pending_id);
> > + self->priv->network_disconnect_pending_id = 0;
> > + }
> > +
> > + G_OBJECT_CLASS (mm_broadband_bearer_cinterion_parent_class)->dispose
> (object);
> > +}
> > +
> > +void
> > +mm_broadband_bearer_cinterion_new (MMBroadbandModemCinterion *modem,
> > + MMBearerProperties *config,
> > + GCancellable *cancellable,
> > + GAsyncReadyCallback callback,
> > + gpointer user_data)
> > +{
> > + g_async_initable_new_async (
> > + MM_TYPE_BROADBAND_BEARER_CINTERION,
> > + G_PRIORITY_DEFAULT,
> > + cancellable,
> > + callback,
> > + user_data,
> > + MM_BASE_BEARER_MODEM, modem,
> > + MM_BASE_BEARER_CONFIG, config,
> > + NULL);
> > +}
> > +
> > +static void
> > +mm_broadband_bearer_cinterion_init (MMBroadbandBearerCinterion *self)
> > +{
> > + // Initialize private data
> > + self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self,
> > + MM_TYPE_BROADBAND_BEARER_
> CINTERION,
> > +
> MMBroadbandBearerCinterionPrivate);
> > +}
> > +
> > +static void
> > +mm_broadband_bearer_cinterion_class_init (
> MMBroadbandBearerCinterionClass *klass)
> > +{
> > + GObjectClass *object_class = G_OBJECT_CLASS (klass);
> > + MMBaseBearerClass *base_bearer_class = MM_BASE_BEARER_CLASS (klass);
> > + MMBroadbandBearerClass *broadband_bearer_class =
> MM_BROADBAND_BEARER_CLASS (klass);
> > +
> > + g_type_class_add_private (object_class, sizeof (
> MMBroadbandBearerCinterionPrivate));
> > +
> > + object_class->dispose = dispose;
> > + base_bearer_class->report_connection_status =
> report_connection_status; //TODO:What triggers this?
> > +
> > + broadband_bearer_class->connect_3gpp = connect_3gpp;
> > + broadband_bearer_class->connect_3gpp_finish = connect_3gpp_finish;
> > + broadband_bearer_class->disconnect_3gpp = disconnect_3gpp;
> > + broadband_bearer_class->disconnect_3gpp_finish =
> disconnect_3gpp_finish;
> > +}
> > diff --git a/plugins/cinterion/mm-broadband-bearer-cinterion.h
> b/plugins/cinterion/mm-broadband-bearer-cinterion.h
> > new file mode 100644
> > index 0000000..8f315db
> > --- /dev/null
> > +++ b/plugins/cinterion/mm-broadband-bearer-cinterion.h
> > @@ -0,0 +1,56 @@
> > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4
> -*- */
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details:
> > + *
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > + * Author: Matthew Stanger <Matthew_Stanger at trimble.com>
> > + */
> > +
> > +#ifndef MM_BROADBAND_BEARER_CINTERION_H
> > +#define MM_BROADBAND_BEARER_CINTERION_H
> > +
> > +#include <glib.h>
> > +#include <glib-object.h>
> > +
> > +#include "mm-broadband-bearer.h"
> > +#include "mm-broadband-modem-cinterion.h"
> > +
> > +#define MM_TYPE_BROADBAND_BEARER_CINTERION
> (mm_broadband_bearer_cinterion_get_type ())
> > +#define MM_BROADBAND_BEARER_CINTERION(obj)
> (G_TYPE_CHECK_INSTANCE_CAST ((obj), MM_TYPE_BROADBAND_BEARER_CINTERION,
> MMBroadbandBearerCinterion))
> > +#define MM_BROADBAND_BEARER_CINTERION_CLASS(klass)
> (G_TYPE_CHECK_CLASS_CAST ((klass), MM_TYPE_BROADBAND_BEARER_CINTERION,
> MMBroadbandBearerCinterionClass))
> > +#define MM_IS_BROADBAND_BEARER_CINTERION(obj)
> (G_TYPE_CHECK_INSTANCE_TYPE ((obj), MM_TYPE_BROADBAND_BEARER_CINTERION))
> > +#define MM_IS_BROADBAND_BEARER_CINTERION_CLASS(klass)
> (G_TYPE_CHECK_CLASS_TYPE ((klass), MM_TYPE_BROADBAND_BEARER_CINTERION))
> > +#define MM_BROADBAND_BEARER_CINTERION_GET_CLASS(obj)
> (G_TYPE_INSTANCE_GET_CLASS ((obj), MM_TYPE_BROADBAND_BEARER_CINTERION,
> MMBroadbandBearerCinterionClass))
> > +
> > +typedef struct _MMBroadbandBearerCinterion MMBroadbandBearerCinterion;
> > +typedef struct _MMBroadbandBearerCinterionClass
> MMBroadbandBearerCinterionClass;
> > +typedef struct _MMBroadbandBearerCinterionPrivate
> MMBroadbandBearerCinterionPrivate;
> > +
> > +struct _MMBroadbandBearerCinterion {
> > + MMBroadbandBearer parent;
> > + MMBroadbandBearerCinterionPrivate *priv;
> > +};
> > +
> > +struct _MMBroadbandBearerCinterionClass {
> > + MMBroadbandBearerClass parent;
> > +};
> > +
> > +GType mm_broadband_bearer_cinterion_get_type (void);
> > +
> > +void mm_broadband_bearer_cinterion_new
> (MMBroadbandModemCinterion *modem,
> > + MMBearerProperties
> *config,
> > + GCancellable
> *cancellable,
> > +
> GAsyncReadyCallback callback,
> > + gpointer
> user_data);
> > +MMBaseBearer *mm_broadband_bearer_cinterion_new_finish (GAsyncResult
> *res,
> > + GError **error);
> > +
> > +#endif /* MM_BROADBAND_BEARER_CINTERION_H */
> > diff --git a/plugins/cinterion/mm-broadband-modem-cinterion.c
> b/plugins/cinterion/mm-broadband-modem-cinterion.c
> > index 4882a41..a7b8afc 100644
> > --- a/plugins/cinterion/mm-broadband-modem-cinterion.c
> > +++ b/plugins/cinterion/mm-broadband-modem-cinterion.c
> > @@ -12,7 +12,9 @@
> > *
> > * Copyright (C) 2011 Ammonit Measurement GmbH
> > * Copyright (C) 2011 Google Inc.
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > * Author: Aleksander Morgado <aleksander at lanedo.com>
> > + * Contributor: Matthew Stanger <matthew_stanger at trimble.com>
> > */
> >
> > #include <config.h>
> > @@ -22,6 +24,8 @@
> > #include <string.h>
> > #include <unistd.h>
> > #include <ctype.h>
> > +#include <gudev/gudev.h>
> > +
> >
> > #include "ModemManager.h"
> > #include "mm-modem-helpers.h"
> > @@ -36,6 +40,7 @@
> > #include "mm-broadband-modem-cinterion.h"
> > #include "mm-modem-helpers-cinterion.h"
> > #include "mm-common-cinterion.h"
> > +#include "mm-broadband-bearer-cinterion.h"
> >
> > static void iface_modem_init (MMIfaceModem *iface);
> > static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface);
> > @@ -50,6 +55,12 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemCinterion,
> mm_broadband_modem_cinterion,
> > G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_MESSAGING,
> iface_modem_messaging_init)
> > G_IMPLEMENT_INTERFACE
> (MM_TYPE_IFACE_MODEM_LOCATION, iface_modem_location_init))
> >
> > +typedef enum {
> > + FEATURE_SUPPORT_UNKNOWN,
> > + FEATURE_NOT_SUPPORTED,
> > + FEATURE_SUPPORTED
> > +} FeatureSupport;
> > +
> > struct _MMBroadbandModemCinterionPrivate {
> > /* Flag to know if we should try AT^SIND or not to get psinfo */
> > gboolean sind_psinfo;
> > @@ -69,6 +80,9 @@ struct _MMBroadbandModemCinterionPrivate {
> > GArray *cnmi_supported_bm;
> > GArray *cnmi_supported_ds;
> > GArray *cnmi_supported_bfr;
> > +
> > + FeatureSupport swwan_support;
> > +
> > };
> >
> > /***********************************************************
> ******************/
> > @@ -117,7 +131,7 @@ cnmi_test_ready (MMBaseModem *self,
> > {
> > GError *error = NULL;
> >
> > - mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res,
> &error);
> > + mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res,
> &error);
> > if (error)
> > g_simple_async_result_take_error (simple, error);
> > else
> > @@ -727,10 +741,16 @@ get_access_technology_from_psinfo (const gchar
> *psinfo,
> > case 9:
> > case 10:
> > return (MM_MODEM_ACCESS_TECHNOLOGY_HSDPA |
> MM_MODEM_ACCESS_TECHNOLOGY_HSUPA);
> > + case 16:
> > + case 17:
> > + return MM_MODEM_ACCESS_TECHNOLOGY_LTE;
> > default:
> > + mm_dbg ("Unable to identify access technology in case:%i",
> psinfoval);
> > break;
> > }
> > }
> > + else
> > + mm_err ("FAILED get_access_technology_from_psinfo-int");
> >
> > g_set_error (error,
> > MM_CORE_ERROR,
> > @@ -1648,6 +1668,259 @@ setup_ports (MMBroadbandModem *self)
> > }
> >
> > /***********************************************************
> ******************/
> > +/* Create Bearer (Modem interface) */
> > +
> > +typedef struct {
> > + MMBroadbandModemCinterion *self;
> > + GSimpleAsyncResult *result;
> > + MMBearerProperties *properties;
> > +} CreateBearerContext;
> > +
> > +static void
> > +create_bearer_context_complete_and_free (CreateBearerContext *ctx)
> > +{
> > + g_simple_async_result_complete (ctx->result);
> > + g_object_unref (ctx->result);
> > + g_object_unref (ctx->self);
> > + g_object_unref (ctx->properties);
> > + g_slice_free (CreateBearerContext, ctx);
> > +}
> > +
> > +static MMBaseBearer *
> > +cinterion_modem_create_bearer_finish (MMIfaceModem *self,
> > + GAsyncResult *res,
> > + GError **error)
> > +{
> > + MMBaseBearer *bearer;
> > +
> > + if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
> (res), error))
> > + return NULL;
> > +
> > + bearer = g_simple_async_result_get_op_res_gpointer
> (G_SIMPLE_ASYNC_RESULT (res));
> > + mm_dbg ("New cinterion bearer created at DBus path '%s'",
> mm_base_bearer_get_path (bearer));
> > + return g_object_ref (bearer);
> > +}
> > +
> > +static void
> > +broadband_bearer_cinterion_new_ready (GObject *source,
> > + GAsyncResult *res,
> > + CreateBearerContext *ctx)
> > +{
> > + MMBaseBearer *bearer;
> > + GError *error = NULL;
> > +
> > + bearer = mm_broadband_bearer_cinterion_new_finish (res, &error);
> > + if (!bearer)
> > + g_simple_async_result_take_error (ctx->result, error);
> > + else
> > + g_simple_async_result_set_op_res_gpointer (ctx->result,
> bearer, (GDestroyNotify)g_object_unref);
> > + create_bearer_context_complete_and_free (ctx);
> > +}
> > +
> > +static void
> > +broadband_bearer_new_ready (GObject *source,
> > + GAsyncResult *res,
> > + CreateBearerContext *ctx)
> > +{
> > + MMBaseBearer *bearer;
> > + GError *error = NULL;
> > +
> > + bearer = mm_broadband_bearer_new_finish (res, &error);
> > + if (!bearer)
> > + g_simple_async_result_take_error (ctx->result, error);
> > + else
> > + g_simple_async_result_set_op_res_gpointer (ctx->result,
> bearer, (GDestroyNotify)g_object_unref);
> > + create_bearer_context_complete_and_free (ctx);
> > +}
> > +
> > +static void
> > +create_bearer_for_net_port (CreateBearerContext *ctx)
> > +{
> > + switch (ctx->self->priv->swwan_support) {
> > + case FEATURE_SUPPORT_UNKNOWN:
> > + g_assert_not_reached ();
> > + case FEATURE_NOT_SUPPORTED:
> > + mm_dbg ("^SWWAN not supported, creating default bearer...");
> > + mm_broadband_bearer_new (MM_BROADBAND_MODEM (ctx->self),
> > + ctx->properties,
> > + NULL, /* cancellable */
> > + (GAsyncReadyCallback)
> broadband_bearer_new_ready,
> > + ctx);
> > + return;
> > + case FEATURE_SUPPORTED:
> > + mm_dbg ("^SWWAN supported, creating cinterion bearer...");
> > + mm_broadband_bearer_cinterion_new
> (MM_BROADBAND_MODEM_CINTERION (ctx->self),
> > + ctx->properties,
> > + NULL, /* cancellable */
> > + (GAsyncReadyCallback)
> broadband_bearer_cinterion_new_ready,
> > + ctx);
> > + return;
> > + }
> > +}
> > +
> > +static MMPortSerialAt *
> > +peek_port_at_for_data (MMBroadbandModemCinterion *self,
> > + MMPort *port)
> > +{
> > + GList *cdc_wdm_at_ports, *l;
>
> cdc_wdm_at_ports doesn't look like a proper name, as you don't have any
> cdc-wdm port, just TTYs :)
>
> > + const gchar *net_port_parent_path;
> > +
> > + g_warn_if_fail (mm_port_get_subsys (port) == MM_PORT_SUBSYS_NET);
> > + net_port_parent_path = mm_port_get_parent_path (port);
> > +
> > + if (!net_port_parent_path) {
> > + g_warning ("(%s) no parent path for net port",
> mm_port_get_device (port));
> > + return NULL;
> > + }
> > +
> > + //Find the port to send AT commands on...
> > + cdc_wdm_at_ports = mm_base_modem_find_ports (MM_BASE_MODEM (self),
> > + MM_PORT_SUBSYS_USB,
> > + MM_PORT_TYPE_NET,
> > + NULL);
> > +
> > +
> > + for (l = cdc_wdm_at_ports; l; l = l->next) {
> > + const gchar *wdm_port_parent_path;
> > +
> > + g_assert (MM_IS_PORT_SERIAL_AT (l->data));
> > + wdm_port_parent_path = mm_port_get_parent_path (MM_PORT
> (l->data));
> > +
> > + if (wdm_port_parent_path && g_str_equal (wdm_port_parent_path,
> net_port_parent_path))
> > + return MM_PORT_SERIAL_AT (l->data);
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +MMPortSerialAt *
> > +mm_broadband_modem_cinterion_peek_port_at_for_data
> (MMBroadbandModemCinterion *self,
> > + MMPort *port)
> > +{
> > + MMPortSerialAt *found;
> > +
> > + g_assert (self->priv->swwan_support == FEATURE_SUPPORTED);
> > +
> > + found = peek_port_at_for_data (self, port);
> > + if (!found)
> > + mm_warn ("Couldn't find associated cdc-wdm port for 'net/%s'",
> > + mm_port_get_device (port));
> > + return found;
> > +}
> > +
> > +static void
> > +ensure_swwan_support_checked (MMBroadbandModemCinterion *self,
> > + MMPort *port)
> > +{
> > + GUdevClient *client;
> > + GUdevDevice *data_device;
> > +
> > + /* Initialize the swwan feature */
> > + if (self->priv->swwan_support != FEATURE_SUPPORT_UNKNOWN)
> > + return;
> > +
> > + /* First, check for devices which could support SWWAN. They
> > + * will be tagged by udev as net devices. */
> > + client = g_udev_client_new (NULL);
> > + data_device = (g_udev_client_query_by_subsystem_and_name (
> > + client,
> > + "net",
> > + mm_port_get_device (port)));
> > +
> > + /* udevadm info for Cinterion PLS8-X v3.017 Modem
> > + P: /devices/pci0000:00/0000:00:14.0/usb3/3-2/3-2:1.10/net/usb0
> > + E: DEVPATH=/devices/pci0000:00/0000:00:14.0/usb3/3-2/3-2:1.
> 10/net/usb0
> > + E: ID_BUS=usb
> > + E: ID_MM_CANDIDATE=1
> > + E: ID_MODEL=LTE_Modem
> > + E: ID_MODEL_ENC=LTE\x20Modem
> > + E: ID_MODEL_FROM_DATABASE=2.0 root hub
> > + E: ID_MODEL_ID=0061
> > + E: ID_NET_NAME_MAC=enxdeadbeef0000
> > + E: ID_NET_NAME_PATH=enp0s20u2i10
> > + E: ID_REVISION=0232
> > + E: ID_SERIAL=Cinterion_LTE_Modem
> > + E: ID_TYPE=generic
> > + E: ID_USB_DRIVER=cdc_ether
> > + E: ID_USB_INTERFACES=:020201:0a0000:020600:
> > + E: ID_USB_INTERFACE_NUM=0a
> > + E: ID_VENDOR=Cinterion
> > + E: ID_VENDOR_ENC=Cinterion
> > + E: ID_VENDOR_FROM_DATABASE=Linux Foundation
> > + E: ID_VENDOR_ID=1e2d
> > + E: IFINDEX=11
> > + E: INTERFACE=usb0
> > + E: SUBSYSTEM=net
> > + E: USEC_INITIALIZED=4142603761
> > +
> > + P: /devices/pci0000:00/0000:00:14.0/usb3/3-2/3-2:1.11
> > + E: DEVPATH=/devices/pci0000:00/0000:00:14.0/usb3/3-2/3-2:1.11
> > + E: DEVTYPE=usb_interface
> > + E: DRIVER=cdc_ether
> > + E: ID_MODEL_FROM_DATABASE=2.0 root hub
> > + E: ID_VENDOR_FROM_DATABASE=Linux Foundation
> > + E: INTERFACE=10/0/0
> > + E: MODALIAS=usb:v1E2Dp0061d0232dc00dsc00dp00ic0Aisc00ip00in0B
> > + E: PRODUCT=1e2d/61/232
> > + E: SUBSYSTEM=usb
> > + E: TYPE=0/0/0
> > + E: USEC_INITIALIZED=604142604196
> > + */
> > +
> > + //Assumption - Cinterion modems that use cdc_ether will support the
> swwan via usb ethernet.
> > + //Cinterion alluded to this idea when I spoke with their
> engineering support but stopped short
> > + //of being able to confirm if all future/current models will be
> this way.
> > + if (data_device && g_str_equal(g_udev_device_get_property(data_device,
> "ID_USB_DRIVER"),"cdc_ether")) {
> > + mm_dbg ("This device (%s) can support swwan feature",
> mm_port_get_device (port));
> > + self->priv->swwan_support = FEATURE_SUPPORTED;
> > + }
> > + else
> > + {
> > + self->priv->swwan_support = FEATURE_NOT_SUPPORTED;
> > + mm_dbg ("This device (%s) can not support swwan feature",
> mm_port_get_device (port));
> > + }
> > +
> > + /* Free the g_object*/
> > + if (data_device)
> > + g_object_unref (data_device);
> > + if (client)
> > + g_object_unref (client);
> > +}
> > +
> > +static void
> > +cinterion_modem_create_bearer (MMIfaceModem *self,
> > + MMBearerProperties *properties,
> > + GAsyncReadyCallback callback,
> > + gpointer user_data)
> > +{
> > + CreateBearerContext *ctx;
> > + MMPort *port;
> > +
> > + ctx = g_slice_new0 (CreateBearerContext);
> > + ctx->self = g_object_ref (self);
> > + ctx->properties = g_object_ref (properties);
> > + ctx->result = g_simple_async_result_new (G_OBJECT (self),
> > + callback,
> > + user_data,
> > +
> cinterion_modem_create_bearer);
> > +
> > + port = mm_base_modem_peek_best_data_port (MM_BASE_MODEM (self),
> MM_PORT_TYPE_NET);
> > +
> > + if (port) {
> > + ensure_swwan_support_checked (ctx->self, port);
> > + create_bearer_for_net_port (ctx);
> > + return;
> > + }
> > +
> > + mm_dbg ("Creating default bearer...");
> > + mm_broadband_bearer_new (MM_BROADBAND_MODEM (self),
> > + properties,
> > + NULL, /* cancellable */
> > + (GAsyncReadyCallback)
> broadband_bearer_new_ready,
> > + ctx);
> > +}
> > +
> > +/**********************************************************
> *******************/
> >
> > MMBroadbandModemCinterion *
> > mm_broadband_modem_cinterion_new (const gchar *device,
> > @@ -1675,6 +1948,7 @@ mm_broadband_modem_cinterion_init
> (MMBroadbandModemCinterion *self)
> >
> > /* Set defaults */
> > self->priv->sind_psinfo = TRUE; /* Initially, always try to get
> psinfo */
> > + self->priv->swwan_support = FEATURE_SUPPORT_UNKNOWN;
> > }
> >
> > static void
> > @@ -1704,6 +1978,8 @@ iface_modem_init (MMIfaceModem *iface)
> > {
> > iface_modem_parent = g_type_interface_peek_parent (iface);
> >
> > + iface->create_bearer = cinterion_modem_create_bearer;
> > + iface->create_bearer_finish = cinterion_modem_create_bearer_finish;
> > iface->load_supported_modes = load_supported_modes;
> > iface->load_supported_modes_finish = load_supported_modes_finish;
> > iface->set_current_modes = set_current_modes;
> > @@ -1764,7 +2040,6 @@ mm_broadband_modem_cinterion_class_init (MMBroadbandModemCinterionClass
> *klass)
> > {
> > GObjectClass *object_class = G_OBJECT_CLASS (klass);
> > MMBroadbandModemClass *broadband_modem_class =
> MM_BROADBAND_MODEM_CLASS (klass);
> > -
> > g_type_class_add_private (object_class, sizeof (
> MMBroadbandModemCinterionPrivate));
> >
> > /* Virtual methods */
> > diff --git a/plugins/cinterion/mm-broadband-modem-cinterion.h
> b/plugins/cinterion/mm-broadband-modem-cinterion.h
> > index 47f0dcb..b675f3d 100644
> > --- a/plugins/cinterion/mm-broadband-modem-cinterion.h
> > +++ b/plugins/cinterion/mm-broadband-modem-cinterion.h
> > @@ -48,4 +48,7 @@ MMBroadbandModemCinterion
> *mm_broadband_modem_cinterion_new (const gchar *device
> > guint16
> vendor_id,
> > guint16
> product_id);
> >
> > +MMPortSerialAt *mm_broadband_modem_cinterion_peek_port_at_for_data
> (MMBroadbandModemCinterion *self,
> > + MMPort
> *port);
> > +
> > #endif /* MM_BROADBAND_MODEM_CINTERION_H */
> > diff --git a/plugins/cinterion/mm-common-cinterion.c
> b/plugins/cinterion/mm-common-cinterion.c
>
> All the changes here look like GPS related, shouldn't go in this patch.
>
> > index 6e4da70..c9119ec 100644
> > --- a/plugins/cinterion/mm-common-cinterion.c
> > +++ b/plugins/cinterion/mm-common-cinterion.c
> > @@ -11,13 +11,15 @@
> > * GNU General Public License for more details:
> > *
> > * Copyright (C) 2014 Ammonit Measurement GmbH
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > * Author: Aleksander Morgado <aleksander at aleksander.es>
> > + * Contributor: Matthew Stanger <matthew_stanger at trimble.com>
> > */
> >
> > #include "mm-common-cinterion.h"
> > #include "mm-base-modem-at.h"
> > +#include "mm-log.h"
> >
> > -static MMIfaceModemLocation *iface_modem_location_parent;
> >
> > /***********************************************************
> ******************/
> >
> > @@ -25,11 +27,40 @@ static MMIfaceModemLocation
> *iface_modem_location_parent;
> > static GQuark cinterion_location_context_quark;
> >
> > /***********************************************************
> ******************/
> > +/* Def's and Enum's */
> > +
> > +typedef enum {
> > + GPS_CONTEXT_STEP_SGPSS = 0,
> > + GPS_CONTEXT_STEP_SGPSC_ANTENNA,
> > + GPS_CONTEXT_STEP_SGPSC_ENGINE,
> > + GPS_CONTEXT_STEP_SGPSC_OUTPUT,
> > + GPS_CONTEXT_STEP_DONE,
> > +} ConnectGpsContextStep;
> > +
> > +typedef struct {
> > + MMBaseModem *self;
> > + GSimpleAsyncResult *result;
> > + MMModemLocationSource source;
> > + ConnectGpsContextStep enable_state;
> > + ConnectGpsContextStep disable_state;
> > + gpointer connect_pending;
> > + gpointer disconnect_pending;
> > + gint gps_retry;
> > +} LocationGatheringContext;
> >
> > typedef struct {
> > MMModemLocationSource enabled_sources;
> > } LocationContext;
> >
> > +static MMIfaceModemLocation *iface_modem_location_parent;
> > +static void try_gps_enable (MMIfaceModemLocation *self,
> > + LocationGatheringContext *ctx);
> > +static void try_gps_disable (MMIfaceModemLocation *self,
> > + LocationGatheringContext *ctx);
> > +static void send_gps_command (MMIfaceModemLocation *self,
> > + LocationGatheringContext *ctx,
> > + gchar **command);
> > +
> > static void
> > location_context_free (LocationContext *ctx)
> > {
> > @@ -127,13 +158,7 @@ mm_common_cinterion_location_load_capabilities
> (MMIfaceModemLocation *self,
> > }
> >
> > /***********************************************************
> ******************/
> > -/* Enable/Disable location gathering (Location interface) */
> > -
> > -typedef struct {
> > - MMBaseModem *self;
> > - GSimpleAsyncResult *result;
> > - MMModemLocationSource source;
> > -} LocationGatheringContext;
> > +/* Common location gathering (Location interface) */
> >
> > static void
> > location_gathering_context_complete_and_free (LocationGatheringContext
> *ctx)
> > @@ -144,6 +169,145 @@ location_gathering_context_complete_and_free
> (LocationGatheringContext *ctx)
> > g_slice_free (LocationGatheringContext, ctx);
> > }
> >
> > +static void
> > +location_gathering_context_complete_and_free_full
> (LocationGatheringContext *ctx)
> > +{
> > + //It's importiant that this parent function remains protected by
> calls where
> > + //both context '*_pending' pointers could be active.
> > + if (ctx->connect_pending != NULL)
> > + ctx->connect_pending = NULL;
> > + else
> > + ctx->disconnect_pending = NULL;
> > +
> > + location_gathering_context_complete_and_free(ctx);
> > +}
> > +
> > +static void
> > +promote_gps_state_from_response (MMBaseModem *self,
> > + GAsyncResult *res,
> > + LocationGatheringContext *ctx)
> > +{
> > + GError *error = NULL;
> > + ConnectGpsContextStep state;
> > + gchar *result;
> > +
> > + //Adjust for whether we're dis/en-ableing
> > + if (ctx->connect_pending != NULL)
> > + state = ctx->enable_state;
> > + else
> > + {
> > + g_assert(ctx->disconnect_pending != NULL);
> > + state = ctx->disable_state;
> > + }
> > +
> > + //We see that the 'Engine,1' command (PLS8) fails for no apparent
> reason
> > + //on ATMEL AT91SAM9263 ARM & not x86. See
> 'mm_common_cinterion_setup_gps_port'
> > + //comments.
> > + result = g_strdup (mm_base_modem_at_command_full_finish (self,
> res, &error));
> > +
> > + //If we get an error don't progress at first, try the same state
> again.
> > + if ((!result && state != GPS_CONTEXT_STEP_SGPSS && ctx-> gps_retry
> < 3))
> > + {
> > + state = GPS_CONTEXT_STEP_SGPSS;
> > + ctx->gps_retry++;
> > + }
> > + else {
> > +
> > +
> > + switch (state) {
> > +
> > + //First step try's the legacy SGPSS command. If it fails don't
> error out
> > + //continue on to send new Cinterion GPS commands and see if they
> work.
> > + case GPS_CONTEXT_STEP_SGPSS: {
> > + if (!result) {
> > + mm_info ("SGPSS command failed, will try another Cinterion
> GPS command.");
> > + state = GPS_CONTEXT_STEP_SGPSC_ANTENNA;
> > + }
> > + else
> > + state = GPS_CONTEXT_STEP_DONE;
> > +
> > + break;
> > + }
> > + //If the new GPS enable comand fails then exit we are out of things
> to try.
> > + //Only connect flow cares about errors.
> > + case GPS_CONTEXT_STEP_SGPSC_ANTENNA: {
> > + if (!result && ctx->disconnect_pending == NULL) {
> > + mm_info ("SGPSC command failed, we are out of things to
> try.");
> > + g_simple_async_result_take_error (ctx->result, error);
> > + location_gathering_context_complete_and_free_full (ctx);
> > + return;
> > + }
> > + else
> > + state = GPS_CONTEXT_STEP_SGPSC_ENGINE;
> > +
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_SGPSC_ENGINE: {
> > + if (!result && ctx->disconnect_pending == NULL) {
> > + g_simple_async_result_take_error (ctx->result, error);
> > + location_gathering_context_complete_and_free_full (ctx);
> > + return;
> > + }
> > + else
> > + state = GPS_CONTEXT_STEP_SGPSC_OUTPUT;
> > +
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_SGPSC_OUTPUT: {
> > + if (!result && ctx->disconnect_pending == NULL) {
> > + g_simple_async_result_take_error (ctx->result, error);
> > + location_gathering_context_complete_and_free_full (ctx);
> > + return;
> > + }
> > + else
> > + state = GPS_CONTEXT_STEP_DONE;
> > +
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_DONE: {
> > + //Shouldn't get here, fall into default error.
> > + }
> > + default: {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_WRONG_STATE,
> > + "Unknown gps state during
> setup.");
> > +
> > + location_gathering_context_complete_and_free_full (ctx);
> > + g_free(result);
> > + return;
> > + }
> > + }
> > + }
> > +
> > + //Save state and go back to dis/en-ableing
> > + if (ctx->connect_pending != NULL) {
> > + ctx->enable_state = state;
> > + try_gps_enable(ctx->connect_pending, ctx);
> > + }
> > + else {
> > + ctx->disable_state = state;
> > + try_gps_disable(ctx->disconnect_pending, ctx);
> > + }
> > + g_free(result);
> > +}
> > +
> > +static void
> > +send_gps_command (MMIfaceModemLocation *self,
> > + LocationGatheringContext *ctx,
> > + gchar **command)
> > +{
> > + mm_base_modem_at_command_full (MM_BASE_MODEM (self),
> > + mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (self), NULL),
> > + *command,
> > + 5,
> > + FALSE,
> > + FALSE, /* raw */
> > + NULL, /* cancellable */
> > + (GAsyncReadyCallback)promote_
> gps_state_from_response,
> > + ctx);
> > +}
> > +
> > /******************************/
> > /* Disable location gathering */
> >
> > @@ -157,15 +321,9 @@ mm_common_cinterion_disable_location_gathering_finish
> (MMIfaceModemLocation *sel
> >
> > static void
> > gps_disabled_ready (MMBaseModem *self,
> > - GAsyncResult *res,
> > LocationGatheringContext *ctx)
> > {
> > - GError *error = NULL;
> > -
> > - if (!mm_base_modem_at_command_full_finish (self, res, &error))
> > - g_simple_async_result_take_error (ctx->result, error);
> > - else
> > - g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> > + g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> >
> > /* Only use the GPS port in NMEA/RAW setups */
> > if (ctx->source & (MM_MODEM_LOCATION_SOURCE_GPS_NMEA |
> > @@ -178,11 +336,57 @@ gps_disabled_ready (MMBaseModem *self,
> > mm_port_serial_close (MM_PORT_SERIAL (gps_port));
> > }
> >
> > - location_gathering_context_complete_and_free (ctx);
> > + location_gathering_context_complete_and_free_full (ctx);
> > +}
> > +
> > +static void
> > +try_gps_disable (MMIfaceModemLocation *self,
> > + LocationGatheringContext *ctx)
> > +{
> > + gchar *gps_command;
> > +
> > + switch (ctx->disable_state) {
> > +
> > + //The old command to enable GPS, works on at least PSX8
> > + case GPS_CONTEXT_STEP_SGPSS: {
> > + gps_command = "^SGPSS=0";
> > + break;
> > + }
> > + //Commands Power/Ant, Engine & Output w/ sgpsc are
> > + //used by PLS8-X & E. Since around versions 2.0+
> > + case GPS_CONTEXT_STEP_SGPSC_ANTENNA: {
> > + gps_command = "^SGPSC=\"Power/Antenna\",\"off\"";
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_SGPSC_ENGINE: {
> > + gps_command = "^SGPSC=\"Engine\",\"0\"";
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_SGPSC_OUTPUT: {
> > + gps_command = "^SGPSC=\"NMEA/Output\",\"off\"";
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_DONE: {
> > + gps_disabled_ready(MM_BASE_MODEM (self), ctx);
> > + return;
> > + }
> > + default: {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_WRONG_STATE,
> > + "Unknown gps state during tear
> down.");
> > +
> > + location_gathering_context_complete_and_free_full (ctx);
> > + return;
> > + }
> > + }
> > +
> > + send_gps_command(self, ctx, &gps_command);
> > + return;
> > }
> >
> > static void
> > -internal_disable_location_gathering (LocationGatheringContext *ctx)
> > +internal_disable_location_gathering (MMIfaceModemLocation *self,
> LocationGatheringContext *ctx)
> > {
> > LocationContext *location_ctx;
> > gboolean stop_gps = FALSE;
> > @@ -202,21 +406,30 @@ internal_disable_location_gathering
> (LocationGatheringContext *ctx)
> > }
> >
> > if (stop_gps) {
> > - /* We disable continuous GPS fixes */
> > - mm_base_modem_at_command_full (MM_BASE_MODEM (ctx->self),
> > - mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (ctx->self), NULL),
> > - "AT^SGPSS=0",
> > - 3,
> > - FALSE,
> > - FALSE, /* raw */
> > - NULL, /* cancellable */
> > - (GAsyncReadyCallback)gps_
> disabled_ready,
> > - ctx);
> > + //Don't allow disconnects while trying to connect.
> > + if (ctx->connect_pending != NULL)
> > + {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_IN_PROGRESS,
> > + "Can't disconnect GPS
> while another process is trying to connect it.");
> > +
> > + //*_complete_and_free_full is safe to use for the
> disconnect flow after this point.
> > + ctx->disconnect_pending = NULL;
> > + location_gathering_context_complete_and_free (ctx);
> > + return;
> > + }
> > +
> > + ctx->disable_state = GPS_CONTEXT_STEP_SGPSS;
> > + ctx->disconnect_pending = self;
> > + try_gps_disable(self, ctx);
> > +
> > return;
> > }
> >
> > /* For any other location (e.g. 3GPP), or if still some GPS needed,
> just return */
> > g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> > + ctx->disconnect_pending = NULL;
> > location_gathering_context_complete_and_free (ctx);
> > }
> >
> > @@ -236,12 +449,13 @@ parent_disable_location_gathering_ready
> (MMIfaceModemLocation *self,
> > } else {
> > /* Fatal */
> > g_simple_async_result_take_error (ctx->result, error);
> > + ctx->disconnect_pending = NULL;
> > location_gathering_context_complete_and_free (ctx);
> > return;
> > }
> > }
> >
> > - internal_disable_location_gathering (ctx);
> > + internal_disable_location_gathering (self, ctx);
> > }
> >
> > void
> > @@ -259,6 +473,9 @@ mm_common_cinterion_disable_location_gathering
> (MMIfaceModemLocation *self,
> > user_data,
> >
> mm_common_cinterion_disable_location_gathering);
> > ctx->source = source;
> > + ctx->connect_pending = NULL;
> > + ctx->disconnect_pending = NULL;
> > + ctx->gps_retry = 0;
> >
> > /* Chain up parent's gathering enable */
> > if (iface_modem_location_parent->disable_location_gathering) {
> > @@ -270,7 +487,7 @@ mm_common_cinterion_disable_location_gathering
> (MMIfaceModemLocation *self,
> > return;
> > }
> >
> > - internal_disable_location_gathering (ctx);
> > + internal_disable_location_gathering (self, ctx);
> > }
> >
> > /***********************************************************
> ******************/
> > @@ -286,16 +503,10 @@ mm_common_cinterion_enable_location_gathering_finish
> (MMIfaceModemLocation *self
> >
> > static void
> > gps_enabled_ready (MMBaseModem *self,
> > - GAsyncResult *res,
> > LocationGatheringContext *ctx)
> > {
> > GError *error = NULL;
> >
> > - if (!mm_base_modem_at_command_full_finish (self, res, &error)) {
> > - g_simple_async_result_take_error (ctx->result, error);
> > - location_gathering_context_complete_and_free (ctx);
> > - return;
> > - }
> >
> > /* Only use the GPS port in NMEA/RAW setups */
> > if (ctx->source & (MM_MODEM_LOCATION_SOURCE_GPS_NMEA |
> > @@ -312,12 +523,60 @@ gps_enabled_ready (MMBaseModem *self,
> > MM_CORE_ERROR,
> > MM_CORE_ERROR_FAILED,
> > "Couldn't open raw GPS
> serial port");
> > - } else
> > + }
> > + else
> > g_simple_async_result_set_op_res_gboolean (ctx->result,
> TRUE);
> > - } else
> > +
> > + }
> > + else
> > g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> >
> > - location_gathering_context_complete_and_free (ctx);
> > + location_gathering_context_complete_and_free_full (ctx);
> > +}
> > +
> > +static void
> > +try_gps_enable (MMIfaceModemLocation *self,
> > + LocationGatheringContext *ctx)
> > +{
> > + gchar *gps_command;
> > +
> > + switch (ctx->enable_state) {
> > +
> > + //The old command to enable GPS, works on at least PSX8
> > + case GPS_CONTEXT_STEP_SGPSS: {
> > + gps_command = "^SGPSS=4";
> > + break;
> > + }
> > + //Commands Power/Ant, Engine & Output w/ sgpsc are
> > + //used by PLS8-X & E. Since versions 2.0+
> > + case GPS_CONTEXT_STEP_SGPSC_ANTENNA: {
> > + gps_command = "^SGPSC=\"Power/Antenna\",\"on\"";
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_SGPSC_ENGINE: {
> > + gps_command = "^SGPSC=\"Engine\",\"1\"";
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_SGPSC_OUTPUT: {
> > + gps_command = "^SGPSC=\"NMEA/Output\",\"on\"";
> > + break;
> > + }
> > + case GPS_CONTEXT_STEP_DONE: {
> > + gps_enabled_ready(MM_BASE_MODEM (self), ctx);
> > + return;
> > + }
> > + default: {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_WRONG_STATE,
> > + "Unknown gps state during
> setup.");
> > + location_gathering_context_complete_and_free_full (ctx);
> > + return;
> > + }
> > + }
> > +
> > + send_gps_command(self, ctx, &gps_command);
> > + return;
> > }
> >
> > static void
> > @@ -338,13 +597,13 @@ parent_enable_location_gathering_ready
> (MMIfaceModemLocation *self,
> > } else {
> > /* Fatal */
> > g_simple_async_result_take_error (ctx->result, error);
> > + ctx->connect_pending = NULL;
> > location_gathering_context_complete_and_free (ctx);
> > return;
> > }
> > }
> >
> > /* Now our own enabling */
> > -
> > location_ctx = get_location_context (MM_BASE_MODEM (self));
> >
> > /* NMEA and RAW are both enabled in the same way */
> > @@ -359,22 +618,33 @@ parent_enable_location_gathering_ready
> (MMIfaceModemLocation *self,
> > location_ctx->enabled_sources |= ctx->source;
> > }
> >
> > +
> > if (start_gps) {
> > - /* We enable continuous GPS fixes */
> > - mm_base_modem_at_command_full (MM_BASE_MODEM (self),
> > - mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (self), NULL),
> > - "AT^SGPSS=4",
> > - 3,
> > - FALSE,
> > - FALSE, /* raw */
> > - NULL, /* cancellable */
> > - (GAsyncReadyCallback)gps_
> enabled_ready,
> > - ctx);
> > + //Can only setup/tear down one thing at a time.
> > + if (ctx->disconnect_pending != NULL)
> > + {
> > + g_simple_async_result_set_error (ctx->result,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_IN_PROGRESS,
> > + "Can't setup GPS while
> another process is trying to disconnect it.");
> > +
> > + //*_complete_and_free_full is safe to use for the connect
> flow after this point.
> > + ctx->connect_pending = NULL;
> > + location_gathering_context_complete_and_free (ctx);
> > + return;
> > + }
> > +
> > + ctx->enable_state = GPS_CONTEXT_STEP_SGPSS;
> > + ctx->connect_pending = self;
> > + ctx->gps_retry = 0;
> > + try_gps_enable(self, ctx);
> > +
> > return;
> > }
> >
> > /* For any other location (e.g. 3GPP), or if GPS already running
> just return */
> > g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> > + ctx->connect_pending = NULL;
> > location_gathering_context_complete_and_free (ctx);
> > }
> >
> > @@ -393,6 +663,8 @@ mm_common_cinterion_enable_location_gathering
> (MMIfaceModemLocation *self,
> > user_data,
> > mm_common_cinterion_enable_
> location_gathering);
> > ctx->source = source;
> > + ctx->connect_pending = NULL;
> > + ctx->disconnect_pending = NULL;
> >
> > /* Chain up parent's gathering enable */
> > iface_modem_location_parent->enable_location_gathering (
> > @@ -443,11 +715,41 @@ mm_common_cinterion_setup_gps_port
> (MMBroadbandModem *self)
> > if (gps_data_port) {
> > /* It may happen that the modem was started with GPS already
> enabled, or
> > * maybe ModemManager got rebooted and it was left enabled
> before. We'll make
> > - * sure that it is disabled when we initialize the modem */
> > + * sure that it is disabled when we initialize the modem.
> > + */
> > +
> > + //It may seem odd that we are turning Antenna & Engine on but
> it is critical for the PLS8
> > + //that this happen here. There is a bug that keeps 'Engine on'
> from succeding on
> > + //the first go around in the 'try_gps_enable' flow. Cinterion
> confirmed this a bug. Retry's in
> > + //the connect flow won't help and the only way found so far is
> to enable the engine here,
> > + //before the modem is enabled. This is not ideal and should be
> redone as soon as the root
> > + //cause of the bug is identified. This bug does not appear on
> my x86 platform. -Matt Stanger
> > + //TODO: Fix bug in 'try_gps_enable' where 'Engine on' command
> gets stuck returning 767 errors
> > + //on ARM AT91SAM9263 processor then change these to default
> off/0.
> > mm_base_modem_at_command_full (MM_BASE_MODEM (self),
> > mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (self), NULL),
> > "AT^SGPSS=0",
> > 3, FALSE, FALSE, NULL, NULL,
> NULL);
> > + g_usleep (100);
> > +
> > + mm_base_modem_at_command_full (MM_BASE_MODEM (self),
> > + mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (self), NULL),
> > + "^SGPSC=\"Power/Antenna\",\"
> on\"",
> > + 3, FALSE, FALSE, NULL, NULL,
> NULL);
> > + g_usleep (100);
> > +
> > + mm_base_modem_at_command_full (MM_BASE_MODEM (self),
> > + mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (self), NULL),
> > + "^SGPSC=\"Engine\",\"1\"",
> > + 3, FALSE, FALSE, NULL, NULL,
> NULL);
> > +
> > + g_usleep (100);
> > +
> > + mm_base_modem_at_command_full (MM_BASE_MODEM (self),
> > + mm_base_modem_peek_best_at_port
> (MM_BASE_MODEM (self), NULL),
> > + "^SGPSC=\"NMEA/Output\",\"off\
> "",
> > + 3, FALSE, FALSE, NULL, NULL,
> NULL);
> > +
> >
> > /* Add handler for the NMEA traces */
> > mm_port_serial_gps_add_trace_handler (gps_data_port,
> > diff --git a/plugins/cinterion/mm-modem-helpers-cinterion.c
> b/plugins/cinterion/mm-modem-helpers-cinterion.c
> > index bffe00a..14b18dd 100644
> > --- a/plugins/cinterion/mm-modem-helpers-cinterion.c
> > +++ b/plugins/cinterion/mm-modem-helpers-cinterion.c
> > @@ -10,12 +10,15 @@
> > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > * GNU General Public License for more details:
> > *
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > * Copyright (C) 2014 Aleksander Morgado <aleksander at aleksander.es>
> > + * Contributor: Matthew Stanger <matthew_stanger at trimble.com>
> > */
> >
> > #include <config.h>
> > #include <string.h>
> > #include <stdlib.h>
> > +#include <stdio.h>
> >
> > #include "ModemManager.h"
> > #define _LIBMM_INSIDE_MM
> > @@ -24,6 +27,7 @@
> > #include "mm-charsets.h"
> > #include "mm-errors-types.h"
> > #include "mm-modem-helpers-cinterion.h"
> > +#include "mm-modem-helpers.h"
> >
> > /* Setup relationship between the 3G band bitmask in the modem and the
> bitmask
> > * in ModemManager. */
> > @@ -482,3 +486,187 @@ mm_cinterion_parse_sind_response (const gchar
> *response,
> >
> > return TRUE;
> > }
> > +
> > +/**********************************************************
> *******************/
> > +/* ^DHCP response parser */
> > +
> > +static gboolean
> > +match_info_to_ip4_addr (GMatchInfo *match_info,
> > + guint match_index,
> > + guint *out_addr)
> > +{
> > + gchar *s, *bin = NULL;
> > + gchar buf[9];
> > + gsize len, bin_len;
> > + gboolean success = FALSE;
> > +
> > + s = g_match_info_fetch (match_info, match_index);
> > + g_return_val_if_fail (s != NULL, FALSE);
> > +
> > + len = strlen (s);
> > + if (len == 1 && s[0] == '0') {
> > + *out_addr = 0;
> > + success = TRUE;
> > + goto done;
> > + }
> > + if (len < 7 || len > 8)
> > + goto done;
> > +
> > + /* Handle possibly missing leading zero */
> > + memset (buf, 0, sizeof (buf));
> > + if (len == 7) {
> > + strcpy (&buf[1], s);
> > + buf[0] = '0';
> > + } else if (len == 8)
> > + strcpy (buf, s);
> > + else
> > + g_assert_not_reached ();
> > +
> > + bin = mm_utils_hexstr2bin (buf, &bin_len);
> > + if (!bin || bin_len != 4)
> > + goto done;
> > +
> > + *out_addr = GUINT32_SWAP_LE_BE (*((guint32 *) bin));
> > + success = TRUE;
> > +
> > +done:
> > + g_free (s);
> > + g_free (bin);
> > + return success;
> > +}
> > +
> > +gboolean
> > +mm_cinterion_parse_dhcp_response (const char *reply,
> > + guint *out_address,
> > + guint *out_prefix,
> > + guint *out_gateway,
> > + guint *out_dns1,
> > + guint *out_dns2,
> > + GError **error)
> > +{
> > + gboolean matched;
> > + GRegex *r;
> > + GMatchInfo *match_info = NULL;
> > + GError *match_error = NULL;
> > +
> > + g_assert (reply != NULL);
> > + g_assert (out_address != NULL);
> > + g_assert (out_prefix != NULL);
> > + g_assert (out_gateway != NULL);
> > + g_assert (out_dns1 != NULL);
> > + g_assert (out_dns2 != NULL);
> > +
> > + /* Format:
> > + *
> > + * ^DHCP: <address>,<netmask>,<gateway>,
> <?>,<dns1>,<dns2>,<uplink>,<downlink>
> > + *
> > + * All numbers are hexadecimal representations of IPv4 addresses,
> with
> > + * least-significant byte first. eg, 192.168.50.32 is expressed as
> > + * "2032A8C0". Sometimes leading zeros are stripped, so "1010A0A"
> is
> > + * actually 10.10.1.1.
> > + */
> > +
> > + r = g_regex_new ("\\^DHCP:\\s*([0-9a-fA-F]+),(
> [0-9a-fA-F]+),([0-9a-fA-F]+),([0-9a-fA-F]+),([0-9a-fA-F]+),([0-9a-fA-F]+),.*$",
> 0, 0, NULL);
> > + g_assert (r != NULL);
> > +
> > + matched = g_regex_match_full (r, reply, -1, 0, 0, &match_info,
> &match_error);
> > + if (!matched) {
> > + if (match_error) {
> > + g_propagate_error (error, match_error);
> > + g_prefix_error (error, "Could not parse ^DHCP results: ");
> > + } else {
> > + g_set_error_literal (error,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_FAILED,
> > + "Couldn't match ^DHCP reply");
> > + }
> > + } else {
> > + guint netmask;
> > +
> > + if (match_info_to_ip4_addr (match_info, 1, out_address) &&
> > + match_info_to_ip4_addr (match_info, 2, &netmask) &&
> > + match_info_to_ip4_addr (match_info, 3, out_gateway) &&
> > + match_info_to_ip4_addr (match_info, 5, out_dns1) &&
> > + match_info_to_ip4_addr (match_info, 6, out_dns2)) {
> > + *out_prefix = mm_count_bits_set (netmask);
> > + matched = TRUE;
> > + }
> > + }
> > +
> > + if (match_info)
> > + g_match_info_free (match_info);
> > + g_regex_unref (r);
> > + return matched;
> > +}
> > +
> > +/**********************************************************
> *******************/
> > +/* ^SWWAN read parser
> > + * Description: Parses <cid>, <state>[, <WWAN adapter>] or CME ERROR
> from SWWAN.
> > + * Return: False if error occured while trying to parse SWWAN.
> > + * Requires: Check for AT Error response before calling.
> > + * Read Command
> > + * AT^SWWAN?
> > + * Response(s)
> > + * [^SWWAN: <cid>, <state>[, <WWAN adapter>]]
> > + * [^SWWAN: ...]
> > + * OK
> > + * ERROR
> > + * +CME ERROR: <err>
> > + *
> > + * Examples:
> > + * OK - If no WWAN connection is active, then read
> command just returns OK
> > + * ^SWWAN: 3,1,1 - 3rd PDP Context, Activated, First WWAN
> Adaptor
> > + * +CME ERROR: ? -
> > +*/
> > +
>
> This is much better than returning a blind GList of integers where the
> caller needs to know which integer is which:
>
> gboolean
> mm_cinterion_parse_swwan_response (
> const gchar *response,
> guint *cid,
> guint *state,
> guint *wwan_adapter,
> GError **error);
>
> Also, please write unit tests for the newly written parsers.
>
> > +gboolean
> > +mm_cinterion_parse_swwan_response (const gchar *response,
> > + GList **result,
> > + GError **error)
> > +{
> > + if (!response){
> > + g_set_error (error,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_FAILED,
> > + "Recieved NULL from SWWAN response.");
> > + return FALSE;
> > + }
> > +
> > + //Parse for [^SWWAN: <cid>, <state>[, <WWAN adapter>]]
> > + if (strcasestr (response, "SWWAN"))
> > + {
> > + gint matched;
> > + guint cid, state, wwan_adapter;
> > + matched = sscanf (response, "^SWWAN: %d,%d,%d",
> > + &cid,
> > + &state,
> > + &wwan_adapter);
> > +
> > + if (matched != 3) {
> > + g_set_error (error,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_FAILED,
> > + "Could not parse SWWAN response: '%s'",
> > + response);
> > + return FALSE;
> > + }
> > +
> > + *result = g_list_append (*result, GINT_TO_POINTER(cid));
> > + *result = g_list_append (*result, GINT_TO_POINTER(state));
> > + *result = g_list_append (*result, GINT_TO_POINTER(wwan_adapter))
> ;
> > + }
> > + //TODO: It'd be nice to get 'OK' from response so we don't have to
> assume that
> > + //zero length response means 'OK' or am I doing it wrong...
> > + else if (!g_utf8_strlen (response, 100))
> > + *result = g_list_append (*result, GINT_TO_POINTER(0));
> > + else {
> > + g_set_error (error,
> > + MM_CORE_ERROR,
> > + MM_CORE_ERROR_FAILED,
> > + "Unknown SWWAN response, unable to parse.");
> > + return FALSE;
> > + }
> > +
> > +
> > + return TRUE;
> > +}
> > diff --git a/plugins/cinterion/mm-modem-helpers-cinterion.h
> b/plugins/cinterion/mm-modem-helpers-cinterion.h
> > index d37bcb0..11754ae 100644
> > --- a/plugins/cinterion/mm-modem-helpers-cinterion.h
> > +++ b/plugins/cinterion/mm-modem-helpers-cinterion.h
> > @@ -10,7 +10,9 @@
> > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > * GNU General Public License for more details:
> > *
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > * Copyright (C) 2014 Aleksander Morgado <aleksander at aleksander.es>
> > + * Contributor: Matthew Stanger <matthew_stanger at trimble.com>
> > */
> >
> > #ifndef MM_MODEM_HELPERS_CINTERION_H
> > @@ -63,4 +65,19 @@ gboolean mm_cinterion_parse_sind_response (const
> gchar *response,
> > guint *value,
> > GError **error);
> >
> > +/**********************************************************
> *******************/
> > +/* ^DHCP response parser */
> > +gboolean mm_cinterion_parse_dhcp_response (const char *reply,
> > + guint *out_address,
> > + guint *out_prefix,
> > + guint *out_gateway,
> > + guint *out_dns1,
> > + guint *out_dns2,
> > + GError **error);
> > +
> > +/**********************************************************
> *******************/
> > +/* ^SWWAN response parser */
> > +gboolean mm_cinterion_parse_swwan_response (const gchar *response,
> > + GList **result,
> > + GError **error);
> > #endif /* MM_MODEM_HELPERS_CINTERION_H */
> > diff --git a/plugins/cinterion/mm-plugin-cinterion.c
> b/plugins/cinterion/mm-plugin-cinterion.c
> > index 8af0a2a..d944c00 100644
> > --- a/plugins/cinterion/mm-plugin-cinterion.c
> > +++ b/plugins/cinterion/mm-plugin-cinterion.c
> > @@ -18,7 +18,9 @@
> > *
> > * Copyright (C) 2011 Ammonit Measurement GmbH
> > * Copyright (C) 2011 - 2012 Google Inc.
> > + * Copyright (C) 2016 Trimble Navigation Limited
> > * Author: Aleksander Morgado <aleksander at lanedo.com>
> > + * Contributor: Matthew Stanger <matthew_stanger at trimble.com>
> > */
> >
> > #include <string.h>
> > @@ -45,6 +47,7 @@ int mm_plugin_minor_version = MM_PLUGIN_MINOR_VERSION;
> >
> > #define TAG_CINTERION_APP_PORT "cinterion-app-port"
> > #define TAG_CINTERION_MODEM_PORT "cinterion-modem-port"
> > +#define TAG_CINTERION_GPS_PORT "ID_MM_CINTERION_PORT_TYPE_GPS"
> >
> > typedef struct {
> > MMPortProbe *probe;
> > @@ -175,9 +178,14 @@ grab_port (MMPlugin *self,
> > mm_dbg ("(%s/%s)' Port flagged as PPP",
> > mm_port_probe_get_port_subsys (probe),
> > mm_port_probe_get_port_name (probe));
> > - pflags = MM_PORT_SERIAL_AT_FLAG_PPP;
> > +
> > + //Assuming PLS8 will stay idProduct 0061 and for that product
> we'll use SWWAN, not PPP.
> > + if ((g_strcmp0 (g_udev_device_get_property(mm_port_probe_peek_port
> (probe),"ID_MODEL_ID"),"0061") == 0))
> > + pflags = MM_PORT_SERIAL_AT_FLAG_SECONDARY;
> > + else
> > + pflags = MM_PORT_SERIAL_AT_FLAG_PPP;
> > } else if (g_udev_device_get_property_as_boolean
> (mm_port_probe_peek_port (probe),
> > -
> "ID_MM_CINTERION_PORT_TYPE_GPS")) {
> > +
> TAG_CINTERION_GPS_PORT)) {
> > mm_dbg ("(%s/%s)' Port flagged as GPS",
>
> Ignore the GPS flag for now in this patch.
>
> > mm_port_probe_get_port_subsys (probe),
> > mm_port_probe_get_port_name (probe));
> > @@ -185,6 +193,135 @@ grab_port (MMPlugin *self,
> > g_warn_if_fail (ptype == MM_PORT_TYPE_UNKNOWN);
> > ptype = MM_PORT_TYPE_GPS;
> > }
> > + //TODO: This should be handled by uDev w/
> cinterion-port-types.rules but I can't get
> > + //it to work. Added the rule in there to set the ENV variable
> TAG_CINTERION_GPS_PORT.
> > + //Once that is fixed, it will set the ENV var above & you can
> delete this else if.
> > + else if ((g_strcmp0 (g_udev_device_get_property(mm_port_probe_peek_port
> (probe),"ID_USB_INTERFACE_NUM"),"04") == 0) &&
> > + (g_strcmp0 (g_udev_device_get_property(mm_port_probe_peek_port
> (probe),"ID_MODEL_ID"),"0061") == 0)) {
> > + mm_dbg ("(%s/%s)' Port flagged as GPS",
> > + mm_port_probe_get_port_subsys (probe),
> > + mm_port_probe_get_port_name (probe));
> > + /* Not an AT port, but the port to grab GPS traces */
> > + g_warn_if_fail (ptype == MM_PORT_TYPE_UNKNOWN);
> > + ptype = MM_PORT_TYPE_GPS;
>
> Ignore the GPS flag for now in this patch.
>
> > + }
> > +
> > + /* udev info for PLS8-X of GPS Port, for troubleshooting above
> > + looking at device '/devices/pci0000:00/0000:00:
> 14.0/usb3/3-2/3-2:1.4/tty/ttyACM2':
> > + KERNEL=="ttyACM2"
> > + SUBSYSTEM=="tty"
> > + DRIVER==""
> > +
> > + looking at parent device '/devices/pci0000:00/0000:00:
> 14.0/usb3/3-2/3-2:1.4':
> > + KERNELS=="3-2:1.4"
> > + SUBSYSTEMS=="usb"
> > + DRIVERS=="cdc_acm"
> > + ATTRS{bInterfaceClass}=="02"
> > + ATTRS{iad_bFunctionClass}=="02"
> > + ATTRS{bmCapabilities}=="2"
> > + ATTRS{iad_bFirstInterface}=="04"
> > + ATTRS{bInterfaceSubClass}=="02"
> > + ATTRS{bInterfaceProtocol}=="01"
> > + ATTRS{bNumEndpoints}=="01"
> > + ATTRS{iad_bFunctionSubClass}=="02"
> > + ATTRS{iad_bFunctionProtocol}=="01"
> > + ATTRS{supports_autosuspend}=="1"
> > + ATTRS{iad_bInterfaceCount}=="02"
> > + ATTRS{bAlternateSetting}==" 0"
> > + ATTRS{bInterfaceNumber}=="04"
> > + ATTRS{interface}=="CDC Abstract Control Model (ACM)"
> > +
> > + looking at parent device '/devices/pci0000:00/0000:00:
> 14.0/usb3/3-2':
> > + KERNELS=="3-2"
> > + SUBSYSTEMS=="usb"
> > + DRIVERS=="usb"
> > + ATTRS{bDeviceSubClass}=="00"
> > + ATTRS{bDeviceProtocol}=="00"
> > + ATTRS{devpath}=="2"
> > + ATTRS{idVendor}=="1e2d"
> > + ATTRS{speed}=="480"
> > + ATTRS{bNumInterfaces}=="14"
> > + ATTRS{bConfigurationValue}=="1"
> > + ATTRS{bMaxPacketSize0}=="64"
> > + ATTRS{busnum}=="3"
> > + ATTRS{devnum}=="30"
> > + ATTRS{configuration}==""
> > + ATTRS{bMaxPower}=="20mA"
> > + ATTRS{authorized}=="1"
> > + ATTRS{bmAttributes}=="e0"
> > + ATTRS{bNumConfigurations}=="1"
> > + ATTRS{maxchild}=="0"
> > + ATTRS{bcdDevice}=="0232"
> > + ATTRS{avoid_reset_quirk}=="0"
> > + ATTRS{quirks}=="0x0"
> > + ATTRS{version}==" 2.00"
> > + ATTRS{urbnum}=="3395"
> > + ATTRS{ltm_capable}=="no"
> > + ATTRS{manufacturer}=="Cinterion"
> > + ATTRS{removable}=="removable"
> > + ATTRS{idProduct}=="0061"
> > + ATTRS{bDeviceClass}=="00"
> > + ATTRS{product}=="LTE Modem"
> > +
>
> We don't need your usb host controller details... :)
>
> > + looking at parent device '/devices/pci0000:00/0000:00:14.0/usb3':
> > + KERNELS=="usb3"
> > + SUBSYSTEMS=="usb"
> > + DRIVERS=="usb"
> > + ATTRS{bDeviceSubClass}=="00"
> > + ATTRS{bDeviceProtocol}=="01"
> > + ATTRS{devpath}=="0"
> > + ATTRS{idVendor}=="1d6b"
> > + ATTRS{speed}=="480"
> > + ATTRS{bNumInterfaces}==" 1"
> > + ATTRS{bConfigurationValue}=="1"
> > + ATTRS{bMaxPacketSize0}=="64"
> > + ATTRS{authorized_default}=="1"
> > + ATTRS{busnum}=="3"
> > + ATTRS{devnum}=="1"
> > + ATTRS{configuration}==""
> > + ATTRS{bMaxPower}=="0mA"
> > + ATTRS{authorized}=="1"
> > + ATTRS{bmAttributes}=="e0"
> > + ATTRS{bNumConfigurations}=="1"
> > + ATTRS{maxchild}=="4"
> > + ATTRS{bcdDevice}=="0313"
> > + ATTRS{avoid_reset_quirk}=="0"
> > + ATTRS{quirks}=="0x0"
> > + ATTRS{serial}=="0000:00:14.0"
> > + ATTRS{version}==" 2.00"
> > + ATTRS{urbnum}=="516"
> > + ATTRS{ltm_capable}=="no"
> > + ATTRS{manufacturer}=="Linux 3.13.0-48-generic xhci_hcd"
> > + ATTRS{removable}=="unknown"
> > + ATTRS{idProduct}=="0002"
> > + ATTRS{bDeviceClass}=="09"
> > + ATTRS{product}=="xHCI Host Controller"
> > +
> > + looking at parent device '/devices/pci0000:00/0000:00:14.0':
> > + KERNELS=="0000:00:14.0"
> > + SUBSYSTEMS=="pci"
> > + DRIVERS=="xhci_hcd"
> > + ATTRS{irq}=="42"
> > + ATTRS{subsystem_vendor}=="0x1028"
> > + ATTRS{broken_parity_status}=="0"
> > + ATTRS{class}=="0x0c0330"
> > + ATTRS{consistent_dma_mask_bits}=="64"
> > + ATTRS{dma_mask_bits}=="64"
> > + ATTRS{local_cpus}=="00000000,00000000,00000000,00000000,
> 00000000,00000000,00000000,000000ff"
> > + ATTRS{device}=="0x1e31"
> > + ATTRS{enable}=="1"
> > + ATTRS{msi_bus}==""
> > + ATTRS{local_cpulist}=="0-7"
> > + ATTRS{vendor}=="0x8086"
> > + ATTRS{subsystem_device}=="0x0577"
> > + ATTRS{numa_node}=="-1"
> > + ATTRS{d3cold_allowed}=="1"
> > +
> > + looking at parent device '/devices/pci0000:00':
> > + KERNELS=="pci0000:00"
> > + SUBSYSTEMS==""
> > + DRIVERS==""
> > + */
> >
> > return mm_base_modem_grab_port (modem,
> > mm_port_probe_get_port_subsys
> (probe),
> > diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c
> > index 9cd7c5f..449dddc 100644
> > --- a/src/mm-base-modem.c
> > +++ b/src/mm-base-modem.c
> > @@ -674,6 +674,26 @@ mm_base_modem_peek_best_data_port (MMBaseModem
> *self,
> > return NULL;
> > }
> >
> > +MMPort *
> > +mm_base_modem_peek_current_data_port (MMBaseModem *self,
> > + MMPortType type)
> > +{
> > + GList *l;
> > +
> > + g_return_val_if_fail (MM_IS_BASE_MODEM (self), NULL);
> > +
> > + /* Return first connected data port */
> > + for (l = self->priv->data; l; l = g_list_next (l)) {
> > + if (mm_port_get_connected ((MMPort *)l->data) &&
> > + (mm_port_get_port_type ((MMPort *)l->data) == type ||
> > + type == MM_PORT_TYPE_UNKNOWN)) {
> > + return (MMPort *)l->data;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > GList *
> > mm_base_modem_get_data_ports (MMBaseModem *self)
> > {
> > diff --git a/src/mm-base-modem.h b/src/mm-base-modem.h
> > index 3c0d16f..bbea52d 100644
> > --- a/src/mm-base-modem.h
> > +++ b/src/mm-base-modem.h
> > @@ -131,6 +131,7 @@ MMPortMbim *mm_base_modem_peek_port_mbim_for_data
> (MMBaseModem *self, MMPo
> > #endif
> > MMPortSerialAt *mm_base_modem_peek_best_at_port (MMBaseModem
> *self, GError **error);
> > MMPort *mm_base_modem_peek_best_data_port (MMBaseModem
> *self, MMPortType type);
> > +MMPort *mm_base_modem_peek_current_data_port (MMBaseModem
> *self, MMPortType type);
> > GList *mm_base_modem_peek_data_ports (MMBaseModem
> *self);
> >
> > MMPortSerialAt *mm_base_modem_get_port_primary (MMBaseModem
> *self);
> > --
> > 1.9.1
> >
> >
>
>
>
> --
> Aleksander
> https://aleksander.es
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20161023/e639470b/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cinterion_swwan_support.patch
Type: text/x-patch
Size: 61189 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20161023/e639470b/attachment-0001.bin>
More information about the ModemManager-devel
mailing list