Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Aleksander Morgado
On 02/28/2013 08:11 PM, Ben Chan wrote:
> ---
>  src/mm-iface-modem-3gpp.c | 15 +++
>  1 file changed, 15 insertions(+)
> 

I pushed it after modifying it to include the
clear_deferred_registration_state_update() call in the first
DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said previously, we
don't really mind if we get an unsolicited registration event during the
disabling process, as we're disabling anyway.

Thanks!


> diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
> index fa8da1a..2430056 100644
> --- a/src/mm-iface-modem-3gpp.c
> +++ b/src/mm-iface-modem-3gpp.c
> @@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp 
> *self)
>   
> (GDestroyNotify)registration_check_context_free);
>  }
>  
> +static void
> +clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
> +{
> +RegistrationStateContext *ctx = get_registration_state_context (self);
> +
> +ctx->deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
> +if (ctx->deferred_update_id) {
> +g_source_remove (ctx->deferred_update_id);
> +ctx->deferred_update_id = 0;
> +}
> +}
> +
>  
> /*/
>  
>  typedef struct _DisablingContext DisablingContext;
> @@ -1436,6 +1448,9 @@ interface_disabling_step (DisablingContext *ctx)
>  }
>  
>  case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
> +/* Prevent any deferred registration state update from happening 
> after the modem is disabled */
> +clear_deferred_registration_state_update (ctx->self);
> +
>  if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_registration_events &&
>  MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_registration_events_finish) {
>  MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_registration_events (
> 


-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


RE: [MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread Ori Inbar
> -Original Message-
> From: networkmanager-list [mailto:networkmanager-list-
> boun...@gnome.org] On Behalf Of Dan Williams
> Sent: Thursday, February 28, 2013 11:09 AM
> To: Aleksander Morgado
> Cc: networkmanager-list@gnome.org
> Subject: Re: [MM] [PATCH v3] serial: Add support to optionally send line-feed
> at the end of AT commands.
>
> On Thu, 2013-02-28 at 17:59 +0100, Dan Williams wrote:
> > On Thu, 2013-02-28 at 16:34 +0100, Aleksander Morgado wrote:
> > > On 02/28/2013 03:20 PM, Dan Williams wrote:
> > > >> > Also, are you going to suggest a new plugin for MM using this
> > > >> > property set to TRUE? If so, it may be a good idea to send all
> > > >> > relevant commits, including this one, in the same patch series, along
> with the plugin.
> > > > Now that I think about it, is there any reason we're not doing
> > > > this for
> > > > *all* devices?  Do you think any devices would care?  I'm not near
> > > > my pile-of-modems at this time, but at least the E362 and my
> > > > Longcheer-based Zoom 4597 don't care whether there's a  at the
> > > > end of every command.
> > >
> > > Good point, it probably isn't a big deal if we add that by default.
> > > Ori, are you able to provide a patch for that?
> >
> > I did hack that up already when testing with my Zoom and the E362, so
> > here we go as a first-pass.  Ori, can you test this and make sure it
> > works on your device?
>
> Bad news; the Zoom 4597 (Longcheer) apparently crashes when you send
> "AT
> +CPNNUM", but seems to be fine with "AT+CPNNUM" :(  All
> the
> other commands are fine up until that point (eg ATI, AT+CPIN?, +CRSM,
> etc) but it's somewhat telling that the first custom command crashes it :)
> Perhaps Longcheer forgot to teach the custom AT parser they added to the
> firmware about ?
>
> I'd like to test a few more devices and write a quick tool to make sure the
> extra  is the problem, but in the mean time, if anyone else can test the
> patch and see if it causes problems that would be great.
>
> Dan
>

Hi Dan
I think it will best to add that property where each plugin can choose if it 
needs the  or not.
This way we won't break existing working plugins. It is safer this way.
Yes this is needed for a new Altair LTE plugin I will add soon.

Ori
>
> ___
> networkmanager-list mailing list
> networkmanager-list@gnome.org
> https://mail.gnome.org/mailman/listinfo/networkmanager-list



Important Notice: This transmission and any files attached to it, may contain 
confidential and/or privileged information and is intended only for the named 
recipient. If you are not the intended recipient, you are hereby notified that 
any disclosure, reproduction, retransmission, dissemination, copying or any 
other use of the information or files contained is strictly prohibited. If you 
have received this transmission in error, please notify the sender by reply 
transmission and delete this electronic mail
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Ben Chan
---
 src/mm-iface-modem-3gpp.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index fa8da1a..2430056 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp 
*self)
  (GDestroyNotify)registration_check_context_free);
 }
 
