From f325ef59020b0acc33b6d05e96c8df7b58373715 Mon Sep 17 00:00:00 2001
From: Aleksander Morgado <aleksander@aleksander.es>
Date: Fri, 27 Mar 2020 16:24:48 +0100
Subject: [PATCH] api,bearer: add 'attempts' and 'failed-attempts' statistics

When we're reusing over and over the same bearer object, we can
provide statistical information about the number of connection
attempts that have been done and how many of them failed.
---
 cli/mmcli-bearer.c                            | 16 +++-
 cli/mmcli-output.c                            |  2 +
 cli/mmcli-output.h                            |  2 +
 .../libmm-glib/libmm-glib-sections.txt        |  4 +
 .../org.freedesktop.ModemManager1.Bearer.xml  | 36 +++++--
 libmm-glib/mm-bearer-stats.c                  | 93 ++++++++++++++++++-
 libmm-glib/mm-bearer-stats.h                  | 16 ++--
 src/mm-base-bearer.c                          | 32 ++++---
 8 files changed, 168 insertions(+), 33 deletions(-)

diff --git a/cli/mmcli-bearer.c b/cli/mmcli-bearer.c
index 279e568a..2dfdab35 100644
--- a/cli/mmcli-bearer.c
+++ b/cli/mmcli-bearer.c
@@ -248,6 +248,8 @@ print_bearer_info (MMBearer *bearer)
         gchar *duration = NULL;
         gchar *bytes_rx = NULL;
         gchar *bytes_tx = NULL;
+        gchar *attempts = NULL;
+        gchar *failed_attempts = NULL;
 
         if (stats) {
             guint64 val;
@@ -261,11 +263,19 @@ print_bearer_info (MMBearer *bearer)
             val = mm_bearer_stats_get_tx_bytes (stats);
             if (val)
                 bytes_tx = g_strdup_printf ("%" G_GUINT64_FORMAT, val);
+            val = mm_bearer_stats_get_attempts (stats);
+            if (val)
+                attempts = g_strdup_printf ("%" G_GUINT64_FORMAT, val);
+            val = mm_bearer_stats_get_failed_attempts (stats);
+            if (val)
+                failed_attempts = g_strdup_printf ("%" G_GUINT64_FORMAT, val);
         }
 
-        mmcli_output_string_take (MMC_F_BEARER_STATS_DURATION, duration);
-        mmcli_output_string_take (MMC_F_BEARER_STATS_BYTES_RX, bytes_rx);
-        mmcli_output_string_take (MMC_F_BEARER_STATS_BYTES_TX, bytes_tx);
+        mmcli_output_string_take (MMC_F_BEARER_STATS_DURATION,        duration);
+        mmcli_output_string_take (MMC_F_BEARER_STATS_BYTES_RX,        bytes_rx);
+        mmcli_output_string_take (MMC_F_BEARER_STATS_BYTES_TX,        bytes_tx);
+        mmcli_output_string_take (MMC_F_BEARER_STATS_ATTEMPTS,        attempts);
+        mmcli_output_string_take (MMC_F_BEARER_STATS_FAILED_ATTEMPTS, failed_attempts);
     }
 
     mmcli_output_dump ();
