Opened 14 years ago
Closed 10 years ago
#1630 closed enhancement (fixed)
Support for mobile broadband provider database
Reported by: | aa | Owned by: | aa |
---|---|---|---|
Priority: | major | Milestone: | Unspecified |
Component: | Sugar | Version: | Git as of bugdate |
Severity: | Minor | Keywords: | 3g r! |
Cc: | tomeu, tch, rgs, erikos, garycmartin, manuq, m_anish, sascha_silbe, ajay_garg, pbrobinson | Distribution/OS: | OLPC |
Bug Status: | Unconfirmed |
Description
The NetworkManager project provides an XML database containing carrier network details for a large number of countries. Support for parsing this database would greatly benefit the user experience when configuring a mobile broadband modem.
The database package is usually called mobile-broadband-provider-info.
The attached patch implements this feature atop the current work in #1622, so rebasing will be needed when those patches are r+.
Attachments (9)
Change History (40)
Changed 14 years ago by aa
comment:1 Changed 14 years ago by rgs
- Cc tch rgs added
Changed 14 years ago by aa
comment:2 Changed 14 years ago by tomeu
- Keywords 3g added
Changed 14 years ago by aa
comment:3 Changed 14 years ago by aa
comment:4 Changed 14 years ago by erikos
- Milestone changed from Unspecified by Release Team to 0.90
First of all, thanks for your work!
Sadly, as this is an enhancement and we are in Freeze already, I move it out to 0.90. If you (deployments) care strongly about it - please ask here for inclusion into a bugfix release 0.88.x.
Thanks!
comment:5 Changed 13 years ago by aa
Submitted latest version to mailing list: http://lists.sugarlabs.org/archive/sugar-devel/2010-June/024675.html
comment:6 follow-up: ↓ 7 Changed 13 years ago by tomeu
- Keywords r+ added; r? removed
Commented in the mailing list. r+ provided the design team doesn't want changes and the review comments are addressed.
comment:7 in reply to: ↑ 6 Changed 13 years ago by garycmartin
Replying to tomeu:
Commented in the mailing list. r+ provided the design team doesn't want changes and the review comments are addressed.
Only trivial design niggles from me would be to right align the text labels next to the drop down menus, matching the rest of the existing fields; perhaps also make the drop down menus the same width as the existing input fields.
comment:8 Changed 13 years ago by erikos
That one should not miss the boat! We have to land it before UI-Freeze the 23th of August.
comment:9 Changed 13 years ago by erikos
aa stated on irc that he will try to address what gary requested in time for the next tarball, yay!
Changed 13 years ago by aa
Changed 13 years ago by aa
Changed 13 years ago by aa
comment:10 follow-up: ↓ 11 Changed 13 years ago by aa
- Cc erikos garycmartin added
I attached the patches, along with a screenshot of the latest version with the modifications suggested by Gary.
comment:11 in reply to: ↑ 10 Changed 13 years ago by garycmartin
Replying to aa:
I attached the patches, along with a screenshot of the latest version with the modifications suggested by Gary.
Fab, that looking great.
comment:12 Changed 13 years ago by erikos
- Keywords r! added; r+ removed
Great that Gary is now happy with the design. Thanks for your great work so far. I have a few code picks:
try: 84 tree = ElementTree(file=PROVIDERS_PATH) 85 elem = tree.getroot() 86 if elem is None or elem.get('format') != PROVIDERS_FORMAT_SUPPORTED: 87 return False 88 return True 89 except IOError: 90 return False
Please only put in the try/except block what you would expect an IOError from. Otherwise you hide errors. You can use an else block.
COUNTRY_CODE = locale.getdefaultlocale()[0][3:5].lower()
This looks dangerous. Will that always return what you expect? It is good to put that in a few lines, too and do error handling.
LANG_NS_ATTR = '{http://www.w3.org/XML/1998/namespace}lang' LANG = locale.getdefaultlocale()[0][:2] DEFAULT_NUMBER = '*99#'
If those are only used locally please make them private.
gtk.HBox.__init__(self, spacing=style.DEFAULT_SPACING*2)
Please use spaces. (There are a few more)
# Copyright (C) 2010 Andrés Ambrois
Character.
In general, I am a bit worried about the error handling in http://bugs.sugarlabs.org/attachment/ticket/1630/0001-Add-models-for-detecting-and-parsing-the-providers-D.patch If you have not done so yet, running that code with a lot of data for testing would be good.
comment:13 Changed 13 years ago by erikos
Note: the current patch does not work for sugar-jhbuild. The paths constructed in config.py point to /home/erikos/sugar-jhbuild/install/share/zoneinfo/iso3166.tab which is not there.
comment:14 Changed 13 years ago by erikos
- Keywords r? added; r! removed
comment:15 follow-up: ↓ 16 Changed 13 years ago by tomeu
- Component changed from sugar to python-xklavier
- Keywords r! added; r? removed
- Priority changed from normal to major
Please don't address review comments in a separate patch.
Removing from the queue because there's work going on about using a XDG spec to locate the files.
Changed 13 years ago by aa
comment:16 in reply to: ↑ 15 Changed 13 years ago by aa
Replying to tomeu:
Please don't address review comments in a separate patch.
There. Fixed that for you.
Removing from the queue because there's work going on about using a XDG spec to locate the files.
Shouldn't that be part of a separate effort? I'm sure there will be other parts of Sugar that will need patching if we stop using the autofoo crap to locate files anyway, so it shouldn't be a blocker for this.
Anyway, there's a line in the patch that still bugs me:
LANG = locale.getdefaultlocale()[0][:2]
That isn't guaranteed to work... I'm surely missing something obvious, but do you know a better way to get the current locale?
comment:17 Changed 13 years ago by erikos
language_code, encoding_ = locale.getdefaultlocale() LANG = language_code[:2]
I find something like this more readable. I looked a bit around but did not find a better way to get the LANG.
Please make that part a method to not have a so long constructor like:
if model.has_providers_db(): self._add_provider_information()
Please include the the with statement by doing: from future import with_statement so we do not depend on 2.6
For XDG, would be great if you could change it to use it. We can change other parts of sugar then too in 0.92. The current patch does not work on sugar-jhbuild for example.
comment:18 Changed 13 years ago by sascha_silbe
- Component changed from python-xklavier to sugar
Fixing component (probably changed by mistake).
comment:19 Changed 11 years ago by ajay_garg
- Cc manuq added
- Distribution/OS changed from Unspecified to OLPC
- Milestone changed from 0.90 to 0.98
- Version changed from Unspecified to Git as of bugdate
0.98-compliant patch (pending review) at ::
http://patchwork.sugarlabs.org/patch/1592/
comment:20 Changed 11 years ago by ajay_garg
- Cc m_anish sascha_silbe added
comment:21 Changed 11 years ago by ajay_garg
- Cc ajay_garg added
comment:22 Changed 11 years ago by pbrobinson
Is there any reason why this code is reading the provider xml database independantly of ModemManager?
We should just use ModemManager to configure this and provide a UX to ModemManager. That way the configuration and settings etc would be shared between Gnome and Sugar and it's also distribution independent and if the provider DB should happen to change format between releases for what ever reason MM will deal with that. This is the same way we moved with NetworkManager 0.9 and later so we needed need to implement it all manually and integrates with NM properly for the various network/IP components. It's also distribution/technology agnostic.
Details on ModemManager are
https://mail.gnome.org/archives/networkmanager-list/2009-February/msg00079.html
http://cgit.freedesktop.org/ModemManager/ModemManager/
comment:23 Changed 11 years ago by ajay_garg
Hi Peter.
As far as I know, the current mainline code already stores the settings as system-settings.
So far so good.
Earlier, in the "ModemConfiguration" CP applet, user would enter some strings (even if they were useless) manually (on the UI). These (even if useless) strings would then be stored as GSM-connection-settings as system-settings.
The current patch simply automates the filling up of this strings (at the UI level).
These strings continue to be saved as system-settings. No change on that front.
Even in gnome, these populating-of-strings-on-the-UI is handled by nm-applet.
So, in Gnome ::
a)
The strings are populated via nm-applet.
b)
Settings are saved as system-settings.
c)
These settings are used by MM.
In Sugar,
a)
The strings are populated via "ModemConfiguration" CP applet.
b)
Settings are saved as system-settings.
c)
These settings are used by MM.
comment:24 Changed 11 years ago by ajay_garg
- Cc pbrobinson added
comment:25 Changed 11 years ago by ajay_garg
Version-2 (0.98 sugar) patch at ::
http://patchwork.sugarlabs.org/patch/1644/
Changes of version-2 over version-1 ::
======================================
a)
Corrected the makefile, for addition of "config.py".
Thanks Ruben, for the fix :)
b)
Now, in the countries' dropdown, the countries are shown sorted by name, and not by code.
Also, I don't think it is necessary to localize the countries-names as per say.
However, opinions are welcome :)
Thanks Ruben :)
c)
Removed horizontal scroll bar.
Thanks Ruben :)
ALSO, PLEASE NOTE :::::::
=========================
This patch is based, after the following two patches have been applied ::
The first patch helps "truly" show the waiting-icon while the section-view
is loaded.
The second patch, solves converting the method-to-fetch-gsm-system-settings
from pseudo-asynchronous to purely asynchronous.
This second patch is needed, as a result of first patch. The first patch worked for all,
except the "ModemConfiguration" section.
Thanks to Ruben for the motivation, of truly showing the waiting-cursor. :)
comment:26 Changed 11 years ago by ajay_garg
Version-3 (0.98 sugar) patch at ::
http://patchwork.sugarlabs.org/patch/1646/
Changes of version-3 over version-2 ::
======================================
Corrected the Makefile :-\
Sorry, as I don't have the "actual" environment, to build the sugar-rpm for mainline.
Anyways, now the makefile is right.
comment:27 Changed 11 years ago by erikos
- Milestone changed from 0.98 to 1.0
This is still a gtk2 patch. Moving out as we are way late in this cycle.
comment:28 Changed 10 years ago by dnarvaez
This was proposed for 0.100, let's see if it actually gets submitted to github.
comment:29 Changed 10 years ago by dnarvaez
- Milestone changed from 1.0 to Unspecified
comment:30 Changed 10 years ago by dnarvaez
- Bug Status changed from New to Unconfirmed
comment:31 Changed 10 years ago by dnarvaez
- Resolution set to fixed
- Status changed from new to closed
Attached latest work, this applies to git HEAD. There are several visual modifications, so please let me know what you think.
Also new in this version is a config.py.in for generating config.py by autotools, and some minor nitpicks to shut pylint up :).