RE: [PATCH 2/2] add imei modem property



> -----Original Message-----
> From: Dan Williams [mailto:dcbw redhat com]
> Sent: den 2 juni 2010 09:19
> To: Jason Glasgow
> Cc: Torgny Johansson; networkmanager-list gnome org; David Rochberg
> Subject: Re: [PATCH 2/2] add imei modem property
>
> On Thu, 2010-05-27 at 13:28 -0400, Jason Glasgow wrote:
> > I'd like agree with David's suggestion.  Adding this
> property to the
> > Modem object and calling it EquipmentIdentifier will
> simplify the use
> > case -- allowing clients of the Modem Manager DBUS interface to
> > uniquely identify devices with a persistent id independent of modem
> > type.
>
> Yeah, that sounds good to me.  The property's spec should state that
> (currently) the value is a string which will be either the
> IMEI (for GSM-based devices) or (for CDMA) the hex-format ESN/MEID.

Ok, I've changed the patch so that the property (on the modem interface) is now called EquipmentIdentity.

I've implemented it for GSM-devices but not for CDMA-devices.

The new patch is attached and it should be applied _after_ the 0001-Added-UnlockRetries-property-to-the-modem-interface.patch that I sent a few days back.

/Torgny

>
> Dan
>
> >
> > -Jason
> >
> >
> > On Thu, May 27, 2010 at 5:57 AM, Torgny Johansson
> > <torgny johansson ericsson com> wrote:
> >         > -----Original Message-----
> >         > From: rochberg google com [mailto:rochberg google com] On
> >         > Behalf Of David Rochberg
> >         > Sent: den 26 maj 2010 15:31
> >         > To: Torgny Johansson
> >         > Cc: Dan Williams; networkmanager-list gnome org
> >         > Subject: Re: [PATCH 2/2] add imei modem property
> >         >
> >
> >         > For things like this with similar semantics but different
> >         > names, would it make sense to hang them off of modem and
> >         > define the semantics according to the protocol?
> That is, a
> >         > property like EquipmentIdentifier, with GSM and
> CDMA phones
> >         > supplying IMEI and MEID, respectively.
> >
> >
> >         I've moved the property to gsm card but I'll wait until this
> >         has been discussed before submitting it.
> >
> >         To me your suggestion sounds good. Which way do you
> want to go
> >         Dan?
> >
> >         /Torgny
> >
> >
> >         >
> >         > -david
> >         >
> >         >
> >         > On Wed, May 26, 2010 at 3:38 AM, Torgny Johansson
> >         > <torgny johansson ericsson com> wrote:
> >         >
> >         >
> >         >       onsdagen den 26 maj 2010 07.57.10 skrev
> Dan Williams:
> >         >
> >         >       > On Tue, 2010-05-25 at 10:42 +0200, Torgny
> Johansson
> >         wrote:
> >         >       > > Hi,
> >         >       > >
> >         >       > > this patch adds the IMEI of the modem as a
> >         property
> >         > so that it is
> >         >       > > available at an early stage to simplify the
> >         connect
> >         > sequence.
> >         >       >
> >         >       > So this can't be a property on the base
> modem object
> >         > because CDMA
> >         >       > devices don't have an IMEI (well, not
> entirely true,
> >         > but mostly true)
> >         >       > but instead have an MEID.  I'd rather have the
> >         property on the
> >         >       > org.freedesktop.ModemManager.Gsm.Card interface
> >         > (mm-modem-gsm-card.c for
> >         >       > the property's definition) instead and
> overridden by
> >         > the generic GSM
> >         >       > class.  Does that sound OK?
> >         >
> >         >
> >         >       Ah, makes sense. I'll try to move it.
> >         >
> >         >       Thanks!
> >         >
> >         >       /Torgny
> >         >
> >         >
> >         >       >
> >         >       > Dan
> >         >       >
> >         >       > > Regards
> >         >       > > Torgny Johansson
> >         >       > >
> >         >       > > ---
> >         >       > >
> >         >       > > diff --git a/introspection/mm-modem.xml
> >         > b/introspection/mm-modem.xml
> >         >       > > index 09512f8..8eff674 100644
> >         >       > > --- a/introspection/mm-modem.xml
> >         >       > > +++ b/introspection/mm-modem.xml
> >         >       > > @@ -113,6 +113,12 @@
> >         >       > >
> >         >       > >        </tp:docstring>
> >         >       > >
> >         >       > >      </property>
> >         >       > >
> >         >       > > +   <property name="Imei" type="s"
> access="read">
> >         >       > > +      <tp:docstring>
> >         >       > > +        The IMEI of the device.
> >         >       > > +      </tp:docstring>
> >         >       > > +    </property>
> >         >       > > +
> >         >       > >
> >         >       > >      <property name="UnlockRequired" type="s"
> >         access="read">
> >         >       > >
> >         >       > >        <tp:docstring>
> >         >       > >
> >         >       > >          Empty if the device is usable
> without an
> >         > unlock code or has
> >         >       > >          already
> >         >       > >
> >         >       > > diff --git a/src/mm-generic-gsm.c
> >         b/src/mm-generic-gsm.c
> >         >       > > index e028a93..7320fb7 100644
> >         >       > > --- a/src/mm-generic-gsm.c
> >         >       > > +++ b/src/mm-generic-gsm.c
> >         >       > > @@ -281,10 +281,22 @@ check_pin (MMGenericGsm
> >         *modem,
> >         >       > >
> >         >       > >  }
> >         >       > >
> >         >       > >  static void
> >         >       > >
> >         >       > > +get_imei_cb (MMModem *modem,
> >         >       > > +             const char *result,
> >         >       > > +             GError *error,
> >         >       > > +             gpointer user_data)
> >         >       > > +{
> >         >       > > +    if (!error)
> >         >       > > +        mm_modem_base_set_imei (MM_MODEM_BASE
> >         > (modem), result);
> >         >       > > +    else
> >         >       > > +        mm_modem_base_set_imei (MM_MODEM_BASE
> >         (modem), "");
> >         >       > > +}
> >         >       > > +
> >         >       > > +static void
> >         >       > >
> >         >       > >  get_unlock_retries_cb (MMModem *modem,
> >         >       > >
> >         >       > > -
> guint32 result,
> >         >       > > -
> GError *error,
> >         >       > > -                                  gpointer
> >         user_data)
> >         >       > > +                       guint32 result,
> >         >       > > +                       GError *error,
> >         >       > > +                       gpointer user_data)
> >         >       > >
> >         >       > >  {
> >         >       > >
> >         >       > >      if (!error)
> >         >       > >
> >         >       > >          mm_modem_base_set_unlock_retries
> >         > (MM_MODEM_BASE (modem),
> >         >       > >          result);
> >         >       > >
> >         >       > > @@ -405,6 +417,34 @@ initial_pin_check
> >         (MMGenericGsm *self)
> >         >       > >
> >         >       > >      }
> >         >       > >
> >         >       > >  }
> >         >       > >
> >         >       > > +static void
> >         >       > > +initial_imei_check (MMGenericGsm *self)
> >         >       > > +{
> >         >       > > +    GError *error = NULL;
> >         >       > > +    MMGenericGsmPrivate *priv;
> >         >       > > +
> >         >       > > +    g_return_if_fail
> (MM_IS_GENERIC_GSM (self));
> >         >       > > +    priv = MM_GENERIC_GSM_GET_PRIVATE (self);
> >         >       > > +
> >         >       > > +    g_return_if_fail (priv->primary != NULL);
> >         >       > > +
> >         >       > > +    if (mm_serial_port_open (MM_SERIAL_PORT
> >         > (priv->primary), &error)) {
> >         >       > > +        /* Make sure echoing is off */
> >         >       > > +        mm_at_serial_port_queue_command
> >         > (priv->primary, "E0", 3, NULL,
> >         >       > > NULL); +
> >         >       > > +        /* Get modem's imei number */
> >         >       > > +        mm_modem_gsm_card_get_imei
> >         > (MM_MODEM_GSM_CARD (self),
> >         >       > > +
> get_imei_cb,
> >         >       > > +                                    NULL);
> >         >       > > +    } else {
> >         >       > > +        g_warning ("%s: failed to open serial
> >         > port: (%d) %s",
> >         >       > > +                   __func__,
> >         >       > > +                   error ? error->code : -1,
> >         >       > > +                   error && error->message ?
> >         > error->message :
> >         >       > > "(unknown)"); +        g_clear_error (&error);
> >         >       > > +    }
> >         >       > > +}
> >         >       > > +
> >         >       > >
> >         >       > >  static gboolean
> >         >       > >  owns_port (MMModem *modem, const char *subsys,
> >         > const char *name)
> >         >       > >  {
> >         >       > >
> >         >       > > @@ -458,6 +498,9 @@ mm_generic_gsm_grab_port
> >         > (MMGenericGsm *self,
> >         >       > >
> >         >       > >              /* Get modem's initial lock/unlock
> >         state */
> >         >       > >              initial_pin_check (self);
> >         >       > >
> >         >       > > +            /* Get modem's IMEI number */
> >         >       > > +            initial_imei_check (self);
> >         >       > > +
> >         >       > >
> >         >       > >          } else if (ptype ==
> >         MM_PORT_TYPE_SECONDARY)
> >         >       > >
> >         >       > >              priv->secondary = MM_AT_SERIAL_PORT
> >         (port);
> >         >       > >
> >         >       > >      } else if (MM_IS_QCDM_SERIAL_PORT (port)) {
> >         >       > >
> >         >       > > diff --git a/src/mm-modem-base.c
> >         b/src/mm-modem-base.c
> >         >       > > index 611b1a1..1afadf6 100644
> >         >       > > --- a/src/mm-modem-base.c
> >         >       > > +++ b/src/mm-modem-base.c
> >         >       > > @@ -42,6 +42,7 @@ typedef struct {
> >         >       > >
> >         >       > >      char *driver;
> >         >       > >      char *plugin;
> >         >       > >      char *device;
> >         >       > >
> >         >       > > +    char *imei;
> >         >       > >
> >         >       > >      char *unlock_required;
> >         >       > >      guint32 unlock_retries;
> >         >       > >      guint32 ip_method;
> >         >       > >
> >         >       > > @@ -193,6 +194,47 @@ mm_modem_base_get_valid
> >         > (MMModemBase *self)
> >         >       > >
> >         >       > >  }
> >         >       > >
> >         >       > >  const char *
> >         >       > >
> >         >       > > +mm_modem_base_get_imei (MMModemBase *self)
> >         >       > > +{
> >         >       > > +    g_return_val_if_fail (self != NULL, NULL);
> >         >       > > +    g_return_val_if_fail (MM_IS_MODEM_BASE
> >         (self), NULL);
> >         >       > > +
> >         >       > > +    return MM_MODEM_BASE_GET_PRIVATE
> >         (self)->imei;
> >         >       > > +}
> >         >       > > +
> >         >       > > +void
> >         >       > > +mm_modem_base_set_imei (MMModemBase
> *self, const
> >         > char *imei)
> >         >       > > +{
> >         >       > > +    MMModemBasePrivate *priv;
> >         >       > > +    const char *dbus_path;
> >         >       > > +
> >         >       > > +    g_return_if_fail (self != NULL);
> >         >       > > +    g_return_if_fail (MM_IS_MODEM_BASE (self));
> >         >       > > +
> >         >       > > +    priv = MM_MODEM_BASE_GET_PRIVATE (self);
> >         >       > > +
> >         >       > > +    /* Only do something if the value
> changes */
> >         >       > > +    if (  (priv->imei == imei)
> >         >       > > +       || (   priv->imei
> >         >       > > +           && imei
> >         >       > > +           && !strcmp (priv->imei, imei)))
> >         >       > > +       return;
> >         >       > > +
> >         >       > > +    g_free (priv->imei);
> >         >       > > +    priv->imei = g_strdup (imei);
> >         >       > > +
> >         >       > > +    dbus_path = (const char *)
> g_object_get_data
> >         > (G_OBJECT (self),
> >         >       > > DBUS_PATH_TAG); +    if (dbus_path) {
> >         >       > > +        if (priv->imei)
> >         >       > > +            g_message ("Modem %s: IMEI
> set (%s)",
> >         > dbus_path,
> >         >       > > priv->imei); +        else
> >         >       > > +            g_message ("Modem %s: IMEI
> not set",
> >         > dbus_path);
> >         >       > > +    }
> >         >       > > +
> >         >       > > +    g_object_notify (G_OBJECT (self),
> >         MM_MODEM_IMEI);
> >         >       > > +}
> >         >       > > +
> >         >       > > +const char *
> >         >       > >
> >         >       > >  mm_modem_base_get_unlock_required (MMModemBase
> >         *self)
> >         >       > >  {
> >         >       > >
> >         >       > >      g_return_val_if_fail (self != NULL, NULL);
> >         >       > >
> >         >       > > @@ -523,6 +565,9 @@ mm_modem_base_init
> >         (MMModemBase *self)
> >         >       > >
> >         >       > >
> >         >  MM_MODEM_ENABLED,
> >         >       > >
> >         >  MM_MODEM_DBUS_INTERF
> >         >       > >
> >            ACE);
> >         >       > >
> >         >       > >
> >          mm_properties_changed_signal_register_property
> >         > (G_OBJECT (self),
> >         >       > >
> >         >       > > +
> >         >  MM_MODEM_IMEI,
> >         >       > > +
> >         >       > > MM_MODEM_DBUS_INTERFACE); +
> >         >       > > mm_properties_changed_signal_register_property
> >         > (G_OBJECT (self),
> >         >       > >
> >         >       > >
> >         >  MM_MODEM_UNLOCK_REQU
> >         >       > >
> >            IRED,
> >         >       > >
> >         >  MM_MODEM_DBUS_INTER
> >         >       > >
> >            FACE);
> >         >       > >
> >         >       > >
> >          mm_properties_changed_signal_register_property
> >         > (G_OBJECT (self),
> >         >       > >
> >         >       > > @@ -576,6 +621,7 @@ set_property
> (GObject *object,
> >         > guint prop_id,
> >         >       > >
> >         >       > >      case MM_MODEM_PROP_VALID:
> >         >       > >      case MM_MODEM_PROP_TYPE:
> >         >       > >
> >         >       > >      case MM_MODEM_PROP_ENABLED:
> >         >       > > +    case MM_MODEM_PROP_IMEI:
> >         >       > >      case MM_MODEM_PROP_UNLOCK_REQUIRED:
> >         >       > >
> >         >       > >      case MM_MODEM_PROP_UNLOCK_RETRIES:
> >         >       > >          break;
> >         >       > >
> >         >       > > @@ -619,6 +665,9 @@ get_property
> (GObject *object,
> >         > guint prop_id,
> >         >       > >
> >         >       > >      case MM_MODEM_PROP_ENABLED:
> >         >       > >          g_value_set_boolean (value, is_enabled
> >         > (priv->state));
> >         >       > >          break;
> >         >       > >
> >         >       > > +    case MM_MODEM_PROP_IMEI:
> >         >       > > +        g_value_set_string (value, priv->imei);
> >         >       > > +        break;
> >         >       > >
> >         >       > >      case MM_MODEM_PROP_UNLOCK_REQUIRED:
> >         >       > >          g_value_set_string (value,
> >         priv->unlock_required);
> >         >       > >          break;
> >         >       > >
> >         >       > > @@ -643,6 +692,7 @@ finalize (GObject *object)
> >         >       > >
> >         >       > >      g_free (priv->driver);
> >         >       > >      g_free (priv->plugin);
> >         >       > >      g_free (priv->device);
> >         >       > >
> >         >       > > +    g_free (priv->imei);
> >         >       > >
> >         >       > >      g_free (priv->unlock_required);
> >         >       > >
> >         >       > >      G_OBJECT_CLASS
> >         > (mm_modem_base_parent_class)->finalize (object);
> >         >       > >
> >         >       > > @@ -697,6 +747,10 @@ mm_modem_base_class_init
> >         > (MMModemBaseClass *klass)
> >         >       > >
> >         >       > >
> >          MM_MODEM_ENABLED);
> >         >       > >
> >         >       > >      g_object_class_override_property
> >         (object_class,
> >         >       > >
> >         >       > > +
> >          MM_MODEM_PROP_IMEI,
> >         >       > > +
> >          MM_MODEM_IMEI);
> >         >       > > +
> >         >       > > +    g_object_class_override_property
> >         (object_class,
> >         >       > >
> >         >       > >
> >         > MM_MODEM_PROP_UNLOCK_REQUIRED,
> >         >       > >
> >         > MM_MODEM_UNLOCK_REQUIRED);
> >         >       > >
> >         >       > > diff --git a/src/mm-modem-base.h
> >         b/src/mm-modem-base.h
> >         >       > > index 8eec0e4..19843a0 100644
> >         >       > > --- a/src/mm-modem-base.h
> >         >       > > +++ b/src/mm-modem-base.h
> >         >       > > @@ -62,6 +62,11 @@ void mm_modem_base_set_valid
> >         > (MMModemBase *self,
> >         >       > >
> >         >       > >  gboolean mm_modem_base_get_valid (MMModemBase
> >         *self);
> >         >       > >
> >         >       > > +const char *mm_modem_base_get_imei (MMModemBase
> >         *self);
> >         >       > > +
> >         >       > > +void mm_modem_base_set_imei (MMModemBase *self,
> >         >       > > +                                        const
> >         char *imei);
> >         >       > > +
> >         >       > >
> >         >       > >  const char *mm_modem_base_get_unlock_required
> >         > (MMModemBase *self);
> >         >       > >
> >         >       > >  void mm_modem_base_set_unlock_required
> >         (MMModemBase *self,
> >         >       > >
> >         >       > > diff --git a/src/mm-modem.c b/src/mm-modem.c
> >         >       > > index b378fff..548a4c3 100644
> >         >       > > --- a/src/mm-modem.c
> >         >       > > +++ b/src/mm-modem.c
> >         >       > > @@ -805,6 +805,14 @@ mm_modem_init (gpointer
> >         g_iface)
> >         >       > >
> >         >       > >      g_object_interface_install_property
> >         >       > >
> >         >       > >          (g_iface,
> >         >       > >
> >         >       > > +         g_param_spec_string (MM_MODEM_IMEI,
> >         >       > > +                               "Imei",
> >         >       > > +                               "The IMEI of the
> >         device",
> >         >       > > +                               NULL,
> >         >       > > +
> >         G_PARAM_READABLE));
> >         >       > > +
> >         >       > > +    g_object_interface_install_property
> >         >       > > +        (g_iface,
> >         >       > >
> >         >       > >           g_param_spec_string
> >         (MM_MODEM_UNLOCK_REQUIRED,
> >         >       > >
> >         >       > >
> "UnlockRequired",
> >         >       > >                                 "Whether or not
> >         the
> >         > modem requires an
> >         >       > >                                 unlock "
> >         >       > >
> >         >       > > diff --git a/src/mm-modem.h b/src/mm-modem.h
> >         >       > > index d2863e4..21d7a5b 100644
> >         >       > > --- a/src/mm-modem.h
> >         >       > > +++ b/src/mm-modem.h
> >         >       > > @@ -58,8 +58,9 @@ typedef enum {
> >         >       > >
> >         >       > >  #define MM_MODEM_TYPE          "type"
> >         >       > >  #define MM_MODEM_IP_METHOD     "ip-method"
> >         >       > >  #define MM_MODEM_ENABLED       "enabled"
> >         >       > >
> >         >       > > +#define MM_MODEM_IMEI          "imei"
> >         >       > >
> >         >       > >  #define MM_MODEM_UNLOCK_REQUIRED
> >          "unlock-required"
> >         >       > >
> >         >       > > -#define MM_MODEM_UNLOCK_RETRIES
> "unlock-retries"
> >         >       > > +#define MM_MODEM_UNLOCK_RETRIES
> >         "unlock-retries"
> >         >       > >
> >         >       > >  #define MM_MODEM_VALID         "valid"      /*
> >         not
> >         > exported */
> >         >       > >  #define MM_MODEM_PLUGIN        "plugin"     /*
> >         not
> >         > exported */
> >         >       > >  #define MM_MODEM_STATE         "state"      /*
> >         not
> >         > exported */
> >         >       > >
> >         >       > > @@ -84,6 +85,7 @@ typedef enum {
> >         >       > >
> >         >       > >      MM_MODEM_PROP_PLUGIN,      /* Not
> exported */
> >         >       > >      MM_MODEM_PROP_STATE,       /* Not
> exported */
> >         >       > >      MM_MODEM_PROP_ENABLED,
> >         >       > >
> >         >       > > +    MM_MODEM_PROP_IMEI,
> >         >       > >
> >         >       > >      MM_MODEM_PROP_UNLOCK_REQUIRED,
> >         >       > >      MM_MODEM_PROP_UNLOCK_RETRIES
> >         >       > >
> >         >       > >  } MMModemProp;
> >         >       > >
> >         >       > > _______________________________________________
> >         >       > > networkmanager-list mailing list
> >         >       > > networkmanager-list gnome org
> >         >       > >
> >         http://mail.gnome.org/mailman/listinfo/networkmanager-list
> >         >       _______________________________________________
> >         >       networkmanager-list mailing list
> >         >       networkmanager-list gnome org
> >         >
> >         http://mail.gnome.org/mailman/listinfo/networkmanager-list
> >         >
> >         >
> >         >
> >         >
> >         _______________________________________________
> >         networkmanager-list mailing list
> >         networkmanager-list gnome org
> >         http://mail.gnome.org/mailman/listinfo/networkmanager-list
> >
> >
> >
> > _______________________________________________
> > networkmanager-list mailing list
> > networkmanager-list gnome org
> > http://mail.gnome.org/mailman/listinfo/networkmanager-list
>
>
>

Attachment: 0002-Added-EQUIPMENT_IDENTITY-property-to-modem-interface.patch
Description: 0002-Added-EQUIPMENT_IDENTITY-property-to-modem-interface.patch



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]