gnome-power-manager r2773 - in trunk: . src

[rhughes svn gnome org]

Author: rhughes
Date: Thu Apr 10 14:20:59 2008
New Revision: 2773
URL: http://svn.gnome.org/viewvc/gnome-power-manager?rev=2773&view=rev

Log:
2008-04-10  Richard Hughes  <richard hughsie com>

* src/gpm-brightness.c: (gpm_brightness_trust_cache),
(gpm_brightness_set), (gpm_brightness_get), (gpm_brightness_up),
(gpm_brightness_down), (gpm_brightness_changed_cb),
(gpm_brightness_init):
* src/gpm-brightness.h:
Allow some data to be cached to speed up querying brightess.

* src/gpm-backlight.c: (gpm_backlight_set_brightness),
(gpm_backlight_brightness_evaluate_and_set),
(gpm_backlight_button_pressed_cb):
* src/gpm-brightness-hal.c: (gpm_brightness_hal_set_hw),
(gpm_brightness_hal_dim_hw_step), (gpm_brightness_hal_set),
(gpm_brightness_hal_up), (gpm_brightness_hal_down),
(gpm_brightness_hal_init):
* src/gpm-brightness-hal.h:
* src/gpm-brightness-xrandr.c:
(gpm_brightness_xrandr_output_set_internal),
(gpm_brightness_xrandr_output_set), (gpm_brightness_xrandr_set),
(gpm_brightness_xrandr_get), (gpm_brightness_xrandr_up),
(gpm_brightness_xrandr_down), (gpm_brightness_xrandr_init):
* src/gpm-brightness-xrandr.h:
Make the methods sane, so we can tell the difference between a failed method, and one
that did not change the hardware brightness. This should fix lots of issues with the
brightness applet.


Modified:
   trunk/ChangeLog
   trunk/src/gpm-backlight.c
   trunk/src/gpm-brightness-hal.c
   trunk/src/gpm-brightness-hal.h
   trunk/src/gpm-brightness-xrandr.c
   trunk/src/gpm-brightness-xrandr.h
   trunk/src/gpm-brightness.c
   trunk/src/gpm-brightness.h

Modified: trunk/src/gpm-backlight.c
==============================================================================
--- trunk/src/gpm-backlight.c	(original)
+++ trunk/src/gpm-backlight.c	Thu Apr 10 14:20:59 2008
@@ -65,7 +65,7 @@
 struct GpmBacklightPrivate
 {
 	GpmAcAdapter		*ac_adapter;
-	GpmBrightness	*brightness;
+	GpmBrightness		*brightness;
 	GpmButton		*button;
 	GpmConf			*conf;
 	GpmFeedback		*feedback;
@@ -299,9 +299,10 @@
  * gpm_backlight_set_brightness:
  **/
 gboolean
-gpm_backlight_set_brightness (GpmBacklight *backlight, guint brightness, GError **error)
+gpm_backlight_set_brightness (GpmBacklight *backlight, guint percentage, GError **error)
 {
 	gboolean ret;
+	gboolean hw_changed;
 
 	g_return_val_if_fail (backlight != NULL, FALSE);
 	g_return_val_if_fail (GPM_IS_BACKLIGHT (backlight), FALSE);
@@ -314,16 +315,21 @@
 		return FALSE;
 	}
 
-	/* just set the AC brightness for now, don't try to be clever */
-	backlight->priv->master_percentage = brightness;
+	/* just set the master percentage for now, don't try to be clever */
+	backlight->priv->master_percentage = percentage;
 
 	/* sets the current policy brightness */
-	ret = gpm_brightness_set (backlight->priv->brightness, brightness);
+	ret = gpm_brightness_set (backlight->priv->brightness, percentage, &hw_changed);
 	if (ret == FALSE) {
 		*error = g_error_new (gpm_backlight_error_quark (),
 				      GPM_BACKLIGHT_ERROR_GENERAL,
 				      "Cannot set policy brightness");
 	}
+	/* we emit a signal for the brightness applet */
+	if (ret && hw_changed) {
+		gpm_debug ("emitting brightness-changed : %i", percentage);
+		g_signal_emit (backlight, signals [BRIGHTNESS_CHANGED], 0, percentage);
+	}
 	return ret;
 }
 
