Merge branch 'iphb'

8 files changed, 490 insertions, 64 deletions

diff --git a/configure.ac b/configure.ac
index 95d3f90..d79bf93 100644
--- a/configure.ac
+++ b/configure.ac
@@ -171,6 +171,14 @@ AM_CONDITIONAL(HAVE_LIBSASL2, test "x$HAVE_LIBSASL2" = "xyes")
 AC_SUBST(LIBXML2_CFLAGS)
 AC_SUBST(LIBXML2_LIBS)
 
+PKG_CHECK_MODULES(LIBIPHB, [libiphb],
+    [AC_DEFINE(HAVE_IPHB, 1, [libiphb is available])
+     have_iphb=yes
+    ],
+    [have_iphb=no])
+AC_SUBST(LIBIPHB_CFLAGS)
+AC_SUBST(LIBIPHB_LIBS)
+
 AC_ARG_ENABLE(coverage, AC_HELP_STRING([--enable-coverage],
       [compile with coverage profiling instrumentation (gcc only)]),
     [
@@ -220,4 +228,5 @@ Configure summary:
         TLS Backend..........: ${with_tls}
         SASL2 Tests..........: ${HAVE_LIBSASL2}
         gtk-doc documentation: ${enable_gtk_doc}
+        libiphb integration..: ${have_iphb}
 "
diff --git a/tests/wocky-ping-test.c b/tests/wocky-ping-test.c
index cd64a80..58ac668 100644
--- a/tests/wocky-ping-test.c
+++ b/tests/wocky-ping-test.c
@@ -12,10 +12,15 @@
 #include "wocky-test-stream.h"
 #include "wocky-test-helper.h"
 
-#define PING_COUNT 3
+#define PING_COUNT 2
 #define PING_INTERVAL 1
 
-#define TOTAL_TIMEOUT (PING_COUNT * PING_INTERVAL + 1)
+/* We expect PING_COUNT pings, followed by disabling pings and waiting for
+ * PING_COUNT * PING_INTERVAL to see if we get any pings we didn't want,
+ * followed by turning pings back on again and testing if we get any. The +1 is
+ * a fudge factor. ;-)
+ */
+#define TOTAL_TIMEOUT (PING_COUNT * PING_INTERVAL + 1) * 3
 
 static gboolean
 ping_recv_cb (WockyPorter *porter, WockyStanza *stanza, gpointer user_data)
@@ -27,17 +32,30 @@ ping_recv_cb (WockyPorter *porter, WockyStanza *stanza, gpointer user_data)
   wocky_porter_send (porter, reply);
   g_object_unref (reply);
 
+  g_assert_cmpuint (data->outstanding, >, 0);
   data->outstanding--;
   g_main_loop_quit (data->loop);
   return TRUE;
 }
 
+static gboolean
+we_have_waited_long_enough (gpointer user_data)
+{
+  test_data_t *test = user_data;
+
+  g_assert_cmpuint (test->outstanding, ==, 1);
+  test->outstanding--;
+  g_main_loop_quit (test->loop);
+  return FALSE;
+}
+
 static void
 test_periodic_ping (void)
 {
   WockyPing *ping;
   test_data_t *test = setup_test_with_timeout (TOTAL_TIMEOUT);
 
+  /* First, we ping every n seconds */
   ping = wocky_ping_new (test->sched_in, PING_INTERVAL);
 
   test_open_both_connections (test);
@@ -56,6 +74,18 @@ test_periodic_ping (void)
 
   test_wait_pending (test);
 
+  /* Now, we disable pings, and wait briefly to see if we get any pings. */
+  g_object_set (ping, "ping-interval", 0, NULL);
+  g_timeout_add_seconds (PING_INTERVAL * PING_COUNT,
+      we_have_waited_long_enough, test);
+  test->outstanding = 1;
+  test_wait_pending (test);
+
+  /* And then we enable pings again, and wait for one more. */
+  g_object_set (ping, "ping-interval", PING_INTERVAL, NULL);
+  test->outstanding += 1;
+  test_wait_pending (test);
+
   test_close_both_porters (test);
   g_object_unref (ping);
 
diff --git a/wocky/Makefile.am b/wocky/Makefile.am
index 5dd49c2..ec685b2 100644
--- a/wocky/Makefile.am
+++ b/wocky/Makefile.am
@@ -54,6 +54,7 @@ handwritten_headers = \
   wocky-contact-factory.h \
   wocky-data-form.h \
   wocky-debug.h \
+  wocky-heartbeat-source.h \
   wocky-jabber-auth.h \
   wocky-jabber-auth-digest.h \
   wocky-jabber-auth-password.h \
@@ -99,6 +100,7 @@ handwritten_sources = \
   wocky-contact-factory.c \
   wocky-data-form.c \
   wocky-debug.c \
+  wocky-heartbeat-source.c \
   wocky-jabber-auth.c \
   wocky-jabber-auth-digest.c \
   wocky-jabber-auth-password.c \
@@ -187,9 +189,10 @@ wocky-signals-marshal.list: $(handwritten_sources) Makefile.am
 
 AM_CFLAGS = $(ERROR_CFLAGS) $(GCOV_CFLAGS) \
             @GLIB_CFLAGS@ @LIBXML2_CFLAGS@ @TLS_CFLAGS@ \
+            @LIBIPHB_CFLAGS@ \
             -DG_LOG_DOMAIN=\"wocky\"
 
-AM_LDFLAGS = $(GCOV_LIBS) @GLIB_LIBS@ @LIBXML2_LIBS@ @TLS_LIBS@
+AM_LDFLAGS = $(GCOV_LIBS) @GLIB_LIBS@ @LIBXML2_LIBS@ @TLS_LIBS@ @LIBIPHB_LIBS@
 
 # rules for making the glib enum objects
 # The shell variable $long is something like WOCKY_XMPP_ERROR for
diff --git a/wocky/wocky-debug.c b/wocky/wocky-debug.c
index 95c7a35..1d05159 100644
--- a/wocky/wocky-debug.c
+++ b/wocky/wocky-debug.c
@@ -30,6 +30,7 @@ static GDebugKey keys[] = {
   { "pubsub",            DEBUG_PUBSUB            },
   { "dataform",          DEBUG_DATA_FORM         },
   { "ping",              DEBUG_PING              },
+  { "heartbeat",         DEBUG_HEARTBEAT         },
   { 0, },
 };
 
diff --git a/wocky/wocky-debug.h b/wocky/wocky-debug.h
index 0aa6f66..281ac9c 100644
--- a/wocky/wocky-debug.h
+++ b/wocky/wocky-debug.h
@@ -32,6 +32,7 @@ typedef enum
   DEBUG_PUBSUB            = 1 << 15,
   DEBUG_DATA_FORM         = 1 << 16,
   DEBUG_PING              = 1 << 17,
+  DEBUG_HEARTBEAT         = 1 << 18,
 } DebugFlags;
 
 #define DEBUG_XMPP (DEBUG_XMPP_READER | DEBUG_XMPP_WRITER)
diff --git a/wocky/wocky-heartbeat-source.c b/wocky/wocky-heartbeat-source.c
new file mode 100644
index 0000000..c6b17ac
--- /dev/null
+++ b/wocky/wocky-heartbeat-source.c
@@ -0,0 +1,377 @@
+/*
+ * wocky-heartbeat-source.c: a GSource wrapping libiphb.
+ * Copyright © 2010 Collabora Ltd. <http://www.collabora.co.uk>
+ * Copyright © 2010 Nokia Corporation
+ * @author Will Thompson <will.thompson@collabora.co.uk>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "wocky-heartbeat-source.h"
+
+#include <errno.h>
+
+#define DEBUG_FLAG DEBUG_HEARTBEAT
+#include "wocky-debug.h"
+
+#ifdef HAVE_IPHB
+# include <iphbd/libiphb.h>
+#endif
+
+typedef struct _WockyHeartbeatSource {
+    GSource parent;
+
+#ifdef HAVE_IPHB
+    iphb_t heartbeat;
+    GPollFD fd;
+#endif
+
+    guint max_interval;
+
+    GTimeVal next_wakeup;
+} WockyHeartbeatSource;
+
+#if HAVE_IPHB
+static void
+wocky_heartbeat_source_degrade (WockyHeartbeatSource *self)
+{
+  /* If we were using the heartbeat before, stop using it. */
+  if (self->heartbeat != NULL)
+    {
+      DEBUG ("closing heartbeat connection");
+      g_source_remove_poll ((GSource *) self, &self->fd);
+
+      iphb_close (self->heartbeat);
+      self->heartbeat = NULL;
+    }
+}
+
+static void
+wocky_heartbeat_source_wait (
+    WockyHeartbeatSource *self,
+    guint min_interval,
+    guint max_interval)
+{
+  int ret;
+
+  g_return_if_fail (max_interval == 0 || min_interval < max_interval);
+
+  if (self->heartbeat == NULL)
+    return;
+
+  if (max_interval > 0)
+    ret = iphb_wait (self->heartbeat, min_interval, max_interval, 0);
+  else
+    ret = iphb_I_woke_up (self->heartbeat);
+
+  if (ret == -1)
+    {
+      DEBUG ("waiting (%u, %u) failed: %s; falling back to internal timeouts",
+          min_interval, max_interval, g_strerror (errno));
+      wocky_heartbeat_source_degrade (self);
+    }
+}
+#endif
+
+static gboolean
+wocky_heartbeat_source_prepare (
+    GSource *source,
+    gint *msec_to_poll)
+{
+  WockyHeartbeatSource *self = (WockyHeartbeatSource *) source;
+  GTimeVal now;
+
+#if HAVE_IPHB
+  /* If we're listening to the system heartbeat, always rely on it to wake us
+   * up.
+   */
+  if (self->heartbeat != NULL)
+    {
+      *msec_to_poll = -1;
+      return FALSE;
+    }
+#endif
+
+  if (self->max_interval == 0)
+    return FALSE;
+
+  g_source_get_current_time (source, &now);
+
+  /* If now > self->next_wakeup, it's already time to wake up. */
+  if (now.tv_sec > self->next_wakeup.tv_sec ||
+      (now.tv_sec == self->next_wakeup.tv_sec &&
+       now.tv_usec >= self->next_wakeup.tv_usec))
+    {
+      DEBUG ("ready to wake up (at %li.%li)", now.tv_sec, now.tv_usec);
+      return TRUE;
+    }
+
+  /* Otherwise, we should only go back to sleep for a period of
+   * (self->next_wakeup - now). Inconveniently, GTimeVal gives us µs but we
+   * need to return ms; hence the scaling.
+   *
+   * The value calculated here will always be positive. The difference in
+   * seconds is non-negative; if it's zero, the difference in microseconds is
+   * positive.
+   */
+  *msec_to_poll = (self->next_wakeup.tv_sec - now.tv_sec) * 1000
+      + (self->next_wakeup.tv_usec - now.tv_usec) / 1000;
+
+  DEBUG ("want to wake up in %ums", *msec_to_poll);
+  return FALSE;
+}
+
+static gboolean
+wocky_heartbeat_source_check (
+    GSource *source)
+{
+  WockyHeartbeatSource *self = (WockyHeartbeatSource *) source;
+  GTimeVal now;
+
+#ifdef HAVE_IPHB
+  if (self->heartbeat != NULL)
+    {
+      if ((self->fd.revents & (G_IO_ERR | G_IO_HUP)) != 0)
+        {
+          DEBUG ("Heartbeat closed unexpectedly: %hu; "
+              "falling back to internal timeouts", self->fd.revents);
+          wocky_heartbeat_source_degrade (self);
+          return FALSE;
+        }
+      else if ((self->fd.revents & G_IO_IN) != 0)
+        {
+          DEBUG ("Heartbeat fired");
+          return TRUE;
+        }
+      else
+        {
+          return FALSE;
+        }
+    }
+#endif
+
+  if (self->max_interval == 0)
+    return FALSE;
+
+  g_source_get_current_time (source, &now);
+
+  return (now.tv_sec > self->next_wakeup.tv_sec ||
+      (now.tv_sec == self->next_wakeup.tv_sec &&
+       now.tv_usec >= self->next_wakeup.tv_usec));
+}
+
+#if HAVE_IPHB
+static inline guint
+get_min_interval (
+    WockyHeartbeatSource *self)
+{
+  /* We allow the heartbeat service to wake us up up to a minute early. */
+  return self->max_interval > 60 ? self->max_interval - 60 : 0;
+}
+#endif
+
+static gboolean
+wocky_heartbeat_source_dispatch (
+    GSource *source,
+    GSourceFunc callback,
+    gpointer user_data)
+{
+  WockyHeartbeatSource *self = (WockyHeartbeatSource *) source;
+
+  if (callback == NULL)
+    {
+      g_warning ("No callback set for WockyHeartbeatSource %p", self);
+      return FALSE;
+    }
+
+  /* Call our callback. We don't currently allow callbacks to stop future
+   * heartbeats from occurring: this source is used for keepalives from the
+   * time we're connected until we disconnect.
+   */
+  if (DEBUGGING)
+    {
+      GTimeVal now;
+
+      g_source_get_current_time (source, &now);
+      DEBUG ("calling %p (%p) at %li.%li", callback, user_data,
+          now.tv_sec, now.tv_usec);
+    }
+
+  ((WockyHeartbeatCallback) callback) (user_data);
+
+#if HAVE_IPHB
+  wocky_heartbeat_source_wait (self, get_min_interval (self),
+      self->max_interval);
+#endif
+
+  /* Record the time we next want to wake up. */
+  g_source_get_current_time (source, &self->next_wakeup);
+  self->next_wakeup.tv_sec += self->max_interval;
+  DEBUG ("next wakeup at %li.%li", self->next_wakeup.tv_sec,
+      self->next_wakeup.tv_usec);
+
+  return TRUE;
+}
+
+static void
+wocky_heartbeat_source_finalize (GSource *source)
+{
+#ifdef HAVE_IPHB
+  WockyHeartbeatSource *self = (WockyHeartbeatSource *) source;
+
+  wocky_heartbeat_source_degrade (self);
+#endif
+}
+
+static GSourceFuncs wocky_heartbeat_source_funcs = {
+    wocky_heartbeat_source_prepare,
+    wocky_heartbeat_source_check,
+    wocky_heartbeat_source_dispatch,
+    wocky_heartbeat_source_finalize,
+    NULL,
+    NULL
+};
+
+#if HAVE_IPHB
+static void
+connect_to_heartbeat (
+    WockyHeartbeatSource *self)
+{
+  GSource *source = (GSource *) self;
+
+  self->heartbeat = iphb_open (NULL);
+
+  if (self->heartbeat == NULL)
+    {
+      DEBUG ("Couldn't open connection to heartbeat service: %s",
+          g_strerror (errno));
+      return;
+    }
+
+  self->fd.fd = iphb_get_fd (self->heartbeat);
+  self->fd.events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+  g_source_add_poll (source, &self->fd);
+
+  /* We initially wait for anywhere between (0, max_interval) rather than
+   * (min_interval, max_interval) to fall into step with other connections,
+   * which may have started waiting at slightly different times.
+   */
+  wocky_heartbeat_source_wait (self, 0, self->max_interval);
+}
+#endif
+
+/**
+ * wocky_heartbeat_source_new:
+ * @max_interval: the maximum interval between calls to the source's callback,
+ *                in seconds. Pass 0 to prevent the callback being called.
+ *
+ * Creates a source which calls its callback at least every @max_interval
+ * seconds. This is similar to g_timeout_source_new_seconds(), except that the
+ * callback may be called slightly earlier than requested, in sync with other
+ * periodic network activity (from other XMPP connections, or other
+ * applications entirely).
+ *
+ * When calling g_source_set_callback() on this source, the supplied callback's
+ * signature should match #WockyHeartbeatCallback.
+ *
+ * Returns: the newly-created source.
+ */
+GSource *
+wocky_heartbeat_source_new (
+    guint max_interval)
+{
+  GSource *source = g_source_new (&wocky_heartbeat_source_funcs,
+      sizeof (WockyHeartbeatSource));
+  WockyHeartbeatSource *self = (WockyHeartbeatSource *) source;
+
+  /* We can't just call wocky_heartbeat_source_update_interval() because it
+   * assumes that we're attached to a main context. I think this is probably a
+   * reasonable assumption.
+   */
+  self->max_interval = max_interval;
+
+  g_get_current_time (&self->next_wakeup);
+  self->next_wakeup.tv_sec += max_interval;
+
+#if HAVE_IPHB
+  connect_to_heartbeat (self);
+#endif
+
+  return source;
+}
+
+/**
+ * wocky_heartbeat_source_update_interval:
+ * @source: a source returned by wocky_heartbeat_source_new()
+ * @max_interval: the new maximum interval between calls to the source's
+ *                callback, in seconds. Pass 0 to stop the callback being
+ *                called.
+ *
+ * Updates the interval between calls to @source's callback. The new interval
+ * may not take effect until after the next call to the callback.
+ */
+void
+wocky_heartbeat_source_update_interval (
+    GSource *source,
+    guint max_interval)
+{
+  WockyHeartbeatSource *self = (WockyHeartbeatSource *) source;
+
+  if (self->max_interval == max_interval)
+    return;
+
+  /* If we're not using the heartbeat, the new interval takes effect
+   * immediately.
+   *
+   * If we are, we just wait for the next heartbeat to fire as
+   * normal, and then use these new values when we ask it to wait again.
+   * (Except if the heartbeat was previously disabled, or is being disabled, in
+   * which case we have to be sure to schedule a wakeup, or cancel the pending
+   * wakeup, respectively.)
+   *
+   * We could alternatively calculate the time already elapsed since we last
+   * called iphb_wait(), and from that calculate how much longer we want to
+   * wait with these new values, taking care to deal with the cases where one
+   * or both of min_interval and max_interval have already passed. But life is
+   * too short.
+   */
+
+#ifdef HAVE_IPHB
+  /* We specify 0 as the lower bound here to give us a better chance of falling
+   * into step with other connections, which may have started waiting at
+   * slightly different times.
+   */
+  if (self->max_interval == 0 || max_interval == 0)
+    wocky_heartbeat_source_wait (self, 0, max_interval);
+#endif
+
+  /* If we were previously disabled, we need to re-initialize next_wakeup, not
+   * just update it.
+   */
+  if (self->max_interval == 0)
+    g_source_get_current_time (source, &self->next_wakeup);
+
+  /* If this moves self->next_wakeup into the past, then we'll wake up ASAP,
+   * which is what we want.
+   */
+  self->next_wakeup.tv_sec += (max_interval - self->max_interval);
+  self->max_interval = max_interval;
+
+  if (self->max_interval == 0)
+    DEBUG ("heartbeat disabled");
+  else
+    DEBUG ("next wakeup at or before %li.%li", self->next_wakeup.tv_sec,
+        self->next_wakeup.tv_usec);
+}
diff --git a/wocky/wocky-heartbeat-source.h b/wocky/wocky-heartbeat-source.h
new file mode 100644
index 0000000..73009e1
--- /dev/null
+++ b/wocky/wocky-heartbeat-source.h
@@ -0,0 +1,40 @@
+/*
+ * wocky-heartbeat-source.h: header for a GSource wrapping libiphb.
+ * Copyright © 2010 Collabora Ltd. <http://www.collabora.co.uk>
+ * Copyright © 2010 Nokia Corporation
+ * @author Will Thompson <will.thompson@collabora.co.uk>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#ifndef WOCKY_HEARTBEAT_SOURCE_H
+#define WOCKY_HEARTBEAT_SOURCE_H
+
+#include <glib.h>
+
+G_BEGIN_DECLS
+
+typedef void (*WockyHeartbeatCallback) (
+    gpointer user_data);
+
+GSource *wocky_heartbeat_source_new (
+    guint max_interval);
+
+void wocky_heartbeat_source_update_interval (
+    GSource *source,
+    guint max_interval);
+
+G_END_DECLS
+
+#endif /* WOCKY_HEARTBEAT_SOURCE_H */
diff --git a/wocky/wocky-ping.c b/wocky/wocky-ping.c
index 4ed059f..ce69f30 100644
--- a/wocky/wocky-ping.c
+++ b/wocky/wocky-ping.c
@@ -28,6 +28,7 @@
 
 #include "wocky-ping.h"
 
