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

Proposed by Tony Espy
Status: Rejected
Rejected by: Tony Espy
Proposed branch: lp://qastaging/~phablet-team/network-manager/lp1361864
Merge into: lp://qastaging/~phablet-team/network-manager/vivid-phone-overlay
Diff against target: 613 lines (+549/-36)
4 files modified
debian/changelog (+9/-0)
debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch (+0/-36)
debian/patches/lp1361864-add-ofono-preferred-contexts.patch (+539/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp://qastaging/~phablet-team/network-manager/lp1361864
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Needs Fixing
Review via email: mp+263855@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 :

See inline comments. Overall, looks good.

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

Comments addressed, will re-push shortly after a quick sanity test.

969. By Tony Espy

Updates to patch based on code-review.

One big change is that most context properties are no longer
added to the context GHashtable in read_imsi_contexts, as
other then ID, IMSI, and Name, none were actually used for
anything.

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

Re-submitted at https://code.launchpad.net/~phablet-team/network-manager/lp1361864-2/+merge/263982.

Changing the status of this merge to 'Rejected'.

Unmerged revisions

969. By Tony Espy

Updates to patch based on code-review.

One big change is that most context properties are no longer
added to the context GHashtable in read_imsi_contexts, as
other then ID, IMSI, and Name, none were actually used for
anything.

968. By Tony Espy

Update debian/changelog

967. By Tony Espy

Revised lp1361864-add-ofono-preferred-contexts.patch for multi-sim.

966. By Tony Espy

Remove NMDeviceModem's disable autoconnect code

Code was added to NMDeviceModem to fix a problem where NM took ~5m
to re-connect after a radio technology change (LP: #1418077). This
code is no longer needed after the re-factoring of NMModemOfono
that was done to fix flight-modem/modem state handling (LP: #1445080).

965. By Tony Espy

Added debian/patches/lp1361864-add-ofono-preferred-contexts.patch

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: