Re: tny_device_force_online() result?

[Murray Cumming <murrayc murrayc com>]

On Fri, 2007-04-27 at 08:30 +0200, Murray Cumming wrote:
> I guess that tny_device_force_online() is really just meant to _try_ to
> go online, with no guarantee of success. And I wonder if it's meant to
> block, or if its documentation should instead recommend a strategy for
> responding to success or failure asynchronously.
> 
> The GNOME platform implementation doesn't seem to do anything, but I'll
> need to make this clear for the TnyMaemoConicDevice implementation.

After talking on irc with Philip, I know now that this "force" stuff is
not really meant to make/close actual connections.

This patch documents that and improves the gnome and maemo
implementations. If this is OK, I guess I should fix the other
implementations.

-- 
Murray Cumming
murrayc murrayc com
www.murrayc.com
www.openismus.com

Index: libtinymail-maemo/tny-maemo-conic-device.c
===================================================================
--- libtinymail-maemo/tny-maemo-conic-device.c	(revision 1842)
+++ libtinymail-maemo/tny-maemo-conic-device.c	(working copy)
@@ -30,6 +30,7 @@
 	ConIcConnection *cnx;
 	gboolean        is_online;
 	gchar     *iap;
+	gboolean 	forced; /* Whether the is_online value is forced rather than real. */
 } TnyMaemoConicDevicePriv;
 
 #define TNY_MAEMO_CONIC_DEVICE_GET_PRIVATE(o)	\
@@ -39,7 +40,19 @@
 static void 
 tny_maemo_conic_device_reset (TnyDevice *device)
 {
-	g_message (__FUNCTION__);
+	TnyMaemoConicDevice *self;
+	TnyMaemoConicDevicePriv *priv;
+	g_return_if_fail (TNY_IS_DEVICE(device));
+	self = TNY_MAEMO_CONIC_DEVICE (device);
+	priv = TNY_MAEMO_CONIC_DEVICE_GET_PRIVATE (self);
+
+	const gboolean status_before = tny_maemo_conic_device_is_online (device);
+
+	priv->forced = FALSE;
+
+	if (status_before != tny_maemo_conic_device_is_online (device))
+		g_signal_emit (device, tny_device_signals [TNY_DEVICE_CONNECTION_CHANGED],
+		       0, !status_before);
 }
 
 
@@ -97,6 +110,7 @@
 	}
 
 	priv->is_online = is_online;
+	priv->forced = FALSE; /* is_online is now accurate. */
 	g_signal_emit (device, tny_device_signals [TNY_DEVICE_CONNECTION_CHANGED],
 		       0, is_online);
 }
@@ -274,40 +288,52 @@
 
 
 static void 
-tny_maemo_conic_device_force_online (TnyDevice *self)
+tny_maemo_conic_device_force_online (TnyDevice *device)
 {
-	TnyMaemoConicDevicePriv *priv;	
+	TnyMaemoConicDevice *self;
+	TnyMaemoConicDevicePriv *priv;
+	g_return_if_fail (TNY_IS_DEVICE(device));
+	self = TNY_MAEMO_CONIC_DEVICE (device);
+	priv = TNY_MAEMO_CONIC_DEVICE_GET_PRIVATE (self);
 
 #ifdef MAEMO_CONIC_DUMMY
 	return;
 #endif /*MAEMO_CONIC_DUMMY*/
-	
-	g_return_if_fail (TNY_IS_DEVICE(self));
-	g_return_if_fail (priv->cnx);
 
-	priv = TNY_MAEMO_CONIC_DEVICE_GET_PRIVATE (self);
+	const gboolean already_online = tny_maemo_conic_device_is_online (device);
 
-	if (!con_ic_connection_connect (priv->cnx, CON_IC_CONNECT_FLAG_NONE))
-			g_warning ("could not send connect dbus message");
+	priv->forced = TRUE;
+
+	/* Signal if it changed: */
+	if (!already_online)
+		g_signal_emit (device, tny_device_signals [TNY_DEVICE_CONNECTION_CHANGED],
+		       0, TRUE);
 }
 
 
 static void