+static void
+clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
+{
+RegistrationStateContext *ctx = get_registration_state_context (self);
+
+ctx->deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+if (ctx->deferred_update_id) {
+g_source_remove (ctx->deferred_update_id);
+ctx->deferred_update_id = 0;
+}
+}
+
 /*/
 
 typedef struct _DisablingContext DisablingContext;
@@ -1436,6 +1448,9 @@ interface_disabling_step (DisablingContext *ctx)
 }
 
 case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
+/* Prevent any deferred registration state update from happening after 
the modem is disabled */
+clear_deferred_registration_state_update (ctx->self);
+
 if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx->self)->cleanup_unsolicited_registration_events &&
 MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx->self)->cleanup_unsolicited_registration_events_finish) {
 MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx->self)->cleanup_unsolicited_registration_events (
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread Dan Williams
On Thu, 2013-02-28 at 17:59 +0100, Dan Williams wrote:
> On Thu, 2013-02-28 at 16:34 +0100, Aleksander Morgado wrote:
> > On 02/28/2013 03:20 PM, Dan Williams wrote:
> > >> > Also, are you going to suggest a new plugin for MM using this property
> > >> > set to TRUE? If so, it may be a good idea to send all relevant commits,
> > >> > including this one, in the same patch series, along with the plugin.
> > > Now that I think about it, is there any reason we're not doing this for
> > > *all* devices?  Do you think any devices would care?  I'm not near my
> > > pile-of-modems at this time, but at least the E362 and my
> > > Longcheer-based Zoom 4597 don't care whether there's a  at the end
> > > of every command.
> > 
> > Good point, it probably isn't a big deal if we add that by default. Ori,
> > are you able to provide a patch for that?
> 
> I did hack that up already when testing with my Zoom and the E362, so
> here we go as a first-pass.  Ori, can you test this and make sure it
> works on your device?

Bad news; the Zoom 4597 (Longcheer) apparently crashes when you send "AT
+CPNNUM", but seems to be fine with "AT+CPNNUM" :(  All the
other commands are fine up until that point (eg ATI, AT+CPIN?, +CRSM,
etc) but it's somewhat telling that the first custom command crashes
it :)  Perhaps Longcheer forgot to teach the custom AT parser they added
to the firmware about ?

I'd like to test a few more devices and write a quick tool to make sure
the extra  is the problem, but in the mean time, if anyone else can
test the patch and see if it causes problems that would be great.

Dan


___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread Dan Williams
On Thu, 2013-02-28 at 16:34 +0100, Aleksander Morgado wrote:
> On 02/28/2013 03:20 PM, Dan Williams wrote:
> >> > Also, are you going to suggest a new plugin for MM using this property
> >> > set to TRUE? If so, it may be a good idea to send all relevant commits,
> >> > including this one, in the same patch series, along with the plugin.
> > Now that I think about it, is there any reason we're not doing this for
> > *all* devices?  Do you think any devices would care?  I'm not near my
> > pile-of-modems at this time, but at least the E362 and my
> > Longcheer-based Zoom 4597 don't care whether there's a  at the end
> > of every command.
> 
> Good point, it probably isn't a big deal if we add that by default. Ori,
> are you able to provide a patch for that?

I did hack that up already when testing with my Zoom and the E362, so
here we go as a first-pass.  Ori, can you test this and make sure it
works on your device?

Thanks!
Dan

diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
index 7962622..7e3da4c 100644
--- a/src/mm-at-serial-port.c
+++ b/src/mm-at-serial-port.c
@@ -306,8 +306,10 @@ at_command_to_byte_array (const char *command, gboolean 
is_raw)
 
 if (!is_raw) {
 /* Make sure there's a trailing carriage return */
-if (command[cmdlen - 1] != '\r')
+if (command[cmdlen - 1] != '\r' && command[cmdlen - 2] != '\r')
 g_byte_array_append (buf, (const guint8 *) "\r", 1);
+if (command[cmdlen - 1] != '\n' && command[cmdlen - 2] != '\n')
+g_byte_array_append (buf, (const guint8 *) "\n", 1);
 }
 
 return buf;


___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread Aleksander Morgado
On 02/28/2013 03:20 PM, Dan Williams wrote:
>> > Also, are you going to suggest a new plugin for MM using this property
>> > set to TRUE? If so, it may be a good idea to send all relevant commits,
>> > including this one, in the same patch series, along with the plugin.
> Now that I think about it, is there any reason we're not doing this for
> *all* devices?  Do you think any devices would care?  I'm not near my
> pile-of-modems at this time, but at least the E362 and my
> Longcheer-based Zoom 4597 don't care whether there's a  at the end
> of every command.

Good point, it probably isn't a big deal if we add that by default. Ori,
are you able to provide a patch for that?

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread Dan Williams
On Thu, 2013-02-28 at 09:38 +0100, Aleksander Morgado wrote:
> On 02/28/2013 09:24 AM, ori.in...@altair-semi.com wrote:
> > From: ori inbar 
> > 
> 
> Looks good; some minor comments below.
> 
> Also, are you going to suggest a new plugin for MM using this property
> set to TRUE? If so, it may be a good idea to send all relevant commits,
> including this one, in the same patch series, along with the plugin.

Now that I think about it, is there any reason we're not doing this for
*all* devices?  Do you think any devices would care?  I'm not near my
pile-of-modems at this time, but at least the E362 and my
Longcheer-based Zoom 4597 don't care whether there's a  at the end
of every command.

Dan

> 
> > ---
> >  src/mm-at-serial-port.c |   34 +++---
> >  src/mm-at-serial-port.h |2 ++
> >  src/mm-plugin.c |   19 +++
> >  src/mm-plugin.h |1 +
> >  src/mm-port-probe.c |5 +
> >  src/mm-port-probe.h |1 +
> >  6 files changed, 59 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
> > index 840f38a..7ed7a82 100644
> > --- a/src/mm-at-serial-port.c
> > +++ b/src/mm-at-serial-port.c
> > @@ -31,6 +31,7 @@ G_DEFINE_TYPE (MMAtSerialPort, mm_at_serial_port, 
> > MM_TYPE_SERIAL_PORT)
> >  enum {
> >  PROP_0,
> >  PROP_REMOVE_ECHO,
> > +PROP_SEND_LF,
> >  LAST_PROP
> >  };
> >  
> > @@ -45,6 +46,7 @@ typedef struct {
> >  MMAtPortFlag flags;
> >  
> >  gboolean remove_echo;
> > +gboolean send_lf;
> >  } MMAtSerialPortPrivate;
> >  
> >  
> > /*/
> > @@ -281,7 +283,7 @@ parse_unsolicited (MMSerialPort *port, GByteArray 
> > *response)
> >  
> > /*/
> >  
> >  static GByteArray *
> > -at_command_to_byte_array (const char *command, gboolean is_raw)
> > +at_command_to_byte_array (const char *command, gboolean is_raw, gboolean 
> > send_lf)
> >  {
> >  GByteArray *buf;
> >  int cmdlen;
> > @@ -303,6 +305,12 @@ at_command_to_byte_array (const char *command, 
> > gboolean is_raw)
> >  /* Make sure there's a trailing carriage return */
> >  if (command[cmdlen - 1] != '\r')
> >  g_byte_array_append (buf, (const guint8 *) "\r", 1);
> > +if (send_lf)
> > +{
> 
> Braces in the same line with the if()
> 
> 
> > +/* Make sure there's a trailing line-feed */
> > +if (command[cmdlen - 1] != '\n')
> > +g_byte_array_append (buf, (const guint8 *) "\n", 1);
> > +}
> >  }
> >  
> >  return buf;
> > @@ -318,12 +326,13 @@ mm_at_serial_port_queue_command (MMAtSerialPort *self,
> >   gpointer user_data)
> >  {
> >  GByteArray *buf;
> > +MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
> >  
> >  g_return_if_fail (self != NULL);
> >  g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
> >  g_return_if_fail (command != NULL);
> >  
> > -buf = at_command_to_byte_array (command, is_raw);
> > +buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
> >  g_return_if_fail (buf != NULL);
> >  
> >  mm_serial_port_queue_command (MM_SERIAL_PORT (self),
> > @@ -345,12 +354,13 @@ mm_at_serial_port_queue_command_cached 
> > (MMAtSerialPort *self,
> >  gpointer user_data)
> >  {
> >  GByteArray *buf;
> > +MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
> >  
> >  g_return_if_fail (self != NULL);
> >  g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
> >  g_return_if_fail (command != NULL);
> >  
> > -buf = at_command_to_byte_array (command, is_raw);
> > +buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
> >  g_return_if_fail (buf != NULL);
> >  
> >  mm_serial_port_queue_command_cached (MM_SERIAL_PORT (self),
> > @@ -434,6 +444,9 @@ mm_at_serial_port_init (MMAtSerialPort *self)
> >  
> >  /* By default, remove echo */
> >  priv->remove_echo = TRUE;
> > +
> > +/* By default, don't send line feed */
> > +priv->send_lf = FALSE;
> >  }
> >  
> >  static void
> > @@ -446,6 +459,10 @@ set_property (GObject *object, guint prop_id,
> >  case PROP_REMOVE_ECHO:
> >  priv->remove_echo = g_value_get_boolean (value);
> >  break;
> > +
> 
> No need for the empty line here.
> 
> > +case PROP_SEND_LF:
> > +priv->send_lf = g_value_get_boolean (value);
> > +break;
> >  default:
> >  G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> >  break;
> > @@ -462,6 +479,9 @@ get_property (GObject *object, guint prop_id,
> >  case PROP_REMOVE_ECHO:
> >  g_value_set_boolean (value, priv->remove_echo);
> >  break;
> > +case PROP_SEND_LF:
> > +g_value_set_bool

[PATCH] Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread ori.in...@altair-semi.com
From: ori inbar 

Change-Id: Id2df103232174d7a2b8575aaf04ed196e7a901d5
---
 src/mm-at-serial-port.c |   34 +++---
 src/mm-at-serial-port.h |2 ++
 src/mm-plugin.c |   19 +++
 src/mm-plugin.h |1 +
 src/mm-port-probe.c |5 +
 src/mm-port-probe.h |1 +
 6 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
index 840f38a..7ed7a82 100644
--- a/src/mm-at-serial-port.c
+++ b/src/mm-at-serial-port.c
@@ -31,6 +31,7 @@ G_DEFINE_TYPE (MMAtSerialPort, mm_at_serial_port, 
MM_TYPE_SERIAL_PORT)
 enum {
 PROP_0,
 PROP_REMOVE_ECHO,
+PROP_SEND_LF,
 LAST_PROP
 };
 
@@ -45,6 +46,7 @@ typedef struct {
 MMAtPortFlag flags;
 
 gboolean remove_echo;
+gboolean send_lf;
 } MMAtSerialPortPrivate;
 
 /*/
@@ -281,7 +283,7 @@ parse_unsolicited (MMSerialPort *port, GByteArray *response)
 /*/
 
 static GByteArray *
-at_command_to_byte_array (const char *command, gboolean is_raw)
+at_command_to_byte_array (const char *command, gboolean is_raw, gboolean 
send_lf)
 {
 GByteArray *buf;
 int cmdlen;
@@ -303,6 +305,12 @@ at_command_to_byte_array (const char *command, gboolean 
is_raw)
 /* Make sure there's a trailing carriage return */
 if (command[cmdlen - 1] != '\r')
 g_byte_array_append (buf, (const guint8 *) "\r", 1);
+if (send_lf)
+{
+/* Make sure there's a trailing line-feed */
+if (command[cmdlen - 1] != '\n')
+g_byte_array_append (buf, (const guint8 *) "\n", 1);
+}
 }
 
 return buf;
@@ -318,12 +326,13 @@ mm_at_serial_port_queue_command (MMAtSerialPort *self,
  gpointer user_data)
 {
 GByteArray *buf;
+MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
 
 g_return_if_fail (self != NULL);
 g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
 g_return_if_fail (command != NULL);
 
-buf = at_command_to_byte_array (command, is_raw);
+buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
 g_return_if_fail (buf != NULL);
 
 mm_serial_port_queue_command (MM_SERIAL_PORT (self),
@@ -345,12 +354,13 @@ mm_at_serial_port_queue_command_cached (MMAtSerialPort 
*self,
 gpointer user_data)
 {
 GByteArray *buf;
+MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
 
 g_return_if_fail (self != NULL);
 g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
 g_return_if_fail (command != NULL);
 
-buf = at_command_to_byte_array (command, is_raw);
+buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
 g_return_if_fail (buf != NULL);
 
 mm_serial_port_queue_command_cached (MM_SERIAL_PORT (self),
@@ -434,6 +444,9 @@ mm_at_serial_port_init (MMAtSerialPort *self)
 
 /* By default, remove echo */
 priv->remove_echo = TRUE;
+
+/* By default, don't send line feed */
+priv->send_lf = FALSE;
 }
 
 static void
@@ -446,6 +459,10 @@ set_property (GObject *object, guint prop_id,
 case PROP_REMOVE_ECHO:
 priv->remove_echo = g_value_get_boolean (value);
 break;
+
+case PROP_SEND_LF:
+priv->send_lf = g_value_get_boolean (value);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -462,6 +479,9 @@ get_property (GObject *object, guint prop_id,
 case PROP_REMOVE_ECHO:
 g_value_set_boolean (value, priv->remove_echo);
 break;
+case PROP_SEND_LF:
+g_value_set_boolean (value, priv->send_lf);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -517,4 +537,12 @@ mm_at_serial_port_class_init (MMAtSerialPortClass *klass)
"Built-in echo removal should be applied",
TRUE,
G_PARAM_READWRITE));
+
+g_object_class_install_property
+(object_class, PROP_SEND_LF,
+ g_param_spec_boolean (MM_AT_SERIAL_PORT_SEND_LF,
+   "Send LF",
+   "Send line-feed at the end of each AT command 
sent",
+   FALSE,
+   G_PARAM_READWRITE));
 }
diff --git a/src/mm-at-serial-port.h b/src/mm-at-serial-port.h
index 682e238..16d07a6 100644
--- a/src/mm-at-serial-port.h
+++ b/src/mm-at-serial-port.h
@@ -67,6 +67,8 @@ typedef void (*MMAtSerialResponseFn) (MMAtSerialPort 
*port,
 
 #define MM_AT_SERIAL_PORT_REMOVE_ECHO "remove-echo"
 
+#define MM_AT_SERIAL_PORT_SEND_LF "send-lf"
+
 struct _MMAtSerialPort {
 MMSerialPort parent;
 };
diff --git a/src/mm-plugin.c b/src/mm-plugin

[no subject]

2013-02-28 Thread ori.in...@altair-semi.com
To support modems that require a  line-feed at the end of AT commands sent
for example altair-lte modem.
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH] Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread ori.in...@altair-semi.com
From: ori inbar 

Change-Id: Id2df103232174d7a2b8575aaf04ed196e7a901d5
---
 src/mm-at-serial-port.c |   34 +++---
 src/mm-at-serial-port.h |2 ++
 src/mm-plugin.c |   19 +++
 src/mm-plugin.h |1 +
 src/mm-port-probe.c |5 +
 src/mm-port-probe.h |1 +
 6 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
index 840f38a..7ed7a82 100644
--- a/src/mm-at-serial-port.c
+++ b/src/mm-at-serial-port.c
@@ -31,6 +31,7 @@ G_DEFINE_TYPE (MMAtSerialPort, mm_at_serial_port, 
MM_TYPE_SERIAL_PORT)
 enum {
 PROP_0,
 PROP_REMOVE_ECHO,
+PROP_SEND_LF,
 LAST_PROP
 };
 
@@ -45,6 +46,7 @@ typedef struct {
 MMAtPortFlag flags;
 
 gboolean remove_echo;
+gboolean send_lf;
 } MMAtSerialPortPrivate;
 
 /*/
@@ -281,7 +283,7 @@ parse_unsolicited (MMSerialPort *port, GByteArray *response)
 /*/
 
 static GByteArray *
-at_command_to_byte_array (const char *command, gboolean is_raw)
+at_command_to_byte_array (const char *command, gboolean is_raw, gboolean 
send_lf)
 {
 GByteArray *buf;
 int cmdlen;
@@ -303,6 +305,12 @@ at_command_to_byte_array (const char *command, gboolean 
is_raw)
 /* Make sure there's a trailing carriage return */
 if (command[cmdlen - 1] != '\r')
 g_byte_array_append (buf, (const guint8 *) "\r", 1);
+if (send_lf)
+{
+/* Make sure there's a trailing line-feed */
+if (command[cmdlen - 1] != '\n')
+g_byte_array_append (buf, (const guint8 *) "\n", 1);
+}
 }
 
 return buf;
@@ -318,12 +326,13 @@ mm_at_serial_port_queue_command (MMAtSerialPort *self,
  gpointer user_data)
 {
 GByteArray *buf;
+MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
 
 g_return_if_fail (self != NULL);
 g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
 g_return_if_fail (command != NULL);
 
-buf = at_command_to_byte_array (command, is_raw);
+buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
 g_return_if_fail (buf != NULL);
 
 mm_serial_port_queue_command (MM_SERIAL_PORT (self),
@@ -345,12 +354,13 @@ mm_at_serial_port_queue_command_cached (MMAtSerialPort 
*self,
 gpointer user_data)
 {
 GByteArray *buf;
+MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
 
 g_return_if_fail (self != NULL);
 g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
 g_return_if_fail (command != NULL);
 
-buf = at_command_to_byte_array (command, is_raw);
+buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
 g_return_if_fail (buf != NULL);
 
 mm_serial_port_queue_command_cached (MM_SERIAL_PORT (self),
@@ -434,6 +444,9 @@ mm_at_serial_port_init (MMAtSerialPort *self)
 
 /* By default, remove echo */
 priv->remove_echo = TRUE;
+
+/* By default, don't send line feed */
+priv->send_lf = FALSE;
 }
 
 static void
@@ -446,6 +459,10 @@ set_property (GObject *object, guint prop_id,
 case PROP_REMOVE_ECHO:
 priv->remove_echo = g_value_get_boolean (value);
 break;
+
+case PROP_SEND_LF:
+priv->send_lf = g_value_get_boolean (value);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -462,6 +479,9 @@ get_property (GObject *object, guint prop_id,
 case PROP_REMOVE_ECHO:
 g_value_set_boolean (value, priv->remove_echo);
 break;
+case PROP_SEND_LF:
+g_value_set_boolean (value, priv->send_lf);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -517,4 +537,12 @@ mm_at_serial_port_class_init (MMAtSerialPortClass *klass)
"Built-in echo removal should be applied",
TRUE,
G_PARAM_READWRITE));
+
+g_object_class_install_property
+(object_class, PROP_SEND_LF,
+ g_param_spec_boolean (MM_AT_SERIAL_PORT_SEND_LF,
+   "Send LF",
+   "Send line-feed at the end of each AT command 
sent",
+   FALSE,
+   G_PARAM_READWRITE));
 }
diff --git a/src/mm-at-serial-port.h b/src/mm-at-serial-port.h
index 682e238..16d07a6 100644
--- a/src/mm-at-serial-port.h
+++ b/src/mm-at-serial-port.h
@@ -67,6 +67,8 @@ typedef void (*MMAtSerialResponseFn) (MMAtSerialPort 
*port,
 
 #define MM_AT_SERIAL_PORT_REMOVE_ECHO "remove-echo"
 
+#define MM_AT_SERIAL_PORT_SEND_LF "send-lf"
+
 struct _MMAtSerialPort {
 MMSerialPort parent;
 };
diff --git a/src/mm-plugin.c b/src/mm-plugin

[no subject]

2013-02-28 Thread ori.in...@altair-semi.com
To support modems that require a  line-feed at the end of AT commands sent
for example altair-lte modem.
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Aleksander Morgado

>>
>> When I reviewed your original patch I actually checked for when we were
>> removing the timeout when disabling, and assumed it was done when we set
>> the GObject's data to NULL, but I mixed the two data objects we handle
>> here, as one of them (the one where this new timeout id is stored) isn't
>> removed during disabling. So, yes, we need this method to remove the
>> timeout. Now, I would add the clearing method just when we start the
>> disabling sequence, next to the periodic_registration_check_disable()
>> call. Or was it added in CLEANUP_UNSOLICITED_REGISTRATION_EVENTS for
>> some reason?
>>
> 
> Could an unsolicited registration state update happen between
> invocations of  interface_disabling_step()? I was wondering if the
> deferred_update_id should be cleared after unsolicited registration
> event handling is removed.
> 

It could, yes, but we wouldn't really care as we're disabling anyway.

-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Ben Chan
One question inline

On Feb 28, 2013 12:23 AM, "Aleksander Morgado" 
wrote:
>
> On 02/27/2013 11:04 PM, Ben Chan wrote:
> > ---
> >  src/mm-iface-modem-3gpp.c | 18 +-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
>
> When I reviewed your original patch I actually checked for when we were
> removing the timeout when disabling, and assumed it was done when we set
> the GObject's data to NULL, but I mixed the two data objects we handle
> here, as one of them (the one where this new timeout id is stored) isn't
> removed during disabling. So, yes, we need this method to remove the
> timeout. Now, I would add the clearing method just when we start the
> disabling sequence, next to the periodic_registration_check_disable()
> call. Or was it added in CLEANUP_UNSOLICITED_REGISTRATION_EVENTS for
> some reason?
>

Could an unsolicited registration state update happen between invocations
of  interface_disabling_step()? I was wondering if the deferred_update_id
should be cleared after unsolicited registration event handling is removed.

> Also some minor things below.
>
> >
> > diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
> > index fa8da1a..3f3a803 100644
> > --- a/src/mm-iface-modem-3gpp.c
> > +++ b/src/mm-iface-modem-3gpp.c
> > @@ -1325,6 +1325,18 @@ periodic_registration_check_enable
(MMIfaceModem3gpp *self)
> >
(GDestroyNotify)registration_check_context_free);
> >  }
> >
> > +static void
> > +clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
> > +{
> > +RegistrationStateContext *ctx = get_registration_state_context
(self);
> > +
> > +ctx->deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
> > +if (ctx->deferred_update_id) {
> > +g_source_remove (ctx->deferred_update_id);
> > +ctx->deferred_update_id = 0;
> > +}
> > +}
> > +
> >
 /*/
> >
> >  typedef struct _DisablingContext DisablingContext;
> > @@ -1435,7 +1447,10 @@ interface_disabling_step (DisablingContext *ctx)
> >  ctx->step++;
> >  }
> >
> > -case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
> > +case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: {
>
> No real need to add braces here, unless we're adding new variables.
>
> > +/* Prevent any deferred registration state update from
happending after the modem is disabled */
>
>
> happending doesn't seem an English word to me :)
>

Oops... typo :)

>
> > +clear_deferred_registration_state_update (ctx->self);
> > +
> >  if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx->self)->cleanup_unsolicited_registration_events &&
> >  MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx->self)->cleanup_unsolicited_registration_events_finish) {
> >  MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx->self)->cleanup_unsolicited_registration_events (
> > @@ -1446,6 +1461,7 @@ interface_disabling_step (DisablingContext *ctx)
> >  }
> >  /* Fall down to next step */
> >  ctx->step++;
> > +}
> >
> >  case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
> >  if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx->self)->cleanup_unsolicited_events &&
> >
>
>
> --
> Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: NetworkManager C99 compiler requirement for 0.9.10 (and therefore 0.9.9)

2013-02-28 Thread Bastien Nocera
On Thu, 2013-02-28 at 04:57 -0500, Pavel Simerda wrote:
> Hi all,
> 
> I would like to do *occasional* C99-style declarations in the code to avoid 
> ugly hacks when using conditionally built code. That means bumping up the 
> compiler requirements to the C99 standard. As we currently only target to GCC 
> and possibly LLVM, I don't think it would be a problem.
> 
> I will modify the configure.ac to require and use C99, if there are no 
> objections on the list.
> 
> Below you can see an example of code that would benefit from moving 
> conditional stuff together.

You can also do:
#if !KERNEL_HACKS
{
gboolean link_was_up;

/* Current kernels may break IPv6 networking when performing 
this test:
 *
 * https://bugzilla.redhat.com/show_bug.cgi?id=886116
 *
 * We're therefore disabling it as a hack to preserve 
connectivity on
 * kernels that would break it.
 */
/* Loopback can be set up and down */
link_was_up = nm_platform_link_is_up (LO_INDEX);
g_assert (nm_platform_link_set_up (LO_INDEX));
g_assert (nm_platform_link_is_up (LO_INDEX));
g_assert (nm_platform_link_is_connected (LO_INDEX));
}
#endif

No need to bump the requirements. Or you could move the hacks to their
own separate function.

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


NetworkManager C99 compiler requirement for 0.9.10 (and therefore 0.9.9)

2013-02-28 Thread Pavel Simerda
Hi all,

I would like to do *occasional* C99-style declarations in the code to avoid 
ugly hacks when using conditionally built code. That means bumping up the 
compiler requirements to the C99 standard. As we currently only target to GCC 
and possibly LLVM, I don't think it would be a problem.

I will modify the configure.ac to require and use C99, if there are no 
objections on the list.

Below you can see an example of code that would benefit from moving conditional 
stuff together.

Cheers,

Pavel


static void
test_loopback ()
{
#if !KERNEL_HACKS
gboolean link_was_up;
#endif
gboolean link_exists = FALSE;

/* Loopback device exists and has the correct name and index */
g_assert (nm_platform_link_exists (LO_NAME));
g_assert (nm_platform_link_get_type (LO_INDEX) == NM_LINK_TYPE_GENERIC);
g_assert (nm_platform_link_get_index (LO_NAME) == LO_INDEX);
g_assert (!g_strcmp0(nm_platform_link_get_name (LO_INDEX), LO_NAME));

/* Foreach */
nm_platform_link_foreach (loopback_check, &link_exists);
g_assert (link_exists);

#if !KERNEL_HACKS
/* Current kernels may break IPv6 networking when performing this test:
 *
 * https://bugzilla.redhat.com/show_bug.cgi?id=886116
 *
 * We're therefore disabling it as a hack to preserve connectivity on
 * kernels that would break it.
 */
/* Loopback can be set up and down */
link_was_up = nm_platform_link_is_up (LO_INDEX);
g_assert (nm_platform_link_set_up (LO_INDEX));
g_assert (nm_platform_link_is_up (LO_INDEX));
g_assert (nm_platform_link_is_connected (LO_INDEX));
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread Aleksander Morgado
On 02/28/2013 09:24 AM, ori.in...@altair-semi.com wrote:
> From: ori inbar 
> 

Looks good; some minor comments below.

Also, are you going to suggest a new plugin for MM using this property
set to TRUE? If so, it may be a good idea to send all relevant commits,
including this one, in the same patch series, along with the plugin.


> ---
>  src/mm-at-serial-port.c |   34 +++---
>  src/mm-at-serial-port.h |2 ++
>  src/mm-plugin.c |   19 +++
>  src/mm-plugin.h |1 +
>  src/mm-port-probe.c |5 +
>  src/mm-port-probe.h |1 +
>  6 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
> index 840f38a..7ed7a82 100644
> --- a/src/mm-at-serial-port.c
> +++ b/src/mm-at-serial-port.c
> @@ -31,6 +31,7 @@ G_DEFINE_TYPE (MMAtSerialPort, mm_at_serial_port, 
> MM_TYPE_SERIAL_PORT)
>  enum {
>  PROP_0,
>  PROP_REMOVE_ECHO,
> +PROP_SEND_LF,
>  LAST_PROP
>  };
>  
> @@ -45,6 +46,7 @@ typedef struct {
>  MMAtPortFlag flags;
>  
>  gboolean remove_echo;
> +gboolean send_lf;
>  } MMAtSerialPortPrivate;
>  
>  
> /*/
> @@ -281,7 +283,7 @@ parse_unsolicited (MMSerialPort *port, GByteArray 
> *response)
>  
> /*/
>  
>  static GByteArray *
> -at_command_to_byte_array (const char *command, gboolean is_raw)
> +at_command_to_byte_array (const char *command, gboolean is_raw, gboolean 
> send_lf)
>  {
>  GByteArray *buf;
>  int cmdlen;
> @@ -303,6 +305,12 @@ at_command_to_byte_array (const char *command, gboolean 
> is_raw)
>  /* Make sure there's a trailing carriage return */
>  if (command[cmdlen - 1] != '\r')
>  g_byte_array_append (buf, (const guint8 *) "\r", 1);
> +if (send_lf)
> +{

Braces in the same line with the if()


> +/* Make sure there's a trailing line-feed */
> +if (command[cmdlen - 1] != '\n')
> +g_byte_array_append (buf, (const guint8 *) "\n", 1);
> +}
>  }
>  
>  return buf;
> @@ -318,12 +326,13 @@ mm_at_serial_port_queue_command (MMAtSerialPort *self,
>   gpointer user_data)
>  {
>  GByteArray *buf;
> +MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
>  
>  g_return_if_fail (self != NULL);
>  g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
>  g_return_if_fail (command != NULL);
>  
> -buf = at_command_to_byte_array (command, is_raw);
> +buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
>  g_return_if_fail (buf != NULL);
>  
>  mm_serial_port_queue_command (MM_SERIAL_PORT (self),
> @@ -345,12 +354,13 @@ mm_at_serial_port_queue_command_cached (MMAtSerialPort 
> *self,
>  gpointer user_data)
>  {
>  GByteArray *buf;
> +MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
>  
>  g_return_if_fail (self != NULL);
>  g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
>  g_return_if_fail (command != NULL);
>  
> -buf = at_command_to_byte_array (command, is_raw);
> +buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
>  g_return_if_fail (buf != NULL);
>  
>  mm_serial_port_queue_command_cached (MM_SERIAL_PORT (self),
> @@ -434,6 +444,9 @@ mm_at_serial_port_init (MMAtSerialPort *self)
>  
>  /* By default, remove echo */
>  priv->remove_echo = TRUE;
> +
> +/* By default, don't send line feed */
> +priv->send_lf = FALSE;
>  }
>  
>  static void
> @@ -446,6 +459,10 @@ set_property (GObject *object, guint prop_id,
>  case PROP_REMOVE_ECHO:
>  priv->remove_echo = g_value_get_boolean (value);
>  break;
> +

No need for the empty line here.

> +case PROP_SEND_LF:
> +priv->send_lf = g_value_get_boolean (value);
> +break;
>  default:
>  G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>  break;
> @@ -462,6 +479,9 @@ get_property (GObject *object, guint prop_id,
>  case PROP_REMOVE_ECHO:
>  g_value_set_boolean (value, priv->remove_echo);
>  break;
> +case PROP_SEND_LF:
> +g_value_set_boolean (value, priv->send_lf);
> +break;
>  default:
>  G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>  break;
> @@ -517,4 +537,12 @@ mm_at_serial_port_class_init (MMAtSerialPortClass *klass)
> "Built-in echo removal should be applied",
> TRUE,
> G_PARAM_READWRITE));
> +
> +g_object_class_install_property
> +(object_class, PROP_SEND_LF,
> + g_param_spec_boolean (MM_AT_SERIAL_PORT_SEND_LF,
> +   "Send LF",
> +

[MM] [PATCH v3] serial: Add support to optionally send line-feed at the end of AT commands.

2013-02-28 Thread ori.in...@altair-semi.com
From: ori inbar 

---
 src/mm-at-serial-port.c |   34 +++---
 src/mm-at-serial-port.h |2 ++
 src/mm-plugin.c |   19 +++
 src/mm-plugin.h |1 +
 src/mm-port-probe.c |5 +
 src/mm-port-probe.h |1 +
 6 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
index 840f38a..7ed7a82 100644
--- a/src/mm-at-serial-port.c
+++ b/src/mm-at-serial-port.c
@@ -31,6 +31,7 @@ G_DEFINE_TYPE (MMAtSerialPort, mm_at_serial_port, 
MM_TYPE_SERIAL_PORT)
 enum {
 PROP_0,
 PROP_REMOVE_ECHO,
+PROP_SEND_LF,
 LAST_PROP
 };
 
@@ -45,6 +46,7 @@ typedef struct {
 MMAtPortFlag flags;
 
 gboolean remove_echo;
+gboolean send_lf;
 } MMAtSerialPortPrivate;
 
 /*/
@@ -281,7 +283,7 @@ parse_unsolicited (MMSerialPort *port, GByteArray *response)
 /*/
 
 static GByteArray *
-at_command_to_byte_array (const char *command, gboolean is_raw)
+at_command_to_byte_array (const char *command, gboolean is_raw, gboolean 
send_lf)
 {
 GByteArray *buf;
 int cmdlen;
@@ -303,6 +305,12 @@ at_command_to_byte_array (const char *command, gboolean 
is_raw)
 /* Make sure there's a trailing carriage return */
 if (command[cmdlen - 1] != '\r')
 g_byte_array_append (buf, (const guint8 *) "\r", 1);
+if (send_lf)
+{
+/* Make sure there's a trailing line-feed */
+if (command[cmdlen - 1] != '\n')
+g_byte_array_append (buf, (const guint8 *) "\n", 1);
+}
 }
 
 return buf;
@@ -318,12 +326,13 @@ mm_at_serial_port_queue_command (MMAtSerialPort *self,
  gpointer user_data)
 {
 GByteArray *buf;
+MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
 
 g_return_if_fail (self != NULL);
 g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
 g_return_if_fail (command != NULL);
 
-buf = at_command_to_byte_array (command, is_raw);
+buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
 g_return_if_fail (buf != NULL);
 
 mm_serial_port_queue_command (MM_SERIAL_PORT (self),
@@ -345,12 +354,13 @@ mm_at_serial_port_queue_command_cached (MMAtSerialPort 
*self,
 gpointer user_data)
 {
 GByteArray *buf;
+MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self);
 
 g_return_if_fail (self != NULL);
 g_return_if_fail (MM_IS_AT_SERIAL_PORT (self));
 g_return_if_fail (command != NULL);
 
-buf = at_command_to_byte_array (command, is_raw);
+buf = at_command_to_byte_array (command, is_raw, priv->send_lf);
 g_return_if_fail (buf != NULL);
 
 mm_serial_port_queue_command_cached (MM_SERIAL_PORT (self),
@@ -434,6 +444,9 @@ mm_at_serial_port_init (MMAtSerialPort *self)
 
 /* By default, remove echo */
 priv->remove_echo = TRUE;
+
+/* By default, don't send line feed */
+priv->send_lf = FALSE;
 }
 
 static void
@@ -446,6 +459,10 @@ set_property (GObject *object, guint prop_id,
 case PROP_REMOVE_ECHO:
 priv->remove_echo = g_value_get_boolean (value);
 break;
+
+case PROP_SEND_LF:
+priv->send_lf = g_value_get_boolean (value);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -462,6 +479,9 @@ get_property (GObject *object, guint prop_id,
 case PROP_REMOVE_ECHO:
 g_value_set_boolean (value, priv->remove_echo);
 break;
+case PROP_SEND_LF:
+g_value_set_boolean (value, priv->send_lf);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -517,4 +537,12 @@ mm_at_serial_port_class_init (MMAtSerialPortClass *klass)
"Built-in echo removal should be applied",
TRUE,
G_PARAM_READWRITE));
+
+g_object_class_install_property
+(object_class, PROP_SEND_LF,
+ g_param_spec_boolean (MM_AT_SERIAL_PORT_SEND_LF,
+   "Send LF",
+   "Send line-feed at the end of each AT command 
sent",
+   FALSE,
+   G_PARAM_READWRITE));
 }
diff --git a/src/mm-at-serial-port.h b/src/mm-at-serial-port.h
index 682e238..16d07a6 100644
--- a/src/mm-at-serial-port.h
+++ b/src/mm-at-serial-port.h
@@ -67,6 +67,8 @@ typedef void (*MMAtSerialResponseFn) (MMAtSerialPort 
*port,
 
 #define MM_AT_SERIAL_PORT_REMOVE_ECHO "remove-echo"
 
+#define MM_AT_SERIAL_PORT_SEND_LF "send-lf"
+
 struct _MMAtSerialPort {
 MMSerialPort parent;
 };
diff --git a/src/mm-plugin.c b/src/mm-plugin.c
index b6889ef..9a863d8 100644
--- a/src/mm-plugin.

Re: [MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Aleksander Morgado
On 02/27/2013 11:04 PM, Ben Chan wrote:
> ---
>  src/mm-iface-modem-3gpp.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)

When I reviewed your original patch I actually checked for when we were
removing the timeout when disabling, and assumed it was done when we set
the GObject's data to NULL, but I mixed the two data objects we handle
here, as one of them (the one where this new timeout id is stored) isn't
removed during disabling. So, yes, we need this method to remove the
timeout. Now, I would add the clearing method just when we start the
disabling sequence, next to the periodic_registration_check_disable()
call. Or was it added in CLEANUP_UNSOLICITED_REGISTRATION_EVENTS for
some reason?

Also some minor things below.

> 
> diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
> index fa8da1a..3f3a803 100644
> --- a/src/mm-iface-modem-3gpp.c
> +++ b/src/mm-iface-modem-3gpp.c
> @@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp 
> *self)
>   
> (GDestroyNotify)registration_check_context_free);
>  }
>  
> +static void
> +clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
> +{
> +RegistrationStateContext *ctx = get_registration_state_context (self);
> +
> +ctx->deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
> +if (ctx->deferred_update_id) {
> +g_source_remove (ctx->deferred_update_id);
> +ctx->deferred_update_id = 0;
> +}
> +}
> +
>  
> /*/
>  
>  typedef struct _DisablingContext DisablingContext;
> @@ -1435,7 +1447,10 @@ interface_disabling_step (DisablingContext *ctx)
>  ctx->step++;
>  }
>  
> -case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
> +case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: {

No real need to add braces here, unless we're adding new variables.

> +/* Prevent any deferred registration state update from happending 
> after the modem is disabled */


happending doesn't seem an English word to me :)


> +clear_deferred_registration_state_update (ctx->self);
> +
>  if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_registration_events &&
>  MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_registration_events_finish) {
>  MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_registration_events (
> @@ -1446,6 +1461,7 @@ interface_disabling_step (DisablingContext *ctx)
>  }
>  /* Fall down to next step */
>  ctx->step++;
> +}
>  
>  case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
>  if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
> (ctx->self)->cleanup_unsolicited_events &&
> 


-- 
Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list