RE: [PATCH 2/2] add imei modem property
- From: Dan Williams <dcbw redhat com>
- To: Torgny Johansson <torgny johansson ericsson com>
- Cc: "David menubar gnome org" <David menubar gnome org>, "networkmanager-list gnome org" <networkmanager-list gnome org>, Rochberg <rochberg chromium org>
- Subject: RE: [PATCH 2/2] add imei modem property
- Date: Tue, 22 Jun 2010 17:14:51 -0700
On Fri, 2010-06-11 at 10:57 +0200, Torgny Johansson wrote:
>
> > -----Original Message-----
> > From: networkmanager-list-bounces gnome org
> > [mailto:networkmanager-list-bounces gnome org] On Behalf Of
> > Torgny Johansson
> > Sent: den 4 juni 2010 11:45
> > To: Dan Williams
> > Cc: Rochberg; networkmanager-list gnome org; David menubar gnome org
> > Subject: 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
> > >
> > >
> > >
> >
>
> Hi!
>
> Attached is an updated patch where EquipmentIdentity has been renamed to EquipmentIdentifier.
>
> Thanks to Jason Glasgow for making the change!
Pushed, thanks.
Dan
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
