Opened 8 years ago

Closed 4 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)

gsm-provider-db.patch (7.0 KB) - added by aa 8 years ago.
gsm-provider-db-v2-1630.patch (8.6 KB) - added by aa 8 years ago.
gsm-provider-db-v3-1630.patch (16.3 KB) - added by aa 7 years ago.
0001-Add-models-for-detecting-and-parsing-the-providers-D.patch (5.5 KB) - added by aa 7 years ago.
0002-Show-Country-Provider-Plan-comboboxes-if-DB-exists.patch (11.5 KB) - added by aa 7 years ago.
0003-Add-config.py.in-and-update-AC_CONFIG_FILES.patch (3.1 KB) - added by aa 7 years ago.
3g-db.png (42.1 KB) - added by aa 7 years ago.
screenshot
0001-Refinements-of-1630.patch (6.2 KB) - added by erikos 7 years ago.
Patch that addresses the issues raised by me
gsm-provider-db-v4-1630.patch (20.1 KB) - added by aa 7 years ago.

Download all attachments as: .zip

Change History (40)

Changed 8 years ago by aa

comment:1 Changed 8 years ago by rgs

  • Cc tch rgs added

Changed 8 years ago by aa

comment:2 Changed 7 years ago by tomeu

  • Keywords 3g added

Changed 7 years ago by aa

comment:3 Changed 7 years ago by aa

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 :).

comment:4 Changed 7 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:6 follow-up: Changed 7 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 7 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 7 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 7 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 7 years ago by aa

screenshot

comment:10 follow-up: Changed 7 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 7 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 7 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.

Changed 7 years ago by erikos

Patch that addresses the issues raised by me

comment:13 Changed 7 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 7 years ago by erikos

  • Keywords r? added; r! removed

comment:15 follow-up: Changed 7 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 7 years ago by aa

comment:16 in reply to: ↑ 15 Changed 7 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 7 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 6 years ago by sascha_silbe

  • Component changed from python-xklavier to sugar

Fixing component (probably changed by mistake).

comment:19 Changed 5 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 5 years ago by ajay_garg

  • Cc m_anish sascha_silbe added

comment:21 Changed 5 years ago by ajay_garg

  • Cc ajay_garg added

comment:22 Changed 5 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 5 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 5 years ago by ajay_garg

  • Cc pbrobinson added

comment:25 Changed 5 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 ::

(i)
http://patchwork.sugarlabs.org/patch/1642/

(ii)
http://patchwork.sugarlabs.org/patch/1643/

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 5 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 5 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 4 years ago by dnarvaez

This was proposed for 0.100, let's see if it actually gets submitted to github.

comment:29 Changed 4 years ago by dnarvaez

  • Milestone changed from 1.0 to Unspecified

comment:30 Changed 4 years ago by dnarvaez

  • Bug Status changed from New to Unconfirmed

comment:31 Changed 4 years ago by dnarvaez

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.