>-----Original Message-----
>From: Yan Leo [mailto:[email protected]] 
>Sent: Thursday, September 16, 2010 5:55 AM
>To: Rongas Timo
>Cc: [email protected]
>Subject: [patch] ALS and Proximity Adaptors of sensor 
>framework on Medfield
>
>Hi Timo,
>I think we can use the basic input device
>accelerometer(adaptors/accelerometeradaptor) on Medfield.
>
>Here are the patches for ALS and Proximity. Both of them are different
>from the basic adaptors because of the output format of driver.Please
>check and give your opinion.

Hi Leo,

These look good! I actually think that these can be generalised for use by 
others as well. If possible, could you do the following modifications please:

* Rename from mfldXXadaptor to XXadaptor-ascii. If another driver with ascii 
interface appears, these plugins can probably be patched to support both. We 
came up with this <pluginname>-<interfacetype>-[<additional detail>] naming 
scheme a while back, but I haven't written it down anywhere yet.

* Add LGPL disclaimer (copy from any other file and place your credentials 
there)

* Use smaller buffersize. This is an evolving issue, but having a buffer of 
e.g. 16 samples should be more than enough for proximity and als. Perhaps this 
gets automatic someday.

* Use sensordLog<T|D|W|C>() for errors. I would consider being unable to open a 
driver interface worth a warning. I know, our code might be slacky on this in 
parts as well, but I can try to prevent any new qDebugs() from seeping in ;)

* Please use spaces for indent (not tabs)

* This final point is actually something that should be turned into a 
configuration option (more of a note than request, my job to implement):

Can your drivers provide sane data when the display is blanked? In both 
adaptors, you have overwritten the default setStandbyOverride() to always 
reject requests to keep sensors on while display is off. I think you should 
allow (*crossing fingers for users thinking about power saving*) users to keep 
the sensors on with display off, if it's possible. For various reasons, the 
"default" als adaptor does not allow display off usage.

But as said, that should be turned into option, so the same plugin could use 
different behavior upon different drivers. 

Providing these as generic plugins for cases where data is read as ascii might 
need some more stuff (paths etc) to go into config file. But those mods can be 
done when the need arises.

Thank you!

// Timo

-- 
Timo Rongas
Project Manager

Digia Plc
Hämeentie 135 A, FI-00350 HELSINKI, FINLAND
Email: [email protected]
Gsm: +358 50 344 1529 Fax: +358 40 307 4089
Visit us at: www.digia.com
------------------------------------------------------------------
PRIVACY AND CONFIDENTIALITY NOTICE
This message and any attachments are intended only for use by 
the named addressee and may contain privileged and/or 
confidential information. If you are not the named addressee 
you should not disseminate, copy or take any action in 
reliance on it. If you have received this message in error, 
please contact the sender immediately and delete the message 
and any attachments accompanying it. Digia Plc does not accept 
liability for any corruption, interception, amendment, 
tampering or viruses occurring to this message.
------------------------------------------------------------------
_______________________________________________
MeeGo-dev mailing list
[email protected]
http://lists.meego.com/listinfo/meego-dev

Reply via email to