broadband-modem: rework flow control selection logic

Try to simplify a bit the logic that selects the flow control to be
used during connectivity:

 * We detect early if the requested flow control (via udev tags) is
   actually supported by the modem (as eported by AT+IFC=?), and if it
   isn't, we error out directly.

 * We try to set in a single place the AT commands to be used to setup
   the flow control, once we have decided which one we're going to
   use.

 * Use the string builder helpers to log the flow control values.

1 files changed, 56 insertions, 55 deletions

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 1ca614971..3c5b73759 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -3311,11 +3311,13 @@ ifc_test_ready (MMBaseModem  *_self,
     MMBroadbandModem *self;
     GError           *error = NULL;
     const gchar      *response;
-    MMFlowControl     mask;
     const gchar      *cmd;
-    MMFlowControl     flow_control = MM_FLOW_CONTROL_UNKNOWN;
+    MMFlowControl     flow_control_supported;
+    MMFlowControl     flow_control_selected = MM_FLOW_CONTROL_UNKNOWN;
+    MMFlowControl     flow_control_requested;
+    gchar            *flow_control_supported_str = NULL;
+    gchar            *flow_control_selected_str = NULL;
     MMPortSerialAt   *port;
-    const gchar      *err_str = NULL;
 
     self = MM_BROADBAND_MODEM (_self);
 
@@ -3325,75 +3327,79 @@ ifc_test_ready (MMBaseModem  *_self,
         goto out;
 
     /* Parse response */
-    mask = mm_parse_ifc_test_response (response, &error);
-    if (mask == MM_FLOW_CONTROL_UNKNOWN)
+    flow_control_supported = mm_parse_ifc_test_response (response, &error);
+    if (flow_control_supported == MM_FLOW_CONTROL_UNKNOWN)
         goto out;
+    flow_control_supported_str = mm_flow_control_build_string_from_mask (flow_control_supported);
 
     port = mm_base_modem_peek_best_at_port (_self, &error);
     if (!port)
         goto out;
 
-    flow_control = mm_port_serial_get_flow_control (MM_PORT_SERIAL (port));
+    flow_control_requested = mm_port_serial_get_flow_control (MM_PORT_SERIAL (port));
+    if (flow_control_requested != MM_FLOW_CONTROL_UNKNOWN) {
+        gchar *flow_control_requested_str;
 
-    switch (flow_control) {
-    case MM_FLOW_CONTROL_RTS_CTS:
-        if (mask & MM_FLOW_CONTROL_RTS_CTS) {
-            self->priv->flow_control = MM_FLOW_CONTROL_RTS_CTS;
-            cmd = "+IFC=2,2";
-        } else
-            err_str = "RTS/CTS";
-        break;
-    case MM_FLOW_CONTROL_XON_XOFF:
-        if (mask & MM_FLOW_CONTROL_XON_XOFF) {
-            self->priv->flow_control = MM_FLOW_CONTROL_XON_XOFF;
-            cmd = "+IFC=1,1";
-        } else
-            err_str = "xon/xoff";
-        break;
-    case MM_FLOW_CONTROL_NONE:
-        if (mask & MM_FLOW_CONTROL_NONE) {
-            self->priv->flow_control = MM_FLOW_CONTROL_NONE;
-            cmd = "+IFC=0,0";
-        } else
-            err_str = "none";
-        break;
-    case MM_FLOW_CONTROL_UNKNOWN:
+        flow_control_requested_str = mm_flow_control_build_string_from_mask (flow_control_requested);
+
+        /* If flow control settings requested via udev tag are not supported by
+         * the modem, we trigger a fatal error */
+        if (!(flow_control_supported & flow_control_requested)) {
+            g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_UNSUPPORTED,
+                                     "Explicitly requested flow control settings (%s) are not supported by the device (%s)",
+                                     flow_control_requested_str, flow_control_supported_str);
+            g_object_unref (task);
+            g_free (flow_control_requested_str);
+            g_free (flow_control_supported_str);
+            return;
+        }
+
+        mm_dbg ("Flow control settings explicitly requested (%s)", flow_control_requested_str);
+        flow_control_selected = flow_control_requested;
+        flow_control_selected_str = flow_control_requested_str;
+    } else {
         /* If flow control is not set explicitly by udev tags,
          * we prefer the methods in this order:
          *  RTS/CTS
          *  XON/XOFF
          *  None.
          */
-        if (mask & MM_FLOW_CONTROL_RTS_CTS) {
-            self->priv->flow_control = MM_FLOW_CONTROL_RTS_CTS;
-            cmd = "+IFC=2,2";
-        } else if (mask & MM_FLOW_CONTROL_XON_XOFF) {
-            self->priv->flow_control = MM_FLOW_CONTROL_XON_XOFF;
-            cmd = "+IFC=1,1";
-        } else if (mask & MM_FLOW_CONTROL_NONE) {
-            self->priv->flow_control = MM_FLOW_CONTROL_NONE;
-            cmd = "+IFC=0,0";
-        } else
+        if (flow_control_supported & MM_FLOW_CONTROL_RTS_CTS)
+            flow_control_selected = MM_FLOW_CONTROL_RTS_CTS;
+        else if (flow_control_supported & MM_FLOW_CONTROL_XON_XOFF)
+            flow_control_selected = MM_FLOW_CONTROL_XON_XOFF;
+        else if (flow_control_supported & MM_FLOW_CONTROL_NONE)
+            flow_control_selected = MM_FLOW_CONTROL_NONE;
+        else
             g_assert_not_reached ();
-        break;
+        flow_control_selected_str = mm_flow_control_build_string_from_mask (flow_control_selected);
+        mm_dbg ("Flow control settings automatically selected (%s)", flow_control_selected_str);
     }
 
-    if (err_str) {
-        g_set_error (&error,
-                     MM_CORE_ERROR,
-                     MM_CORE_ERROR_FAILED,
-                     "%s: failed to set serial flow control to %s",
-                     __func__, err_str);
-        goto fatal;
-    }
-
-    /* Notify the flow control property update */
+    /* Select flow control for all connections */
+    self->priv->flow_control = flow_control_selected;
     g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_FLOW_CONTROL]);
 
     /* Set flow control settings and ignore result */
+    switch (flow_control_selected) {
+    case MM_FLOW_CONTROL_RTS_CTS:
+        cmd = "+IFC=2,2";
+        break;
+    case MM_FLOW_CONTROL_XON_XOFF:
+        cmd = "+IFC=1,1";
+        break;
+    case MM_FLOW_CONTROL_NONE:
+        cmd = "+IFC=0,0";
+        break;
+    default:
+        g_assert_not_reached ();
+    }
     mm_base_modem_at_command (_self, cmd, 3, FALSE, NULL, NULL);
 
 out:
+    g_free (flow_control_supported_str);
+    g_free (flow_control_selected_str);
+
     /* Ignore errors */
     if (error) {
         mm_dbg ("couldn't load supported flow control methods: %s", error->message);
@@ -3402,11 +3408,6 @@ out:
 
     g_task_return_boolean (task, TRUE);
     g_object_unref (task);
-    return;
-
-fatal:
-    g_task_return_error (task, error);
-    g_object_unref (task);
 }
 
 static void