Merge request !1240 from 'port-delay'

port-probe: allow per-port ID_MM_TTY_AT_PROBE_TRIES tag for custom number of AT probes

https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/merge_requests/1240

6 files changed, 231 insertions, 74 deletions

diff --git a/include/ModemManager-tags.h b/include/ModemManager-tags.h
index fc2136ae..57067785 100644
--- a/include/ModemManager-tags.h
+++ b/include/ModemManager-tags.h
@@ -307,6 +307,22 @@
  */
 #define ID_MM_MAX_MULTIPLEXED_LINKS "ID_MM_MAX_MULTIPLEXED_LINKS"
 
+/**
+ * ID_MM_TTY_AT_PROBE_TRIES:
+ *
+ * For ports that require a longer time to become ready to respond to AT
+ * commands, this tag specifies maximum number of AT probes to try as an
+ * integer between 1 and 20 (inclusive). Each probe attempt has a three-second
+ * timeout before the next probe is tried. Values outside the allowed range
+ * will be clamped to the min/max.
+ *
+ * Plugins implementing custom initialization without opting into this tag
+ * will ignore it.
+ *
+ * Since: 1.24
+ */
+#define ID_MM_TTY_AT_PROBE_TRIES "ID_MM_TTY_AT_PROBE_TRIES"
+
 /*
  * The following symbols are deprecated. We don't add them to -compat
  * because this -tags file is not really part of the installed API.
diff --git a/src/mm-port-probe-at.c b/src/mm-port-probe-at.c
index 37a5ab27..4505fc76 100644
--- a/src/mm-port-probe-at.c
+++ b/src/mm-port-probe-at.c
@@ -44,7 +44,10 @@ mm_port_probe_response_processor_is_at (const gchar *command,
          * they will just go on to the next command. */
         if (g_error_matches (error,
                              MM_SERIAL_ERROR,
-                             MM_SERIAL_ERROR_RESPONSE_TIMEOUT)) {
+                             MM_SERIAL_ERROR_RESPONSE_TIMEOUT) ||
+            g_error_matches (error,
+                             MM_SERIAL_ERROR,
+                             MM_SERIAL_ERROR_SEND_FAILED)) {
             return FALSE;
         }
 
