<div dir="ltr">I guess we can't expect too much from AT commands/responses, except them being a string... Perhaps it's time to move on to MBIM :)<div> </div><div>In the meantime, we can probably proceed with these two patches:</div> <div><a href="http://lists.freedesktop.org/archives/modemmanager-devel/2013-August/000113.html">http://lists.freedesktop.org/archives/modemmanager-devel/2013-August/000113.html</a> </div><div><a href="http://lists.freedesktop.org/archives/modemmanager-devel/2013-August/000114.html">http://lists.freedesktop.org/archives/modemmanager-devel/2013-August/000114.html</a> </div><div> </div><div>Thanks, <div>Ben</div></div></div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Aug 6, 2013 at 10:52 AM, Dan Williams <<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Tue, 2013-08-06 at 10:15 -0700, Ben Chan wrote: > Thanks Dan. Looks like we sometimes need to ignore the ^PDPDEACT > unsolicited messages as well. > > <debug> [000141.977013] [mm-at-serial-port.c:408] debug_log(): (ttyUSB0): > <-- '<CR><LF>^RSSI: > 10<CR><LF><CR><LF>^CSNR:-101,-8<CR><LF><CR><LF>^PDPDEACT: > 3<CR><LF><CR><LF>^NDISSTAT: 0,,,"IPV4"<CR><LF>' > > I'm not sure if there is any other unsolicited message that should be > ignored. Perhaps we need a catch-all regex for handling uninterested > messages? </div>If you've got some time, you can investigate having the parser code ignore unknown unsolicited responses and look for the expected response. There are a number of corner cases though, where some devices and some commands don't send the original command back at all (eg, they just do "\r\n<data>" instead of "\r\nORIGCMD:<data>") or they send back a different command than the original command. There's pretty incredible variation in firmware :) Dan <div class="HOEnZb"><div class="h5"> > Ben > > > On Tue, Aug 6, 2013 at 7:26 AM, Dan Williams <<a href="mailto:dcbw@redhat.com">dcbw@redhat.com</a>> wrote: > > > 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 <<a href="mailto:dcbw@redhat.com">dcbw@redhat.com</a>> 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 <<a href="mailto:fangxiaozhi@huawei.com">fangxiaozhi@huawei.com</a>> > > > > > > > > > > And then revised by: > > > > > Ben Chan <<a href="mailto:benchan@chromium.org">benchan@chromium.org</a>> > > > > > --- > > > > > 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 > > > <a href="mailto:ModemManager-devel@lists.freedesktop.org">ModemManager-devel@lists.freedesktop.org</a> > > > <a href="http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel</a> > > > > > > > _______________________________________________ > ModemManager-devel mailing list > <a href="mailto:ModemManager-devel@lists.freedesktop.org">ModemManager-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel</a> </div></div></blockquote></div> </div>