gsm: prefer AT+CIND signal quality for modems that support it (lp:682282) (bgo #636040)

Some devices always reply with 99 for AT+CSQ when in UMTS mode (Linktop LW273)
so if the modem supports it, use CIND/CIEV instead.

src/mm-generic-gsm.c

@@ -97,9 +97,9 @@ typedef struct {
     gboolean manual_reg;
 
     gboolean cmer_enabled;
-    gint roam_ind;
+    guint roam_ind;
-    gint signal_ind;
+    guint signal_ind;
-    gint service_ind;
+    guint service_ind;
 
     guint signal_quality_id;
     time_t signal_emit_timestamp;
@@ -1260,7 +1260,7 @@ cind_cb (MMAtSerialPort *port,
     self = MM_GENERIC_GSM (user_data);
     priv = MM_GENERIC_GSM_GET_PRIVATE (self);
 
-    indicators = mm_parse_cind_response (response->str, NULL);
+    indicators = mm_parse_cind_test_response (response->str, NULL);
     if (indicators) {
         CindResponse *r;
 
@@ -3296,95 +3296,48 @@ mm_generic_gsm_update_signal_quality (MMGenericGsm *self, guint32 quality)
 #define CIND_TAG "+CIND:"
 
 static void
-cind_signal_cb (MMAtSerialPort *port,
+get_cind_signal_done (MMAtSerialPort *port,
-                GString *response,
+                      GString *response,
-                GError *error,
+                      GError *error,
-                gpointer user_data)
+                      gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    const char *p = response->str;
+    MMGenericGsmPrivate *priv;
-    GRegex *r = NULL;
+    GByteArray *indicators;
-    GMatchInfo *match_info;
+    guint quality;
-    int i = 1, quality = -1;
 
     info->error = mm_modem_check_removed (info->modem, error);
-    if (info->error)
+    if (!info->error) {
-        goto done;
+        priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem);
 
-    if (!g_str_has_prefix (p, CIND_TAG)) {
+        indicators = mm_parse_cind_query_response (response->str, &info->error);
-        info->error = g_error_new_literal (MM_MODEM_ERROR,
+        if (indicators) {
-                                           MM_MODEM_ERROR_GENERAL,
+            if (indicators->len >= priv->signal_ind) {
-                                           "Could not parse the +CIND response");
+                quality = g_array_index (indicators, guint8, priv->signal_ind);
-        goto done;
+                quality = CLAMP (quality, 0, 5) * 20;
-    }
+                mm_generic_gsm_update_signal_quality (MM_GENERIC_GSM (info->modem), quality);
-
+                mm_callback_info_set_result (info, GUINT_TO_POINTER (quality), NULL);
-    p += strlen (CIND_TAG);
-    while (isspace (*p))
-        p++;
-
-    r = g_regex_new ("(\\d+)[^0-9]+", G_REGEX_UNGREEDY, 0, NULL);
-    if (!r) {
-        info->error = g_error_new_literal (MM_MODEM_ERROR,
-                                           MM_MODEM_ERROR_GENERAL,
-                                           "Internal failure attempting to parse +CIND response");
-        goto done;
-    }
-
-    if (!g_regex_match_full (r, p, strlen (p), 0, 0, &match_info, NULL)) {
-        info->error = g_error_new_literal (MM_MODEM_ERROR,
-                                           MM_MODEM_ERROR_GENERAL,
-                                           "Failure parsing the +CIND response");
-        goto done;
-    }
-
-    while (g_match_info_matches (match_info)) {
-        if (MM_GENERIC_GSM_GET_PRIVATE (info->modem)->signal_ind == i++) {
-            char *str = g_match_info_fetch (match_info, 1);
-
-            if (str) {
-                quality = atoi (str);
-                mm_generic_gsm_update_signal_quality (MM_GENERIC_GSM (info->modem), quality * 20);
-                g_free (str);
-                break;
             }
+            g_byte_array_free (indicators, TRUE);
         }
-        g_match_info_next (match_info, NULL);
     }
-    g_match_info_free (match_info);
-
-done:
-    if (r)
-        g_regex_unref (r);
 
     mm_callback_info_schedule (info);
 }
 
 static void
-get_signal_quality_done (MMAtSerialPort *port,
+get_csq_done (MMAtSerialPort *port,
-                         GString *response,
+              GString *response,
-                         GError *error,
+              GError *error,
-                         gpointer user_data)
+              gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
     char *reply = response->str;
     gboolean parsed = FALSE;
 
     info->error = mm_modem_check_removed (info->modem, error);
-    if (info->error) {
+    if (info->error)
-        if (info->modem) {
-            MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem);
-
-            /* Modem won't do unsolicited reporting via +CMER/+CIEV, but still
-             * supports +CIND.
-             */
-            if (!priv->cmer_enabled && (priv->signal_ind >= 0)) {
-                g_clear_error (&info->error);
-                mm_at_serial_port_queue_command (port, "+CIND?", 3, cind_signal_cb, info);
-                return;
-            }
-        }
         goto done;
-    }
 
     if (!strncmp (reply, "+CSQ: ", 6)) {
         /* Got valid reply */
@@ -3429,9 +3382,13 @@ get_signal_quality (MMModemGsmNetwork *modem,
     info = mm_callback_info_uint_new (MM_MODEM (modem), callback, user_data);
 
     port = mm_generic_gsm_get_best_at_port (MM_GENERIC_GSM (modem), NULL);
-    if (port)
+    if (port) {
-        mm_at_serial_port_queue_command (port, "+CSQ", 3, get_signal_quality_done, info);
+        /* Prefer +CIND if the modem supports it, fall back to +CSQ otherwise */
-    else {
+        if (priv->signal_ind)
+            mm_at_serial_port_queue_command (port, "+CIND?", 3, get_cind_signal_done, info);
+        else
+            mm_at_serial_port_queue_command (port, "+CSQ", 3, get_csq_done, info);
+    } else {
         /* Use cached signal quality */
         mm_callback_info_set_result (info, GUINT_TO_POINTER (priv->signal_quality), NULL);
         mm_callback_info_schedule (info);
@@ -4694,9 +4651,6 @@ mm_generic_gsm_init (MMGenericGsm *self)
     priv->act = MM_MODEM_GSM_ACCESS_TECH_UNKNOWN;
     priv->reg_regex = mm_gsm_creg_regex_get (TRUE);
     priv->roam_allowed = TRUE;
-    priv->signal_ind = -1;
-    priv->roam_ind = -1;
-    priv->service_ind = -1;
 
     mm_properties_changed_signal_register_property (G_OBJECT (self),
                                                     MM_MODEM_GSM_NETWORK_ALLOWED_MODE,

src/mm-modem-helpers.c

@@ -888,13 +888,13 @@ mm_create_device_identifier (guint vid,
 
 struct CindResponse {
     char *desc;
-    gint idx;
+    guint idx;
     gint min;
     gint max;
 };
 
 static CindResponse *
-cind_response_new (const char *desc, gint idx, gint min, gint max)
+cind_response_new (const char *desc, guint idx, gint min, gint max)
 {
     CindResponse *r;
     char *p;
@@ -936,10 +936,10 @@ cind_response_get_desc (CindResponse *r)
     return r->desc;
 }
 
-gint
+guint
 cind_response_get_index (CindResponse *r)
 {
-    g_return_val_if_fail (r != NULL, -1);
+    g_return_val_if_fail (r != NULL, 0);
 
     return r->idx;
 }
@@ -963,12 +963,12 @@ cind_response_get_max (CindResponse *r)
 #define CIND_TAG "+CIND:"
 
 GHashTable *
-mm_parse_cind_response (const char *reply, GError **error)
+mm_parse_cind_test_response (const char *reply, GError **error)
 {
     GHashTable *hash;
     GRegex *r;
     GMatchInfo *match_info;
-    gint idx = 1;
+    guint idx = 1;
 
     g_return_val_if_fail (reply != NULL, NULL);
 
@@ -1019,3 +1019,71 @@ mm_parse_cind_response (const char *reply, GError **error)
     return hash;
 }
 
+GByteArray *
+mm_parse_cind_query_response(const char *reply, GError **error)
+{
+    GByteArray *array = NULL;
+    const char *p = reply;
+    GRegex *r = NULL;
+    GMatchInfo *match_info;
+    guint8 t = 0;
+
+    g_return_val_if_fail (reply != NULL, NULL);
+
+    if (!g_str_has_prefix (p, CIND_TAG)) {
+        g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL,
+                             "Could not parse the +CIND response");
+        return NULL;
+    }
+
+    p += strlen (CIND_TAG);
+    while (isspace (*p))
+        p++;
+
+    r = g_regex_new ("(\\d+)[^0-9]+", G_REGEX_UNGREEDY, 0, NULL);
+    if (!r) {
+        g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL,
+                             "Internal failure attempting to parse +CIND response");
+        return NULL;
+    }
+
+    if (!g_regex_match_full (r, p, strlen (p), 0, 0, &match_info, NULL)) {
+        g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL,
+                             "Failure parsing the +CIND response");
+        goto done;
+    }
+
+    array = g_byte_array_sized_new (g_match_info_get_match_count (match_info));
+
+    /* Add a zero element so callers can use 1-based indexes returned by
+     * cind_response_get_index().
+     */
+    g_byte_array_append (array, &t, 1);
+
+    while (g_match_info_matches (match_info)) {
+        char *str;
+        gulong val;
+
+        str = g_match_info_fetch (match_info, 1);
+
+        errno = 0;
+        val = strtoul (str, NULL, 10);
+
+        t = 0;
+        if ((errno == 0) && (val < 255))
+            t = (guint8) val;
+        /* FIXME: indicate errors somehow? */
+        g_byte_array_append (array, &t, 1);
+
+        g_free (str);
+        g_match_info_next (match_info, NULL);
+    }
+    g_match_info_free (match_info);
+
+done:
+    if (r)
+        g_regex_unref (r);
+
+    return array;
+}
+

src/mm-modem-helpers.h

@@ -70,11 +70,13 @@ char *mm_create_device_identifier (guint vid,
                                    gboolean debug);
 
 typedef struct CindResponse CindResponse;
-GHashTable *mm_parse_cind_response  (const char *reply, GError **error);
+GHashTable *mm_parse_cind_test_response (const char *reply, GError **error);
-const char *cind_response_get_desc  (CindResponse *r);
+const char *cind_response_get_desc      (CindResponse *r);
-gint        cind_response_get_index (CindResponse *r);
+guint       cind_response_get_index     (CindResponse *r);
-gint        cind_response_get_min   (CindResponse *r);
+gint        cind_response_get_min       (CindResponse *r);
-gint        cind_response_get_max   (CindResponse *r);
+gint        cind_response_get_max       (CindResponse *r);
+
+GByteArray *mm_parse_cind_query_response(const char *reply, GError **error);
 
 #endif  /* MM_MODEM_HELPERS_H */
 

src/tests/test-modem-helpers.c

@@ -1095,7 +1095,7 @@ test_cind_results (const char *desc,
 
     g_print ("\nTesting %s +CIND response...\n", desc);
 
-    results = mm_parse_cind_response (reply, &error);
+    results = mm_parse_cind_test_response (reply, &error);
     g_assert (results);
     g_assert (error == NULL);