Review Request: Adds preliminary ModemManager support to solid
Lamarque Vieira Souza
lamarque at gmail.com
Thu May 27 03:12:57 CEST 2010
> On 2010-05-26 22:03:54, Will Stephenson wrote:
> > Generally fine, but udi() is not necessary, i18nc() should be used and
some accessors are not marked const.
udi is necessary because uni() returns a NetworkManager id
(/org/freedesktop/NetworkManager/Devices/0) and I need a ModemManager id
(/org/freedesktop/ModemManager/Modems/3), which is returned by udi(). I will
reimplement everything, the current implementation is too bound to
GsmNeworkInterface, ModemManager can be used standalone and supports Cdma and
Serial modems too, so I will reimplement this patch as a solid backend. The
change is huge and today I have got some other work to do from my employer
that is going to finish only in August.
> On 2010-05-26 22:03:54, Will Stephenson wrote:
> > /trunk/KDE/kdebase/workspace/libs/solid/control/networkgsminterface.h,
lines 27-28
> > <http://reviewboard.kde.org/r/3769/diff/6/?file=27196#file27196line27>
> >
> > Don't use 'using' in headers; fully qualify names instead
Ok.
> On 2010-05-26 22:03:54, Will Stephenson wrote:
> > /trunk/KDE/kdebase/workspace/libs/solid/control/networkgsminterface.cpp,
lines 150-163
> > <http://reviewboard.kde.org/r/3769/diff/6/?file=27197#file27197line150>
> >
> > Use i18nc(comment, string-to-translate) to give context to your
translations.
Ok.
> On 2010-05-26 22:03:54, Will Stephenson wrote:
> > /trunk/KDE/kdebase/workspace/libs/solid/control/networkinterface.h, line
138
> > <http://reviewboard.kde.org/r/3769/diff/6/?file=27198#file27198line138>
> >
> > udi() is not necessary; uni() represents it in the Solid::Control API.
udi() is necessary for ModemManager, uni() returns something like
/org/freedesktop/NetworkManager/Devices/0 udi() returns something like
/org/freedesktop/ModemManager/Modems/18 (the final number differs). A little
bit off topic: in kdereview/networkmanagement/libs/internals/uiutils.cpp I had
to change the code in UiUtils::interfaceNameLabel to use the interface name
(eth0, eth1, usb0, etc) to search for a Solid::Device object to make
UiUtils::interfaceNameLabel work as expected. Passing uni() as parameter to
the Solid::Device constructor always returns a null/invalid Solid::Device.
> On 2010-05-26 22:03:54, Will Stephenson wrote:
> > /trunk/KDE/kdebase/workspace/libs/solid/control/modemmanager.h, lines
70-75
> > <http://reviewboard.kde.org/r/3769/diff/6/?file=27193#file27193line70>
> >
> > These enums should be written in camel case so that they match KDE/Qt
coding styles and don't conflict with enums from a future ModemManager.h
Ok, I am using the specification enums to find them easily inside the source
code. There are several files to change to implement one single method/signal
and having to translate the enums to something else all the time would make
lose time. When everything is finsished I am going to change them.
> On 2010-05-26 22:03:54, Will Stephenson wrote:
> >
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkgsminterface.h,
line 54
> > <http://reviewboard.kde.org/r/3769/diff/6/?file=27216#file27216line54>
> >
> > Why did you make this change?
Because accoding to NM-0.7 specification although Gsm devices have a
propertiesChanged signal they do not have any properties, so this seemed
useless to me. On the other hand ModemManager also emmits a propertiesChanged
signal for Gsm devices with usefull information AllowedMode (Any, 2G, 3G, etc)
and AccessTechnology (GPRS, Edge, HDSDPA, etc). Ok, that breaks the
specification, I can bring it back.
- Lamarque
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3769/#review5877
-----------------------------------------------------------
On 2010-05-20 01:01:06, Lamarque Souza wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3769/
> -----------------------------------------------------------
>
> (Updated 2010-05-20 01:01:06)
>
>
> Review request for Network Management and KNetworkManager.
>
>
> Summary
> -------
>
> This patch adds preliminary ModemManager support to solid. The dbus methods
can be tested in command line with:
>
> dbus-send --system --print-reply --dest=org.freedesktop.ModemManager
/org/freedesktop/ModemManager org.freedesktop.ModemManager.EnumerateDevices
>
> dbus-send --system --print-reply --dest=org.freedesktop.ModemManager
/org/freedesktop/ModemManager/Modems/<modem number>
org.freedesktop.ModemManager.Modem.Gsm.Network.GetSignalQuality
>
> dbus-send --system --print-reply --dest=org.freedesktop.ModemManager
/org/freedesktop/ModemManager/Modems/<modem number>
org.freedesktop.ModemManager.Modem.Gsm.Network.GetRegistrationInfo
>
> dbus-send --system --print-reply --dest=org.freedesktop.ModemManager
/org/freedesktop/ModemManager/Modems/<modem number>
org.freedesktop.DBus.Properties.GetAll string:<interface name>
>
> <modem number> is returned by the first method (EnumerateDevices).
> <interface name> is either of
>
> org.freedesktop.ModemManager (do not use /Modems/<modem number> with this
one)
> org.freedesktop.DBus.Properties (do not use /Modems/<modem number> with this
one)
> org.freedesktop.ModemManager.Modem
> org.freedesktop.ModemManager.Modem.Simple
> org.freedesktop.ModemManager.Modem.Location
> org.freedesktop.ModemManager.Modem.Cdma
> org.freedesktop.ModemManager.Modem.Gsm
> org.freedesktop.ModemManager.Modem.Gsm.Card
> org.freedesktop.ModemManager.Modem.Gsm.Contacts
> org.freedesktop.ModemManager.Modem.Gsm.Network
> org.freedesktop.ModemManager.Modem.Gsm.SMS
> org.freedesktop.ModemManager.Modem.Gsm.Hso
> org.freedesktop.ModemManager.Modem.Gsm.Ussd
>
> There are still some problems with this patch that and I need advice as to
the best way to solve them:
>
> 1. [FIXED] It only works for one 3G modem. NetworkManager and ModemManager
have their own list of devices, I need to match the NetworkManager device id
to the ModemManager to that I can use
/org/freedesktop/ModemManager/Modems/<modem number> for more than one modem.
For now the implementation uses the first modem found.
>
> 2. The ModemManager object is in NMGsmNetworkInterfacePrivate class. Maybe
there is a better place to put it so other device types (CDMA, serial) could
make use of ModemManager.
>
> 3. The patch has only been tested with my Sony Ericsson MD300 modem.
>
> 4. When I hook up a second 3G modem (my cell phone actually) in my notebook
ModemManager disconnects the first modem from Internet. This is not a KDE
problem, just to warn everyone that tries this patch.
>
> ModemManager dbus specification is in here
http://projects.gnome.org/NetworkManager/developers/mm-spec-04.html in case
someone wants to help me implement the other methods. ModemManager-0.3 does
not implement all methods/signal from that specification. I am doing tests
with git NetworkManager and ModemManager as of May 16, 2010. To verify which
interfaces ModemManager and your modem supports do:
>
> dbus-send --system --print-reply --dest=org.freedesktop.ModemManager
/org/freedesktop/ModemManager org.freedesktop.DBus.Introspectable.Introspect
>
> dbus-send --system --print-reply --dest=org.freedesktop.ModemManager
/org/freedesktop/ModemManager/Modems/<modem number>
org.freedesktop.DBus.Introspectable.Introspect
>
> Current implemenation status is
>
> org.freedesktop.ModemManager
> - fully implemented
> - still needs testing: signals DeviceAdded and DeviceRemoved
>
> org.freedesktop.DBus.Properties
> - fully implemented and tested
>
> org.freedesktop.ModemManager.Modem
> - modem MD300 does not supports: method GetIP4Config
>
> org.freedesktop.ModemManager.Modem.Simple
> - to be implemented
>
> org.freedesktop.ModemManager.Modem.Location
> - modem MD300 does not support this interface (it is not listed when
calling org.freedesktop.DBus.Introspectable.Introspect)
>
> org.freedesktop.ModemManager.Modem.Cdma
> - modem MD300 does not support this interface
>
> org.freedesktop.ModemManager.Modem.Gsm
> - fully implemented and tested
>
> org.freedesktop.ModemManager.Modem.Gsm.Card
> - to be implemented
>
> org.freedesktop.ModemManager.Modem.Gsm.Contacts
> - modem MD300 does not support this interface
>
> org.freedesktop.ModemManager.Modem.Gsm.Network
> - fully implemented
> - to be tested: methods Register and SetApn
> - still not working (gives error): method scan
> - modem MD300 does not support: method GetBand and probably method
SetBand too
>
> org.freedesktop.ModemManager.Modem.Gsm.SMS
> - modem MD300 does not support method List and probably all other
methods in this interface.
>
> org.freedesktop.ModemManager.Modem.Gsm.Hso
> - modem MD300 does not support this interface
>
> org.freedesktop.ModemManager.Modem.Gsm.Ussd
> - modem MD300 does not support this interface
>
>
> Diffs
> -----
>
> /trunk/KDE/kdebase/workspace/libs/solid/control/CMakeLists.txt 1128694
>
/trunk/KDE/kdebase/workspace/libs/solid/control/backends/fakenet/fakeaccesspoint.h
1128694
>
/trunk/KDE/kdebase/workspace/libs/solid/control/backends/fakenet/fakeaccesspoint.cpp
1128694
>
/trunk/KDE/kdebase/workspace/libs/solid/control/backends/fakenet/fakenetworkinterface.h
1128694
>
/trunk/KDE/kdebase/workspace/libs/solid/control/backends/fakenet/fakenetworkinterface.cpp
1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/CMakeLists.txt
1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/modemmanager.h PRE-
CREATION
> /trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/modemmanager.cpp
PRE-CREATION
>
/trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/networkgsminterface.h
1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/networkinterface.h
1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/modemmanager.h PRE-
CREATION
> /trunk/KDE/kdebase/workspace/libs/solid/control/modemmanager.cpp PRE-
CREATION
> /trunk/KDE/kdebase/workspace/libs/solid/control/modemmanager_p.h PRE-
CREATION
> /trunk/KDE/kdebase/workspace/libs/solid/control/networkgsminterface.h
1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/networkgsminterface.cpp
1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/networkinterface.h 1128694
> /trunk/KDE/kdebase/workspace/libs/solid/control/networkinterface.cpp
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/CMakeLists.txt
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/accesspoint.h
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/CMakeLists.txt
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/hand-edits.diff
1128694
>
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/introspection/all.xml
1128694
>
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/introspection/mm-
gsm-network.xml PRE-CREATION
>
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/introspection/mm-
manager-client.xml PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/mm-gsm-
networkinterface.h PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/mm-gsm-
networkinterface.cpp PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/mm-manager-
clientinterface.h PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/mm-manager-
clientinterface.cpp PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/modemmanager-
types.h PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/dbus/modemmanager-
types.cpp PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/modemmanager.h PRE-
CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/modemmanager.cpp
PRE-CREATION
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/modemmanager_p.h
PRE-CREATION
>
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkgsminterface.h
1128694
>
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkgsminterface.cpp
1128694
>
/trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkgsminterface_p.h
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkinterface.h
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkinterface.cpp
1128694
> /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkinterface_p.h
1128694
> /trunk/KDE/kdebase/workspace/solid/wicd/networkinterface.h 1128694
>
> Diff: http://reviewboard.kde.org/r/3769/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lamarque
>
>
More information about the kde-networkmanager
mailing list