@@ -348,10 +354,12 @@
 {
 	gfloat brightness;
 	gfloat scale;
+	gboolean ret;
 	gboolean on_ac;
 	gboolean do_laptop_lcd;
 	gboolean enable_action;
 	gboolean battery_reduce;
+	gboolean hw_changed;
 	guint value;
 	guint old_value;
 
@@ -442,11 +450,12 @@
 		gpm_feedback_display_value (backlight->priv->feedback, (float) brightness);
 	}
 
+	ret = gpm_brightness_set (backlight->priv->brightness, value, &hw_changed);
 	/* we emit a signal for the brightness applet */
-	gpm_debug ("emitting brightness-changed : %i", value);
-	g_signal_emit (backlight, signals [BRIGHTNESS_CHANGED], 0, value);
-
-	gpm_brightness_set (backlight->priv->brightness, value);
+	if (ret && hw_changed) {
+		gpm_debug ("emitting brightness-changed : %i", value);
+		g_signal_emit (backlight, signals [BRIGHTNESS_CHANGED], 0, value);
+	}
 	return TRUE;
 }
 
@@ -515,27 +524,42 @@
 gpm_backlight_button_pressed_cb (GpmButton *button, const gchar *type, GpmBacklight *backlight)
 {
 	guint percentage;
+	gboolean ret;
+	gboolean hw_changed;
 	gpm_debug ("Button press event type=%s", type);
 
 	if (strcmp (type, GPM_BUTTON_BRIGHT_UP) == 0) {
 		/* go up one step */
-		gpm_brightness_up (backlight->priv->brightness);
+		ret = gpm_brightness_up (backlight->priv->brightness, &hw_changed);
 
-		/* get the new value */
-		gpm_brightness_get (backlight->priv->brightness, &percentage);
-		gpm_feedback_display_value (backlight->priv->feedback, (float) percentage/100.0f);
-		/* save the new percentage */
-		backlight->priv->master_percentage = percentage;
+		/* show the new value */
+		if (ret) {
+			gpm_brightness_get (backlight->priv->brightness, &percentage);
+			gpm_feedback_display_value (backlight->priv->feedback, (float) percentage/100.0f);
+			/* save the new percentage */
+			backlight->priv->master_percentage = percentage;
+		}
+		/* we emit a signal for the brightness applet */
+		if (ret && hw_changed) {
+			gpm_debug ("emitting brightness-changed : %i", percentage);
+			g_signal_emit (backlight, signals [BRIGHTNESS_CHANGED], 0, percentage);
+		}
 	} else if (strcmp (type, GPM_BUTTON_BRIGHT_DOWN) == 0) {
 		/* go up down step */
-		gpm_brightness_down (backlight->priv->brightness);
+		ret = gpm_brightness_down (backlight->priv->brightness, &hw_changed);
 
-		/* get the new value */
-		gpm_brightness_get (backlight->priv->brightness, &percentage);
-		gpm_feedback_display_value (backlight->priv->feedback, (float) percentage/100.0f);
-
-		/* save the new percentage */
-		backlight->priv->master_percentage = percentage;
+		/* show the new value */
+		if (ret) {
+			gpm_brightness_get (backlight->priv->brightness, &percentage);
+			gpm_feedback_display_value (backlight->priv->feedback, (float) percentage/100.0f);
+			/* save the new percentage */
+			backlight->priv->master_percentage = percentage;
+		}
+		/* we emit a signal for the brightness applet */
+		if (ret && hw_changed) {
+			gpm_debug ("emitting brightness-changed : %i", percentage);
+			g_signal_emit (backlight, signals [BRIGHTNESS_CHANGED], 0, percentage);
+		}
 	} else if (strcmp (type, GPM_BUTTON_LID_OPEN) == 0) {
 		/* make sure we undim when we lift the lid */
 		gpm_backlight_brightness_evaluate_and_set (backlight, FALSE);

Modified: trunk/src/gpm-brightness-hal.c
==============================================================================
--- trunk/src/gpm-brightness-hal.c	(original)
+++ trunk/src/gpm-brightness-hal.c	Thu Apr 10 14:20:59 2008
@@ -56,6 +56,7 @@
 	guint			 level_std_hw;
 	guint			 levels;
 	gchar			*udi;
+	gboolean		 hw_changed;
 	DbusProxy		*gproxy;
 };
 
@@ -165,6 +166,11 @@
 		return FALSE;
 	}
 
