After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 687359 - Integrate the new ModemManager1 interface
Integrate the new ModemManager1 interface
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: network-indicator
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: Aleksander Morgado
gnome-shell-maint
Depends on: 688221
Blocks:
 
 
Reported: 2012-11-01 17:24 UTC by Aleksander Morgado
Modified: 2013-02-05 14:40 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch to integrate the new ModemManager interface (16.15 KB, patch)
2012-11-01 17:26 UTC, Aleksander Morgado 		none Details | Review
Patch for the issue (13.24 KB, patch)
2012-11-06 10:33 UTC, Aleksander Morgado 		reviewed Details | Review
Updated patch (13.12 KB, patch)
2012-11-12 07:46 UTC, Aleksander Morgado 		none Details | Review
Rebased patch (13.12 KB, patch)
2013-02-05 09:23 UTC, Aleksander Morgado 		committed Details | Review

Description Aleksander Morgado 2012-11-01 17:24:42 UTC 
ModemManager >= 0.7 comes with a new DBus interface. NetworkManager will report modems handled by the new ModemManager as NMDevices with a udi starting with the "/org/freedesktop/ModemManager1/Modem" prefix. The shell should detect this and use the new DBus interface with this devices, for operator name and signal quality retrieval.
Comment 1 Aleksander Morgado 2012-11-01 17:26:23 UTC 
Created attachment 227828 [details] [review]
Patch to integrate the new ModemManager interface
Comment 2 Aleksander Morgado 2012-11-05 21:25:36 UTC 
Comment on attachment 227828 [details] [review]
Patch to integrate the new ModemManager interface

Marking this patch as obsolete; need to rebase it on top of 70736be4eb5d9de0cee52aa47bb68ea6a7d8d518.
Comment 3 Aleksander Morgado 2012-11-06 10:33:08 UTC 
Created attachment 228231 [details] [review]
Patch for the issue

Rebased the original patch on top of 70736be.
Comment 4 Giovanni Campagna 2012-11-11 17:47:26 UTC 
Review of attachment 228231 [details] [review]:

Almost ok, one minor style nit.

::: js/ui/status/network.js
@@ +724,1 @@
         if (this._capabilities & NetworkManager.DeviceModemCapabilities.GSM_UMTS) {

Wrong indentation, it seems.

You can also merge the else and the if if you don't want to reindent everything.
Comment 5 Aleksander Morgado 2012-11-12 07:46:24 UTC 
Created attachment 228758 [details] [review]
Updated patch

Patch updated, I merged the if and the else as suggested. Not sure why I didn't see the indentation issue :-)
Comment 6 Aleksander Morgado 2012-11-13 07:29:55 UTC 
Adding dependency on the NetworkManager integration of the MM1 interface.
Comment 7 Aleksander Morgado 2012-11-13 07:30:59 UTC 
oops, resetting severity, not sure how it got changed
Comment 8 Aleksander Morgado 2013-02-05 09:23:18 UTC 
Created attachment 235198 [details] [review]
Rebased patch

Rebased and retested the patch on top of git master.

NetworkManager already can support the new MM1 interface, so there isn't anything stopping us from merging this. Should I merge it?
Comment 9 Giovanni Campagna 2013-02-05 14:02:39 UTC 
Review of attachment 235198 [details] [review]:

Looks good to me
Comment 10 Aleksander Morgado 2013-02-05 14:40:30 UTC 
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.