[Sugar-devel] [PATCH v2 sugar] Wireless network Frame device: add suport for disabling wireless devices

[Gary Martin]

Hi Sascha,

On 30 Jun 2011, at 22:34, Sascha Silbe <silbe at activitycentral.com> wrote:

> This duplicates the existing functionality in the Control Panel for more
> convenient access. Disabling wifi devices can save power, thereby increasing
> battery life.
> 
> Signed-off-by: Sascha Silbe <silbe at activitycentral.com>
> ---
> v1->v2: Set primary palette text
> 
> I didn't change the option labels because none of the strings that were
> mentioned or that I could think of are significantly better. "Wireless
> radio" is redundant and "disable my wireless network" sounds like
> shutting down the entire network (i.e. the AP) rather than just
> powering down the client device. "Disable radio" is a bit too generic
> for my taste. Come to think of it, "Disable wireless network device"
> instead of the current "Disable wireless device" would be easier to
> understand, albeit rather long. If "WiFi" weren't a trademark, "Disable
> wifi radio" might be an option.

Is there a reason in your current strings for:

a) the disable label uses the string 'all' and the enable label doesn't i.e 'Disable all wireless devices' vs. 'Enable wireless devices'. For me 'Disable all wireless devices' sounds more like a admin label for something that shuts down the remote ap devices. That's why I used 'my'.

b) you use the plural 'devices', is this trying to cover users with multiple wifi devices attached to their hardware (e.g. built-in wifi and a USB wifi dongle)?

Given the above, how about:

'Disable my wireless device' vs. 'Enable my wireless device'

> There's also no interaction with the Ad Hoc networks feature yet. Read:
> The Ad Hoc icons will stay in the Neighbourhood, but not show an option
> to enable wifi. This happens even if you use the Control Panel to disable
> wifi, so we can tackle it independently. Personally I'd just make those
> icons disappear like the APs. That would be consistent with their role
> as (pre-defined) network settings rather than actual devices.

Assuming that NM is powering down your wifi device, hiding mesh/adhoc icons would seem to be the logical choice.

Regards,
--Gary