-tny_maemo_conic_device_force_offline (TnyDevice *self)
+tny_maemo_conic_device_force_offline (TnyDevice *device)
 {
-	TnyMaemoConicDevicePriv *priv;	
+	TnyMaemoConicDevice *self;
+	TnyMaemoConicDevicePriv *priv;
+	g_return_if_fail (TNY_IS_DEVICE(device));
+	self = TNY_MAEMO_CONIC_DEVICE (device);
+	priv = TNY_MAEMO_CONIC_DEVICE_GET_PRIVATE (self);
 
 #ifdef MAEMO_CONIC_DUMMY
 	return;
 #endif /*MAEMO_CONIC_DUMMY*/
 
-	g_return_if_fail (TNY_IS_DEVICE(self));
-	g_return_if_fail (priv->cnx);
 
-	priv   = TNY_MAEMO_CONIC_DEVICE_GET_PRIVATE (self);
 
-	if (!con_ic_connection_disconnect (priv->cnx))
-		g_warning ("could not send disconnect dbus message");
+	const gboolean already_offline = !tny_maemo_conic_device_is_online (device);
+
+	priv->forced = TRUE;
+
+	/* Signal if it changed: */
+	if (!already_offline)
+		g_signal_emit (device, tny_device_signals [TNY_DEVICE_CONNECTION_CHANGED],
+		       0, FALSE);
 }
 
 
@@ -347,6 +373,7 @@
 	
 	
 	priv->is_online     = FALSE; 
+	priv->forced = FALSE;
 }
 
 
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 1842)
+++ ChangeLog	(working copy)
@@ -1,6 +1,24 @@
 2007-04-27  Murray Cumming  <murrayc murrayc com>
 
+	* libtinymail/tny-device.c: Improve the documentation for 
+	tny_device_force_online(), tny_device_force_offline(), and 
+	tny_device_reset(), describing why/when you might want to use them, 
+	and describing how they affect the behaviour of this object. This 
+	helps us to implement the underlying vfuncs.
+
+	* libtinymail-gnome-desktop/tny-gnome-device.c:
+	(tny_gnome_device_reset), (tny_gnome_device_force_online),
+	(tny_gnome_device_force_offline):
 	* libtinymail-maemo/tny-maemo-conic-device.c:
+	(tny_maemo_conic_device_reset), (on_connection_event),
+	(tny_maemo_conic_device_force_online),
+	(tny_maemo_conic_device_force_offline),
+	(tny_maemo_conic_device_instance_init): Improve the implementation, 
+	to match the documented behaviour.
+
+2007-04-27  Murray Cumming  <murrayc murrayc com>
+
+	* libtinymail-maemo/tny-maemo-conic-device.c:
 	(on_connection_event), (tny_maemo_conic_device_get_iap): Added some comments and 
 	improved the (temporary) debug output so its more obvious what is happening.
 
Index: libtinymail-gnome-desktop/tny-gnome-device.c
===================================================================
--- libtinymail-gnome-desktop/tny-gnome-device.c	(revision 1842)
+++ libtinymail-gnome-desktop/tny-gnome-device.c	(working copy)
@@ -62,10 +62,14 @@
 {
 	TnyGnomeDevicePriv *priv = TNY_GNOME_DEVICE_GET_PRIVATE (self);
 
+	const gboolean status_before = tny_gnome_device_is_online (self);
+
 	priv->fset = FALSE;
 	priv->forced = FALSE;
 
-	emit_status (self);
+	/* Signal if it changed: */
+	if (status_before != tny_gnome_device_is_online (self))
+		emit_status (self);
 }
 
 static void 
