On 12/02/2013 01:37 PM, Vratislav Podzimek wrote:
On Fri, 2013-11-29 at 13:17 +0100, Radek Vykydal wrote:
@@ -352,30 +434,28 @@ class NetworkControlBox(object): selection.select_iter(itr)
def refresh(self):
device = self.selected_device()
self.refresh_ui(device)
self.refresh_ui() # Signal handlers.
- def on_device_selection_changed(self, *args):
device = self.selected_device()
if not device:
return
- def on_nm_state_changed(self, *args):
self.emit("nm-state-changed")
log.debug("network: selected device %s" % device.get_iface())
self.refresh_ui(device)
- def on_device_selection_changed(self, *args):
self.refresh_ui()
- def on_device_state_changed(self, *args):
device = args[0]
new_state = args[1]
- def on_device_state_changed(self, device, new_state, *args):
self.emit("device-state-changed", device.get_iface(), new_state, *args) if new_state == NetworkManager.DeviceState.SECONDARIES: return self._refresh_carrier_info()
if device == self.selected_device():
self.refresh_ui(device, new_state)
dev_cfg = self.selected_dev_cfg()
if dev_cfg and dev_cfg.device == device:
self.refresh_ui(state=new_state) def on_device_config_changed(self, device, *args):
if device == self.selected_device():
self.refresh_ui(device)
dev_cfg = self.selected_dev_cfg()
if dev_cfg and dev_cfg.device == device:
self.refresh_ui() def on_wireless_ap_changed_cb(self, combobox, *args): if self._updating_device:
@@ -384,13 +464,13 @@ class NetworkControlBox(object): if not iter: return
device = self.selected_device()
device = self.selected_dev_cfg().device
The value returned from self.selected_dev_cfg() should be checked for not being None first. At least that's what is done in all the other places.
Unlike in other places the race condition the check is for shouldn't occur here, but adding the check to be on safe side won't hurt.
@@ -483,28 +555,29 @@ class NetworkControlBox(object): return
active = switch.get_active()
device = self.selected_device()
dev_cfg = self.selected_dev_cfg()
Same check here? Or is it for sure some device will be selected here? `
I'd think so, but I'll add the check.
log.info("network: device %s switched %s" %
(device.get_iface(), "on" if active else "off"))
(dev_cfg.get_iface(), "on" if active else "off"))
dev_type = device.get_device_type()
if dev_type in self.wired_ui_device_types:
if dev_cfg.device_type == NetworkManager.DeviceType.WIFI:
self.client.wireless_set_enabled(active)
else: if active:
uuid = nm.nm_device_setting_value(device.get_iface(), "connection", "uuid")
if uuid:
try:
nm.nm_activate_device_connection(device.get_iface(), uuid)
except nm.UnmanagedDeviceError as e:
log.debug("network: on_device_off_toggled: %s" % e)
else:
self.client.add_and_activate_connection(None, device, None,
None, None)
dev_name = dev_cfg.device and dev_cfg.device.get_iface()
if not dev_cfg.con_uuid:
log.debug("network: on_device_off_toggled: no connection for %s"
% dev_name)
return
try:
nm.nm_activate_device_connection(dev_name, dev_cfg.con_uuid)
except (nm.UnmanagedDeviceError, nm.UnknownDeviceError) as e:
log.debug("network: on_device_off_toggled: %s" % e) else:
device.disconnect(None, None)
elif dev_type == NetworkManager.DeviceType.WIFI:
self.client.wireless_set_enabled(active)
try:
nm.nm_disconnect_device(dev_cfg.get_iface())
except (nm.UnmanagedDeviceError, nm.DeviceNotActiveError) as e:
log.debug("network: on_device_off_toggled: %s" % e) def on_add_device_clicked(self, *args): dialog = self.builder.get_object("add_device_dialog")
@@ -532,130 +605,129 @@ class NetworkControlBox(object): self._running_nmce = None network.logIfcfgFiles("nm-c-e run")
- def selected_device(self):
- def selected_dev_cfg(self): selection = self.builder.get_object("treeview_devices").get_selection()
(model, iter) = selection.get_selected()
model, iter = selection.get_selected() if not iter: return None
return model.get(iter, DEVICES_COLUMN_OBJECT)[0]
return model[iter][DEVICES_COLUMN_OBJECT]
- def _device_is_stored(self, nm_device):
"""Check that device with Udi of nm_device is already in liststore"""
udi = nm_device.get_udi()
model = self.builder.get_object("liststore_devices")
for row in model:
if udi == row[DEVICES_COLUMN_OBJECT].get_udi():
return True
return False
- def add_dev_cfg(self, dev_cfg):
log.debug ("network: GUI, device configuration added: connection %s device %s"
% (dev_cfg.con_uuid, dev_cfg.get_iface()))
self.builder.get_object("liststore_devices").append([
self._dev_icon_name(dev_cfg),
self.device_type_sort_value.get(dev_cfg.device_type, "100"),
self._dev_title(dev_cfg),
dev_cfg
])
The "liststore_devices" store should be pulled only once from the builder, not with every single item added.
Okay, adding the item happens only a couple of times, but the dev_cfg below is more frequent, and other places are calling for it too, I'll add it.
if dev_cfg.device:
if (dev_cfg.device.get_state() == NetworkManager.DeviceState.UNAVAILABLE
and dev_cfg.device.get_device_type() == NetworkManager.DeviceType.ETHERNET
and not dev_cfg.device.get_carrier()):
# Translators: ethernet cable is unplugged
We use TRANSLATORS in other places.
I'll make it consistent.
title = ""
return title
- def dev_cfg(self, uuid=None, device=None):
model = self.builder.get_object("liststore_devices")
Should be self._liststore_devices pulled only once.
Yep.
# TODO: is this necessary? Isn't it static from glade? self.builder.get_object("label_%s_device" % dev_type_str).set_label(
"%s (%s)" % (self._dev_type_str(device), device.get_iface()))
"%s (%s)" % (self.device_type_name.get(dev_cfg.device_type, ""), dev_cfg.get_iface()))
I think a temporary variable for the label would clarify this piece of code a bit.
I agree.
if __name__ == "__main__":
win = Gtk.Window() win.connect("delete-event", Gtk.main_quit)
Do you still use this main block for testing?
Yes, this should more or less work, perhaps requires some paths tweaking. I'll look at it some time.