+	/* we changed the hardware */
+	if (ret) {
+		brightness->priv->hw_changed = TRUE;
+	}
+
 	brightness->priv->last_set_hw = value_hw;
 	return TRUE;
 }
@@ -181,7 +187,7 @@
 {
 	guint last_set_hw;
 	gint a;
-	gboolean ret;
+	gboolean ret = FALSE;
 
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_HAL (brightness), FALSE);
 
@@ -214,7 +220,7 @@
 			g_usleep (1000 * GPM_BRIGHTNESS_DIM_INTERVAL);
 		}
 	}
-	return TRUE;
+	return ret;
 }
 
 /**
@@ -264,19 +270,30 @@
  * gpm_brightness_hal_set:
  * @brightness: This brightness class instance
  * @percentage: The percentage brightness
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  **/
 gboolean
-gpm_brightness_hal_set (GpmBrightnessHal *brightness, guint	percentage)
+gpm_brightness_hal_set (GpmBrightnessHal *brightness, guint percentage, gboolean *hw_changed)
 {
 	guint level_hw;
+	gboolean ret;
 
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_HAL (brightness), FALSE);
+	g_return_val_if_fail (hw_changed != NULL, FALSE);
+
+	/* reset to not-changed */
+	brightness->priv->hw_changed = FALSE;
 
 	level_hw = gpm_percent_to_discrete (percentage, brightness->priv->levels);
 	brightness->priv->level_std_hw = level_hw;
 
 	/* update */
-	return gpm_brightness_hal_dim_hw (brightness, level_hw);
+	ret = gpm_brightness_hal_dim_hw (brightness, level_hw);
+
+	/* did the hardware have to be modified? */
+	*hw_changed = brightness->priv->hw_changed;
+	return ret;
 }
 
 /**
@@ -300,17 +317,24 @@
 /**
  * gpm_brightness_hal_up:
  * @brightness: This brightness class instance
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  *
  * If possible, put the brightness of the LCD up one unit.
  **/
 gboolean
-gpm_brightness_hal_up (GpmBrightnessHal *brightness)
+gpm_brightness_hal_up (GpmBrightnessHal *brightness, gboolean *hw_changed)
 {
+	gboolean ret = FALSE;
 	gint step;
 	gint percentage;
 	guint current_hw;
 
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_HAL (brightness), FALSE);
+	g_return_val_if_fail (hw_changed != NULL, FALSE);
+
+	/* reset to not-changed */
+	brightness->priv->hw_changed = FALSE;
 
 	/* check to see if the panel has changed */
 	gpm_brightness_hal_get_hw (brightness, &current_hw);
@@ -325,26 +349,35 @@
 		if (brightness->priv->last_set_hw + step > brightness->priv->levels - 1) {
 			step = (brightness->priv->levels - 1) - brightness->priv->last_set_hw;
 		}
-		gpm_brightness_hal_set_hw (brightness, brightness->priv->last_set_hw + step);
+		ret = gpm_brightness_hal_set_hw (brightness, brightness->priv->last_set_hw + step);
 	}
 
-	return TRUE;
+	/* did the hardware have to be modified? */
+	*hw_changed = brightness->priv->hw_changed;
+	return ret;
 }
 
 /**
  * gpm_brightness_hal_down:
  * @brightness: This brightness class instance
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  *
  * If possible, put the brightness of the LCD down one unit.
  **/
 gboolean
-gpm_brightness_hal_down (GpmBrightnessHal *brightness)
+gpm_brightness_hal_down (GpmBrightnessHal *brightness, gboolean *hw_changed)
 {
+	gboolean ret = FALSE;
 	gint step;
 	gint percentage;
 	guint current_hw;
 
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_HAL (brightness), FALSE);
+	g_return_val_if_fail (hw_changed != NULL, FALSE);
+
+	/* reset to not-changed */
+	brightness->priv->hw_changed = FALSE;
 
 	/* check to see if the panel has changed */
 	gpm_brightness_hal_get_hw (brightness, &current_hw);
