ayatana-commits team mailing list archive

[Conor Curran <conor.curran@xxxxxxxxxxxxx>]

> Some questions
> 
> 1. I don't see where the new ido signals are connected and handled: it seems
> everything was removed or commented out. Is that managed in another part of
> the code?

line 265.266
+ g_signal_connect(volume_slider, "slider-grabbed", G_CALLBACK(slider_grabbed), NULL);
+ g_signal_connect(volume_slider, "slider-released", G_CALLBACK(slider_released), NULL); 

The signals are handles here:
534	+
535	+static void slider_grabbed (GtkWidget *widget, gpointer user_data)
536	+{
537	+ slider_in_direct_use = TRUE;
538	+ g_debug ("!!!!!! grabbed\n");
539	+}
540	+
541	+static void slider_released (GtkWidget *widget, gpointer user_data)
542	+{
543	+ slider_in_direct_use = FALSE;
544	+ g_debug ("!!!!!! released\n");
545	+}

 slider_in_direct_use is then used in catch_signal_sink_volume_update to determine whether the slider should listen to volume updates. If it is not 'in use' then it will listen otherwise ignore.

> 
> 2. if device_available == FALSE, shouldn't you set the state to some
> reasonable default anyway? so that if the device availability fluctuates, you
> do not rely on the last state when it was available?
I indicator-sound I assume there are devices 
122: static gboolean device_available = TRUE;
This is because 99% of the time there will be some device. Why not default it to cater for the norm ? 

> 3. What happens if the state changes while an animation happens? is the icon
> at the end of the animation going to reflect the new state, or the old one, or
> just the end of the animation?
The animation will end abruptly and the icon updated to its new state. The callback cancelled in the main loop. The idea being when in mute state only an unmute signal can change things. Therefore only in the catch unmute method do we need to control this.
line 535 540 indicator-sound.c
        if(animation_id != 0){
            g_debug("about to remove the animation_id callback from the mainloop!!**");
            g_source_remove(animation_id);
            animation_id = 0;
        }
    }

> 4. in catch_signal_sink_mute_update() in the if/else block it's not clear what
> happens: i'd suggest putting pre-conditions at the entry of the function with
> some g_exit_if_fail (device_available != TRUE) or something similar

I would think until the application is feature complete and tests are wrote can i begin a refactor which will make things more clear. Basically a unified state object shared over dbus between the indicator and the sound-service.c

> 5. in catch_signal_sink_availability_update(), shouldn't you call update_state
> whatever the new value is?
I assume there are always devices therefore all I am really interested in is where there is none. 

-- 
https://code.launchpad.net/~cjcurran/indicator-sound/slider_length_fixing/+merge/21132
Your team ayatana-commits is subscribed to branch lp:indicator-sound.