[Kde-hardware-devel] Re: Review Request: Fill in vendor and model names using Solid's udev backend when possible

[Dario Freddi]

> On May 2, 2011, 10:50 a.m., Dario Freddi wrote:
> > solid/solid/devicemanager.cpp, line 21
> > <http://git.reviewboard.kde.org/r/101270/diff/2/?file=15781#file15781line21>
> >
> >     This include should go below devicenotifier.h and devicemanager_p.h, ideally below all includes
> 
> Lamarque Vieira Souza wrote:
>     Any particular reason for that? Most people, including me, are used to add non-project includes earlier. This can prevents undesired side effects. Imagine QFile includes lines like: #ifdef DEFINE #define DEFINE2 int #else #define DEFINE2 long int #endif, and one of my project's header includes a line #define DEFINE 1, but my DEFINE has nothing to do with the one in QFile I can trigger a side effect. It is rare but possible.
> 
> Dario Freddi wrote:
>     Actually, it's the other way round :) Please read http://techbase.kde.org/Policies/Library_Code_Policy#Getting_.23includes_right for an insight on this issue
> 
> Lamarque Vieira Souza wrote:
>     That doc talks about compiler error, compiler error is easy to detect, side effects are not.

However, such things as duplicate defines should never happen, whereas compiler errors are not easy to work around if you export the headers (even if that is not the case). Moreover, KDE's coding policies clearly define the headers' order to be as mentioned in the doc - however I'll let Kevin or Alex decide on this matter


- Dario


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


On May 1, 2011, 11:27 p.m., Lamarque Vieira Souza wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101270/
> -----------------------------------------------------------
> 
> (Updated May 1, 2011, 11:27 p.m.)
> 
> 
> Review request for Solid, Kevin Ottens and Alex Fiestas.
> 
> 
> Summary
> -------
> 
> solid-hardware does not report my 3G modem's vendor and model names. This patch fix that:
> 
> [lamarque at evolucao ~]$ solid-hardware details /sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1
> udi = '/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1'
>   parent = '/org/kde/solid/udev'  (string)
>   vendor = 'Sony Ericsson'  (string)
>   product = 'Sony Ericsson MD300'  (string)
>   description = ''  (string)
>   Block.major = 189  (0xbd)  (int)
>   Block.minor = 175  (0xaf)  (int)
>   Block.device = '/dev/bus/usb/002/048'  (string)
> 
> The patch basically tests if /sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1 exists, if affirmative it creates a UDevDevice, which makes it possible to report the available device details instead of just ignoring them completely.
> 
> 'solid-hardware list' still does not list the device. I am still not sure if I should fix that since it would give the false impression the device is supported.
> 
> The motivation for this change is Plasma NM. The patch allows me inform modem's vendor and model names
> in Mobile Connection Wizard (when creating Gsm/Cdma connections) and in PinDialog (when requesting PIN unlock code from user). It is basically usability fix.
> 
> 
> Diffs
> -----
> 
>   solid/solid/backends/udev/udevdevice.cpp 4f34382 
>   solid/solid/backends/udev/udevmanager.cpp e08fcde 
>   solid/solid/devicemanager.cpp 59f32a7 
> 
> Diff: http://git.reviewboard.kde.org/r/101270/diff
> 
> 
> Testing
> -------
> 
> I have been using kdelibs-4.6.2 compiled with the patch since yesterday, not problems so far.
> 
> 
> Thanks,
> 
> Lamarque Vieira
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20110502/86344121/attachment-0001.htm