> > Attached a patch for review, which does this echo removal in the v1 > parser. I'm assuming here that all known devices do prefix their replies > with <CR><LF> :-) If that is not the case, I can improve the patch to > make this behaviour configurable per plugin (even for the probing > process). At least it works super well for my use case. If we do accept > this patch, we could possibly remove from the sources the v1_e1 parser, > which wouldn't be needed any more. >
Attaching updated version of this patch to handle some corner cases; and also including some unit tests. -- Aleksander
>From 48ce30df846bb2a742943d7b4d4bb1d695cc0d9c Mon Sep 17 00:00:00 2001 From: Aleksander Morgado <aleksan...@lanedo.com> Date: Wed, 11 Jan 2012 01:33:05 +0100 Subject: [PATCH] at-serial-port: implement built-in echo/garbage removal We expect the responses to start always with <CR><LF>. We just remove anything that comes before that. --- .gitignore | 1 + src/mm-at-serial-port.c | 27 +++++++++++++ src/mm-at-serial-port.h | 4 +- src/tests/Makefile.am | 15 +++++++- src/tests/test-at-serial-port.c | 83 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 src/tests/test-at-serial-port.c diff --git a/.gitignore b/.gitignore index 64e7b89..384f08a 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ test/lsudev src/tests/test-modem-helpers src/tests/test-charsets src/tests/test-qcdm-serial-port +src/tests/test-at-serial-port src/tests/test-sms policy/org.freedesktop.modem-manager.policy include/ diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c index 30da3a3..140d093 100644 --- a/src/mm-at-serial-port.c +++ b/src/mm-at-serial-port.c @@ -58,6 +58,27 @@ mm_at_serial_port_set_response_parser (MMAtSerialPort *self, priv->response_parser_notify = notify; } +void +mm_at_serial_port_remove_echo (GByteArray *response) +{ + guint i; + + if (response->len <= 2) + return; + + for (i = 0; i < (response->len - 2); i++) { + /* If there is any content before the first + * <CR><LF>, assume it's echo or garbage, and skip it */ + if (response->data[i] == '\r' && response->data[i + 1] == '\n') { + if (i > 0) + g_byte_array_remove_range (response, 0, i); + else + /* good, we're already started with <CR><LF> */ + break; + } + } +} + static gboolean parse_response (MMSerialPort *port, GByteArray *response, GError **error) { @@ -68,6 +89,9 @@ parse_response (MMSerialPort *port, GByteArray *response, GError **error) g_return_val_if_fail (priv->response_parser_fn != NULL, FALSE); + /* Remove echo */ + mm_at_serial_port_remove_echo (response); + /* Construct the string that AT-parsing functions expect */ string = g_string_sized_new (response->len + 1); g_string_append_len (string, (const char *) response->data, response->len); @@ -159,6 +183,9 @@ parse_unsolicited (MMSerialPort *port, GByteArray *response) MMAtSerialPortPrivate *priv = MM_AT_SERIAL_PORT_GET_PRIVATE (self); GSList *iter; + /* Remove echo */ + mm_at_serial_port_remove_echo (response); + for (iter = priv->unsolicited_msg_handlers; iter; iter = iter->next) { MMAtUnsolicitedMsgHandler *handler = (MMAtUnsolicitedMsgHandler *) iter->data; GMatchInfo *match_info; diff --git a/src/mm-at-serial-port.h b/src/mm-at-serial-port.h index cec5dc3..689c184 100644 --- a/src/mm-at-serial-port.h +++ b/src/mm-at-serial-port.h @@ -80,5 +80,7 @@ void mm_at_serial_port_queue_command_cached (MMAtSerialPort *self, MMAtSerialResponseFn callback, gpointer user_data); -#endif /* MM_AT_SERIAL_PORT_H */ +/* Just for unit tests */ +void mm_at_serial_port_remove_echo (GByteArray *response); +#endif /* MM_AT_SERIAL_PORT_H */ diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index cc47e66..0ed88c2 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -5,6 +5,7 @@ noinst_PROGRAMS = \ test-modem-helpers \ test-charsets \ test-qcdm-serial-port \ + test-at-serial-port \ test-sms test_modem_helpers_SOURCES = \ @@ -41,6 +42,19 @@ test_qcdm_serial_port_LDADD = \ $(top_builddir)/libqcdm/src/libqcdm.la \ -lutil +test_at_serial_port_SOURCES = \ + test-at-serial-port.c + +test_at_serial_port_CPPFLAGS = \ + $(MM_CFLAGS) \ + -I$(top_srcdir) + +test_at_serial_port_LDADD = \ + $(MM_LIBS) \ + $(top_builddir)/src/libserial.la \ + $(top_builddir)/src/libmodem-helpers.la \ + -lutil + test_sms_SOURCES = \ test-sms.c @@ -60,4 +74,3 @@ check-local: test-modem-helpers $(abs_builddir)/test-sms endif - diff --git a/src/tests/test-at-serial-port.c b/src/tests/test-at-serial-port.c new file mode 100644 index 0000000..76ccadc --- /dev/null +++ b/src/tests/test-at-serial-port.c @@ -0,0 +1,83 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details: + * + * Copyright (C) 2012 Aleksander Morgado <aleksan...@gnu.org> + */ + +#include <config.h> +#include <string.h> +#include <glib.h> + +#include "mm-errors.h" +#include "mm-at-serial-port.h" +#include "mm-log.h" + +typedef struct { + gchar *original; + gchar *without_echo; +} EchoRemovalTest; + +static const EchoRemovalTest echo_removal_tests[] = { + { "\r\n", "\r\n" }, + { "\r", "\r" }, + { "\n", "\n" }, + { "this is a string that ends just with <CR>\r", "this is a string that ends just with <CR>\r" }, + { "this is a string that ends just with <CR>\n", "this is a string that ends just with <CR>\n" }, + { "\r\nthis is valid", "\r\nthis is valid" }, + { "a\r\nthis is valid", "\r\nthis is valid" }, + { "a\r\n", "\r\n" }, + { "all this string is to be considered echo\r\n", "\r\n" }, + { "all this string is to be considered echo\r\nthis is valid", "\r\nthis is valid" }, +}; + +static void +at_serial_echo_removal (void) +{ + guint i; + + for (i = 0; i < G_N_ELEMENTS (echo_removal_tests); i++) { + GByteArray *ba; + + /* Note that we add last NUL also to the byte array, so that we can compare + * C strings later on */ + ba = g_byte_array_sized_new (strlen (echo_removal_tests[i].original) + 1); + g_byte_array_prepend (ba, + (guint8 *)echo_removal_tests[i].original, + strlen (echo_removal_tests[i].original) + 1); + + mm_at_serial_port_remove_echo (ba); + + g_assert_cmpstr ((gchar *)ba->data, ==, echo_removal_tests[i].without_echo); + + g_byte_array_unref (ba); + } +} + +void +_mm_log (const char *loc, + const char *func, + guint32 level, + const char *fmt, + ...) +{ + /* Dummy log function */ +} + +int main (int argc, char **argv) +{ + g_type_init (); + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/ModemManager/AT-serial/echo-removal", at_serial_echo_removal); + + return g_test_run (); +} -- 1.7.5.4
_______________________________________________ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list