diff --git a/src/mm-port-probe.c b/src/mm-port-probe.c
index 45ffdde8..c01cbddb 100644
--- a/src/mm-port-probe.c
+++ b/src/mm-port-probe.c
@@ -120,6 +120,162 @@ static const MMStringUintMap port_subsys_map[] = {
 
 /*****************************************************************************/
 
+static const MMPortProbeAtCommand at_probing[] = {
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { "AT",  3, mm_port_probe_response_processor_is_at },
+    { NULL }
+};
+
+typedef struct {
+    MMPortSerialAt             *serial;
+    const MMPortProbeAtCommand *at_commands;
+    guint                       at_commands_limit;
+} EarlyAtProbeContext;
+
+static void
+early_at_probe_context_free (EarlyAtProbeContext *ctx)
+{
+    g_clear_object (&ctx->serial);
+    g_slice_free (EarlyAtProbeContext, ctx);
+}
+
+gboolean
+mm_port_probe_run_early_at_probe_finish (MMPortProbe   *self,
+                                         GAsyncResult  *result,
+                                         GError       **error)
+{
+    return g_task_propagate_boolean (G_TASK (result), error);
+}
+
+static void
+early_at_probe_parse_response (MMPortSerialAt *serial,
+                               GAsyncResult   *res,
+                               GTask          *task)
+{
+    g_autoptr(GVariant)  result = NULL;
+    g_autoptr(GError)    result_error = NULL;
+    g_autofree gchar    *response = NULL;
+    g_autoptr(GError)    command_error = NULL;
+    EarlyAtProbeContext *ctx;
+    MMPortProbe         *self;
+    gboolean             is_at = FALSE;
+
+    ctx = g_task_get_task_data (task);
+    self = g_task_get_source_object (task);
+
+    /* If already cancelled, do nothing else */
+    if (g_task_return_error_if_cancelled (task)) {
+        g_object_unref (task);
+        return;
+    }
+
+    response = mm_port_serial_at_command_finish (serial, res, &command_error);
+    if (!ctx->at_commands->response_processor (ctx->at_commands->command,
+                                               response,
+                                               !!ctx->at_commands[1].command,
+                                               command_error,
+                                               &result,
+                                               &result_error)) {
+        /* Were we told to abort the whole probing? */
+        if (result_error) {
+            g_task_return_new_error (task,
+                                     MM_CORE_ERROR,
+                                     MM_CORE_ERROR_UNSUPPORTED,
+                                     "(%s/%s) error while probing AT features: %s",
+                                     mm_kernel_device_get_subsystem (self->priv->port),
+                                     mm_kernel_device_get_name (self->priv->port),
+                                     result_error->message);
+            g_object_unref (task);
+            return;
+        }
+
+        /* Go on to next command */
+        ctx->at_commands++;
+        ctx->at_commands_limit--;
+        if (ctx->at_commands->command && ctx->at_commands_limit > 0) {
+            /* More commands in the group? */
+            mm_port_serial_at_command (
+                ctx->serial,
+                ctx->at_commands->command,
+                ctx->at_commands->timeout,
+                FALSE, /* raw */
+                FALSE, /* allow_cached */
+                g_task_get_cancellable (task),
+                (GAsyncReadyCallback)early_at_probe_parse_response,
+                task);
+            return;
+        }
+
+        /* No more commands in the group; end probing; not AT */
+    } else if (result) {
+        /* If any result given, it must be a boolean */
+        g_assert (g_variant_is_of_type (result, G_VARIANT_TYPE_BOOLEAN));
+        is_at = g_variant_get_boolean (result);
+    }
+
+    mm_port_probe_set_result_at (self, is_at);
+    g_task_return_boolean (task, is_at);
+    g_object_unref (task);
+}
+
+gboolean
+mm_port_probe_run_early_at_probe (MMPortProbe         *self,
+                                  MMPortSerialAt      *serial,
+                                  GCancellable        *cancellable,
+                                  GAsyncReadyCallback  callback,
+                                  gpointer             user_data)
+{
+    GTask               *task;
+    EarlyAtProbeContext *ctx;
+    gint                 tries;
+
+    tries = mm_kernel_device_get_global_property_as_int (mm_port_probe_peek_port (self),
+                                                         ID_MM_TTY_AT_PROBE_TRIES);
+    if (tries == 0) {
+        /* Early probing not required */
+        return FALSE;
+    }
+
+    task = g_task_new (self, cancellable, callback, user_data);
+
+    ctx = g_slice_new0 (EarlyAtProbeContext);
+    ctx->serial                = g_object_ref (serial);
+    ctx->at_commands           = at_probing;
+    ctx->at_commands_limit     = CLAMP (tries, 1, (gint) G_N_ELEMENTS (at_probing));
+    g_task_set_task_data (task, ctx, (GDestroyNotify) early_at_probe_context_free);
+
+    mm_port_serial_at_command (
+        ctx->serial,
+        ctx->at_commands->command,
+        ctx->at_commands->timeout,
+        FALSE, /* raw */
+        FALSE, /* allow_cached */
+        g_task_get_cancellable (task),
+        (GAsyncReadyCallback)early_at_probe_parse_response,
+        task);
+    return TRUE;
+}
+
+/*****************************************************************************/
+
 static void
 mm_port_probe_clear (MMPortProbe *self)
 {
@@ -420,6 +576,8 @@ typedef struct {
     const MMPortProbeAtCommand *at_custom_probe;
     /* Current group of AT commands to be sent */
     const MMPortProbeAtCommand *at_commands;
+    /* Maximum number of at_commands to be sent */
+    guint at_commands_limit;
     /* Seconds between each AT command sent in the group */
     guint at_commands_wait_secs;
     /* Current AT Result processor */
@@ -1071,7 +1229,8 @@ probe_at_parse_response (MMPortSerialAt *port,
 
         /* Go on to next command */
         ctx->at_commands++;
-        if (!ctx->at_commands->command) {
+        ctx->at_commands_limit--;
+        if (!ctx->at_commands->command || ctx->at_commands_limit == 0) {
             /* Was it the last command in the group? If so,
              * end this partial probing */
             ctx->at_result_processor (self, NULL);
@@ -1129,16 +1288,6 @@ probe_at (MMPortProbe *self)
     return G_SOURCE_REMOVE;
 }
 
-static const MMPortProbeAtCommand at_probing[] = {
-    { "AT",  3, mm_port_probe_response_processor_is_at },
-    { "AT",  3, mm_port_probe_response_processor_is_at },
-    { "AT",  3, mm_port_probe_response_processor_is_at },
-    { "AT",  3, mm_port_probe_response_processor_is_at },
-    { "AT",  3, mm_port_probe_response_processor_is_at },
-    { "AT",  3, mm_port_probe_response_processor_is_at },
-    { NULL }
-};
-
 static const MMPortProbeAtCommand vendor_probing[] = {
     { "+CGMI", 3, mm_port_probe_response_processor_string },
     { "+GMI",  3, mm_port_probe_response_processor_string },
@@ -1339,6 +1488,8 @@ serial_open_at (MMPortProbe *self)
                              MM_PORT_PROBE_AT_ICERA | \
                              MM_PORT_PROBE_AT_XMM)
 
+#define AT_PROBING_DEFAULT_TRIES 6
+
 static void
 probe_step (MMPortProbe *self)
 {
@@ -1355,6 +1506,7 @@ probe_step (MMPortProbe *self)
     ctx->at_result_processor   = NULL;
     ctx->at_commands           = NULL;
     ctx->at_commands_wait_secs = 0;
+    ctx->at_commands_limit     = G_MAXUINT; /* run all given AT probes */
 
     switch (ctx->step) {
     case PROBE_STEP_FIRST:
@@ -1411,7 +1563,20 @@ probe_step (MMPortProbe *self)
         if ((ctx->flags & MM_PORT_PROBE_AT) && !(self->priv->flags & MM_PORT_PROBE_AT)) {
             mm_obj_msg (self, "probe step: AT");
             /* Prepare AT probing */
-            ctx->at_commands = ctx->at_custom_probe ? ctx->at_custom_probe : at_probing;
+            if (ctx->at_custom_probe)
+                ctx->at_commands = ctx->at_custom_probe;
+            else {
+                gint at_probe_tries;
+
+                /* NOTE: update ID_MM_TTY_AT_PROBE_TRIES documentation when changing min/max/default */
+                at_probe_tries = mm_kernel_device_get_property_as_int (mm_port_probe_peek_port (self),
+                                                                       ID_MM_TTY_AT_PROBE_TRIES);
+                /* If no tag, use default number of tries */
+                if (at_probe_tries <= 0)
+                    at_probe_tries = AT_PROBING_DEFAULT_TRIES;
+                ctx->at_commands_limit = MIN (at_probe_tries, (gint) G_N_ELEMENTS (at_probing));
+                ctx->at_commands = at_probing;
+            }
             ctx->at_result_processor = probe_at_result_processor;
             ctx->source_id = g_idle_add ((GSourceFunc) probe_at, self);
             return;
diff --git a/src/mm-port-probe.h b/src/mm-port-probe.h
index fd427575..000b40da 100644
--- a/src/mm-port-probe.h
+++ b/src/mm-port-probe.h
@@ -128,6 +128,17 @@ gboolean mm_port_probe_run_finish (MMPortProbe *self,
 
 gboolean mm_port_probe_run_cancel_at_probing (MMPortProbe *self);
 
+/* Run early AT probes from plugin custom init hooks */
+gboolean mm_port_probe_run_early_at_probe (MMPortProbe         *self,
+                                           MMPortSerialAt      *serial,
+                                           GCancellable        *cancellable,
+                                           GAsyncReadyCallback  callback,
+                                           gpointer             user_data);
+
+gboolean mm_port_probe_run_early_at_probe_finish (MMPortProbe   *self,
+                                                  GAsyncResult  *result,
+                                                  GError       **error);
+
 /* Probing result getters */
 MMPortType    mm_port_probe_get_port_type    (MMPortProbe *self);
 gboolean      mm_port_probe_is_at            (MMPortProbe *self);
diff --git a/src/plugins/telit/77-mm-telit-port-types.rules b/src/plugins/telit/77-mm-telit-port-types.rules
index 8a5849f7..48b40f4a 100644
--- a/src/plugins/telit/77-mm-telit-port-types.rules
+++ b/src/plugins/telit/77-mm-telit-port-types.rules
@@ -149,26 +149,26 @@ ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="7011", ENV{.MM_USBIFNUM}=="03", SUBS
 ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="7011", ENV{.MM_USBIFNUM}=="04", SUBSYSTEM=="tty", ENV{ID_MM_PORT_TYPE_AT_PRIMARY}="1"
 
 # LM940/960 initial port delay
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1040", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1041", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1042", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1043", ENV{ID_MM_TELIT_PORT_DELAY}="1"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1040", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1041", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1042", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1043", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
 
 # FN980 initial port delay
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1050", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1051", ENV{ID_MM_TELIT_PORT_DELAY}="1"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1050", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1051", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
 
 # LN920 initial port delay
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1060", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1061", ENV{ID_MM_TELIT_PORT_DELAY}="1"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1060", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1061", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
 
 # FN990 initial port delay
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1070", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1071", ENV{ID_MM_TELIT_PORT_DELAY}="1"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1070", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1071", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
 
 # FN912C04 initial port delay
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="3000", ENV{ID_MM_TELIT_PORT_DELAY}="1"
-ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="3001", ENV{ID_MM_TELIT_PORT_DELAY}="1"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="3000", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
+ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="3001", ENV{ID_MM_TTY_AT_PROBE_TRIES}="14"
 
 GOTO="mm_telit_end"
 
diff --git a/src/plugins/telit/mm-common-telit.c b/src/plugins/telit/mm-common-telit.c
index a401b1ae..10aba7a6 100644
--- a/src/plugins/telit/mm-common-telit.c
+++ b/src/plugins/telit/mm-common-telit.c
@@ -277,34 +277,6 @@ out:
     g_object_unref (task);
 }
 
-static void at_ready (MMPortSerialAt *port,
-                      GAsyncResult   *res,
-                      GTask          *task);
-
-static void
-wait_for_ready (GTask *task)
-{
-    TelitCustomInitContext *ctx;
-
-    ctx = g_task_get_task_data (task);
-
-    if (ctx->port_responsive_retries == 0) {
-        telit_custom_init_step (task);
-        return;
-    }
-    ctx->port_responsive_retries--;
-
-    mm_port_serial_at_command (
-        ctx->port,
-        "AT",
-        5,
-        FALSE, /* raw */
-        FALSE, /* allow_cached */
-        g_task_get_cancellable (task),
-        (GAsyncReadyCallback)at_ready,
-        task);
-}
-
 static void
 at_ready (MMPortSerialAt *port,
           GAsyncResult   *res,
@@ -315,26 +287,17 @@ at_ready (MMPortSerialAt *port,
 
     probe = g_task_get_source_object (task);
 
-    mm_port_serial_at_command_finish (port, res, &error);
-    if (error) {
-        /* On a timeout or send error, wait */
-        if (g_error_matches (error, MM_SERIAL_ERROR, MM_SERIAL_ERROR_RESPONSE_TIMEOUT) ||
-            g_error_matches (error, MM_SERIAL_ERROR, MM_SERIAL_ERROR_SEND_FAILED)) {
-            wait_for_ready (task);
-            return;
-        }
-        /* On an unknown error, make it fatal */
-        if (!mm_serial_parser_v1_is_known_error (error)) {
+    if (!mm_port_probe_run_early_at_probe_finish (probe, res, &error)) {
+        /* Not a usable AT port or an error occurred */
+        if (error)
             mm_obj_warn (probe, "custom port initialization logic failed: %s", error->message);
-            g_task_return_boolean (task, TRUE);
-            g_object_unref (task);
-            return;
-        }
+        g_task_return_boolean (task, TRUE); /* continue with probing */
+        g_object_unref (task);
+        return;
     }
 
     /* When successful mark the port as AT and continue checking #PORTCFG */
     mm_obj_dbg (probe, "port is AT");
-    mm_port_probe_set_result_at (probe, TRUE);
     telit_custom_init_step (task);
 }
 
@@ -347,13 +310,11 @@ telit_custom_init (MMPortProbe *probe,
 {
     TelitCustomInitContext *ctx;
     GTask *task;
-    gboolean wait_needed;
 
     ctx = g_slice_new (TelitCustomInitContext);
     ctx->port = g_object_ref (port);
     ctx->getportcfg_done = FALSE;
     ctx->getportcfg_retries = 3;
-    ctx->port_responsive_retries = TELIT_PORT_CHECK_RETRIES;
     task = g_task_new (probe, cancellable, callback, user_data);
     g_task_set_check_cancellable (task, FALSE);
     g_task_set_task_data (task, ctx, (GDestroyNotify)telit_custom_init_context_free);
@@ -361,11 +322,12 @@ telit_custom_init (MMPortProbe *probe,
     /* Some Telit modems require an initial delay for the ports to be responsive
      * If no explicit tag is present, the modem does not need this step
      * and can directly look for #PORTCFG support */
-    wait_needed = mm_kernel_device_get_global_property_as_boolean (mm_port_probe_peek_port (probe),
-                                                                   "ID_MM_TELIT_PORT_DELAY");
-    if (wait_needed) {
+    if (mm_port_probe_run_early_at_probe (probe,
+                                          port,
+                                          cancellable,
+                                          (GAsyncReadyCallback) at_ready,
+                                          task)) {
         mm_obj_dbg (probe, "Start polling for port responsiveness");
-        wait_for_ready (task);
         return;
     }