Review Request: Adds preliminary ModemManager support to solid

[Lamarque Vieira Souza]

Em Quinta-feira 27 Maio 2010, you wrote:
> > 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.
> > 
> > Lamarque Souza wrote:
> >     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.
> 
> Ok, then we need to rethink the naming of these methods because udi()
> sounds like a HAL id.

	Actually according to NM-0.7 specification this udi() method IS the HAL 
udi for the device, although it is not valid in Solid::Device for instance (I 
have tested using solid-hardware command).
 
> I was going to ask for a 4.5 feature freeze exemption but I'll wait until
> 4.6 if you are reimplmeenting.

	It is better wait, even now I have implemented only one third of the 
specification, the one third usefull for 2G/3G modems in general. There are 
other parts of the specification like location, cdma, contacts, sms, hso and 
ussd that do not work with my 3G modem so I postponed them.
 
> Let's discuss the redesign in more detail on the kde-networkmanager list.

	Ok. I have already converted the interface 
(kdebase/workspace/libs/solid/control/*) to be used as backend and reorganized 
parts of the source code to be more like solid NM interface. I still need to 
create a kdebase/workspace/solid/modemmanager-0.4 directory and move the MM 
stuff from kdebase/workspace/solid/networkmanager-0.7 to there.

	Another thing to discuss is implementing NM-0.8 specification, there is 
at least one method there that can be usefull in Plasma NM. Currently when 
someone tries to disconnect an "auto-connect" connection Plasma NM reconnects 
the interface afterwards, I think the connection should stay disconnected in 
that case but NM-0.7 seems not to support that but NM-0.8 have a method to do 
that (I think).
 
> > On 2010-05-26 22:03:54, Will Stephenson wrote:
> > > /trunk/KDE/kdebase/workspace/solid/networkmanager-0.7/networkgsminterfa
> > > ce.h, line 54
> > > <http://reviewboard.kde.org/r/3769/diff/6/?file=27216#file27216line54>
> > > 
> > >     Why did you make this change?
> > 
> > Lamarque Souza wrote:
> >     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.
> 
> The idea of the solid backend abstraction is to decouple us from
> Networkmanager so we can use other things like the Nokia QtMobility
> backend too. So I always put our own API consistency above following the
> NM APIs.

	Ok.
 
> > 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
> > 
> > Lamarque Souza wrote:
> >     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.
> 
> Use kdevelop's refactoring support to do this rename?

	Well actually I use gvim here :-) I used to use vim even before KDE was 
created, so I am used to it. I can do the rename by hand or create a PERL 
script to do that, this is not a big deal for me. Next time I send the patch 
to reviewboard it will have the enum names converted.
 
> > 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.
> > 
> > Lamarque Souza wrote:
> >     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.
> 
> Is this change in the networkmanagement patch here on reviewboard?  using
> interface names sounds un-portable.

	It is in networkmangement since yesterday, well the old method did not 
work at all here. solid-hardware list always returns udis different from NM's 
uni() and udi().
 
-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://www.kde-mg.org