Merge lp:~cjcurran/indicator-sound/keyboard-ui-controls into lp:indicator-sound/0.1

[Conor Curran]

[Conor Curran (cjcurran)]

In this branch:

Keyboard controls of the sound menu.
 - +/- 5% inc/dec
 - Arrow left/right 5% inc/dec
 - Ctlr left arrow - volume 0
 - Ctlr right arrow - Max vol

New slider.

volume control far smoother - proper use of the API for the current UI spec.

New icons now being pulled in.

[David Barth (dbarth)]

LGTM, improvements and simplifications. Caution with the GtkRange cast.

+1

review: Approve

=== modified file 'src/indicator-sound.c'
--- src/indicator-sound.c	2010-02-15 15:23:54 +0000
+++ src/indicator-sound.c	2010-02-16 18:00:37 +0000
@@ -20,10 +20,11 @@
 You should have received a copy of the GNU General Public License along 
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
-
+#include <math.h>
 #include <glib.h>
 #include <glib-object.h>
 #include <gtk/gtk.h>
+#include <gdk/gdkkeysyms.h>
 #include <libdbusmenu-gtk/menu.h>
 #include <libido/idoscalemenuitem.h>
 
@@ -83,6 +84,7 @@
 static void slider_prop_change_cb (DbusmenuMenuitem * mi, gchar * prop, GValue * value, GtkWidget *widget);
 static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer  user_data);
 static gboolean value_changed_event_cb(GtkRange *range, gpointer user_data);
+static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data);
 
 // DBUS communication
 static DBusGProxy *sound_dbus_proxy = NULL;
@@ -162,14 +164,13 @@
 {
     // TODO we need three more images
     volume_states = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_free);
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED), g_strdup("audio-volume-muted"));
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_ZERO), g_strdup(/*"audio-volume-zero"*/"audio-volume-muted"));
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_LOW), g_strdup("audio-volume-low"));
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MEDIUM), g_strdup("audio-volume-medium"));
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_HIGH), g_strdup("audio-volume-high"));
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED_WHILE_INPUT), g_strdup(/*"audio-volume-muted-blocking"*/"audio-volume-muted"));
-    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_SINKS_NONE), g_strdup(/*"audio-output-none"*/"audio-volume-muted"));
-    //test_images_hash();
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED), g_strdup("audio-volume-muted-panel"));
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_ZERO), g_strdup("audio-volume-zero-panel"));
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_LOW), g_strdup("audio-volume-low-panel"));
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MEDIUM), g_strdup("audio-volume-medium-panel"));
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_HIGH), g_strdup("audio-volume-high-panel"));
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED_WHILE_INPUT), g_strdup("audio-volume-muted-blocking-panel"));
+    g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_SINKS_NONE), g_strdup("audio-output-none-panel"));
 }
 
 static void
@@ -257,10 +258,20 @@
 
 static void catch_signal_sink_volume_update(DBusGProxy *proxy, gdouble volume_percent, gpointer userdata)
 {
-    g_debug("signal caught - update sink volume with value : %f", volume_percent);
     GtkWidget *slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);
     GtkRange *range = (GtkRange*)slider;   
-    gtk_range_set_value(range, volume_percent); 
+    
+    // DEBUG
+    gdouble current_value = gtk_range_get_value(range);
+    g_debug("SIGNAL- update sink volume - current_value : %f and new value : %f", current_value, volume_percent);
+
+    // Don't like this solution - too fuzzy
+    // Need the ability to detect if the slider is grabbed
+    if(floor(current_value) != floor(volume_percent))
+    {
+        g_debug("Going to update slider value");
+        gtk_range_set_value(range, volume_percent); 
+    }
     determine_state_from_volume(volume_percent);
 }
 
