Opened 12 years ago

Closed 12 years ago

#2 closed defect (fixed)

wpa support for NM07 in sugar head

Reported by: erikos Owned by: erikos
Priority: major Milestone:
Component: Sugar Version:
Severity: Keywords: r+
Cc: morgan.collett@… Distribution/OS:
Bug Status:

Description

  • adds the loading and saving of the connections file
  • support of wpa
  • cleaning up the dialogs (looked what nm-applet is doing)

Attachments (1)

nm07_wpa.patch (28.0 KB) - added by erikos 12 years ago.
does include better error handling for reading the config file and move the settings and secrets to python objects

Download all attachments as: .zip

Change History (8)

comment:1 Changed 12 years ago by 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.

comment:2 Changed 12 years ago by marcopg

  • Component changed from glucose to sugar
  • Owner changed from tomeu to marcopg

comment:3 Changed 12 years ago by erikos

  • Owner changed from marcopg to erikos
  • Status changed from new to assigned

comment:4 Changed 12 years ago by morgs

  • Cc morgan.collett@… added

Changed 12 years ago by erikos

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

comment:5 Changed 12 years ago by erikos

  • Keywords r? added

comment:6 Changed 12 years ago by tomeu

  • Keywords r+ added; r? removed

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

comment:7 Changed 12 years ago by erikos

  • Resolution set to fixed
  • Status changed from assigned to closed

Done and pushed - thanks for the review!

Note: See TracTickets for help on using tickets.