@@ -359,10 +392,13 @@
 		if (brightness->priv->last_set_hw < step) {
 			step = brightness->priv->last_set_hw;
 		}
-		gpm_brightness_hal_set_hw (brightness, brightness->priv->last_set_hw - step);
+		ret = gpm_brightness_hal_set_hw (brightness, brightness->priv->last_set_hw - step);
 	}
 
-	return TRUE;
+
+	/* did the hardware have to be modified? */
+	*hw_changed = brightness->priv->hw_changed;
+	return ret;
 }
 
 /**
@@ -431,6 +467,7 @@
 
 	brightness->priv = GPM_BRIGHTNESS_HAL_GET_PRIVATE (brightness);
 	brightness->priv->gproxy = NULL;
+	brightness->priv->hw_changed = FALSE;
 
 	/* save udi of lcd adapter */
 	manager = hal_gmanager_new ();

Modified: trunk/src/gpm-brightness-hal.h
==============================================================================
--- trunk/src/gpm-brightness-hal.h	(original)
+++ trunk/src/gpm-brightness-hal.h	Thu Apr 10 14:20:59 2008
@@ -52,12 +52,15 @@
 GpmBrightnessHal *gpm_brightness_hal_new	(void);
 
 gboolean	 gpm_brightness_hal_has_hw	(GpmBrightnessHal	*brightness);
-gboolean	 gpm_brightness_hal_up		(GpmBrightnessHal	*brightness);
-gboolean	 gpm_brightness_hal_down	(GpmBrightnessHal	*brightness);
+gboolean	 gpm_brightness_hal_up		(GpmBrightnessHal	*brightness,
+						 gboolean		*hw_changed);
+gboolean	 gpm_brightness_hal_down	(GpmBrightnessHal	*brightness,
+						 gboolean		*hw_changed);
 gboolean	 gpm_brightness_hal_get		(GpmBrightnessHal	*brightness,
 						 guint			*percentage);
 gboolean	 gpm_brightness_hal_set		(GpmBrightnessHal	*brightness,
-						 guint			 percentage);
+						 guint			 percentage,
+						 gboolean		*hw_changed);
 
 G_END_DECLS
 

Modified: trunk/src/gpm-brightness-xrandr.c
==============================================================================
--- trunk/src/gpm-brightness-xrandr.c	(original)
+++ trunk/src/gpm-brightness-xrandr.c	Thu Apr 10 14:20:59 2008
@@ -58,6 +58,7 @@
 	Display			*dpy;
 	guint			 shared_value;
 	gboolean		 has_extension;
+	gboolean		 hw_changed;
 };
 
 enum {
@@ -127,6 +128,10 @@
 		gpm_warning ("failed to XRRChangeOutputProperty for brightness %i", value);
 		ret = FALSE;
 	}
+	/* we changed the hardware */
+	if (ret) {
+		brightness->priv->hw_changed = TRUE;
+	}
 	return ret;
 }
 
