altair-lte: ignore invalid bands in %BANDCAP / %GETCFG="BAND" responses

Due to a firmware issue, the modem may reply an invalid band value, such
as 0, for the %BANDCAP or %GETCFG="BAND" command. This patch moves
parse_bands_response() to mm-modem-helpers-altair-lte.c, modifies the
function to ignore any invalid band outside the range of E-UTRAN
operating bands, and add unit tests for the function.

4 files changed, 73 insertions, 33 deletions

diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c
index 8240185c..d313a51b 100644
--- a/plugins/altair/mm-broadband-modem-altair-lte.c
+++ b/plugins/altair/mm-broadband-modem-altair-lte.c
@@ -264,37 +264,6 @@ load_current_capabilities (MMIfaceModem *self,
 }
 
 /*****************************************************************************/
-/* supported/current Bands helpers*/
-
-static GArray *
-parse_bands_response (const gchar *response)
-{
-    guint32 bandval;
-    MMModemBand band;
-    gchar **split;
-    guint i, num_of_bands;
-    GArray *bands;
-
-    split = g_strsplit_set (response, ",", -1);
-    if (!split)
-        return NULL;
-
-    num_of_bands = g_strv_length (split);
-
-    bands = g_array_sized_new (FALSE, FALSE, sizeof (MMModemBand), num_of_bands);
-
-    for (i = 0; split[i]; i++) {
-        bandval = (guint32)strtoul (split[i], NULL, 10);
-        band = MM_MODEM_BAND_EUTRAN_I - 1 + bandval;
-        g_array_append_val (bands, band);
-    }
-
-    g_strfreev (split);
-
-    return bands;
-}
-
-/*****************************************************************************/
 /* Load supported bands (Modem interface) */
 
 static GArray *
@@ -334,7 +303,7 @@ load_supported_bands_done (MMIfaceModem *self,
      */
     response = mm_strip_tag (response, BANDCAP_TAG);
 
-    bands = parse_bands_response (response);
+    bands = mm_altair_parse_bands_response (response);
     if (!bands) {
         mm_dbg ("Failed to parse supported bands response");
         g_simple_async_result_set_error (
@@ -415,7 +384,7 @@ load_current_bands_done (MMIfaceModem *self,
      */
     response = mm_strip_tag (response, CFGBANDS_TAG);
 
-    bands = parse_bands_response (response);
+    bands = mm_altair_parse_bands_response (response);
     if (!bands) {
         mm_dbg ("Failed to parse current bands response");
         g_simple_async_result_set_error (
diff --git a/plugins/altair/mm-modem-helpers-altair-lte.c b/plugins/altair/mm-modem-helpers-altair-lte.c
index e19f147b..b369e29c 100644
--- a/plugins/altair/mm-modem-helpers-altair-lte.c
+++ b/plugins/altair/mm-modem-helpers-altair-lte.c
@@ -14,6 +14,7 @@
  *
  */
 
+#include <stdlib.h>
 #include <string.h>
 
 #include <ModemManager.h>
@@ -23,6 +24,44 @@
 #include "mm-modem-helpers-altair-lte.h"
 
 /*****************************************************************************/
+/* Bands response parser */
+
+GArray *
+mm_altair_parse_bands_response (const gchar *response)
+{
+    gchar **split;
+    GArray *bands;
+    guint i;
+
+    /*
+     * Response is "<band>[,<band>...]"
+     */
+    split = g_strsplit_set (response, ",", -1);
+    if (!split)
+        return NULL;
+
+    bands = g_array_sized_new (FALSE, FALSE, sizeof (MMModemBand), g_strv_length (split));
+
+    for (i = 0; split[i]; i++) {
+        guint32 band_value;
+        MMModemBand band;
+
+        band_value = (guint32)strtoul (split[i], NULL, 10);
+        band = MM_MODEM_BAND_EUTRAN_I - 1 + band_value;
+
+        /* Due to a firmware issue, the modem may incorrectly includes 0 in the
+         * bands response. We thus ignore any band value outside the range of
+         * E-UTRAN operating bands. */
+        if (band >= MM_MODEM_BAND_EUTRAN_I && band <= MM_MODEM_BAND_EUTRAN_XLIV)
+            g_array_append_val (bands, band);
+    }
+
+    g_strfreev (split);
+
+    return bands;
+}
+
+/*****************************************************************************/
 /* +CEER response parser */
 
 gchar *
diff --git a/plugins/altair/mm-modem-helpers-altair-lte.h b/plugins/altair/mm-modem-helpers-altair-lte.h
index c6fbf342..56a6e150 100644
--- a/plugins/altair/mm-modem-helpers-altair-lte.h
+++ b/plugins/altair/mm-modem-helpers-altair-lte.h
@@ -19,6 +19,9 @@
 
 #include <glib.h>
 
+/* Bands response parser */
+GArray *mm_altair_parse_bands_response (const gchar *response);
+
 /* +CEER response parser */
 gchar *mm_altair_parse_ceer_response (const gchar *response,
                                       GError **error);
diff --git a/plugins/altair/tests/test-modem-helpers-altair-lte.c b/plugins/altair/tests/test-modem-helpers-altair-lte.c
index 2df3eefa..46f70b61 100644
--- a/plugins/altair/tests/test-modem-helpers-altair-lte.c
+++ b/plugins/altair/tests/test-modem-helpers-altair-lte.c
@@ -20,9 +20,37 @@
 #include <glib-object.h>
 #include <locale.h>
 
+#include <ModemManager.h>
+#define _LIBMM_INSIDE_MM
+#include <libmm-glib.h>
+
 #include "mm-modem-helpers-altair-lte.h"
 
 /*****************************************************************************/
+/* Test bands response parsing */
+
+static void
+test_parse_bands (void)
+{
+    GArray *bands;
+
+    bands = mm_altair_parse_bands_response ("");
+    g_assert (bands != NULL);
+    g_assert_cmpuint (bands->len, ==, 0);
+    g_array_free (bands, TRUE);
+
+    /* 0 and 45 are outside the range of E-UTRAN operating bands and should be ignored. */
+    bands = mm_altair_parse_bands_response ("0, 0, 1, 4,13,44,45");
+    g_assert (bands != NULL);
+    g_assert_cmpuint (bands->len, ==, 4);
+    g_assert_cmpuint (g_array_index (bands, MMModemBand, 0), ==, MM_MODEM_BAND_EUTRAN_I);
+    g_assert_cmpuint (g_array_index (bands, MMModemBand, 1), ==, MM_MODEM_BAND_EUTRAN_IV);
+    g_assert_cmpuint (g_array_index (bands, MMModemBand, 2), ==, MM_MODEM_BAND_EUTRAN_XIII);
+    g_assert_cmpuint (g_array_index (bands, MMModemBand, 3), ==, MM_MODEM_BAND_EUTRAN_XLIV);
+    g_array_free (bands, TRUE);
+}
+
+/*****************************************************************************/
 /* Test +CEER responses */
 
 typedef struct {
@@ -105,6 +133,7 @@ int main (int argc, char **argv)
     g_type_init ();
     g_test_init (&argc, &argv, NULL);
 
+    g_test_add_func ("/MM/altair/parse_bands", test_parse_bands);
     g_test_add_func ("/MM/altair/ceer", test_ceer);
     g_test_add_func ("/MM/altair/parse_cid", test_parse_cid);
     g_test_add_func ("/MM/altair/parse_vendor_pco_info", test_parse_vendor_pco_info);