[PATCH] huawei: fix parsing of ^NDISSTATQRY? response
Dan Williams
dcbw at redhat.com
Tue Aug 6 07:26:44 PDT 2013
On Mon, 2013-08-05 at 21:58 -0700, Ben Chan wrote:
> Hi Dan,
>
> Now that you mentioned it, the issue isn't quite obvious to me by looking
> at the log. I printed out the "response" passed to
> mm_huawei_parse_ndisstatqry_response and observed the "^RSSI: 16^M^M"
> message in front of the "^NDISSTATQRY" response:
I think I see it now. The modem puts a space between the : and the
number, and the regex doesn't handle that:
self->priv->rssi_regex = g_regex_new ("\\r\\n\\^RSSI:(\\d+)\\r\\n",
so I think it should be a simple fixup to all the regexes in Huawei to
discard spaces ("\\s*") between the : and the interesting stuff?
Dan
> <debug> [000027.596303] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> --> 'AT^NDISSTATQRY?<CR>'
> <debug> [000027.617122] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> <-- '<CR><LF>^NDISSTATQRY: 0,,,"IPV4"<CR><LF><CR><LF>OK<CR><LF>'
> <debug> [000027.617316] [mm-serial-port.c:1018] mm_serial_port_close():
> (ttyUSB0) device open count is 1 (close)
> <debug> [000028.561321] [mm-serial-port.c:972] mm_serial_port_open():
> (ttyUSB0) device open count is 2 (open)
> <debug> [000028.561476] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> --> 'AT^NDISSTATQRY?<CR>'
> <debug> [000028.584342] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> <-- '<CR><LF>^NDISSTATQRY: 0,,,"IPV4"<CR><LF><CR><LF>OK<CR><LF>'
> <debug> [000028.584642] [mm-serial-port.c:1018] mm_serial_port_close():
> (ttyUSB0) device open count is 1 (close)
> <debug> [000029.218829] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> <-- '<CR><LF>^RSSI: 16<CR><LF><CR><LF>^CSNR:-87,-5<CR><LF>'
> <debug> [000029.561465] [mm-serial-port.c:972] mm_serial_port_open():
> (ttyUSB0) device open count is 2 (open)
> <debug> [000029.561841] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> --> 'AT^NDISSTATQRY?<CR>'
> <debug> [000029.582647] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0):
> <-- '<CR><LF>^NDISSTATQRY: 0,,,"IPV4"<CR><LF><CR><LF>OK<CR><LF>'
> <debug> [000029.582735] [huawei/mm-modem-helpers-huawei.c:42]
> mm_huawei_parse_ndisstatqry_response(): Missing ^NDISSTATQRY prefix:
> '^RSSI: 16^M
> ^M
> ^NDISSTATQRY: 0,,,"IPV4"'
> <debug> [000029.582747] [huawei/mm-broadband-bearer-huawei.c:133]
> connect_ndisstatqry_check_ready(): Modem doesn't properly support
> ^NDISSTATQRY command: Missing ^NDISSTATQRY prefix
> <debug> [000029.582767] [mm-serial-port.c:1018] mm_serial_port_close():
> (ttyUSB0) device open count is 1 (close)
> <debug> [000029.582865] [mm-bearer.c:461] connect_ready(): Couldn't connect
> bearer '/org/freedesktop/ModemManager1/Bearer/0': 'Connection attempt not
> supported'
> <info> [000029.582960] [mm-iface-modem.c:1203]
> __iface_modem_update_state_internal(): Modem
> /org/freedesktop/ModemManager1/Modem/0: state changed (connecting ->
> registered)
> <debug> [000029.583709] [mm-iface-modem-simple.c:221]
> connect_bearer_ready(): Couldn't connect bearer: 'Connection attempt not
> supported'
>
> Thanks,
> Ben
>
> On Mon, Aug 5, 2013 at 9:08 PM, Dan Williams <dcbw at redhat.com> wrote:
>
> > On Mon, 2013-08-05 at 19:46 -0700, Ben Chan wrote:
> > > This patch modifies mm_huawei_parse_ndisstatqry_response() to properly
> > > handle a ^NDISSTATQRY? response when it is mixed with another unsolicited
> > > response, e.g.
> > >
> > > ^RSSI: 16\r\n^NDISSTATQRY: 0,,,"IPV4"
> >
> > We'd expect the unsolicited responses to be cleared out of the response
> > string already by the serial parser. Is that not happening? At least
> > for ^RSSI, the regex expects a leading \r\n though. Are there any logs
> > you've got that show a more complete command sequence?
> >
> > Dan
> >
> > > This patch is originally developed by:
> > > Franko Fang <fangxiaozhi at huawei.com>
> > >
> > > And then revised by:
> > > Ben Chan <benchan at chromium.org>
> > > ---
> > > plugins/huawei/mm-modem-helpers-huawei.c | 6 +-
> > > plugins/huawei/tests/test-modem-helpers-huawei.c | 70
> > +++++++++++++++---------
> > > 2 files changed, 48 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/plugins/huawei/mm-modem-helpers-huawei.c
> > b/plugins/huawei/mm-modem-helpers-huawei.c
> > > index d9b038d..e1e05c4 100644
> > > --- a/plugins/huawei/mm-modem-helpers-huawei.c
> > > +++ b/plugins/huawei/mm-modem-helpers-huawei.c
> > > @@ -20,6 +20,7 @@
> > > #define _LIBMM_INSIDE_MM
> > > #include <libmm-glib.h>
> > >
> > > +#include "mm-log.h"
> > > #include "mm-modem-helpers-huawei.h"
> > >
> > >
> > /*****************************************************************************/
> > > @@ -37,7 +38,10 @@ mm_huawei_parse_ndisstatqry_response (const gchar
> > *response,
> > > GMatchInfo *match_info;
> > > GError *inner_error = NULL;
> > >
> > > - if (!response || !g_str_has_prefix (response, "^NDISSTATQRY:")) {
> > > + if (response)
> > > + response = g_strstr_len (response, -1, "^NDISSTATQRY:");
> > > +
> > > + if (!response) {
> > > g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> > "Missing ^NDISSTATQRY prefix");
> > > return FALSE;
> > > }
> > > diff --git a/plugins/huawei/tests/test-modem-helpers-huawei.c
> > b/plugins/huawei/tests/test-modem-helpers-huawei.c
> > > index c1bc0ed..5e2e83d 100644
> > > --- a/plugins/huawei/tests/test-modem-helpers-huawei.c
> > > +++ b/plugins/huawei/tests/test-modem-helpers-huawei.c
> > > @@ -17,6 +17,9 @@
> > > #include <glib-object.h>
> > > #include <locale.h>
> > >
> > > +#include <ModemManager.h>
> > > +
> > > +#include "mm-errors-types.h"
> > > #include "mm-modem-helpers-huawei.h"
> > >
> > >
> > /*****************************************************************************/
> > > @@ -24,6 +27,7 @@
> > >
> > > typedef struct {
> > > const gchar *str;
> > > + gboolean expected_result;
> > > gboolean expected_ipv4_available;
> > > gboolean expected_ipv4_connected;
> > > gboolean expected_ipv6_available;
> > > @@ -31,39 +35,48 @@ typedef struct {
> > > } NdisstatqryTest;
> > >
> > > static const NdisstatqryTest ndisstatqry_tests[] = {
> > > - { "^NDISSTATQRY: 1,,,IPV4\r\n", TRUE, TRUE, FALSE, FALSE },
> > > - { "^NDISSTATQRY: 0,,,IPV4\r\n", TRUE, FALSE, FALSE, FALSE },
> > > - { "^NDISSTATQRY: 1,,,IPV6\r\n", FALSE, FALSE, TRUE, TRUE },
> > > - { "^NDISSTATQRY: 0,,,IPV6\r\n", FALSE, FALSE, TRUE, FALSE },
> > > + { "^NDISSTATQRY: 1,,,IPV4\r\n", TRUE, TRUE, TRUE, FALSE, FALSE },
> > > + { "^NDISSTATQRY: 0,,,IPV4\r\n", TRUE, TRUE, FALSE, FALSE, FALSE },
> > > + { "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE, FALSE, FALSE, TRUE, TRUE },
> > > + { "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE, FALSE, FALSE, TRUE, FALSE },
> > > { "^NDISSTATQRY: 1,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE, TRUE, TRUE, TRUE },
> > > + "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE, TRUE, TRUE, TRUE, TRUE },
> > > { "^NDISSTATQRY: 1,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE, TRUE, TRUE, FALSE },
> > > + "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE, TRUE, TRUE, TRUE, FALSE },
> > > { "^NDISSTATQRY: 0,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE, FALSE, TRUE, TRUE },
> > > + "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE, TRUE, FALSE, TRUE, TRUE },
> > > { "^NDISSTATQRY: 0,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE, FALSE, TRUE, FALSE },
> > > - { "^NDISSTATQRY: 1,,,IPV4", TRUE, TRUE, FALSE, FALSE },
> > > - { "^NDISSTATQRY: 0,,,IPV4", TRUE, FALSE, FALSE, FALSE },
> > > - { "^NDISSTATQRY: 1,,,IPV6", FALSE, FALSE, TRUE, TRUE },
> > > - { "^NDISSTATQRY: 0,,,IPV6", FALSE, FALSE, TRUE, FALSE },
> > > + "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE, TRUE, FALSE, TRUE, FALSE },
> > > + { "^NDISSTATQRY: 1,,,IPV4", TRUE, TRUE, TRUE, FALSE, FALSE },
> > > + { "^NDISSTATQRY: 0,,,IPV4", TRUE, TRUE, FALSE, FALSE, FALSE },
> > > + { "^NDISSTATQRY: 1,,,IPV6", TRUE, FALSE, FALSE, TRUE, TRUE },
> > > + { "^NDISSTATQRY: 0,,,IPV6", TRUE, FALSE, FALSE, TRUE, FALSE },
> > > { "^NDISSTATQRY: 1,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 1,,,IPV6", TRUE, TRUE, TRUE, TRUE },
> > > + "^NDISSTATQRY: 1,,,IPV6", TRUE, TRUE, TRUE, TRUE, TRUE },
> > > { "^NDISSTATQRY: 1,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 0,,,IPV6", TRUE, TRUE, TRUE, FALSE },
> > > + "^NDISSTATQRY: 0,,,IPV6", TRUE, TRUE, TRUE, TRUE, FALSE },
> > > { "^NDISSTATQRY: 0,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 1,,,IPV6", TRUE, FALSE, TRUE, TRUE },
> > > + "^NDISSTATQRY: 1,,,IPV6", TRUE, TRUE, FALSE, TRUE, TRUE },
> > > { "^NDISSTATQRY: 0,,,IPV4\r\n"
> > > - "^NDISSTATQRY: 0,,,IPV6", TRUE, FALSE, TRUE, FALSE },
> > > - { NULL, FALSE, FALSE, FALSE, FALSE }
> > > + "^NDISSTATQRY: 0,,,IPV6", TRUE, TRUE, FALSE, TRUE, FALSE },
> > > + { "^RSSI: 16\r\n"
> > > + "^NDISSTATQRY: 0,,,IPV4", TRUE, TRUE, FALSE, FALSE, FALSE
> > },
> > > + { "^RSSI: 16\r\n"
> > > + "^NDISSTATQRY: 0,,,IPV4\r\n"
> > > + "^NDISSTATQRY: 0,,,IPV6", TRUE, TRUE, FALSE, TRUE, FALSE },
> > > + { "^RSSI: 16\r\n", FALSE, FALSE, FALSE, FALSE, FALSE },
> > > + { "OK\r\n", FALSE, FALSE, FALSE, FALSE, FALSE },
> > > + { "\r\n", FALSE, FALSE, FALSE, FALSE, FALSE },
> > > + { "", FALSE, FALSE, FALSE, FALSE, FALSE },
> > > + { NULL, FALSE, FALSE, FALSE, FALSE, FALSE }
> > > };
> > >
> > > static void
> > > test_ndisstatqry (void)
> > > {
> > > - guint i;
> > > + size_t i;
> > >
> > > - for (i = 0; ndisstatqry_tests[i].str; i++) {
> > > + for (i = 0; i < sizeof (ndisstatqry_tests) / sizeof
> > (ndisstatqry_tests[0]); i++) {
> > > GError *error = NULL;
> > > gboolean ipv4_available;
> > > gboolean ipv4_connected;
> > > @@ -76,15 +89,18 @@ test_ndisstatqry (void)
> > > &ipv4_connected,
> > > &ipv6_available,
> > > &ipv6_connected,
> > > - &error) == TRUE);
> > > - g_assert_no_error (error);
> > > + &error) == ndisstatqry_tests[i].expected_result);
> > > + if (ndisstatqry_tests[i].expected_result) {
> > > + g_assert_no_error (error);
> > > + g_assert (ipv4_available ==
> > ndisstatqry_tests[i].expected_ipv4_available);
> > > + if (ipv4_available)
> > > + g_assert (ipv4_connected ==
> > ndisstatqry_tests[i].expected_ipv4_connected);
> > > + g_assert (ipv6_available ==
> > ndisstatqry_tests[i].expected_ipv6_available);
> > > + if (ipv6_available)
> > > + g_assert (ipv6_connected ==
> > ndisstatqry_tests[i].expected_ipv6_connected);
> > > + } else
> > > + g_assert_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED);
> > >
> > > - g_assert (ipv4_available ==
> > ndisstatqry_tests[i].expected_ipv4_available);
> > > - if (ipv4_available)
> > > - g_assert (ipv4_connected ==
> > ndisstatqry_tests[i].expected_ipv4_connected);
> > > - g_assert (ipv6_available ==
> > ndisstatqry_tests[i].expected_ipv6_available);
> > > - if (ipv6_available)
> > > - g_assert (ipv6_connected ==
> > ndisstatqry_tests[i].expected_ipv6_connected);
> > > }
> > > }
> > >
> >
> >
> >
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
More information about the ModemManager-devel
mailing list