api,modem: deprecate the 'MaxBearers' property

This property was never initialized with a correct value, we would
always use the same value as used for MaxActiveBearers, which isn't
technically right. Just suggest to ignore this property, and flag
it as deprecated.

In addition to deprecating the API property, we will internally no
longer limit the amount of bearer objects that may be created with
different connection settings. The bearer object exposed in DBus is a
representation of the connection that may be setup, and so we can hold
as many representations as we want as long as the amount of connected
bearers doesn't exceed the maximum amount for each modem.

Leaving around the disconnected modems also serves another purpose.
Each bearer holds information about the amount of times a connection
with it has been attempted, and the amount of data transferred through
it. If we use the Simple.Connect() method to connect the modem using
different bearer settings, we won't lose the information of the past
connection attempts with different settings.

7 files changed, 42 insertions, 152 deletions

diff --git a/introspection/org.freedesktop.ModemManager1.Modem.xml b/introspection/org.freedesktop.ModemManager1.Modem.xml
index 25527394..d75f86fc 100644
--- a/introspection/org.freedesktop.ModemManager1.Modem.xml
+++ b/introspection/org.freedesktop.ModemManager1.Modem.xml
@@ -337,6 +337,12 @@
         while GSM/UMTS devices typically support three or more, and any
         LTE-capable device (whether LTE-only, GSM/UMTS-capable, and/or
         CDMA2000-capable) also typically support three or more.
+
+        Deprecated: 1.18.0. There is no way to query the modem how many bearers
+        it supports, so the value exposed in this property in all the different
+        implementations is always equal to the value in
+        #org.freedesktop.ModemManager1.Modem:MaxActiveBearers, so there is no
+        point in using this property.
     -->
     <property name="MaxBearers" type="u" access="read" />
 
diff --git a/libmm-glib/mm-modem.c b/libmm-glib/mm-modem.c
index 38300d49..e3b29bb5 100644
--- a/libmm-glib/mm-modem.c
+++ b/libmm-glib/mm-modem.c
@@ -395,6 +395,8 @@ mm_modem_get_current_capabilities (MMModem *self)
 
 /*****************************************************************************/
 
+#ifndef MM_DISABLE_DEPRECATED
+
 /**
  * mm_modem_get_max_bearers:
  * @self: a #MMModem.
@@ -412,6 +414,11 @@ mm_modem_get_current_capabilities (MMModem *self)
  * Returns: the maximum number of defined packet data bearers.
  *
  * Since: 1.0
+ * Deprecated: 1.18. There is no way to query the modem how many bearers
+ * it supports, so the value exposed in this property in all the different
+ * implementations is always equal to the one retrieved with
+ * mm_modem_get_max_active_bearers(), so there is no point in using this
+ * method.
  */
 guint
 mm_modem_get_max_bearers (MMModem *self)
@@ -421,6 +428,8 @@ mm_modem_get_max_bearers (MMModem *self)
     return mm_gdbus_modem_get_max_bearers (MM_GDBUS_MODEM (self));
 }
 
+#endif /* MM_DISABLE_DEPRECATED */
+
 /*****************************************************************************/
 
 /**
diff --git a/libmm-glib/mm-modem.h b/libmm-glib/mm-modem.h
index 0d887eae..589ddafa 100644
--- a/libmm-glib/mm-modem.h
+++ b/libmm-glib/mm-modem.h
@@ -89,7 +89,10 @@ gboolean           mm_modem_get_supported_capabilities (MMModem *self,
 
 MMModemCapability  mm_modem_get_current_capabilities (MMModem *self);
 
+#ifndef MM_DISABLE_DEPRECATED
+G_DEPRECATED
 guint              mm_modem_get_max_bearers          (MMModem *self);
+#endif /* MM_DISABLE_DEPRECATED */
 
 guint              mm_modem_get_max_active_bearers   (MMModem *self);
 
diff --git a/src/mm-bearer-list.c b/src/mm-bearer-list.c
index 65717132..8bf6ae05 100644
--- a/src/mm-bearer-list.c
+++ b/src/mm-bearer-list.c
@@ -34,7 +34,6 @@ G_DEFINE_TYPE (MMBearerList, mm_bearer_list, G_TYPE_OBJECT);
 enum {
     PROP_0,
     PROP_NUM_BEARERS,
-    PROP_MAX_BEARERS,
     PROP_MAX_ACTIVE_BEARERS,
     PROP_LAST
 };