@@ -315,11 +326,11 @@
 
 static void update_state(const gint state)
 {
-    g_debug("update state beginning - previous_state = %i", previous_state);
+/*    g_debug("update state beginning - previous_state = %i", previous_state);*/
 
     previous_state = current_state;
 
-    g_debug("update state 3rd line - previous_state = %i", previous_state);
+/*    g_debug("update state 3rd line - previous_state = %i", previous_state);*/
 
     current_state = state;
     gchar* image_name = g_hash_table_lookup(volume_states, GINT_TO_POINTER(current_state));
@@ -338,7 +349,7 @@
 
 static void determine_state_from_volume(gdouble volume_percent)
 {
-    g_debug("determine_state_from_volume - previous_state = %i", previous_state);
+/*    g_debug("determine_state_from_volume - previous_state = %i", previous_state);*/
     gint state = previous_state;
     if (volume_percent < 30.0 && volume_percent > 0){
         state = STATE_LOW;
@@ -366,6 +377,9 @@
 	DbusmenuGtkClient *client = dbusmenu_gtkmenu_get_client(menu);	
     dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_SLIDER_MENUITEM_TYPE, new_slider_item);
 
+    // register Key-press listening on the widget
+    g_signal_connect(menu, "key-press-event", G_CALLBACK(key_press_cb), NULL);         
+
     return GTK_MENU(menu);
 }
 
@@ -378,22 +392,23 @@
 	g_object_set(volume_slider, "reverse-scroll-events", TRUE, NULL);
 
     GtkMenuItem *menu_volume_slider = GTK_MENU_ITEM(volume_slider);
+
 	dbusmenu_gtkclient_newitem_base(DBUSMENU_GTKCLIENT(client), newitem, menu_volume_slider, parent);
 	g_signal_connect(G_OBJECT(newitem), DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, G_CALLBACK(slider_prop_change_cb), volume_slider);
     
+    // register slider changes listening on the range
     GtkWidget* slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);  
     g_signal_connect(slider, "change-value", G_CALLBACK(user_change_value_event_cb), newitem);     
     g_signal_connect(slider, "value-changed", G_CALLBACK(value_changed_event_cb), newitem);     
     
+    // Set images on the ido
     primary_image = ido_scale_menu_item_get_primary_image((IdoScaleMenuItem*)volume_slider);    
     gtk_image_set_from_icon_name(GTK_IMAGE(primary_image), g_hash_table_lookup(volume_states, GINT_TO_POINTER(STATE_ZERO)), GTK_ICON_SIZE_MENU);
     GtkWidget* secondary_image = ido_scale_menu_item_get_secondary_image((IdoScaleMenuItem*)volume_slider);                 
     gtk_image_set_from_icon_name(GTK_IMAGE(secondary_image), g_hash_table_lookup(volume_states, GINT_TO_POINTER(STATE_HIGH)), GTK_ICON_SIZE_MENU);
 
-/*    GtkRange* range = (GtkRange*)slider;   */
-/*    gtk_range_set_value(range, initial_volume_percent);  */
-
     gtk_widget_show_all(volume_slider);
+
 	return TRUE;
 }
 
@@ -417,18 +432,86 @@
 static gboolean value_changed_event_cb(GtkRange *range, gpointer user_data)
 {
     gdouble current_value = gtk_range_get_value(range);        
-    if(current_value == 0 || current_value == 100)
-    {
-        DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
-        GValue value = {0};
-        g_value_init(&value, G_TYPE_DOUBLE);
-        g_value_set_double(&value, current_value);
-        g_debug("Value changed listener - = %f", current_value);
-        dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);        
-    }
-    return FALSE;
-}
-
+/*    if(current_value == 0 || current_value == 100)*/
+/*    {*/
+    DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
+    GValue value = {0};
+    g_value_init(&value, G_TYPE_DOUBLE);
+    g_value_set_double(&value, current_value);
+    g_debug("Value changed callback - = %f", current_value);
+    dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);        
+/*    }*/
+    return FALSE;
+}
+
+/**
+key_press_cb:
+**/
+static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data)
+{
+
+    if(event->length > 0)
+        g_debug("The key event's string is '%s'\n", event->string);
+
+    GtkWidget* slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);
+    GtkRange* range = (GtkRange*)slider;       
+    gdouble current_value = gtk_range_get_value(range);  
+    gdouble new_value = current_value;
+    const gdouble five_percent = 5;
+
+    switch(event->keyval)
+        {
+        case GDK_Right:
+            if(event->state & GDK_CONTROL_MASK)
+            {
+/*                g_debug("right key was pressed with ctrl- volume set to 100");                 */
+                new_value = 100;
+            }
+            else
+            {
+/*                g_debug("right key was pressed - normal 5 percent increase");                */
+                new_value = current_value + five_percent;
+            }
+            break;
+        case GDK_Left:
+            if(event->state & GDK_CONTROL_MASK)
+            {
+/*                g_debug("left key was pressed with ctrl- volume set to 0");                */
+                new_value = 0;
+            }
+            else
+            {
+/*                g_debug("left key was pressed - normal 5 percent decrease");                */
+                new_value = current_value - five_percent;                
+            }
+            break;
+        case GDK_plus:
+/*                g_debug("Plus key was pressed");*/
+                new_value = current_value + five_percent;                
+            break;
+        case GDK_minus:
+/*            g_debug("minus key was pressed");*/
+                new_value = current_value - five_percent;                
+            break;
+        default:
+            break;
+        }    
+        
+/*        g_debug("new range value without being clamped  = %f", new_value);            */
+
+        new_value = CLAMP(new_value, 0, 100);
+        if(new_value != current_value)
+        {
+            g_debug("Attempting to set the range to %f", new_value);        
+            gtk_range_set_value(range, new_value);  
+        }
+    return FALSE;
+}
+
+/**
+This callback should only be called when the user actually drags the slider.
+Turned off for now in favour of the non descriminating call back.
+**/
 static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer  user_data)
 {
     DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;

=== modified file 'src/pulse-manager.c'
--- src/pulse-manager.c	2010-02-15 12:42:08 +0000
+++ src/pulse-manager.c	2010-02-16 18:00:37 +0000
@@ -189,18 +189,15 @@
         g_warning("We have no default sink !!! - returning after not attempting to set any volume of any sink");
         return;
     }
