GNOME Bugzilla – Bug 760284
Add D-Bus API to ask which audio device was plugged in
Last modified: 2016-08-29 13:00:10 UTC
Created attachment 318433 [details] audio-device-selection To implement the feature in https://bugzilla.gnome.org/show_bug.cgi?id=755062 we'll need a system modal dialogue implemented in gnome-shell, which gnome-settings-daemon can use for those machines (laptops/desktops) which cannot detect which type of device got plugged into the jack. A mockup by aday is attached. gnome-settings-daemon needs to be able to: - display the dialogue - get the selection back from the dialogue - be able to dismiss the dialogue (if the jack is unplugged while the dialogue is shown) The dialogue can have 3 states: 1) show the 3 choices as in the mockup 2) same but without the headset choice (the machine supports either input or output but not both) 3) same but without the mic choice (the machine has a separate mic jack)
(In reply to Bastien Nocera from comment #0) > gnome-settings-daemon needs to be able to: > - display the dialogue > - get the selection back from the dialogue > - be able to dismiss the dialogue (if the jack is unplugged while the > dialogue is shown) I've implemented the following interface for this: <interface name="org.gnome.Shell.AudioDeviceSelection"> <method name="Open"> <arg name="devices" direction="in" type="as" /> <arg name="success" direction="out" type="b" /> </method> <method name="Close"> <arg name="success" direction="out" type="b" /> </method> <signal name="DeviceSelected"> <arg name="device" type="s" /> </signal> <signal name="Closed" /> </interface> In the above, devices are simply represented as strings ("headphones", "headset" and "microphone" and are used both for returning the selected device and to request the list of devices which should be displayed. Is this a reasonable API for gnome-settings-daemon?
Not sure the "out" arg for Close is necessary but looks fine.
(In reply to Bastien Nocera from comment #2) > Not sure the "out" arg for Close is necessary but looks fine. Close can fail if there is no dialog or the sender doesn't match the one that called Open, but I guess we can just fail silently - to be honest, I can't really think of a case where a caller would care ...
Created attachment 320882 [details] [review] audioDeviceSelection: Add audio device selection dialog It is not always possible to determine the type of audio device that got plugged in. Add a system modal dialog to query the user in that case and export in on the bus to gnome-settings-daemon.
Review of attachment 320882 [details] [review]: There's some extra space below the device buttons / above the dialog buttons On the g-s-d side (bug 755062) I don't find any use for the "success" return values nor for the "Closed" signal. Feel free to drop those Otherwise looks good. If you have a design ack on this, feel free to go ahead and push ::: js/ui/audioDeviceSelection.js @@ +53,3 @@ + throw new Error('Too few devices for a selection'); + + this.setInitialKeyFocus(this._selectionBox.get_first_child()); It's a bit odd to have an option looking pre-selected @@ +181,3 @@ + return AudioDevice[dev] == device; + })[0].toLowerCase(); + log(deviceName); leftover log() @@ +213,3 @@ + + this._audioSelectionDialog = dialog; + invocation.return_value(GLib.Variant.new('(b)', [true])); I'm seeing a JS warning: anonymous function does not always return a value - I don't think we need to return a value from DBus async handlers
Created attachment 321314 [details] [review] audioDeviceSelection: Add audio device selection dialog (In reply to Rui Matos from comment #5) > Review of attachment 320882 [details] [review] [review]: > > There's some extra space below the device buttons / above the dialog buttons The mockup in attachment 318433 [details] does have some extra spacing. I've toned it down a bit now, let's keep further tweaking to the designers. > On the g-s-d side (bug 755062) I don't find any use for the "success" return > values nor for the "Closed" signal. Feel free to drop those It feels a bit odd to not indicate that the user canceled the selection, but given that this is private API between the shell and g-s-d, there's little point in having unused stuff - removed. > + this.setInitialKeyFocus(this._selectionBox.get_first_child()); > > It's a bit odd to have an option looking pre-selected Mmmh, true. I don't quite like that we end up selecting "Cancel" by default, but I can see how setting the initial selection can be mistaken for a default/safe choice ... > + invocation.return_value(GLib.Variant.new('(b)', [true])); > > I don't think we need to return a value from DBus async handlers It works without return_value(), but the caller will get a timeout error. Or if you mean the "return invocation.return_value(...)" - that was an ugly way to avoid braces around the block. Split into two lines now.
Created attachment 321315 [details] [review] audioDeviceSelection: Add audio selection dialog The corresponding SASS patch.
(In reply to Florian Müllner from comment #6) > It feels a bit odd to not indicate that the user canceled the selection, Right, but in this case there's really nothing useful g-s-d can do. After all, we're asking the user because we don't know what to do automatically, so if the user cancels the dialog there's nothing more to do, we just leave the underlying configuration on its default state. If that doesn't work for users, I'm suppose they'll just try to re-plug the cable at which point we'll pop up the dialog again. > Mmmh, true. I don't quite like that we end up selecting "Cancel" by default, > but I can see how setting the initial selection can be mistaken for a > default/safe choice ... Yep.. we could have a hidden widget for the initial key focus which we could remove after any other widget gets key focus but I don't think that's worth the complexity for a dialog most people will never see. > > + invocation.return_value(GLib.Variant.new('(b)', [true])); > > > > I don't think we need to return a value from DBus async handlers > > It works without return_value(), but the caller will get a timeout error. Or > if you mean the "return invocation.return_value(...)" - that was an ugly way > to avoid braces around the block. Split into two lines now. I meant the latter. Thanks
Review of attachment 321314 [details] [review]: looks good
Comment on attachment 321315 [details] [review] audioDeviceSelection: Add audio selection dialog Attachment 321315 [details] pushed as 9fb3918 - audioDeviceSelection: Add audio selection dialog
Attachment 321314 [details] pushed as 30c7545 - audioDeviceSelection: Add audio device selection dialog
(In reply to Rui Matos from comment #8) > (In reply to Florian Müllner from comment #6) > > It feels a bit odd to not indicate that the user canceled the selection, > > Right, but in this case there's really nothing useful g-s-d can do. It would allow to only keep the proxy/signal connections around when needed. Not super-useful, I know :-) > Yep.. we could have a hidden widget for the initial key focus which we could > remove after any other widget gets key focus but I don't think that's worth > the complexity for a dialog most people will never see. Yeah, I agree that it's not worth it.
Is there any way a user can set the default value and disable the dialog? I plug headphones into my laptop almost every day. Every time I do it, I have to close this dialog (or selecting "headset", anything works fine). This takes a small effort every time, but after a while it became really annoying. So, is it possible to prevent this dialog from appearing?