@@ -44,8 +43,6 @@ static GParamSpec *properties[PROP_LAST];
 struct _MMBearerListPrivate {
     /* List of bearers */
     GList *bearers;
-    /* Max number of bearers */
-    guint max_bearers;
     /* Max number of active bearers */
     guint max_active_bearers;
 };
@@ -53,44 +50,16 @@ struct _MMBearerListPrivate {
 /*****************************************************************************/
 
 guint
-mm_bearer_list_get_max (MMBearerList *self)
-{
-    return self->priv->max_bearers;
-}
-
-guint
 mm_bearer_list_get_max_active (MMBearerList *self)
 {
     return self->priv->max_active_bearers;
 }
 
-guint
-mm_bearer_list_get_count (MMBearerList *self)
-{
-    return g_list_length (self->priv->bearers);
-}
-
-guint
-mm_bearer_list_get_count_active (MMBearerList *self)
-{
-    return 0; /* TODO */
-}
-
 gboolean
 mm_bearer_list_add_bearer (MMBearerList *self,
                            MMBaseBearer *bearer,
                            GError **error)
 {
-    /* Just in case, ensure we don't go off limits */
-    if (g_list_length (self->priv->bearers) == self->priv->max_bearers) {
-        g_set_error (error,
-                     MM_CORE_ERROR,
-                     MM_CORE_ERROR_TOO_MANY,
-                     "Cannot add new bearer: already reached maximum (%u)",
-                     self->priv->max_bearers);
-        return FALSE;
-    }
-
     /* Keep our own reference */
     self->priv->bearers = g_list_prepend (self->priv->bearers, g_object_ref (bearer));
     g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_NUM_BEARERS]);
