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

Proposed by Tony Espy
Status: Merged
Approved by: Mathieu Trudel-Lapierre
Approved revision: 970
Merged at revision: 969
Proposed branch: lp://staging/~phablet-team/network-manager/lp1445080-wily
Merge into: lp://staging/~network-manager/network-manager/ubuntu
Prerequisite: lp://staging/~phablet-team/network-manager/lp1461593-wily
Diff against target: 762 lines (+301/-188)
6 files modified
debian/changelog (+7/-0)
debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch (+215/-183)
debian/patches/lp1445080-modify-device-modem-avail.patch (+44/-0)
debian/patches/lp1445080-nm-modem-check-for-set-mm-enabled-func.patch (+28/-0)
debian/patches/lp1461593-add-modem-reconnect-delay-to-policy.patch (+5/-5)
debian/patches/series (+2/-0)
To merge this branch: bzr merge lp://staging/~phablet-team/network-manager/lp1445080-wily
Reviewer Review Type Date Requested Status
Mathieu Trudel-Lapierre Approve
Review via email: mp+264895@code.staging.launchpad.net

Description of the change

This change re-factors the modem_state handling code in NMModemOfono to solve underlying race conditions in the handling of flight-mode.

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

Have the code bits been discussed with upstream? What is the likely impact on non-ofono modems of the changes to nm_device_modem to not return that connections are unavailable when the modem is still in NM_MODEM_STATE_INITIALIZING state, for example?

As per the other patches, I think the ofono code changes should be merged in to the main patch for the modem support (since we're at a development release, this makes it just one large patch to fixup when there is a need to refresh the patch, rather than conflicting, interdepending change sets), and any core changes should be in a separate, isolated patch so that we can send them upstream ASAP.

Since I don't know whether there has been careful testing of the impact of the NMDeviceModem changes on non-ofono modem -> Needs Information.

review: Needs Information
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)

969. By Tony Espy

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

970. By Tony Espy

debian/patches/0001-wwan-add-support-for-using-oFono-as-a-modem-manager.patch,
debian/patches/lp1445080-modify-device-modem-avail.patch,
debian/patches/lp1445080-nm-modem-check-for-set-mm-enabled-func.patch,
debian/patches/lp1461593-add-modem-reconnect-delay-to-policy.patch: These
changes collectively fix flight-mode on arale ( and other devices ), due to
some fundemental race conditions in the ofono logic. (LP: #1445080, #1440917)

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

Updated to fold the core changes into existing patches, created new patches for core changes, and ensure that new patches have DEP-3 headers.

Yes, some of the core changes were discussed upstream, but as I wasn't getting much traction with upstream and the changes needed to land for phone networking stabilization, the changes were landed in the phone overlay PPA after being reviewed and thoroughly tested.

> What is the likely impact on non-ofono modems of the changes to nm_device_modem to not return that connections are unavailable when the modem is still in NM_MODEM_STATE_INITIALIZING state, for example?

Actually, that's not how the change works. The change ensures that devices with a modem_state <= SEARCHING are considered unavailable. As INITIALIZING is less than SEARCHING, modems in this state will still be considered unavailable. This is consistent with the function documentation for nm_device_is_available () and nm_device_connection_is_available (). Note, this hasn't been tested on non-ofono modems, so this will need to be done. If problems arise, then we will need to add a hook so that NMModemOfono can implement a stricter policy than NMDeviceModem for availability.

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