@@ -244,7 +249,7 @@
 	gpm_debug ("hard value=%i, min=%i, max=%i", cur, min, max);
 	if (cur == min) {
 		gpm_debug ("already min");
-		return FALSE;
+		return TRUE;
 	}
 	cur -= brightness->priv->shared_value;
 	if (cur < min) {
@@ -278,7 +283,7 @@
 	gpm_debug ("hard value=%i, min=%i, max=%i", cur, min, max);
 	if (cur == max) {
 		gpm_debug ("already max");
-		return FALSE;
+		return TRUE;
 	}
 	cur += brightness->priv->shared_value;
 	if (cur > max) {
@@ -322,7 +327,7 @@
 		shared_value_abs = min;
 	if (cur == shared_value_abs) {
 		gpm_debug ("already set %i", cur);
-		return FALSE;
+		return TRUE;
 	}
 
 	/* step the correct way */
@@ -423,28 +428,45 @@
 }
 
 /**
- * gpm_brightness_xrandr_set_std:
+ * gpm_brightness_xrandr_set:
  * @brightness: This brightness class instance
  * @percentage: The percentage brightness
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  **/
 gboolean
-gpm_brightness_xrandr_set (GpmBrightnessXRandR *brightness, guint percentage)
+gpm_brightness_xrandr_set (GpmBrightnessXRandR *brightness, guint percentage, gboolean *hw_changed)
 {
+	gboolean ret;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_XRANDR (brightness), FALSE);
+	g_return_val_if_fail (hw_changed != NULL, FALSE);
+
 	brightness->priv->shared_value = percentage;
-	return gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_SET);
+
+	/* reset to not-changed */
+	brightness->priv->hw_changed = FALSE;
+	ret = gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_SET);
+
+	/* did the hardware have to be modified? */
+	*hw_changed = brightness->priv->hw_changed;
+	return ret;
 }
 
 /**
  * gpm_brightness_xrandr_get:
  * @brightness: This brightness class instance
- * Return value: Success
+ * @percentage: Value to retrieve
+ * Return value: %TRUE if success, %FALSE if there was an error
  **/
 gboolean
 gpm_brightness_xrandr_get (GpmBrightnessXRandR *brightness, guint *percentage)
 {
 	gboolean ret;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_XRANDR (brightness), FALSE);
+	g_return_val_if_fail (percentage != NULL, FALSE);
+
 	ret = gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_GET);
 	*percentage = brightness->priv->shared_value;
 	return ret;
@@ -453,31 +475,57 @@
 /**
  * gpm_brightness_xrandr_up:
  * @brightness: This brightness class instance
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  *
  * If possible, put the brightness of the LCD up one unit.
  **/
 gboolean
-gpm_brightness_xrandr_up (GpmBrightnessXRandR *brightness)
+gpm_brightness_xrandr_up (GpmBrightnessXRandR *brightness, gboolean *hw_changed)
 {
+	gboolean ret;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_XRANDR (brightness), FALSE);
+	g_return_val_if_fail (hw_changed != NULL, FALSE);
+
 	/* single step */
 	brightness->priv->shared_value = 1;
-	return gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_INC);
+
+	/* reset to not-changed */
+	brightness->priv->hw_changed = FALSE;
+	ret = gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_INC);
+
+	/* did the hardware have to be modified? */
+	*hw_changed = brightness->priv->hw_changed;
+	return ret;
 }
 
 /**
  * gpm_brightness_xrandr_down:
  * @brightness: This brightness class instance
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  *
  * If possible, put the brightness of the LCD down one unit.
  **/
 gboolean
-gpm_brightness_xrandr_down (GpmBrightnessXRandR *brightness)
+gpm_brightness_xrandr_down (GpmBrightnessXRandR *brightness, gboolean *hw_changed)
 {
+	gboolean ret;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS_XRANDR (brightness), FALSE);
+	g_return_val_if_fail (hw_changed != NULL, FALSE);
+
 	/* single step */
 	brightness->priv->shared_value = 1;
-	return gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_DEC);
+
+	/* reset to not-changed */
+	brightness->priv->hw_changed = FALSE;
+	ret = gpm_brightness_xrandr_foreach_screen (brightness, ACTION_BACKLIGHT_DEC);
+
+	/* did the hardware have to be modified? */
+	*hw_changed = brightness->priv->hw_changed;
+	return ret;
 }
 
 /**
@@ -566,6 +614,7 @@
 	int ignore;
 
 	brightness->priv = GPM_BRIGHTNESS_XRANDR_GET_PRIVATE (brightness);
+	brightness->priv->hw_changed = FALSE;
 
 	/* can we do this */
 	brightness->priv->has_extension = gpm_brightness_xrandr_setup_display (brightness);

Modified: trunk/src/gpm-brightness-xrandr.h
==============================================================================
--- trunk/src/gpm-brightness-xrandr.h	(original)
+++ trunk/src/gpm-brightness-xrandr.h	Thu Apr 10 14:20:59 2008
@@ -52,12 +52,15 @@
 GpmBrightnessXRandR *gpm_brightness_xrandr_new	(void);
 
 gboolean	 gpm_brightness_xrandr_has_hw	(GpmBrightnessXRandR	*brightness);
