Review Request: Make Network Manager show signal strength in system tray also for mobile broadband connections

[Kai Uwe Broulik]

> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 198
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line198>
> >
> >     remove extra space.

Yes, sorry for the excessive white-space, I just forgot about that :) I will fix all of those ;)


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 238
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line238>
> >
> >     use the variable accesstechnology instead of calling this method again.

That thing was just there for debugging purpose, forgot to remove it.


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 248
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line248>
> >
> >     optimization: remove this else clause and just add 'return QString("phone");' incondicionally.

okay.


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 249
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line249>
> >
> >     I do not think there is a "phone" element in the network.svgz. It must be added for this to work, unless Plasma::Svg is smart enough to search for pixmaps when there is no element.

Nope, there is not. I will make up something for this case ;)


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 291
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line291>
> >
> >     This test is required to prevent crashes with CDMA phones.

okay.


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 292
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line292>
> >
> >     signal signalQualityChanged passes an uint variable, not int. The QObject::connect does not work because of that.

ah, thanks. Will change that.


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > applet/networkmanager.cpp, line 293
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38580#file38580line293>
> >
> >     accessTechnologyChanges passes a variable of type Solid::Control::ModemInterface::AccessTechnology, not int. The QObject::connect does not work because of that.


> On Oct. 12, 2011, 3:46 p.m., Lamarque Vieira Souza wrote:
> > libs/internals/uiutils.cpp, line 125
> > <http://git.reviewboard.kde.org/r/102827/diff/1/?file=38581#file38581line125>
> >
> >     This comment is not necessary, remove it. Uutils::iconName() is for pixmap icons only, not svg.

Okay was just thinking because the WiFi icon also has various icons for the signal strength as non-svg as well, but okay, I will just leave the "phone" here.


- Kai Uwe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102827/#review7264
-----------------------------------------------------------


On Oct. 11, 2011, 4:05 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102827/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2011, 4:05 p.m.)
> 
> 
> Review request for Network Management.
> 
> 
> Description
> -------
> 
> As I now use a Surfstick to get to the internet, I found it handy to see the signal strength right in system tray like WiFi connections do, rather than opening the popup all the time.
> This patch resolves this issue by making the system tray show the signal strength as well as the access technology (Edge, umts, etc) in a similar way like Plasma Active's (fake?) mobilesignal widget does, where I also borrowed the graphics from :P
> I currently only respected the Plasma SVG icon case, i.e. when you do not use SVG graphics, it uses the generic phone icon still, as we do not yet have a icon naming spec for such cases and do not have icons either, so the tooltip and popup have the generic phone icon.
> 
> I added the HSDPA and 3G strength icons to the network.svgz icon file from network manager. See attachments.
> 
> 
> This addresses bug 283776.
>     http://bugs.kde.org/show_bug.cgi?id=283776
> 
> 
> Diffs
> -----
> 
>   applet/networkmanager.cpp 73a9cf4 
>   libs/internals/uiutils.cpp f90e44e 
> 
> Diff: http://git.reviewboard.kde.org/r/102827/diff/diff
> 
> 
> Testing
> -------
> 
> Compiles and works okay.
> It is not yet perfect but I think it is a good start.
> 
> Known issues:
>  - The strength indicator is only updated if you click (hover?) the system tray icon. The konsole output states that the signals "Solid::Control::ModemGsmNetworkInterface::signalQualityChanged()" do not exist (but I don't know why?) and thus not being triggered
>  - When not connected, there is either no icon at all (I wanted to use the no-wifi connected icon as fallback) or the 3G with 50% strength (this is weird)
> 
> 
> Screenshots
> -----------
> 
> HSDPA tray icon
>   http://git.reviewboard.kde.org/r/102827/s/289/
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20111012/ae3197ca/attachment-0001.html>