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

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",
> +                               "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.c
> +++ b/src/mm-plugin.c
> @@ -80,6 +80,7 @@ struct _MMPluginPrivate {
>      MMPortProbeAtCommand *custom_at_probe;
>      guint64 send_delay;
>      gboolean remove_echo;
> +    gboolean send_lf;
>  
>      /* Probing setup and/or post-probing filter.
>       * Plugins may use this method to decide whether they support a given
> @@ -110,6 +111,7 @@ enum {
>      PROP_CUSTOM_INIT,
>      PROP_SEND_DELAY,
>      PROP_REMOVE_ECHO,
> +    PROP_SEND_LF,
>      LAST_PROP
>  };
>  
> @@ -741,6 +743,7 @@ mm_plugin_supports_port (MMPlugin *self,
>                         ctx->flags,
>                         self->priv->send_delay,
>                         self->priv->remove_echo,
> +                       self->priv->send_lf,
>                         self->priv->custom_at_probe,
>                         self->priv->custom_init,
>                         (GAsyncReadyCallback)port_probe_run_ready,
> @@ -874,6 +877,7 @@ mm_plugin_init (MMPlugin *self)
>      /* Defaults */
>      self->priv->send_delay = 100000;
>      self->priv->remove_echo = TRUE;
> +    self->priv->send_lf = FALSE;
>  }
>  
>  static void
> @@ -969,6 +973,10 @@ set_property (GObject *object,
>          /* Construct only */
>          self->priv->remove_echo = g_value_get_boolean (value);
>          break;
> +    case PROP_SEND_LF:
> +        /* Construct only */
> +        self->priv->send_lf = g_value_get_boolean (value);
> +        break;
>      default:
>          G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>          break;
> @@ -1047,6 +1055,9 @@ get_property (GObject *object,
>      case PROP_REMOVE_ECHO:
>          g_value_set_boolean (value, self->priv->remove_echo);
>          break;
> +    case PROP_SEND_LF:
> +        g_value_set_boolean (value, self->priv->send_lf);
> +        break;
>      default:
>          G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>          break;
> @@ -1256,4 +1267,12 @@ mm_plugin_class_init (MMPluginClass *klass)
>                                 "Remove echo out of the AT responses",
>                                 TRUE,
>                                 G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
> +
> +    g_object_class_install_property
> +        (object_class, PROP_SEND_LF,
> +         g_param_spec_boolean (MM_PLUGIN_SEND_LF,
> +                               "Send LF",
> +                               "Send line-feed at the end of each AT command 
> sent",
> +                               FALSE,
> +                               G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
>  }
> diff --git a/src/mm-plugin.h b/src/mm-plugin.h
> index a423ceb..343d5a0 100644
> --- a/src/mm-plugin.h
> +++ b/src/mm-plugin.h
> @@ -59,6 +59,7 @@
>  #define MM_PLUGIN_CUSTOM_AT_PROBE           "custom-at-probe"
>  #define MM_PLUGIN_SEND_DELAY                "send-delay"
>  #define MM_PLUGIN_REMOVE_ECHO               "remove-echo"
> +#define MM_PLUGIN_SEND_LF                   "send-lf"
>  
>  typedef enum {
>      MM_PLUGIN_SUPPORTS_PORT_UNSUPPORTED = 0x0,
> diff --git a/src/mm-port-probe.c b/src/mm-port-probe.c
> index 263b6c4..e4e6569 100644
> --- a/src/mm-port-probe.c
> +++ b/src/mm-port-probe.c
> @@ -85,6 +85,8 @@ typedef struct {
>      guint64 at_send_delay;
>      /* Flag to leave/remove echo in AT responses */
>      gboolean at_remove_echo;
> +    /* Flag to send line-feed at the end of AT commands */
> +    gboolean at_send_lf;
>      /* Number of times we tried to open the AT port */
>      guint at_open_tries;
>      /* Custom initialization setup */
> @@ -975,6 +977,7 @@ serial_open_at (MMPortProbe *self)
>          g_object_set (task->serial,
>                        MM_SERIAL_PORT_SEND_DELAY,     task->at_send_delay,
>                        MM_AT_SERIAL_PORT_REMOVE_ECHO, task->at_remove_echo,
> +                      MM_AT_SERIAL_PORT_SEND_LF,     task->at_send_lf,
>                        MM_PORT_CARRIER_DETECT,        FALSE,
>                        MM_SERIAL_PORT_SPEW_CONTROL,   TRUE,
>                        NULL);
> @@ -1096,6 +1099,7 @@ mm_port_probe_run (MMPortProbe *self,
>                     MMPortProbeFlag flags,
>                     guint64 at_send_delay,
>                     gboolean at_remove_echo,
> +                   gboolean at_send_lf,
>                     const MMPortProbeAtCommand *at_custom_probe,
>                     const MMAsyncMethod *at_custom_init,
>                     GAsyncReadyCallback callback,
> @@ -1115,6 +1119,7 @@ mm_port_probe_run (MMPortProbe *self,
>      task = g_new0 (PortProbeRunTask, 1);
>      task->at_send_delay = at_send_delay;
>      task->at_remove_echo = at_remove_echo;
> +    task->at_send_lf = at_send_lf;
>      task->flags = MM_PORT_PROBE_NONE;
>      task->at_custom_probe = at_custom_probe;
>      task->at_custom_init = at_custom_init ? 
> (MMPortProbeAtCustomInit)at_custom_init->async : NULL;
> diff --git a/src/mm-port-probe.h b/src/mm-port-probe.h
> index 74a3420..dfce77a 100644
> --- a/src/mm-port-probe.h
> +++ b/src/mm-port-probe.h
> @@ -106,6 +106,7 @@ void     mm_port_probe_run        (MMPortProbe *self,
>                                     MMPortProbeFlag flags,
>                                     guint64 at_send_delay,
>                                     gboolean at_remove_echo,
> +                                   gboolean at_send_lf,
>                                     const MMPortProbeAtCommand 
> *at_custom_probe,
>                                     const MMAsyncMethod *at_custom_init,
>                                     GAsyncReadyCallback callback,
> 


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

Reply via email to