-gboolean	 gpm_brightness_xrandr_up	(GpmBrightnessXRandR	*brightness);
-gboolean	 gpm_brightness_xrandr_down	(GpmBrightnessXRandR	*brightness);
+gboolean	 gpm_brightness_xrandr_up	(GpmBrightnessXRandR	*brightness,
+						 gboolean		*hw_changed);
+gboolean	 gpm_brightness_xrandr_down	(GpmBrightnessXRandR	*brightness,
+						 gboolean		*hw_changed);
 gboolean	 gpm_brightness_xrandr_get	(GpmBrightnessXRandR	*brightness,
 						 guint			*percentage);
 gboolean	 gpm_brightness_xrandr_set	(GpmBrightnessXRandR	*brightness,
-						 guint			 percentage);
+						 guint			 percentage,
+						 gboolean		*hw_changed);
 
 G_END_DECLS
 

Modified: trunk/src/gpm-brightness.c
==============================================================================
--- trunk/src/gpm-brightness.c	(original)
+++ trunk/src/gpm-brightness.c	Thu Apr 10 14:20:59 2008
@@ -43,11 +43,15 @@
 #include "gpm-marshal.h"
 
 #define GPM_BRIGHTNESS_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GPM_TYPE_BRIGHTNESS, GpmBrightnessPrivate))
+#define GPM_SOLE_SETTER_USE_CACHE	TRUE	/* this may be insanity */
 
 struct GpmBrightnessPrivate
 {
 	gboolean		 use_xrandr;
 	gboolean		 use_hal;
+	gboolean		 has_changed_events;
+	gboolean		 cache_trusted;
+	guint			 cache_percentage;
 	GpmBrightnessHal	*hal;
 	GpmBrightnessXRandR	*xrandr;
 };
@@ -62,22 +66,73 @@
 static gpointer gpm_brightness_object = NULL;
 
 /**
+ * gpm_brightness_trust_cache:
+ * @brightness: This brightness class instance
+ * Return value: if we can trust the cache
+ **/
+static gboolean
+gpm_brightness_trust_cache (GpmBrightness *brightness)
+{
+	g_return_val_if_fail (GPM_IS_BRIGHTNESS (brightness), FALSE);
+	/* only return the cached value if the cache is trusted and we have change events */
+	if (brightness->priv->cache_trusted && brightness->priv->has_changed_events) {
+		gpm_debug ("using cache for value %u (okay)", brightness->priv->cache_percentage);
+		return TRUE;
+	}
+
+	/* can we trust that if we set a value 5 minutes ago, will it still be valid now?
+	 * if we have multiple things setting policy on the workstation, e.g. fast user switching
+	 * or kpowersave, then this will be invalid -- this logic may be insane */
+	if (GPM_SOLE_SETTER_USE_CACHE && brightness->priv->cache_trusted) {
+		gpm_warning ("using cache for value %u (probably okay)", brightness->priv->cache_percentage);
+		return TRUE;
+	}
+	return FALSE;
+}
+
+/**
  * gpm_brightness_set:
  * @brightness: This brightness class instance
  * @percentage: The percentage brightness
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  **/
 gboolean
-gpm_brightness_set (GpmBrightness *brightness, guint percentage)
+gpm_brightness_set (GpmBrightness *brightness, guint percentage, gboolean *hw_changed)
 {
 	gboolean ret = FALSE;
+	gboolean trust_cache;
+	gboolean hw_changed_local = FALSE;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS (brightness), FALSE);
+
+	/* can we check the new value with the cache? */
+	trust_cache = gpm_brightness_trust_cache (brightness);
+	if (trust_cache && percentage == brightness->priv->cache_percentage) {
+		gpm_debug ("not setting the same value %i", percentage);
+		return TRUE;
+	}
+
+	/* set the hardware */
 	if (brightness->priv->use_xrandr) {
-		ret = gpm_brightness_xrandr_set (brightness->priv->xrandr, percentage);
-		return ret;
+		ret = gpm_brightness_xrandr_set (brightness->priv->xrandr, percentage, &hw_changed_local);
+		goto out;
 	}
 	if (brightness->priv->use_hal) {
-		ret = gpm_brightness_hal_set (brightness->priv->hal, percentage);
-	}	
+		ret = gpm_brightness_hal_set (brightness->priv->hal, percentage, &hw_changed_local);
+		goto out;
+	}
+	gpm_debug ("no hardware support");
+	return FALSE;
+out:
+	/* we did something to the hardware, so untrusted */
+	if (ret) {
+		brightness->priv->cache_trusted = FALSE;
+	}
+	/* is the caller interested? */
+	if (ret && hw_changed != NULL) {
+		*hw_changed = hw_changed_local;
+	}
 	return ret;
 }
 
