[Patch] add serial devices to solid

Harald Fernengel harald at trolltech.com
Tue Jan 20 08:29:46 GMT 2009


Hi,

thanks Richard and Kevin for the review. Comments below...

On Monday 19 January 2009 22:48:33 Richard Moore wrote:
> On Mon, Jan 19, 2009 at 9:15 PM, Kevin Ottens <ervin at kde.org> wrote:
> > On Monday 19 January 2009 21:50:19 Harald Fernengel wrote:
> > Only two things bug me API wise:
> >  * QString ifaceName()
> >   For this kind of things we use QVariant handle() (or something similar)
> > at other places of the API, would this one qualify for such a move?
>
> I'd change ifaceName() etc. to interfaceName() there's no need to
> abbreviate it and using the full word is clearer.

I agree, the reason I chose it was copy/paste from networkinterface.h :)

looking at the other interfaces again, it seems that "QVariant driverHandle() 
const;" would be the consistent choice. I'll add a comment to change 
NetworkInterface::ifaceName() to interfaceName() for KDE 5 ;)

>
> >  * QString serialType()
> >   I'd really like this one use an enum instead, that's what we have
> >   everywhere else in the API for this kind of cases.
>
> I agree with Kevin that an enum would be good here. With Q_ENUM it's
> easy to convert to a string if required.

Trouble is that the HAL spec doesn't specify what serialType() will really be 
- from the spec, it looked more like a readable string to me so you can 
distinguish your USB->serial converter from the built-in serial ports.

> I'm also a bit worried about  virtual int port() const; Doesn't this
> only make sense for RS232 style serial ports?

Again, stolen from the HAL spec. Agree that it only makes sense for the legacy 
serial ports, but there it's key to know the port number. How often did I try 
to send AT commands to my serial mouse instead of my modem in the good old 
days? :)

Harald




More information about the kde-core-devel mailing list