Merge lp://staging/~phablet-team/network-manager/lp1461593-wily into lp://staging/~network-manager/network-manager/ubuntu

Proposed by Tony Espy
Status: Merged
Approved by: Mathieu Trudel-Lapierre
Approved revision: 969
Merged at revision: 968
Proposed branch: lp://staging/~phablet-team/network-manager/lp1461593-wily
Merge into: lp://staging/~network-manager/network-manager/ubuntu
Prerequisite: lp://staging/~phablet-team/network-manager/lp1435776-wily
Diff against target: 331 lines (+223/-10)
6 files modified
debian/changelog (+10/-0)
debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch (+16/-9)
debian/patches/add_ofono_settings_support.patch (+5/-1)
debian/patches/lp1461593-add-modem-reconnect-delay-to-policy.patch (+117/-0)
debian/patches/lp1461593-add-nm-settings-connection-reset-retries-methods.patch (+73/-0)
debian/patches/series (+2/-0)
To merge this branch: bzr merge lp://staging/~phablet-team/network-manager/lp1461593-wily
Reviewer Review Type Date Requested Status
Mathieu Trudel-Lapierre Approve
Review via email: mp+264894@code.staging.launchpad.net

Description of the change

This change adds a 5s delay into NM_POLICY's re-connect logic, specifically so that the retry_limit of the associated connection isn't exhausted immediately whenever the connection drops, which then used to result in a 5m timer being set.

There are two changes...

1. The afore-mentioned 5m timer is now reduced to 30s for modem devices. In theory, connections shouldn't be getting disabled anymore due to the above fix, so this is a just-in-case change.

2. The low-level NM_MODEM_OFONO code wasn't setting the modem state to DISCONNECTING when disconnect() was called. This is now done, and in the associated callback ( disconnect_cb ) the new state is now determined by the ofono cached properties ( online && powered && attached ).

To post a comment you must log in.
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Ofono bits probably should be folded in to the main patch; anything that touches the core NM code should be split out by itself, so that we can upstream just that independently from any ofono support code.

If you want, I could possibly do the patch mangling.

review: Needs Fixing
967. By Tony Espy

debian/patches/add_ofono_settings_support.patch: remove code
that added APN, USERNAME and PASSWORD to NM_SETTING_GSM object.
NM doesn't actually need access to these settings, and USERNAME/
PASSWORD can cause issues with NM's secrets needed logic.

968. By Tony Espy

debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch,
debian/patches/lp1461593-add-nm-settings-connection-reset-retries-methods.patch,
debian/patches/add_ofono_settings_support.patch,
debian/patches/lp1461593-add-modem-reconnect-delay-to-policy.patch: More changes
to NMModemOfono's modem_state handling. Added get/set_reset_retries_timeout
methods to NMSettingsConnection, and use the set method to lower the timeout for
ofono connections to 30s. Finally added a 5s delay to NM_POLICY's activation
logic triggered when a modem device is disconnected. This allows modem time to
settle and NM to process the resulting DBus state changes. (LP: #1461593)

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Looks fine overall, though I've added some nitpicks inline. The extra delay/retries patches will need careful testing in a silo to make sure they don't break ModemManager, but they do look harmless.

review: Needs Fixing
969. By Tony Espy

Updated new patches for DEP-3, and re-ordered ofono patches.

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

Updated patches for DEP-3, and re-ordered based on inline comments.

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) :
review: Approve

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