On 02/28/2013 09:24 AM, [email protected] wrote:
> From: ori inbar <[email protected]>
>
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
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list
