[Sugar-devel] [PATCH v2] Simple NetworkManager-0.9 port
Simon Schampijer
simon at schampijer.de
Mon Nov 14 07:38:14 EST 2011
Thanks Daniel for the new patch. I am splitting this up in smaller chunks:
El 11/11/11 18:17, Daniel Drake escribió:
> Adapt sugar to NetworkManager-0.9 API changes.
>
> The major change here is the removal of the user-level connections
> mechanism; instead of storing connection information in Sugar,
> NetworkManager now Manages our Networks for us.
>
> However, some level of interfacing/monitoring NM is now needed,
> implemented with the Connections and Connection classes in
> jarabe.model.network.
>
> If found, connections in sugar's connections.cfg are automatically
> migrated, and then connections.cfg is deleted. Similarly, if modem
> connection details are found in gconf, they are migrated into NM
> and then the gconf keys are unset.
>
> The existing network code is far from perfect and actually quite messy.
> In this port I've tried not to make fundamental changes to improve this,
> in order to keep task complexity down and aid review.
> In the medium term I do plan to improve this code, by moving it to
> use gobject-introspection on libnm, and rewriting/restructuring at the
> same time. By letting libnm do most of the work for us, this layer can
> be greatly simplified. However, libnm and gobject-introspection
> improvements are needed first, which I will continue to work on.
>
> Modem PUK configuration has been removed as NetworkManager no longer
> has configuration for this. It hasn't been used (and was marked
> deprecated) throughout the NM-0.8 release series.
> ---
> data/sugar.schemas.in | 24 +-
> extensions/cpsection/modemconfiguration/model.py | 106 ++--
> extensions/cpsection/modemconfiguration/view.py | 207 ++-----
> extensions/cpsection/network/model.py | 14 +-
> extensions/deviceicon/network.py | 179 +++---
> src/jarabe/desktop/keydialog.py | 32 +-
> src/jarabe/desktop/meshbox.py | 60 +-
> src/jarabe/desktop/networkviews.py | 169 ++---
> src/jarabe/model/adhoc.py | 115 ++--
> src/jarabe/model/network.py | 754 +++++++++++-----------
> src/jarabe/model/olpcmesh.py | 159 +++--
> 11 files changed, 844 insertions(+), 975 deletions(-)
>
> v2: handle review comments from Simon and Sascha's Oct 24 mail. Only non-minor
> resulting change is a refactoring of modem setting handling so that undo
> is supported again.
>
> diff --git a/data/sugar.schemas.in b/data/sugar.schemas.in
> index 8b3e1ad..aaef381 100644
> --- a/data/sugar.schemas.in
> +++ b/data/sugar.schemas.in
> @@ -280,8 +280,8 @@
> <type>string</type>
> <default></default>
> <locale name="C">
> -<short>GSM network username</short>
> -<long>GSM network username configuration</long>
> +<short>GSM network username (DEPRECATED/UNUSED)</short>
> +<long>GSM network username configuration (DEPRECATED/UNUSED)</long>
> </locale>
> </schema>
> <schema>
> @@ -291,8 +291,8 @@
> <type>string</type>
> <default></default>
> <locale name="C">
> -<short>GSM network password</short>
> -<long>GSM network password configuration</long>
> +<short>GSM network password (DEPRECATED/UNUSED)</short>
> +<long>GSM network password configuration (DEPRECATED/UNUSED)</long>
> </locale>
> </schema>
> <schema>
> @@ -302,8 +302,8 @@
> <type>string</type>
> <default>*99#</default>
> <locale name="C">
> -<short>GSM network number</short>
> -<long>GSM network telephone number configuration</long>
> +<short>GSM network number (DEPRECATED/UNUSED)</short>
> +<long>GSM network telephone number configuration (DEPRECATED/UNUSED)</long>
> </locale>
> </schema>
> <schema>
> @@ -313,8 +313,8 @@
> <type>string</type>
> <default></default>
> <locale name="C">
> -<short>GSM network APN</short>
> -<long>GSM network access point name configuration</long>
> +<short>GSM network APN (DEPRECATED/UNUSED)</short>
> +<long>GSM network access point name configuration (DEPRECATED/UNUSED)</long>
> </locale>
> </schema>
> <schema>
> @@ -324,8 +324,8 @@
> <type>string</type>
> <default></default>
> <locale name="C">
> -<short>GSM network PIN</short>
> -<long>GSM network personal identification number configuration</long>
> +<short>GSM network PIN (DEPRECATED/UNUSED)</short>
> +<long>GSM network personal identification number configuration (DEPRECATED/UNUSED)</long>
> </locale>
> </schema>
> <schema>
> @@ -335,8 +335,8 @@
> <type>string</type>
> <default></default>
> <locale name="C">
> -<short>GSM network PUK</short>
> -<long>GSM network personal unlock key configuration</long>
> +<short>GSM network PUK (DEPRECATED/UNUSED)</short>
> +<long>GSM network personal unlock key configuration (DEPRECATED/UNUSED)</long>
> </locale>
> </schema>
>
> diff --git a/extensions/cpsection/modemconfiguration/model.py b/extensions/cpsection/modemconfiguration/model.py
> index 1e83c44..8b8ef05 100755
> --- a/extensions/cpsection/modemconfiguration/model.py
> +++ b/extensions/cpsection/modemconfiguration/model.py
> @@ -14,68 +14,88 @@
> # along with this program; if not, write to the Free Software
> # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 US
>
> -import gconf
> +import logging
>
> -from jarabe.model.network import GSM_USERNAME_PATH, GSM_PASSWORD_PATH, \
> - GSM_NUMBER_PATH, GSM_APN_PATH, GSM_PIN_PATH, \
> - GSM_PUK_PATH
> +import dbus
> +import gtk
>
> +from jarabe.model import network
>
> -def get_username():
> - client = gconf.client_get_default()
> - return client.get_string(GSM_USERNAME_PATH) or ''
>
> +def get_connection():
> + return network.find_gsm_connection()
Can we make this private? The 'get_*' 'set_*' are listed in the command
line tool.
> -def get_password():
> - client = gconf.client_get_default()
> - return client.get_string(GSM_PASSWORD_PATH) or ''
>
> +def get_modem_settings():
> + modem_settings = {}
> + connection = get_connection()
> + if not connection:
> + return modem_settings
>
> -def get_number():
> - client = gconf.client_get_default()
> - return client.get_string(GSM_NUMBER_PATH) or ''
> + settings = connection.get_settings('gsm')
> + for setting in ('username', 'number', 'apn'):
> + modem_settings[setting] = settings.get(setting, '')
>
> + # use mutable container for nested function control variable
> + secrets_call_done = [False]
>
> -def get_apn():
> - client = gconf.client_get_default()
> - return client.get_string(GSM_APN_PATH) or ''
> + def _secrets_cb(secrets):
> + secrets_call_done[0] = True
> + if not secrets or not 'gsm' in secrets:
> + return
>
> + gsm_secrets = secrets['gsm']
> + modem_settings['password'] = gsm_secrets.get('password', '')
> + modem_settings['pin'] = gsm_secrets.get('pin', '')
>
> -def get_pin():
> - client = gconf.client_get_default()
> - return client.get_string(GSM_PIN_PATH) or ''
> + def _secrets_err_cb(err):
> + secrets_call_done[0] = True
> + if isinstance(err, dbus.exceptions.DBusException) and \
> + err.get_dbus_name() == network.NM_AGENT_MANAGER_ERR_NO_SECRETS:
> + logging.debug('No GSM secrets present')
> + else:
> + logging.error('Error retrieving GSM secrets: %s', err)
>
> + # must be called asynchronously as this re-enters the GTK main loop
> + connection.get_secrets('gsm', _secrets_cb, _secrets_err_cb)
>
> -def get_puk():
> - client = gconf.client_get_default()
> - return client.get_string(GSM_PUK_PATH) or ''
> + # wait til asynchronous execution completes
> + while not secrets_call_done[0]:
> + gtk.main_iteration()
>
> + return modem_settings
>
> -def set_username(username):
> - client = gconf.client_get_default()
> - client.set_string(GSM_USERNAME_PATH, username)
>
> +def _set_or_clear(_dict, key, value):
> + """Update a dictionary value for a specific key. If value is None or
> + zero-length, but the key is present in the dictionary, delete that
> + dictionary entry."""
> + if value:
> + _dict[key] = value
> + return
>
> -def set_password(password):
> - client = gconf.client_get_default()
> - client.set_string(GSM_PASSWORD_PATH, password)
> + if key in _dict:
> + del _dict[key]
>
>
> -def set_number(number):
> - client = gconf.client_get_default()
> - client.set_string(GSM_NUMBER_PATH, number)
> +def set_modem_settings(modem_settings):
> + username = modem_settings.get('username', '')
> + password = modem_settings.get('password', '')
> + number = modem_settings.get('number', '')
> + apn = modem_settings.get('apn', '')
> + pin = modem_settings.get('pin', '')
>
> + connection = get_connection()
> + if not connection:
> + network.create_gsm_connection(username, password, number, apn, pin)
> + return
>
> -def set_apn(apn):
> - client = gconf.client_get_default()
> - client.set_string(GSM_APN_PATH, apn)
> + settings = connection.get_settings()
> + gsm_settings = settings['gsm']
> + _set_or_clear(gsm_settings, 'username', username)
> + _set_or_clear(gsm_settings, 'password', password)
> + _set_or_clear(gsm_settings, 'number', number)
> + _set_or_clear(gsm_settings, 'apn', apn)
> + _set_or_clear(gsm_settings, 'pin', pin)
> + connection.update_settings(settings)
>
> -
> -def set_pin(pin):
> - client = gconf.client_get_default()
> - client.set_string(GSM_PIN_PATH, pin)
> -
> -
> -def set_puk(puk):
> - client = gconf.client_get_default()
> - client.set_string(GSM_PUK_PATH, puk)
> diff --git a/extensions/cpsection/modemconfiguration/view.py b/extensions/cpsection/modemconfiguration/view.py
> index c31edba..78577c2 100644
> --- a/extensions/cpsection/modemconfiguration/view.py
> +++ b/extensions/cpsection/modemconfiguration/view.py
> @@ -14,9 +14,8 @@
> # along with this program; if not, write to the Free Software
> # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 US
>
> -import os
> -import logging
> from gettext import gettext as _
> +import logging
>
> import gtk
> import gobject
> @@ -35,10 +34,6 @@ class EntryWithLabel(gtk.HBox):
> def __init__(self, label_text):
> gtk.HBox.__init__(self, spacing=style.DEFAULT_SPACING)
>
> - self._timeout_sid = 0
> - self._changed_handler = None
> - self._is_valid = True
> -
> self.label = gtk.Label(label_text)
> self.label.modify_fg(gtk.STATE_NORMAL,
> style.COLOR_SELECTION_GREY.get_gdk_color())
> @@ -47,119 +42,13 @@ class EntryWithLabel(gtk.HBox):
> self.label.show()
>
> self._entry = gtk.Entry(25)
> - self._entry.connect('changed', self.__entry_changed_cb)
> self._entry.set_width_chars(25)
> self.pack_start(self._entry, expand=False)
> self._entry.show()
>
> - def __entry_changed_cb(self, widget, data=None):
> - if self._timeout_sid:
> - gobject.source_remove(self._timeout_sid)
> - self._timeout_sid = gobject.timeout_add(APPLY_TIMEOUT,
> - self.__timeout_cb)
> -
> - def __timeout_cb(self):
> - self._timeout_sid = 0
> -
> - if self._entry.get_text() == self.get_value():
> - return False
> -
> - try:
> - self.set_value(self._entry.get_text())
> - except ValueError:
> - self._is_valid = False
> - else:
> - self._is_valid = True
> -
> - self.notify('is-valid')
> -
> - return False
> -
> - def set_text_from_model(self):
> - self._entry.set_text(self.get_value())
> -
> - def get_value(self):
> - raise NotImplementedError
> -
> - def set_value(self):
> - raise NotImplementedError
> -
> - def _get_is_valid(self):
> - return self._is_valid
> - is_valid = gobject.property(type=bool, getter=_get_is_valid, default=True)
> -
> -
> -class UsernameEntry(EntryWithLabel):
> - def __init__(self, model):
> - EntryWithLabel.__init__(self, _('Username:'))
> - self._model = model
> -
> - def get_value(self):
> - return self._model.get_username()
> -
> - def set_value(self, username):
> - self._model.set_username(username)
> -
> -
> -class PasswordEntry(EntryWithLabel):
> - def __init__(self, model):
> - EntryWithLabel.__init__(self, _('Password:'))
> - self._model = model
> -
> - def get_value(self):
> - return self._model.get_password()
> -
> - def set_value(self, password):
> - self._model.set_password(password)
> -
> -
> -class NumberEntry(EntryWithLabel):
> - def __init__(self, model):
> - EntryWithLabel.__init__(self, _('Number:'))
> - self._model = model
> -
> - def get_value(self):
> - return self._model.get_number()
> -
> - def set_value(self, number):
> - self._model.set_number(number)
> -
> -
> -class ApnEntry(EntryWithLabel):
> - def __init__(self, model):
> - EntryWithLabel.__init__(self, _('Access Point Name (APN):'))
> - self._model = model
> -
> - def get_value(self):
> - return self._model.get_apn()
> -
> - def set_value(self, apn):
> - self._model.set_apn(apn)
> -
> -
> -class PinEntry(EntryWithLabel):
> - def __init__(self, model):
> - EntryWithLabel.__init__(self, _('Personal Identity Number (PIN):'))
> - self._model = model
> -
> - def get_value(self):
> - return self._model.get_pin()
> -
> - def set_value(self, pin):
> - self._model.set_pin(pin)
> -
> -
> -class PukEntry(EntryWithLabel):
> - def __init__(self, model):
> - EntryWithLabel.__init__(self, _('Personal Unblocking Key (PUK):'))
> - self._model = model
> -
> - def get_value(self):
> - return self._model.get_puk()
> -
> - def set_value(self, puk):
> - self._model.set_puk(puk)
> -
> + @gobject.property
> + def entry(self):
> + return self._entry
In all the rest of the code we use this style of gobject properties. For
consistency would be nice to use the same style.
def get_entry(self):
return self._entry
entry = gobject.property(type=object, getter=get_entry)
>
> class ModemConfiguration(SectionView):
> def __init__(self, model, alerts=None):
> @@ -167,6 +56,7 @@ class ModemConfiguration(SectionView):
>
> self._model = model
> self.restart_alerts = alerts
> + self._timeout_sid = 0
>
> self.set_border_width(style.DEFAULT_SPACING)
> self.set_spacing(style.DEFAULT_SPACING)
> @@ -182,75 +72,70 @@ class ModemConfiguration(SectionView):
> self.pack_start(self._text, False)
> self._text.show()
>
> - self._username_entry = UsernameEntry(model)
> - self._username_entry.connect('notify::is-valid',
> - self.__notify_is_valid_cb)
> + self._username_entry = EntryWithLabel(_('Username:'))
> + self._username_entry.entry.connect('changed', self.__entry_changed_cb)
> self._group.add_widget(self._username_entry.label)
> self.pack_start(self._username_entry, expand=False)
> self._username_entry.show()
>
> - self._password_entry = PasswordEntry(model)
> - self._password_entry.connect('notify::is-valid',
> - self.__notify_is_valid_cb)
> + self._password_entry = EntryWithLabel(_('Password:'))
> + self._password_entry.entry.connect('changed', self.__entry_changed_cb)
> self._group.add_widget(self._password_entry.label)
> self.pack_start(self._password_entry, expand=False)
> self._password_entry.show()
>
> - self._number_entry = NumberEntry(model)
> - self._number_entry.connect('notify::is-valid',
> - self.__notify_is_valid_cb)
> + self._number_entry = EntryWithLabel(_('Number:'))
> + self._number_entry.entry.connect('changed', self.__entry_changed_cb)
> self._group.add_widget(self._number_entry.label)
> self.pack_start(self._number_entry, expand=False)
> self._number_entry.show()
>
> - self._apn_entry = ApnEntry(model)
> - self._apn_entry.connect('notify::is-valid',
> - self.__notify_is_valid_cb)
> + self._apn_entry = EntryWithLabel(_('Access Point Name (APN):'))
> + self._apn_entry.entry.connect('changed', self.__entry_changed_cb)
> self._group.add_widget(self._apn_entry.label)
> self.pack_start(self._apn_entry, expand=False)
> self._apn_entry.show()
>
> - self._pin_entry = PinEntry(model)
> - self._pin_entry.connect('notify::is-valid',
> - self.__notify_is_valid_cb)
> + self._pin_entry = EntryWithLabel(_('Personal Identity Number (PIN):'))
> + self._pin_entry.entry.connect('changed', self.__entry_changed_cb)
> self._group.add_widget(self._pin_entry.label)
> self.pack_start(self._pin_entry, expand=False)
> self._pin_entry.show()
>
> - self._puk_entry = PukEntry(model)
> - self._puk_entry.connect('notify::is-valid',
> - self.__notify_is_valid_cb)
> - self._group.add_widget(self._puk_entry.label)
> - self.pack_start(self._puk_entry, expand=False)
> - self._puk_entry.show()
> -
> self.setup()
>
> - def setup(self):
> - self._username_entry.set_text_from_model()
> - self._password_entry.set_text_from_model()
> - self._number_entry.set_text_from_model()
> - self._apn_entry.set_text_from_model()
> - self._pin_entry.set_text_from_model()
> - self._puk_entry.set_text_from_model()
> -
> - self.needs_restart = False
> -
> def undo(self):
> self._model.undo()
>
> - def _validate(self):
> - if self._username_entry.is_valid and \
> - self._password_entry.is_valid and \
> - self._number_entry.is_valid and \
> - self._apn_entry.is_valid and \
> - self._pin_entry.is_valid and \
> - self._puk_entry.is_valid:
> - self.props.is_valid = True
> - else:
> - self.props.is_valid = False
> + def _populate_entry(self, entrywithlabel, text):
> + """Populate an entry with text, without triggering its 'changed'
> + handler."""
> + entry = entrywithlabel.entry
> + entry.handler_block_by_func(self.__entry_changed_cb)
> + entry.set_text(text)
> + entry.handler_unblock_by_func(self.__entry_changed_cb)
> +
> + def setup(self):
> + settings = self._model.get_modem_settings()
> + self._populate_entry(self._username_entry, settings.get('username', ''))
> + self._populate_entry(self._number_entry, settings.get('number', ''))
> + self._populate_entry(self._apn_entry, settings.get('apn', ''))
> + self._populate_entry(self._password_entry, settings.get('password', ''))
> + self._populate_entry(self._pin_entry, settings.get('pin', ''))
> +
> + def __entry_changed_cb(self, widget, data=None):
> + if self._timeout_sid:
> + gobject.source_remove(self._timeout_sid)
> + self._timeout_sid = gobject.timeout_add(APPLY_TIMEOUT,
> + self.__timeout_cb)
> +
> + def __timeout_cb(self):
> + self._timeout_sid = 0
> + settings = {}
> + settings['username'] = self._username_entry.entry.get_text()
> + settings['password'] = self._password_entry.entry.get_text()
> + settings['number'] = self._number_entry.entry.get_text()
> + settings['apn'] = self._apn_entry.entry.get_text()
> + settings['pin'] = self._pin_entry.entry.get_text()
> + self._model.set_modem_settings(settings)
>
> - def __notify_is_valid_cb(self, entry, pspec):
> - if entry.is_valid:
> - self.needs_restart = True
> - self._validate()
Can you run pylint and pep8 on those files, there are unused imports and
'blank line at end of file' stuff to be fixed.
Regards,
Simon
More information about the Sugar-devel
mailing list