diff --git a/cli/mmcli-output.c b/cli/mmcli-output.c
index 25877b0b..7418fc18 100644
--- a/cli/mmcli-output.c
+++ b/cli/mmcli-output.c
@@ -226,6 +226,8 @@ static FieldInfo field_infos[] = {
     [MMC_F_BEARER_STATS_DURATION]             = { "bearer.stats.duration",                           "duration",                 MMC_S_BEARER_STATS,            },
     [MMC_F_BEARER_STATS_BYTES_RX]             = { "bearer.stats.bytes-rx",                           "bytes rx",                 MMC_S_BEARER_STATS,            },
     [MMC_F_BEARER_STATS_BYTES_TX]             = { "bearer.stats.bytes-tx",                           "bytes tx",                 MMC_S_BEARER_STATS,            },
+    [MMC_F_BEARER_STATS_ATTEMPTS]             = { "bearer.stats.attempts",                           "attempts",                 MMC_S_BEARER_STATS,            },
+    [MMC_F_BEARER_STATS_FAILED_ATTEMPTS]      = { "bearer.stats.failed-attempts",                    "attempts",                 MMC_S_BEARER_STATS,            },
     [MMC_F_CALL_GENERAL_DBUS_PATH]            = { "call.dbus-path",                                  "dbus path",                MMC_S_CALL_GENERAL,            },
     [MMC_F_CALL_PROPERTIES_NUMBER]            = { "call.properties.number",                          "number",                   MMC_S_CALL_PROPERTIES,         },
     [MMC_F_CALL_PROPERTIES_DIRECTION]         = { "call.properties.direction",                       "direction",                MMC_S_CALL_PROPERTIES,         },
diff --git a/cli/mmcli-output.h b/cli/mmcli-output.h
index bd7b317a..76ca2026 100644
--- a/cli/mmcli-output.h
+++ b/cli/mmcli-output.h
@@ -243,6 +243,8 @@ typedef enum {
     MMC_F_BEARER_STATS_DURATION,
     MMC_F_BEARER_STATS_BYTES_RX,
     MMC_F_BEARER_STATS_BYTES_TX,
+    MMC_F_BEARER_STATS_ATTEMPTS,
+    MMC_F_BEARER_STATS_FAILED_ATTEMPTS,
     MMC_F_CALL_GENERAL_DBUS_PATH,
     MMC_F_CALL_PROPERTIES_NUMBER,
     MMC_F_CALL_PROPERTIES_DIRECTION,
diff --git a/docs/reference/libmm-glib/libmm-glib-sections.txt b/docs/reference/libmm-glib/libmm-glib-sections.txt
index 64bd2e02..83dd52f8 100644
--- a/docs/reference/libmm-glib/libmm-glib-sections.txt
+++ b/docs/reference/libmm-glib/libmm-glib-sections.txt
@@ -1128,6 +1128,8 @@ MMBearerStats
 mm_bearer_stats_get_duration
 mm_bearer_stats_get_rx_bytes
 mm_bearer_stats_get_tx_bytes
+mm_bearer_stats_get_attempts
+mm_bearer_stats_get_failed_attempts
 <SUBSECTION Private>
 mm_bearer_stats_get_dictionary
 mm_bearer_stats_new
@@ -1135,6 +1137,8 @@ mm_bearer_stats_new_from_dictionary
 mm_bearer_stats_set_duration
 mm_bearer_stats_set_rx_bytes
 mm_bearer_stats_set_tx_bytes
+mm_bearer_stats_set_attempts
+mm_bearer_stats_set_failed_attempts
 <SUBSECTION Standard>
 MMBearerStatsClass
 MMBearerStatsPrivate
diff --git a/introspection/org.freedesktop.ModemManager1.Bearer.xml b/introspection/org.freedesktop.ModemManager1.Bearer.xml
index e86ef00d..b82c9d36 100644
--- a/introspection/org.freedesktop.ModemManager1.Bearer.xml
+++ b/introspection/org.freedesktop.ModemManager1.Bearer.xml
@@ -244,28 +244,50 @@
     <!--
         Stats:
 
-        If the modem supports it, this property will show statistics of the
-        ongoing connection.
+        If the modem supports it, this property will show statistics associated
+        to the bearer.
+
+        There are two main different statistic types reported: either applicable
+        to the ongoing connection, or otherwise compiled for all connections
+        that have been done on this bearer object.
 
         When the connection is disconnected automatically or explicitly by the
-        user, the values in this property will show the last values cached.
-        The statistics are reset
+        user, the values applicable to the ongoing connection will show the last
+        values cached.
 
         The following items may appear in the list of statistics:
         <variablelist>
           <varlistentry><term><literal>"rx-bytes"</literal></term>
             <listitem>
-              Number of bytes received without error, given as an unsigned 64-bit integer value (signature <literal>"t"</literal>).
+              Number of bytes received without error in the ongoing connection,
+              given as an unsigned 64-bit integer value (signature
+              <literal>"t"</literal>).
             </listitem>
           </varlistentry>
           <varlistentry><term><literal>"tx-bytes"</literal></term>
             <listitem>
-              Number bytes transmitted without error, given as an unsigned 64-bit integer value (signature <literal>"t"</literal>).
+              Number bytes transmitted without error in the ongoing connection,
+              given as an unsigned 64-bit integer value (signature
+              <literal>"t"</literal>).
             </listitem>
             </varlistentry>
           <varlistentry><term><literal>"duration"</literal></term>
             <listitem>
-              Duration of the connection, in seconds, given as an unsigned integer value (signature <literal>"u"</literal>).
+              Duration of the ongoing connection, in seconds, given as an
+              unsigned integer value (signature <literal>"u"</literal>).
+            </listitem>
+          </varlistentry>
+          <varlistentry><term><literal>"attempts"</literal></term>
+            <listitem>
+              Total number of connection attempts done with this bearer, given
+              as an unsigned integer value (signature <literal>"u"</literal>).
+            </listitem>
+          </varlistentry>
+          <varlistentry><term><literal>"failed-attempts"</literal></term>
+            <listitem>
+              Number of failed connection attempts done with this bearer,
+              given as an unsigned integer value (signature
+              <literal>"u"</literal>).
             </listitem>
           </varlistentry>
         </variablelist>
diff --git a/libmm-glib/mm-bearer-stats.c b/libmm-glib/mm-bearer-stats.c
index df499ca5..42e63839 100644
--- a/libmm-glib/mm-bearer-stats.c
+++ b/libmm-glib/mm-bearer-stats.c
@@ -11,8 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2015 Azimut Electronics
- *
- * Author: Aleksander Morgado <aleksander@aleksander.es>
+ * Copyright (C) 2015-2020 Aleksander Morgado <aleksander@aleksander.es>
  */
 
 #include <string.h>
@@ -34,14 +33,18 @@
 
 G_DEFINE_TYPE (MMBearerStats, mm_bearer_stats, G_TYPE_OBJECT)
 
-#define PROPERTY_DURATION "duration"
-#define PROPERTY_RX_BYTES "rx-bytes"
-#define PROPERTY_TX_BYTES "tx-bytes"
+#define PROPERTY_DURATION        "duration"
+#define PROPERTY_RX_BYTES        "rx-bytes"
+#define PROPERTY_TX_BYTES        "tx-bytes"
+#define PROPERTY_ATTEMPTS        "attempts"
+#define PROPERTY_FAILED_ATTEMPTS "failed-attempts"
 
 struct _MMBearerStatsPrivate {
     guint   duration;
     guint64 rx_bytes;
     guint64 tx_bytes;
+    guint   attempts;
+    guint   failed_attempts;
 };
 
 /*****************************************************************************/
@@ -142,6 +145,70 @@ mm_bearer_stats_set_tx_bytes (MMBearerStats *self,
 
 /*****************************************************************************/
 
+/**
+ * mm_bearer_stats_get_attempts:
+ * @self: a #MMBearerStats.
+ *
+ * Gets the number of connection attempts done with this bearer.
+ *
+ * Returns: a #guint.
+ *
+ * Since: 1.14
+ */
+guint
+mm_bearer_stats_get_attempts (MMBearerStats *self)
+{
+    g_return_val_if_fail (MM_IS_BEARER_STATS (self), 0);
+
+    return self->priv->attempts;
+}
+
+/**
+ * mm_bearer_stats_set_attempts: (skip)
+ */
+void
+mm_bearer_stats_set_attempts (MMBearerStats *self,
+                              guint          attempts)
+{
+    g_return_if_fail (MM_IS_BEARER_STATS (self));
+
+    self->priv->attempts = attempts;
+}
+
+/*****************************************************************************/
+
+/**
+ * mm_bearer_stats_get_failed_attempts:
+ * @self: a #MMBearerStats.
+ *
+ * Gets the number of failed connection attempts done with this bearer.
+ *
+ * Returns: a #guint.
+ *
+ * Since: 1.14
+ */
+guint
+mm_bearer_stats_get_failed_attempts (MMBearerStats *self)
+{
+    g_return_val_if_fail (MM_IS_BEARER_STATS (self), 0);
+
+    return self->priv->failed_attempts;
+}
+
+/**
+ * mm_bearer_stats_set_failed_attempts: (skip)
+ */
+void
+mm_bearer_stats_set_failed_attempts (MMBearerStats *self,
+                                     guint          failed_attempts)
+{
+    g_return_if_fail (MM_IS_BEARER_STATS (self));
+
+    self->priv->failed_attempts = failed_attempts;
+}
+
+/*****************************************************************************/
+
 /**
  * mm_bearer_stats_get_dictionary: (skip)
  */
@@ -167,6 +234,14 @@ mm_bearer_stats_get_dictionary (MMBearerStats *self)
                             "{sv}",
                             PROPERTY_TX_BYTES,
                             g_variant_new_uint64 (self->priv->tx_bytes));
+    g_variant_builder_add  (&builder,
+                            "{sv}",
+                            PROPERTY_ATTEMPTS,
+                            g_variant_new_uint32 (self->priv->attempts));
+    g_variant_builder_add  (&builder,
+                            "{sv}",
+                            PROPERTY_FAILED_ATTEMPTS,
+                            g_variant_new_uint32 (self->priv->failed_attempts));
     return g_variant_builder_end (&builder);
 }
 
@@ -212,6 +287,14 @@ mm_bearer_stats_new_from_dictionary (GVariant *dictionary,
             mm_bearer_stats_set_tx_bytes (
                 self,
                 g_variant_get_uint64 (value));
+        } else if (g_str_equal (key, PROPERTY_ATTEMPTS)) {
+            mm_bearer_stats_set_attempts (
+                self,
+                g_variant_get_uint32 (value));
+        } else if (g_str_equal (key, PROPERTY_FAILED_ATTEMPTS)) {
+            mm_bearer_stats_set_failed_attempts (
+                self,
+                g_variant_get_uint32 (value));
         }
         g_free (key);
         g_variant_unref (value);
diff --git a/libmm-glib/mm-bearer-stats.h b/libmm-glib/mm-bearer-stats.h
index dceb5aeb..000bd07b 100644
--- a/libmm-glib/mm-bearer-stats.h
+++ b/libmm-glib/mm-bearer-stats.h
@@ -58,9 +58,11 @@ struct _MMBearerStatsClass {
 GType mm_bearer_stats_get_type (void);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (MMBearerStats, g_object_unref)
 
-guint   mm_bearer_stats_get_duration (MMBearerStats *self);
-guint64 mm_bearer_stats_get_rx_bytes (MMBearerStats *self);
-guint64 mm_bearer_stats_get_tx_bytes (MMBearerStats *self);
+guint   mm_bearer_stats_get_duration        (MMBearerStats *self);
+guint64 mm_bearer_stats_get_rx_bytes        (MMBearerStats *self);
+guint64 mm_bearer_stats_get_tx_bytes        (MMBearerStats *self);
+guint   mm_bearer_stats_get_attempts        (MMBearerStats *self);
+guint   mm_bearer_stats_get_failed_attempts (MMBearerStats *self);
 
 /*****************************************************************************/
 /* ModemManager/libmm-glib/mmcli specific methods */
@@ -73,9 +75,11 @@ MMBearerStats *mm_bearer_stats_new (void);
 MMBearerStats *mm_bearer_stats_new_from_dictionary (GVariant *dictionary,
                                                     GError **error);
 
-void mm_bearer_stats_set_duration (MMBearerStats *self, guint duration);
-void mm_bearer_stats_set_rx_bytes (MMBearerStats *self, guint64 rx_bytes);
-void mm_bearer_stats_set_tx_bytes (MMBearerStats *self, guint64 tx_bytes);
+void mm_bearer_stats_set_duration             (MMBearerStats *self, guint   duration);
+void mm_bearer_stats_set_rx_bytes             (MMBearerStats *self, guint64 rx_bytes);
+void mm_bearer_stats_set_tx_bytes             (MMBearerStats *self, guint64 tx_bytes);
+void mm_bearer_stats_set_attempts             (MMBearerStats *self, guint   attempts);
+void mm_bearer_stats_set_failed_attempts      (MMBearerStats *self, guint   failed_attempts);
 
 GVariant *mm_bearer_stats_get_dictionary (MMBearerStats *self);
 
diff --git a/src/mm-base-bearer.c b/src/mm-base-bearer.c
index 461c4058..718956fd 100644
--- a/src/mm-base-bearer.c
+++ b/src/mm-base-bearer.c
@@ -254,18 +254,19 @@ bearer_update_interface_stats (MMBaseBearer *self)
 }
 
 static void
-bearer_reset_interface_stats (MMBaseBearer *self)
+bearer_reset_ongoing_interface_stats (MMBaseBearer *self)
 {
-    g_clear_object (&self->priv->stats);
-    mm_gdbus_bearer_set_stats (MM_GDBUS_BEARER (self), NULL);
+    mm_bearer_stats_set_duration (self->priv->stats, 0);
+    mm_bearer_stats_set_tx_bytes (self->priv->stats, 0);
+    mm_bearer_stats_set_rx_bytes (self->priv->stats, 0);
+    bearer_update_interface_stats (self);
 }
 
 static void
 bearer_stats_stop (MMBaseBearer *self)
 {
     if (self->priv->duration_timer) {
-        if (self->priv->stats)
-            mm_bearer_stats_set_duration (self->priv->stats, (guint64) g_timer_elapsed (self->priv->duration_timer, NULL));
+        mm_bearer_stats_set_duration (self->priv->stats, (guint64) g_timer_elapsed (self->priv->duration_timer, NULL));
         g_timer_destroy (self->priv->duration_timer);
         self->priv->duration_timer = NULL;
     }
@@ -337,11 +338,6 @@ stats_update_cb (MMBaseBearer *self)
 static void
 bearer_stats_start (MMBaseBearer *self)
 {
-    /* Allocate new stats object. If there was one already created from a
-     * previous run, deallocate it */
-    g_assert (!self->priv->stats);
-    self->priv->stats = mm_bearer_stats_new ();
-
     /* Start duration timer */
     g_assert (!self->priv->duration_timer);
     self->priv->duration_timer = g_timer_new ();
@@ -731,7 +727,13 @@ connect_ready (MMBaseBearer *self,
     /* NOTE: connect() implementations *MUST* handle cancellations themselves */
     result = MM_BASE_BEARER_GET_CLASS (self)->connect_finish (self, res, &error);
     if (!result) {
-        mm_obj_dbg (self, "couldn't connect: '%s'", error->message);
+        mm_obj_dbg (self, "couldn't connect: %s", error->message);
+
+        /* Update failed attempts */
+        mm_bearer_stats_set_failed_attempts (self->priv->stats,
+                                             mm_bearer_stats_get_failed_attempts (self->priv->stats) + 1);
+        bearer_update_interface_stats (self);
+
         if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
             /* Will launch disconnection */
             launch_disconnect = TRUE;
@@ -862,11 +864,15 @@ mm_base_bearer_connect (MMBaseBearer *self,
         return;
     }
 
+    /* Update total attempts */
+    mm_bearer_stats_set_attempts (self->priv->stats,
+                                  mm_bearer_stats_get_attempts (self->priv->stats) + 1);
+    bearer_reset_ongoing_interface_stats (self);
+
     /* Connecting! */
     mm_obj_dbg (self, "connecting...");
     self->priv->connect_cancellable = g_cancellable_new ();
     bearer_update_status (self, MM_BEARER_STATUS_CONNECTING);
-    bearer_reset_interface_stats (self);
     MM_BASE_BEARER_GET_CLASS (self)->connect (
         self,
         self->priv->connect_cancellable,
@@ -1450,6 +1456,7 @@ mm_base_bearer_init (MMBaseBearer *self)
     self->priv->reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE;
     self->priv->reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE;
     self->priv->default_ip_family = MM_BEARER_IP_FAMILY_IPV4;
+    self->priv->stats = mm_bearer_stats_new ();
 
     /* Set defaults */
     mm_gdbus_bearer_set_interface   (MM_GDBUS_BEARER (self), NULL);
@@ -1462,6 +1469,7 @@ mm_base_bearer_init (MMBaseBearer *self)
                                      mm_bearer_ip_config_get_dictionary (NULL));
     mm_gdbus_bearer_set_ip6_config  (MM_GDBUS_BEARER (self),
                                      mm_bearer_ip_config_get_dictionary (NULL));
+    bearer_update_interface_stats (self);
 }
 
 static void