+#include "wocky-heartbeat-source.h"
 #include "wocky-namespaces.h"
 #include "wocky-stanza.h"
 
@@ -49,16 +50,14 @@ struct _WockyPingPrivate
   WockyPorter *porter;
 
   guint ping_interval;
+  GSource *heartbeat;
 
-  gulong porter_sending_id;
   gulong ping_iq_cb;
-  gulong ping_timeout_id;
 
   gboolean dispose_has_run;
 };
 
-static void reset_ping_timeout (WockyPing *self);
-static void porter_sending_cb (WockyPorter *porter, WockyPing *self);
+static void send_xmpp_ping (WockyPing *self);
 static gboolean ping_iq_cb (WockyPorter *porter, WockyStanza *stanza,
     gpointer data);
 
@@ -85,7 +84,12 @@ wocky_ping_set_property (GObject *object,
       break;
     case PROP_PING_INTERVAL:
       priv->ping_interval = g_value_get_uint (value);
-      reset_ping_timeout (self);
+      DEBUG ("updated ping interval to %u", priv->ping_interval);
+
+      if (priv->heartbeat != NULL)
+        wocky_heartbeat_source_update_interval (priv->heartbeat,
+            priv->ping_interval);
+
       break;
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
@@ -124,16 +128,17 @@ wocky_ping_constructed (GObject *object)
 
   g_assert (priv->porter != NULL);
 
-  priv->porter_sending_id = g_signal_connect (priv->porter, "sending",
-    G_CALLBACK (porter_sending_cb), self);
-  g_assert (priv->porter_sending_id != 0);
-
   priv->ping_iq_cb = wocky_porter_register_handler (priv->porter,
       WOCKY_STANZA_TYPE_IQ, WOCKY_STANZA_SUB_TYPE_GET, NULL,
       WOCKY_PORTER_HANDLER_PRIORITY_NORMAL, ping_iq_cb, self,
       '(', "ping",
           ':', WOCKY_XMPP_NS_PING,
       ')', NULL);
+
+  priv->heartbeat = wocky_heartbeat_source_new (priv->ping_interval);
+  g_source_set_callback (priv->heartbeat, (GSourceFunc) send_xmpp_ping,
+      self, NULL);
+  g_source_attach (priv->heartbeat, NULL);
 }
 
 static void
@@ -147,9 +152,6 @@ wocky_ping_dispose (GObject *object)
 
   priv->dispose_has_run = TRUE;
 
-  g_signal_handler_disconnect (priv->porter, priv->porter_sending_id);
-  priv->porter_sending_id = 0;
-
   if (priv->ping_iq_cb != 0)
     {
       wocky_porter_unregister_handler (priv->porter, priv->ping_iq_cb);
@@ -159,8 +161,9 @@ wocky_ping_dispose (GObject *object)
   g_object_unref (priv->porter);
   priv->porter = NULL;
 
-  priv->ping_interval = 0;
-  reset_ping_timeout (self);
+  g_source_destroy (self->priv->heartbeat);
+  g_source_unref (self->priv->heartbeat);
+  self->priv->heartbeat = NULL;
 
   if (G_OBJECT_CLASS (wocky_ping_parent_class)->dispose)
     G_OBJECT_CLASS (wocky_ping_parent_class)->dispose (object);
@@ -190,7 +193,7 @@ wocky_ping_class_init (WockyPingClass *wocky_ping_class)
   spec = g_param_spec_uint ("ping-interval", "Ping interval",
       "keepalive ping interval in seconds, or 0 to disable",
       0, G_MAXUINT, 0,
-      G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+      G_PARAM_READWRITE | G_PARAM_CONSTRUCT | G_PARAM_STATIC_STRINGS);
   g_object_class_install_property (object_class, PROP_PING_INTERVAL, spec);
 
 }
