wpa support for NM07 in sugar head

[erikos]

• adds the loading and saving of the connections file

• support of wpa

• cleaning up the dialogs (looked what nm-applet is doing)

[erikos]

tomeu comments:

+            # No security
+            return

I would prefer to explicitly return None, and test for that value in the callers. Though the most usual solution would be to return a tuple or an object with those properties as members.

+            if security:

This construction should be used only when security is a boolean or a sequence. If you want to check if it's None, just do "if security is None".

+                key = 'key-mgmt'
+                if self._settings['802-11-wireless-security'].has_key(key):
+                    value = self._settings['802-11-wireless-security'][key]
+                    config.set(identifier, key, value)
+                key = 'proto'
+                if self._settings['802-11-wireless-security'].has_key(key):
+                    value = self._settings['802-11-wireless-security'][key]
+                    config.set(identifier, key, value)
+                key = 'pairwise'
+                if self._settings['802-11-wireless-security'].has_key(key):
+                    value = self._settings['802-11-wireless-security'][key]
+                    config.set(identifier, key, value)
+                key = 'group'
+                if self._settings['802-11-wireless-security'].has_key(key):
+                    value = self._settings['802-11-wireless-security'][key]
+                    config.set(identifier, key, value)

Couldn't this be done in a nice "for key in ['key-mgmt', 'proto', ...]:" loop?

+        else:
+            f = open(config_path, 'w')
+            try:
+                config.write(f)
+            except ConfigParser.Error, e:
+                logging.error('Can not write %s error: %s' % (config_path, e))
+            f.close()

What this else clause does?

+        settings = {'connection': {'id': section}}        

I'm a bit concerned about all these strings in the code, cannot we use a python object with some properties instead?

Globally, this patch sounds pretty good to me.

[erikos]

does include better error handling for reading the config file and move the settings and secrets to python objects

[tomeu]

Though we should make translatable that 'Auto ' string and try to keep abbreviations to the minimum possible.

[erikos]

Done and pushed - thanks for the review!