@@ -93,56 +148,121 @@
 gpm_brightness_get (GpmBrightness *brightness, guint *percentage)
 {
 	gboolean ret = FALSE;
+	gboolean trust_cache;
+	guint percentage_local;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS (brightness), FALSE);
+	g_return_val_if_fail (percentage != NULL, FALSE);
+
+	/* can we use the cache? */
+	trust_cache = gpm_brightness_trust_cache (brightness);
+	if (trust_cache) {
+		*percentage = brightness->priv->cache_percentage;
+		return TRUE;
+	}
+
+	/* get the brightness from hardware -- slow */
 	if (brightness->priv->use_xrandr) {
-		ret = gpm_brightness_xrandr_get (brightness->priv->xrandr, percentage);
-		return ret;
+		ret = gpm_brightness_xrandr_get (brightness->priv->xrandr, &percentage_local);
+		goto out;
 	}
 	if (brightness->priv->use_hal) {
-		ret = gpm_brightness_hal_get (brightness->priv->hal, percentage);
-	}	
+		ret = gpm_brightness_hal_get (brightness->priv->hal, &percentage_local);
+		goto out;
+	}
+	gpm_debug ("no hardware support");
+	return FALSE;
+out:
+	/* valid? */
+	if (percentage_local > 100) {
+		gpm_warning ("percentage value of %i will be ignored", percentage_local);
+		ret = FALSE;
+		return;
+	}
+	/* a new value is always trusted if the method and checks succeed */
+	if (ret) {
+		brightness->priv->cache_percentage = percentage_local;
+		brightness->priv->cache_trusted = TRUE;
+		*percentage = percentage_local;
+	} else {
+		brightness->priv->cache_trusted = FALSE;
+	}
 	return ret;
 }
 
 /**
  * gpm_brightness_up:
  * @brightness: This brightness class instance
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  *
  * If possible, put the brightness of the LCD up one unit.
  **/
 gboolean
-gpm_brightness_up (GpmBrightness *brightness)
+gpm_brightness_up (GpmBrightness *brightness, gboolean *hw_changed)
 {
 	gboolean ret = FALSE;
+	gboolean hw_changed_local = FALSE;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS (brightness), FALSE);
+
 	if (brightness->priv->use_xrandr) {
-		ret = gpm_brightness_xrandr_up (brightness->priv->xrandr);
-		return ret;
+		ret = gpm_brightness_xrandr_up (brightness->priv->xrandr, &hw_changed_local);
+		goto out;
 	}
 	if (brightness->priv->use_hal) {
-		ret = gpm_brightness_hal_up (brightness->priv->hal);
-	}	
+		ret = gpm_brightness_hal_up (brightness->priv->hal, &hw_changed_local);
+		goto out;
+	}
+	gpm_debug ("no hardware support");
+	return FALSE;
+out:
+	/* we did something to the hardware, so untrusted */
+	if (ret) {
+		brightness->priv->cache_trusted = FALSE;
+	}
+	/* is the caller interested? */
+	if (ret && hw_changed != NULL) {
+		*hw_changed = hw_changed_local;
+	}
 	return ret;
 }
 
 /**
  * gpm_brightness_down:
  * @brightness: This brightness class instance
+ * @hw_changed: If the hardware was changed, i.e. the brightness changed
+ * Return value: %TRUE if success, %FALSE if there was an error
  *
  * If possible, put the brightness of the LCD down one unit.
  **/
 gboolean