@@ -209,69 +212,31 @@ wocky_ping_new (WockyPorter *porter, guint interval)
 static void
 send_xmpp_ping (WockyPing *self)
 {
-  WockyPingPrivate *priv = self->priv;
   WockyStanza *iq;
 
+  g_return_if_fail (WOCKY_IS_PING (self));
+
   iq = wocky_stanza_build (WOCKY_STANZA_TYPE_IQ,
       WOCKY_STANZA_SUB_TYPE_GET, NULL, NULL,
       '(', "ping",
           ':', WOCKY_XMPP_NS_PING,
       ')', NULL);
 
-  wocky_porter_send_iq_async (priv->porter, iq, NULL, NULL, NULL);
+  DEBUG ("pinging");
+  wocky_porter_send_iq_async (self->priv->porter, iq, NULL, NULL, NULL);
   g_object_unref (iq);
 }
 
 static gboolean
-ping_timeout_cb (gpointer data)
-{
-  WockyPing *self = WOCKY_PING (data);
-  WockyPingPrivate *priv = self->priv;
-
-  DEBUG ("sending ping after %d seconds", priv->ping_interval);
-
-  /* Sending the ping stanza will call back to us to reset
-   * the timeout, so no need to keep this one running. */
-  priv->ping_timeout_id = 0;
-  send_xmpp_ping (self);
-  return FALSE;
-}
-
-static void
-porter_sending_cb (WockyPorter *porter, WockyPing *ping)
-{
-  g_assert (WOCKY_IS_PING (ping));
-
-  reset_ping_timeout (ping);
-}
-
-static void
-reset_ping_timeout (WockyPing *self)
-{
-  WockyPingPrivate *priv = self->priv;
-
-  if (priv->ping_timeout_id != 0)
-    {
-      g_source_remove (priv->ping_timeout_id);
-      priv->ping_timeout_id = 0;
-    }
-
-  if (priv->ping_interval > 0)
-    {
-      priv->ping_timeout_id = g_timeout_add_seconds (priv->ping_interval,
-        ping_timeout_cb, self);
-    }
-}
-
-static gboolean
 ping_iq_cb (WockyPorter *porter, WockyStanza *stanza, gpointer data)
 {
+#ifdef ENABLE_DEBUG
+  const gchar *from = wocky_stanza_get_from (stanza);
+#endif
   WockyStanza *reply;
 
-  DEBUG ("replying to ping request");
+  DEBUG ("replying to ping from %s", from ? from : "<null>");
 
-  /* As a side-effect of sending pong, our keepalive timer will be
-   * reset, so we don't have to do it manually. */
   reply = wocky_stanza_build_iq_result (stanza, NULL);
   wocky_porter_send (porter, reply);
   g_object_unref (reply);