On Thu, 2013-02-28 at 09:38 +0100, Aleksander Morgado wrote:
> 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.
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 <LF> 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_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,
> >
>
>
_______________________________________________
networkmanager-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list