-    gdouble linear_input = (gdouble)(percent);
-    linear_input /= 100.0;
-    g_debug("linear double input = %f", linear_input);
-    pa_volume_t new_volume = pa_sw_volume_from_linear(linear_input); 
-    // Use this to achieve more accurate scaling using the base volume (in the sink struct already!)
-    //pa_volume_t new_volume = (pa_volume_t) ((GPOINTER_TO_INT(linear_input) * s->base_volume) / 100);
-    g_debug("about to try to set the sw volume to a linear volume of %f", pa_sw_volume_to_linear(new_volume));
-    g_debug("and an actual volume of %f", (gdouble)new_volume);
+
+    sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(DEFAULT_SINK_INDEX));   
+
+    pa_volume_t new_volume = (pa_volume_t) ((percent * PA_VOLUME_NORM) / 100);
+    g_debug("new_volume double check :%f", pa_sw_volume_to_linear(new_volume));
+    g_debug("new volume calculated :%f", (gdouble)new_volume);
     pa_cvolume dev_vol;
-    sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(DEFAULT_SINK_INDEX));   
     pa_cvolume_set(&dev_vol, s->volume.channels, new_volume);   
-    
+    // TODO why don't you update the sink_info here with the appropriate pa_cvolume (&dev_vol)
     pa_operation_unref(pa_context_set_sink_volume_by_index(pulse_context, DEFAULT_SINK_INDEX, &dev_vol, NULL, NULL));
 }
 
@@ -321,7 +318,6 @@
     if(position >= 0) // => index is within the keys of the hash.
     {
         sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(info->index));
-        //g_debug("attempting to update sink with name %s", s->name);
         s->name = g_strdup(info->name);
         s->description = g_strdup(info->description);
         s->icon_name = g_strdup(pa_proplist_gets(info->proplist, PA_PROP_DEVICE_ICON_NAME));
@@ -336,14 +332,11 @@
         {
             //update the UI
             pa_volume_t vol = pa_cvolume_avg(&s->volume);
-            // Use the base of the device to ensure maximum acceptable levels on the hardware
-            gdouble volume_percent = (vol/s->base_volume) * 100;
-            g_debug("When using base volume => volume = %f", volume_percent);
-            g_debug("about to update ui with linear volume of %f", pa_sw_volume_to_linear(vol));            
-            sound_service_dbus_update_sink_volume(dbus_service, pa_sw_volume_to_linear(vol)); 
+            gdouble volume_percent = ((gdouble) vol * 100) / PA_VOLUME_NORM;
+            g_debug("Updating volume from PA manager with volume = %f", volume_percent);
+            sound_service_dbus_update_sink_volume(dbus_service, volume_percent); 
             if (mute_changed == TRUE)     
                 sound_service_dbus_update_sink_mute(dbus_service, s->mute);
-            
             update_mute_ui(s->mute);
         }
     }

=== modified file 'src/sound-service-dbus.c'
--- src/sound-service-dbus.c	2010-02-10 12:45:23 +0000
+++ src/sound-service-dbus.c	2010-02-16 18:00:37 +0000
@@ -194,7 +194,7 @@
 void sound_service_dbus_update_sink_volume(SoundServiceDbus* obj, gdouble sink_volume)
 {
     SoundServiceDbusPrivate *priv = SOUND_SERVICE_DBUS_GET_PRIVATE (obj);
-    priv->volume_percent = sink_volume * 100;            
+    priv->volume_percent = sink_volume;            
 
     g_debug("Emitting signal: SINK_VOLUME_UPDATE, with sink_volme %f", priv->volume_percent);
     g_signal_emit(obj,

=== modified file 'src/sound-service.c'
--- src/sound-service.c	2010-02-10 18:12:23 +0000
+++ src/sound-service.c	2010-02-16 18:00:37 +0000
@@ -124,8 +124,8 @@
 	if (mainloop != NULL) {
 		g_debug("Service shutdown !");
         // TODO: uncomment for release !!
-        close_pulse_activites();
-        g_main_loop_quit(mainloop);
+/*        close_pulse_activites();*/
+/*        g_main_loop_quit(mainloop);*/
 	}
 	return;
 }