-gpm_brightness_down (GpmBrightness *brightness)
+gpm_brightness_down (GpmBrightness *brightness, gboolean *hw_changed)
 {
 	gboolean ret = FALSE;
+	gboolean hw_changed_local = FALSE;
+
 	g_return_val_if_fail (GPM_IS_BRIGHTNESS (brightness), FALSE);
+
 	if (brightness->priv->use_xrandr) {
-		ret = gpm_brightness_xrandr_down (brightness->priv->xrandr);
-		return ret;
+		ret = gpm_brightness_xrandr_down (brightness->priv->xrandr, &hw_changed_local);
+		goto out;
 	}
 	if (brightness->priv->use_hal) {
-		ret = gpm_brightness_hal_down (brightness->priv->hal);
-	}	
+		ret = gpm_brightness_hal_down (brightness->priv->hal, &hw_changed_local);
+		goto out;
+	}
+	gpm_debug ("no hardware support");
+	return FALSE;
+out:
+	/* we did something to the hardware, so untrusted */
+	if (ret) {
+		brightness->priv->cache_trusted = FALSE;
+	}
+	/* is the caller interested? */
+	if (ret && hw_changed != NULL) {
+		*hw_changed = hw_changed_local;
+	}
 	return ret;
 }
 
@@ -196,6 +316,22 @@
 static void
 gpm_brightness_changed_cb (gpointer caller, guint percentage, GpmBrightness *brightness)
 {
+	g_return_if_fail (GPM_IS_BRIGHTNESS (brightness));
+	brightness->priv->cache_trusted = TRUE;
+
+	/* valid? */
+	if (percentage > 100) {
+		gpm_warning ("percentage value of %i will be ignored", percentage);
+		/* no longer trust the cache */
+		brightness->priv->has_changed_events = FALSE;
+		brightness->priv->cache_trusted = FALSE;
+		return;
+	}
+
+	brightness->priv->has_changed_events = TRUE;
+	brightness->priv->cache_trusted = TRUE;
+	brightness->priv->cache_percentage = percentage;
+	/* ONLY EMIT THIS SIGNAL WHEN SOMETHING _ELSE_ HAS CHANGED THE BACKLIGHT */
 	gpm_debug ("emitting brightness-changed (%i)", percentage);
 	g_signal_emit (brightness, signals [BRIGHTNESS_CHANGED], 0, percentage);
 }
@@ -211,6 +347,9 @@
 
 	brightness->priv->use_xrandr = FALSE;
 	brightness->priv->use_hal = FALSE;
+	brightness->priv->cache_trusted = FALSE;
+	brightness->priv->has_changed_events = FALSE;
+	brightness->priv->cache_percentage = 0;
 
 	brightness->priv->xrandr = gpm_brightness_xrandr_new ();
 	if (gpm_brightness_xrandr_has_hw (brightness->priv->xrandr)) {

Modified: trunk/src/gpm-brightness.h
==============================================================================
--- trunk/src/gpm-brightness.h	(original)
+++ trunk/src/gpm-brightness.h	Thu Apr 10 14:20:59 2008
@@ -54,12 +54,15 @@
 GpmBrightness	*gpm_brightness_new		(void);
 
 gboolean	 gpm_brightness_has_hw		(GpmBrightness		*brightness);
-gboolean	 gpm_brightness_up		(GpmBrightness		*brightness);
-gboolean	 gpm_brightness_down		(GpmBrightness		*brightness);
+gboolean	 gpm_brightness_up		(GpmBrightness		*brightness,
+						 gboolean		*hw_changed);
+gboolean	 gpm_brightness_down		(GpmBrightness		*brightness,
+						 gboolean		*hw_changed);
 gboolean	 gpm_brightness_get		(GpmBrightness		*brightness,
-						 guint			*brightness_level);
+						 guint			*percentage);
 gboolean	 gpm_brightness_set		(GpmBrightness		*brightness,
-						 guint			 brightness_level);
+						 guint			 percentage,
+						 gboolean		*hw_changed);
 
 G_END_DECLS