@@ -269,12 +238,10 @@ mm_bearer_list_disconnect_all_bearers (MMBearerList *self,
 /*****************************************************************************/
 
 MMBearerList *
-mm_bearer_list_new (guint max_bearers,
-                    guint max_active_bearers)
+mm_bearer_list_new (guint max_active_bearers)
 {
     /* Create the object */
     return g_object_new  (MM_TYPE_BEARER_LIST,
-                          MM_BEARER_LIST_MAX_BEARERS, max_bearers,
                           MM_BEARER_LIST_MAX_ACTIVE_BEARERS, max_active_bearers,
                           NULL);
 }
@@ -291,9 +258,6 @@ set_property (GObject *object,
     case PROP_NUM_BEARERS:
         g_assert_not_reached ();
         break;
-    case PROP_MAX_BEARERS:
-        self->priv->max_bearers = g_value_get_uint (value);
-        break;
     case PROP_MAX_ACTIVE_BEARERS:
         self->priv->max_active_bearers = g_value_get_uint (value);
         break;
@@ -315,9 +279,6 @@ get_property (GObject *object,
     case PROP_NUM_BEARERS:
         g_value_set_uint (value, g_list_length (self->priv->bearers));
         break;
-    case PROP_MAX_BEARERS:
-        g_value_set_uint (value, self->priv->max_bearers);
-        break;
     case PROP_MAX_ACTIVE_BEARERS:
         g_value_set_uint (value, self->priv->max_active_bearers);
         break;
@@ -371,16 +332,6 @@ mm_bearer_list_class_init (MMBearerListClass *klass)
                            G_PARAM_READABLE);
     g_object_class_install_property (object_class, PROP_NUM_BEARERS, properties[PROP_NUM_BEARERS]);
 
-    properties[PROP_MAX_BEARERS] =
-        g_param_spec_uint (MM_BEARER_LIST_MAX_BEARERS,
-                           "Max bearers",
-                           "Maximum number of bearers the list can handle",
-                           1,
-                           G_MAXUINT,
-                           1,
-                           G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY);
-    g_object_class_install_property (object_class, PROP_MAX_BEARERS, properties[PROP_MAX_BEARERS]);
-
     properties[PROP_MAX_ACTIVE_BEARERS] =
         g_param_spec_uint (MM_BEARER_LIST_MAX_ACTIVE_BEARERS,
                            "Max active bearers",
diff --git a/src/mm-bearer-list.h b/src/mm-bearer-list.h
index 0d220eee..1d8db726 100644
--- a/src/mm-bearer-list.h
+++ b/src/mm-bearer-list.h
@@ -31,7 +31,6 @@
 #define MM_BEARER_LIST_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj),  MM_TYPE_BEARER_LIST, MMBearerListClass))
 
 #define MM_BEARER_LIST_NUM_BEARERS        "num-bearers"
-#define MM_BEARER_LIST_MAX_BEARERS        "max-bearers"
 #define MM_BEARER_LIST_MAX_ACTIVE_BEARERS "max-active-bearers"
 
 typedef struct _MMBearerList MMBearerList;
@@ -50,14 +49,9 @@ struct _MMBearerListClass {
 GType mm_bearer_list_get_type (void);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (MMBearerList, g_object_unref)
 
-MMBearerList *mm_bearer_list_new (guint max_bearers,
-                                  guint max_active_bearers);
+MMBearerList *mm_bearer_list_new (guint max_active_bearers);
 
-GStrv mm_bearer_list_get_paths (MMBearerList *self);
-
-guint mm_bearer_list_get_count (MMBearerList *self);
-guint mm_bearer_list_get_count_active (MMBearerList *self);
-guint mm_bearer_list_get_max (MMBearerList *self);
+GStrv mm_bearer_list_get_paths      (MMBearerList *self);
 guint mm_bearer_list_get_max_active (MMBearerList *self);
 
 gboolean mm_bearer_list_add_bearer (MMBearerList *self,
diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c
index b0cf5e22..d788f677 100644
--- a/src/mm-iface-modem-simple.c
+++ b/src/mm-iface-modem-simple.c
@@ -226,6 +226,7 @@ typedef struct {
     GDBusMethodInvocation *invocation;
     MMIfaceModemSimple *self;
     ConnectionStep step;
+    MMBearerList *bearer_list;
 
     /* Expected input properties */
     GVariant *dictionary;
@@ -263,6 +264,7 @@ connection_context_free (ConnectionContext *ctx)
     cleanup_cancellation (ctx);
     g_clear_object (&ctx->properties);
     g_clear_object (&ctx->bearer);
+    g_clear_object (&ctx->bearer_list);
     g_variant_unref (ctx->dictionary);
     g_object_unref (ctx->skeleton);
     g_object_unref (ctx->invocation);
@@ -477,22 +479,6 @@ update_lock_info_ready (MMIfaceModem *self,
     g_object_unref (sim);
 }
 
-typedef struct {
-    MMBaseBearer *found;
-} BearerListFindContext;
-
-static void
-bearer_list_find_disconnected (MMBaseBearer *bearer,
-                               BearerListFindContext *ctx)
-{
-    /* If already marked one to remove, do nothing */
-    if (ctx->found)
-        return;
-
-    if (mm_base_bearer_get_status (bearer) == MM_BEARER_STATUS_DISCONNECTED)
-        ctx->found = g_object_ref (bearer);
-}
-
 static gboolean
 completed_if_cancelled (ConnectionContext *ctx)
 {
@@ -595,85 +581,26 @@ connection_step (ConnectionContext *ctx)
         /* fall through */
 
     case CONNECTION_STEP_BEARER: {
-        MMBearerList *list = NULL;
-        MMBearerProperties *bearer_properties;
+        g_autoptr(MMBearerProperties) bearer_properties = NULL;
 
         mm_obj_info (ctx->self, "simple connect state (%d/%d): bearer",
                      ctx->step, CONNECTION_STEP_LAST);
-        g_object_get (ctx->self,
-                      MM_IFACE_MODEM_BEARER_LIST, &list,
-                      NULL);
-        if (!list) {
-            g_dbus_method_invocation_return_error (
-                ctx->invocation,
-                MM_CORE_ERROR,
-                MM_CORE_ERROR_FAILED,
-                "Couldn't get the bearer list");
-            connection_context_free (ctx);
-            return;
-        }
 
         bearer_properties = mm_simple_connect_properties_get_bearer_properties (ctx->properties);
 
         /* Check if the bearer we want to create is already in the list */
-        ctx->bearer = mm_bearer_list_find_by_properties (list, bearer_properties);
+        ctx->bearer = mm_bearer_list_find_by_properties (ctx->bearer_list, bearer_properties);
         if (!ctx->bearer) {
             mm_obj_dbg (ctx->self, "creating new bearer...");
-            /* If we don't have enough space to create the bearer, try to remove
-             * a disconnected bearer first. */
-            if (mm_bearer_list_get_max (list) == mm_bearer_list_get_count (list)) {
-                BearerListFindContext foreach_ctx;
-
-                foreach_ctx.found = NULL;
-                mm_bearer_list_foreach (list,
-                                        (MMBearerListForeachFunc)bearer_list_find_disconnected,
-                                        &foreach_ctx);
-
-                /* Found a disconnected bearer, remove it */
-                if (foreach_ctx.found) {
-                    GError *error = NULL;
-
-                    if (!mm_bearer_list_delete_bearer (list,
-                                                       mm_base_bearer_get_path (foreach_ctx.found),
-                                                       &error)) {
-                        mm_obj_dbg (ctx->self, "couldn't delete disconnected bearer at '%s': %s",
-                                    mm_base_bearer_get_path (foreach_ctx.found),
-                                    error->message);
-                        g_error_free (error);
-                    } else
-                        mm_obj_dbg (ctx->self, "deleted disconnected bearer at '%s'",
-                                    mm_base_bearer_get_path (foreach_ctx.found));
-                    g_object_unref (foreach_ctx.found);
-                }
-
-                /* Re-check space, and if we still are in max, return an error */
-                if (mm_bearer_list_get_max (list) == mm_bearer_list_get_count (list)) {
-                    g_dbus_method_invocation_return_error (
-                        ctx->invocation,
-                        MM_CORE_ERROR,
-                        MM_CORE_ERROR_TOO_MANY,
-                        "Cannot create new bearer: all existing bearers are connected");
-                    connection_context_free (ctx);
-                    g_object_unref (list);
-                    g_object_unref (bearer_properties);
-                    return;
-                }
-            }
-
             mm_iface_modem_create_bearer (MM_IFACE_MODEM (ctx->self),
                                           bearer_properties,
                                           (GAsyncReadyCallback)create_bearer_ready,
                                           ctx);
-
-            g_object_unref (list);
-            g_object_unref (bearer_properties);
             return;
         }
 
         mm_obj_dbg (ctx->self, "Using already existing bearer at '%s'...",
                     mm_base_bearer_get_path (ctx->bearer));
-        g_object_unref (list);
-        g_object_unref (bearer_properties);
         ctx->step++;
     } /* fall through */
 
@@ -748,7 +675,15 @@ connect_auth_ready (MMBaseModem *self,
     /* We may be able to skip some steps, so check that before doing anything */
     g_object_get (self,
                   MM_IFACE_MODEM_STATE, &current,
+                  MM_IFACE_MODEM_BEARER_LIST, &ctx->bearer_list,
                   NULL);
+    if (!ctx->bearer_list) {
+        g_dbus_method_invocation_return_error (
+            ctx->invocation, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+            "Couldn't get the bearer list");
+        connection_context_free (ctx);
+        return;
+    }
 
     mm_obj_info (self, "simple connect started...");
 
diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 2e9a3004..5ac3da9c 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -670,17 +670,6 @@ mm_iface_modem_create_bearer (MMIfaceModem *self,
         return;
     }
 
-    if (mm_bearer_list_get_count (ctx->list) == mm_bearer_list_get_max (ctx->list)) {
-        g_task_return_new_error (
-            task,
-            MM_CORE_ERROR,
-            MM_CORE_ERROR_TOO_MANY,
-            "Cannot add new bearer: already reached maximum (%u)",
-            mm_bearer_list_get_count (ctx->list));
-        g_object_unref (task);
-        return;
-    }
-
     MM_IFACE_MODEM_GET_INTERFACE (self)->create_bearer (
         self,
         properties,
@@ -5109,10 +5098,10 @@ interface_initialization_step (GTask *task)
             /* The maximum number of available/connected modems is guessed from
              * the size of the data ports list. */
             n = g_list_length (mm_base_modem_peek_data_ports (MM_BASE_MODEM (self)));
-            mm_obj_dbg (self, "allowed up to %u bearers", n);
+            mm_obj_dbg (self, "allowed up to %u active bearers", n);
 
             /* Create new default list */
-            list = mm_bearer_list_new (n, n);
+            list = mm_bearer_list_new (n);
             g_signal_connect (list,
                               "notify::" MM_BEARER_LIST_NUM_BEARERS,
                               G_CALLBACK (bearer_list_updated),
@@ -5122,14 +5111,17 @@ interface_initialization_step (GTask *task)
                           NULL);
         }
 
-        if (mm_gdbus_modem_get_max_bearers (ctx->skeleton) == 0)
-            mm_gdbus_modem_set_max_bearers (
-                ctx->skeleton,
-                mm_bearer_list_get_max (list));
         if (mm_gdbus_modem_get_max_active_bearers (ctx->skeleton) == 0)
             mm_gdbus_modem_set_max_active_bearers (
                 ctx->skeleton,
                 mm_bearer_list_get_max_active (list));
+
+        /* MaxBearers set equal to MaxActiveBearers */
+        if (mm_gdbus_modem_get_max_bearers (ctx->skeleton) == 0)
+            mm_gdbus_modem_set_max_bearers (
+                ctx->skeleton,
+                mm_gdbus_modem_get_max_active_bearers (ctx->skeleton));
+
         g_object_unref (list);
 
         ctx->step++;