NetworkManager

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

lp1461593-wily
Merge into ubuntu

Proposed by Tony Espy on 2015-07-15

Status:	Merged
Approved by:	Mathieu Trudel-Lapierre on 2015-07-20
Approved revision:	969
Merged at revision:	968
Proposed branch:	lp://qastaging/~phablet-team/network-manager/lp1461593-wily
Merge into:	lp://qastaging/~network-manager/network-manager/ubuntu
Prerequisite:	lp://qastaging/~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://qastaging/~phablet-team/network-manager/lp1461593-wily
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mathieu Trudel-Lapierre		2015-07-15	Approve on 2015-07-20
Review via email: mp+264894@code.qastaging.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 ).

Revision history for this message

Mathieu Trudel-Lapierre (cyphermox) wrote on 2015-07-15:

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

lp://qastaging/~phablet-team/network-manager/lp1461593-wily updated on 2015-07-15

967. By Tony Espy on 2015-07-15: 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 on 2015-07-15: 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 on 2015-07-16:

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

lp://qastaging/~phablet-team/network-manager/lp1461593-wily updated on 2015-07-17