@@ -73,10 +77,14 @@
 {
 	TnyGnomeDevicePriv *priv = TNY_GNOME_DEVICE_GET_PRIVATE (self);
 
+	const gboolean already_online = tny_gnome_device_is_online (self);
+
 	priv->fset = TRUE;
 	priv->forced = TRUE;
 
-	emit_status (self);
+	/* Signal if it changed: */
+	if (!already_online)
+		emit_status (self);
 	
 	return;
 }
@@ -87,10 +95,14 @@
 {
 	TnyGnomeDevicePriv *priv = TNY_GNOME_DEVICE_GET_PRIVATE (self);
 
+	const gboolean already_offline = !tny_gnome_device_is_online (self);
+
 	priv->fset = TRUE;
 	priv->forced = FALSE;
 
-	emit_status (self);
+	/* Signal if it changed: */
+	if (!already_offline)
+		emit_status (self);
 	
 	return;
 }
Index: libtinymail/tny-device.c
===================================================================
--- libtinymail/tny-device.c	(revision 1842)
+++ libtinymail/tny-device.c	(working copy)
@@ -27,7 +27,14 @@
  * tny_device_reset:
  * @self: a #TnyDevice object
  * 
- * Reset the status (unforce the status)
+ * Reset the status (unforce the status).
+ * This reverses the effects of tny_device_force_online() or tny_device_force_offline(), 
+ * so that future changes of connection status will cause the connection_changed signal 
+ * to be emitted, and tny_device_is_online() will return a correct value.
+ *
+ * The connection_changed signal will be emitted if this tny_device_is_online() to 
+ * return a different value than before, for instance if the network connection has actually 
+ * become available or unavailable while the status was forced.
  **/
 void 
 tny_device_reset (TnyDevice *self)
@@ -49,7 +56,17 @@
  * tny_device_force_online:
  * @self: a #TnyDevice object
  * 
- * Force online status
+ * Force online status, so that tny_device_is_online() returns TRUE, 
+ * regardless of whether there is an actual network connection.
+ * The connection_changed signal will be emitted if the online status is changed by this function, 
+ * but note if a real network connection is made or lost later, the connection_changed signal will not be 
+ * emitted again, and tny_device_is_online() will continue to return TRUE;
+ *
+ * This might be used on platforms that cannot detect whether a network connection exists.
+ *
+ * This will usually not attempt to make a real network connection. 
+ *
+ * See also tny_device_force_offline() and tny_device_reset().
  **/
 void 
 tny_device_force_online (TnyDevice *self)
@@ -72,8 +89,19 @@
  * tny_device_force_offline:
  * @self: a #TnyDevice object
  * 
- * Force offline status
- * 
+ * Force offline status, so that tny_device_is_online() returns FALSE, 
+ * regardless of whether there is an actual network connection.
+ * The connection_changed signal will be emitted if the online status is changed by this function, 
+ * but note if a real network connection is made or lost later, the connection_changed signal will not be 
+ * emitted again, and tny_device_is_online() will continue to return FALSE;
+ *
+ * This might be used to mark a device as offline if the connection is partly unusable 
+ * due to some specific error, such as a failure to access a server or to use a particular port, 
+ * or if the user specifically chose "offline mode". 
+ * It might also be used on platforms that cannot detect whether a network connection exists.  
+ *
+ * This will usually not attempt to disconnect a real network connection. 
+ *
  * Example:
  * <informalexample><programlisting>
  * TnyDevice *device = ...
@@ -82,6 +110,8 @@
  *      g_print ("Something is wrong\n");
  * tny_device_reset (device);
  * </programlisting></informalexample>
+ *
+ * See also tny_device_force_online() and tny_device_reset().
  **/
 void
 tny_device_force_offline (TnyDevice *self)
@@ -151,6 +181,8 @@
  * @user_data: user data set when the signal handler was connected.
  *
  * Emitted when the connection status of a device changes.
+ * This signal will not be emitted in response to actual connection changes 
+ * while the status is forced with tny_device_force_online() or tny_device_force_offline().
  */
 		tny_device_signals[TNY_DEVICE_CONNECTION_CHANGED] =
 		   g_signal_new ("connection_changed",