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



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.

-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



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