NetworkManager

Merge lp://qastaging/~phablet-team/network-manager/lp1361864-2 into lp://qastaging/~phablet-team/network-manager/vivid-phone-overlay

  1. lp1361864-2
  2. Merge into vivid-phone-overlay
Proposed by Tony Espy
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: 968
Merged at revision: 965
Proposed branch: lp://qastaging/~phablet-team/network-manager/lp1361864-2
Merge into: lp://qastaging/~phablet-team/network-manager/vivid-phone-overlay
Diff against target: 710 lines (+665/-4)
4 files modified
debian/changelog (+18/-0)
debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch (+3/-4)
debian/patches/lp1361864-add-ofono-preferred-contexts.patch (+643/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp://qastaging/~phablet-team/network-manager/lp1361864-2
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Approve
Review via email: mp+263982@code.qastaging.launchpad.net

Description of the change

This change adds support for the new ofono gprs-context property 'Preferred'.

When a context is set to 'Preferred=true', NM should disregard all other contexts for the the same IMSI. If the a context is connected, and another context is set as 'Preferred', NM will disconnect the existing context, and attempt to activate the 'Preferred' context. If the 'Preferred' context fails, NM will not try to activate any other contexts.

When a context is set to 'Preferred=false', NM will not deactivate the context, if currently connected.

This change also removes the code in NMMOdemOfono which invoked a DBus method (ReadImsiConetexts) provided by SCPlugin-Ofono, as this only had any effect when a new context was added. and this case is already handled by the imsi_monitor.

Finally, the code in NMDeviceModem which disabled and re-enabled autoconnect based upon device and modem_state was removed, as it caused a hang on arale when a deactivate context failed, and the retry limit was hit, which causes a modem reset.

To post a comment you must log in.
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Please see comments below.

review: Needs Fixing
lp://qastaging/~phablet-team/network-manager/lp1361864-2 updated
967. By Tony Espy

Fix formatting: lp1361864-add-ofono-preferred-contexts.patch

Revision history for this message
Tony Espy (awe) wrote :

Minor update based on comments...

lp://qastaging/~phablet-team/network-manager/lp1361864-2 updated
968. By Tony Espy

Addressed review comments for lp1361864-add-ofono-preferred-contexts.patch

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM

review: Approve
lp://qastaging/~phablet-team/network-manager/lp1361864-2 updated
969. By Tony Espy

Removed extra whitespace from latest debian/changelog entry

Revision history for this message
Tony Espy (awe) wrote :

Tested on mako (rc/ubuntu #1); additional packages include the recent landings of lxc-android-config ( just the NM 03mmsproxy script ) and ofono ( 1.12.bzr6900+15.04.20150702.3-0ubuntu1 ).

Ran the Basic Tests from:

https://wiki.ubuntu.com/Process/Merges/TestPlans/network-manager

Also tested the new gprs-context 'Preferred' property and everything works as expected. Note, it can sometimes take 30-45s for a context to be disconnected and the new 'Preferred' context connected.

lp://qastaging/~phablet-team/network-manager/lp1361864-2 updated
970. By Tony Espy

Gobject memory mgmt cleanup in lp1361864-add-ofono-preferred-contexts.patch

In certain instances, references to the NMModemOfono instance weren't being
incremented. This could cause crashes if ofono restarts and the associated
modem instance was referenced after a DBus async operation.

Revision history for this message
Tony Espy (awe) wrote :

Tested krillin (rc-proposed/bq-aquaris.en #60, ~OTA5 ).

Ran Basic, Preferred, and Dual SIM tests from:

https://wiki.ubuntu.com/Process/Merges/TestPlans/network-manager

Encountered a couple of crashes when manually restarting ofono that were caused by sloppy GObject memory management in NMModemOfono. As such, I've updated this merge proposal with a few small changes to fix these problems.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Looks good, but I have detected one code path where you do not decrement the count of the object. See comment below.

review: Needs Fixing
Revision history for this message
Tony Espy (awe) wrote :

Reply added...

lp://qastaging/~phablet-team/network-manager/lp1361864-2 updated
971. By Tony Espy

Minor update to lp1361864-add-ofono-preferred-contexts.patch

Fixed one remaining memory leak.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM

review: Approve
lp://qastaging/~phablet-team/network-manager/lp1361864-2 updated
972. By Tony Espy

Add new changelog entry for NMModemOfono GObject fixes.

Revision history for this message
Tony Espy (awe) wrote :

Re-tested version 15.1.6 on krillin (rc-proposed/bq-aquaris.en #60, ~OTA5 ).

Quick sanity check of boot, flight-mode, and set-radio-tech. Verified that ofono restarts no longer crash NM.

Revision history for this message
Tony Espy (awe) wrote :

Tested arale ( rc-proposed/meizu.en #58 ).

Ran the Basic test cases from:

https://wiki.ubuntu.com/Process/Merges/TestPlans/network-manager

Also verified that the new 'Preferred' functions work as expected.

Updating the silo (006) to 'Tested'.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: