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.  

-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



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