> Updated screenshots are in the wiki [1,2].
> 
> [1] http://wiki.sugarlabs.org/go/File:Screenshot_of_the_wireless_network_Frame_device_in_disabled_state.png
> [2] http://wiki.sugarlabs.org/go/File:Sugar-wifi-conn-disable-2.png
> 
> extensions/deviceicon/network.py |   62 +++++++++++++++++++++++++++++++++++++-
> 1 files changed, 61 insertions(+), 1 deletions(-)
> 
> diff --git a/extensions/deviceicon/network.py b/extensions/deviceicon/network.py
> index bdd2405..c7d6d9f 100644
> --- a/extensions/deviceicon/network.py
> +++ b/extensions/deviceicon/network.py
> @@ -72,12 +72,15 @@ class WirelessPalette(Palette):
>     __gsignals__ = {
>         'deactivate-connection': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
>                                   ([])),
> +        'disable-wifi': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, []),
> +        'enable-wifi': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, []),
>     }
> 
>     def __init__(self, primary_text):
>         Palette.__init__(self, label=primary_text)
> 
>         self._disconnect_item = None
> +        self._enabled = True
> 
>         self._channel_label = gtk.Label()
>         self._channel_label.props.xalign = 0.0
> @@ -109,6 +112,13 @@ class WirelessPalette(Palette):
>                                       self.__disconnect_activate_cb)
>         self.menu.append(self._disconnect_item)
> 
> +        label = glib.markup_escape_text(_('Disable all wireless devices'))
> +        self._powercontrol_item = MenuItem(label, 'system-shutdown')
> +        self._powercontrol_item.connect('activate',
> +                                        self.__powercontrol_activate_cb)
> +        self._powercontrol_item.show()
> +        self.menu.append(self._powercontrol_item)
> +
>     def set_connecting(self):
>         self.props.secondary_text = _('Connecting...')
> 
> @@ -127,15 +137,35 @@ class WirelessPalette(Palette):
>         self._set_channel(channel)
> 
>     def set_disconnected(self):
> -        label = glib.markup_escape_text(_('No wireless connection'))
> +        if self._enabled:
> +            label = glib.markup_escape_text(_('No wireless connection'))
> +        else:
> +            label = glib.markup_escape_text(_('Wireless device deactivated'))
>         self.props.primary_text = label
>         self.props.secondary_text = ''
>         self._disconnect_item.hide()
>         self.set_content(None)
> 
> +    def set_enabled(self, enabled):
> +        self._enabled = enabled
> +        if enabled:
> +            label = glib.markup_escape_text(_('Disable all wireless devices'))
> +        else:
> +            self.set_disconnected()
> +            label = glib.markup_escape_text(_('Enable wireless devices'))
> +
> +        # FIXME: what's the right way to do this?
> +        self._powercontrol_item.child.set_markup(label)
> +
>     def __disconnect_activate_cb(self, menuitem):
>         self.emit('deactivate-connection')
> 
> +    def __powercontrol_activate_cb(self, menuitem):
> +        if self._enabled:
> +            self.emit('disable-wifi')
> +        else:
> +            self.emit('enable-wifi')
> +
>     def _set_frequency(self, frequency):
>         channel = network.frequency_to_channel(frequency)
>         self._set_channel(channel)
> @@ -403,6 +433,8 @@ class WirelessDeviceView(ToolButton):
>         self._palette = WirelessPalette(self._name)
>         self._palette.connect('deactivate-connection',
>                               self.__deactivate_connection_cb)
> +        self._palette.connect('disable-wifi', self.__disable_wifi_cb)
> +        self._palette.connect('enable-wifi', self.__enable_wifi_cb)
>         self.set_palette(self._palette)
>         self._palette.set_group_id('frame')
> 
> @@ -421,6 +453,16 @@ class WirelessDeviceView(ToolButton):
>                                       path=self._device.object_path,
>                                       dbus_interface=_NM_DEVICE_IFACE)
> 
> +        obj = self._bus.get_object(_NM_SERVICE, _NM_PATH)
> +        network_manager = dbus.Interface(obj, _NM_IFACE)
> +        self._nm_props = dbus.Interface(network_manager, dbus.PROPERTIES_IFACE)
> +        self._nm_props.connect_to_signal('PropertiesChanged',
> +                                         self.__nm_props_changed_cb,
> +                                         dbus_interface=_NM_IFACE)
> +        self._nm_props.Get(_NM_IFACE, 'XWirelessEnabled',
> +                           reply_handler=self.__nm_wifi_enabled_reply_cb,
> +                           error_handler=self.__nm_wifi_enabled_error_cb)
> +
>     def disconnect(self):
>         self._bus.remove_signal_receiver(self.__state_changed_cb,
>                                          signal_name='StateChanged',
> @@ -579,6 +621,24 @@ class WirelessDeviceView(ToolButton):
>     def __activate_error_cb(self, err):
>         logging.debug('Failed to create network: %s', err)
> 
> +    def __nm_wifi_enabled_reply_cb(self, enabled):
> +        self._palette.set_enabled(enabled)
> +
> +    def __nm_wifi_enabled_error_cb(self, error):
> +        logging.error('Could not determine whether wifi is enabled: %r', error)
> +
> +    def __nm_props_changed_cb(self, changed):
> +        if 'WirelessEnabled' not in changed:
> +            return
> +
> +        self._palette.set_enabled(changed['WirelessEnabled'])
> +
> +    def __disable_wifi_cb(self, palette_):
> +        self._nm_props.Set(_NM_IFACE, 'WirelessEnabled', False)
> +
> +    def __enable_wifi_cb(self, palette_):
> +        self._nm_props.Set(_NM_IFACE, 'WirelessEnabled', True)
> +
> 
> class OlpcMeshDeviceView(ToolButton):
>     _ICON_NAME = 'network-mesh'
> --
> 1.7.2.5
>