[PATCH v3] core: process SMS +CGMR response with regex
Aleksander Morgado
aleksander at aleksander.es
Tue Sep 22 00:37:22 PDT 2015
On Thu, Sep 17, 2015 at 11:31 PM, Nick Stevens <Nick.Stevens at digi.com> wrote:
> Variability in the response style from certain modems causes the parsing
> of the +CGMR response to fail. For example, the Telit HE910 inserts an
> empty string ("") in the second field of the response, causing the
> sscanf implementation to fail.
>
> This patch converts the parsing of the CGMR response to a regex that
> allows for more flexibility and robustness, and adds unit tests around
> the parsing call.
>
> Signed-off-by: Nick Stevens <Nick.Stevens at digi.com>
Thanks; pushed to git master and mm-1-4.
Also fixed a memleak in the unit tests in a follow-up commit.
> ---
> V2->V3:
> * Refactored parsing logic to use "goto" on error, simplifying logic for error
> handling
>
> V1->V2:
> * Moved parsing logic to mm-modem-helpers.[ch]
> * Added unit tests for CGMR parsing call
> - Generic case based on CGML test case
> - Telit-specific case that contains extra empty string in <alpha> field
>
> src/mm-broadband-modem.c | 17 ++++------
> src/mm-modem-helpers.c | 73 +++++++++++++++++++++++++++++++++++++++++-
> src/mm-modem-helpers.h | 5 +++
> src/mm-sms-part-3gpp.h | 2 --
> src/tests/test-modem-helpers.c | 64 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 148 insertions(+), 13 deletions(-)
>
> diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> index 9093db5..2013b78 100644
> --- a/src/mm-broadband-modem.c
> +++ b/src/mm-broadband-modem.c
> @@ -5603,8 +5603,7 @@ sms_part_ready (MMBroadbandModem *self,
> SmsPartContext *ctx)
> {
> MMSmsPart *part;
> - gint rv, status, tpdu_len;
> - gchar pdu[MM_SMS_PART_3GPP_MAX_PDU_LEN + 1];
> + MM3gppPduInfo *info;
> const gchar *response;
> GError *error = NULL;
>
> @@ -5622,19 +5621,16 @@ sms_part_ready (MMBroadbandModem *self,
> return;
> }
>
> - rv = sscanf (response, "+CMGR: %d,,%d %" G_STRINGIFY (MM_SMS_PART_3GPP_MAX_PDU_LEN) "s",
> - &status, &tpdu_len, pdu);
> - if (rv != 3) {
> - error = g_error_new (MM_CORE_ERROR,
> - MM_CORE_ERROR_FAILED,
> - "Failed to parse CMGR response (parsed %d items)", rv);
> - mm_warn ("Couldn't retrieve SMS part: '%s'", error->message);
> + info = mm_3gpp_parse_cmgr_read_response (response, ctx->idx, &error);
> + if (!info) {
> + mm_warn ("Couldn't parse SMS part: '%s'",
> + error->message);
> g_simple_async_result_take_error (ctx->result, error);
> sms_part_context_complete_and_free (ctx);
> return;
> }
>
> - part = mm_sms_part_3gpp_new_from_pdu (ctx->idx, pdu, &error);
> + part = mm_sms_part_3gpp_new_from_pdu (info->index, info->pdu, &error);
> if (part) {
> mm_dbg ("Correctly parsed PDU (%d)", ctx->idx);
> mm_iface_modem_messaging_take_part (MM_IFACE_MODEM_MESSAGING (self),
> @@ -5648,6 +5644,7 @@ sms_part_ready (MMBroadbandModem *self,
> }
>
> /* All done */
> + mm_3gpp_pdu_info_free (info);
> g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> sms_part_context_complete_and_free (ctx);
> }
> diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
> index d40082d..a11bbbc 100644
> --- a/src/mm-modem-helpers.c
> +++ b/src/mm-modem-helpers.c
> @@ -1161,6 +1161,77 @@ mm_3gpp_parse_cmgf_test_response (const gchar *reply,
>
> /*************************************************************************/
>
> +MM3gppPduInfo *
> +mm_3gpp_parse_cmgr_read_response (const gchar *reply,
> + guint index,
> + GError **error)
> +{
> + GRegex *r;
> + GMatchInfo *match_info;
> + gint count;
> + gint status;
> + gchar *pdu;
> + MM3gppPduInfo *info = NULL;
> +
> + /* +CMGR: <stat>,<alpha>,<length>(whitespace)<pdu> */
> + /* The <alpha> and <length> fields are matched, but not currently used */
> + r = g_regex_new ("\\+CMGR:\\s*(\\d+)\\s*,([^,]*),\\s*(\\d+)\\s*([^\\r\\n]*)", 0, 0, NULL);
> + g_assert (r);
> +
> + if (!g_regex_match_full (r, reply, strlen (reply), 0, 0, &match_info, NULL)) {
> + g_set_error (error,
> + MM_CORE_ERROR,
> + MM_CORE_ERROR_FAILED,
> + "Failed to parse CMGR read result: response didn't match '%s'",
> + reply);
> + goto done;
> + }
> +
> + /* g_match_info_get_match_count includes match #0 */
> + if ((count = g_match_info_get_match_count (match_info)) != 5) {
> + g_set_error (error,
> + MM_CORE_ERROR,
> + MM_CORE_ERROR_FAILED,
> + "Failed to match CMGR fields (matched %d) '%s'",
> + count,
> + reply);
> + goto done;
> + }
> +
> + if (!mm_get_int_from_match_info (match_info, 1, &status)) {
> + g_set_error (error,
> + MM_CORE_ERROR,
> + MM_CORE_ERROR_FAILED,
> + "Failed to extract CMGR status field '%s'",
> + reply);
> + goto done;
> + }
> +
> +
> + pdu = mm_get_string_unquoted_from_match_info (match_info, 4);
> + if (!pdu) {
> + g_set_error (error,
> + MM_CORE_ERROR,
> + MM_CORE_ERROR_FAILED,
> + "Failed to extract CMGR pdu field '%s'",
> + reply);
> + goto done;
> + }
> +
> + info = g_new0 (MM3gppPduInfo, 1);
> + info->index = index;
> + info->status = status;
> + info->pdu = pdu;
> +
> +done:
> + g_match_info_free (match_info);
> + g_regex_unref (r);
> +
> + return info;
> +}
> +
> +/*************************************************************************/
> +
> static MMSmsStorage
> storage_from_str (const gchar *str)
> {
> @@ -1658,7 +1729,7 @@ done:
>
> /*************************************************************************/
>
> -static void
> +void
> mm_3gpp_pdu_info_free (MM3gppPduInfo *info)
> {
> g_free (info->pdu);
> diff --git a/src/mm-modem-helpers.h b/src/mm-modem-helpers.h
> index 0ec59af..edc33f9 100644
> --- a/src/mm-modem-helpers.h
> +++ b/src/mm-modem-helpers.h
> @@ -181,10 +181,15 @@ typedef struct {
> gint status;
> gchar *pdu;
> } MM3gppPduInfo;
> +void mm_3gpp_pdu_info_free (MM3gppPduInfo *info);
> void mm_3gpp_pdu_info_list_free (GList *info_list);
> GList *mm_3gpp_parse_pdu_cmgl_response (const gchar *str,
> GError **error);
>
> +/* AT+CMGR (Read message) response parser */
> +MM3gppPduInfo *mm_3gpp_parse_cmgr_read_response (const gchar *reply,
> + guint index,
> + GError **error);
>
> /* Additional 3GPP-specific helpers */
>
> diff --git a/src/mm-sms-part-3gpp.h b/src/mm-sms-part-3gpp.h
> index 82709a2..a21d734 100644
> --- a/src/mm-sms-part-3gpp.h
> +++ b/src/mm-sms-part-3gpp.h
> @@ -22,8 +22,6 @@
>
> #include "mm-sms-part.h"
>
> -#define MM_SMS_PART_3GPP_MAX_PDU_LEN 344
> -
> MMSmsPart *mm_sms_part_3gpp_new_from_pdu (guint index,
> const gchar *hexpdu,
> GError **error);
> diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
> index b493e6b..db8fc89 100644
> --- a/src/tests/test-modem-helpers.c
> +++ b/src/tests/test-modem-helpers.c
> @@ -196,6 +196,67 @@ test_cmgl_response_pantech_multiple (void *f, gpointer d)
> }
>
> /*****************************************************************************/
> +/* Test CMGR responses */
> +
> +static void
> +test_cmgr_response (const gchar *str,
> + const MM3gppPduInfo *expected)
> +{
> + MM3gppPduInfo *info;
> + GError *error = NULL;
> +
> + info = mm_3gpp_parse_cmgr_read_response (str, 0, &error);
> + g_assert_no_error (error);
> + g_assert (info != NULL);
> +
> + /* Ignore index, it is not included in CMGR response */
> + g_assert_cmpint (info->status, ==, expected->status);
> + g_assert_cmpstr (info->pdu, ==, expected->pdu);
> +}
> +
> +static void
> +test_cmgr_response_generic (void *f, gpointer d)
> +{
> + const gchar *str =
> + "+CMGR: 1,,147 07914306073011F00405812261F700003130916191314095C27"
> + "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
> + "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
> + "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
> + "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07";
> +
> + const MM3gppPduInfo expected = {
> + .index = 0,
> + .status = 1,
> + .pdu = "07914306073011F00405812261F700003130916191314095C27"
> + "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
> + "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
> + "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
> + "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07"
> + };
> +
> + test_cmgr_response (str, &expected);
> +}
> +
> +/* Telit HE910 places empty quotation marks in the <alpha> field and a CR+LF
> + * before the PDU */
> +static void
> +test_cmgr_response_telit (void *f, gpointer d)
> +{
> + const gchar *str =
> + "+CMGR: 0,\"\",50\r\n07916163838428F9040B916121021021F7000051905141642"
> + "20A23C4B0BCFD5E8740C4B0BCFD5E83C26E3248196687C9A0301D440DBBC3677918";
> +
> + const MM3gppPduInfo expected = {
> + .index = 0,
> + .status = 0,
> + .pdu = "07916163838428F9040B916121021021F7000051905141642"
> + "20A23C4B0BCFD5E8740C4B0BCFD5E83C26E3248196687C9A0301D440DBBC3677918"
> + };
> +
> + test_cmgr_response (str, &expected);
> +}
> +
> +/*****************************************************************************/
> /* Test COPS responses */
>
> static void
> @@ -2497,6 +2558,9 @@ int main (int argc, char **argv)
> g_test_suite_add (suite, TESTCASE (test_cmgl_response_pantech, NULL));
> g_test_suite_add (suite, TESTCASE (test_cmgl_response_pantech_multiple, NULL));
>
> + g_test_suite_add (suite, TESTCASE (test_cmgr_response_generic, NULL));
> + g_test_suite_add (suite, TESTCASE (test_cmgr_response_telit, NULL));
> +
> g_test_suite_add (suite, TESTCASE (test_supported_mode_filter, NULL));
>
> g_test_suite_add (suite, TESTCASE (test_supported_capability_filter, NULL));
> --
> 1.9.1
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
--
Aleksander
https://aleksander.es
More information about the ModemManager-devel
mailing list