Generic Sensor API : Ambient Light Sensor API.

Generic Sensor API : Ambient Light Sensor API.

[DO NOT LAND]: This CL is for comments.

This CL implements the new Ambient Light Sensor API
for chrome on android based on the
Generic Sensor API.

The ED for the Generic Sensor API is here -
https://w3c.github.io/sensors/

The ED for the Ambient Light Sensor API is here -
http://w3c.github.io/ambient-light/

The new way to use the ambient light sensor readings is -

var s = new AmbientLightSensor();
s.start();
s.onchange = event => console.log(event.reading.illuminance);
s.stop();

BUG=

[riju_, 2016-04-21 12:55:47]

Description was changed from

==========
Generic Sensor API : Ambient Light Sensor API.

[DO NOT LAND]: This CL is for comments.

This CL implements the new Ambient Light Sensor API
for chrome on android based on the
Generic Sensor API.

The ED for the Generic Sensor API is here -
https://w3c.github.io/sensors/

The ED for the Ambient Light Sensor API is here -
http://w3c.github.io/ambient-light/

The new way to use the ambient light sensor readings is -

var s = new AmbientLightSensor();
s.start();
sensor.onchange = event => console.log(event.reading.illuminance);
s.stop();

BUG=
==========

to

==========
Generic Sensor API : Ambient Light Sensor API.

[DO NOT LAND]: This CL is for comments.

This CL implements the new Ambient Light Sensor API
for chrome on android based on the
Generic Sensor API.

The ED for the Generic Sensor API is here -
https://w3c.github.io/sensors/

The ED for the Ambient Light Sensor API is here -
http://w3c.github.io/ambient-light/

The new way to use the ambient light sensor readings is -

var s = new AmbientLightSensor();
s.start();
s.onchange = event => console.log(event.reading.illuminance);
s.stop();

BUG=
==========

[riju_, 2016-04-21 12:55:47]

rijubrata.bhaumik@intel.com changed reviewers:
+ timvolodine@chromium.org

[timvolodine, 2016-05-06 17:05:41]

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensor.cpp (right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensor.cpp:119:
m_hasEventListener = false;
could this go to the Sensor superclass?

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensor.h (right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensor.h:66:
Member<AmbientLightSensorReading> m_lightReading;
does this mean we have two readings now? one from Sensor an this one

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensorDispatcher.cpp
(right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensorDispatcher.cpp:62:
Platform::current()->serviceRegistry()->connectToRemoteService(mojo::GetProxy(&m_lightSensor));
does this mean that for for each blink instance we would have a separate mojo
service? i.e. multiple registrations for 'light sensor' on the platform side. if
so, are there any issues with that?

[riju_, 2016-05-12 13:43:35]

Patchset #2 (id:20001) has been deleted

[riju_, 2016-05-16 16:58:52]

hi Tim, PTAL.

https://codereview.chromium.org/1892083002/diff/1/device/sensor/android/java/...
File
device/sensor/android/java/src/org/chromium/device/sensor/AmbientLightSensorImpl.java
(right):

https://codereview.chromium.org/1892083002/diff/1/device/sensor/android/java/...
device/sensor/android/java/src/org/chromium/device/sensor/AmbientLightSensorImpl.java:91:
public void onSensorChanged(SensorEvent event) {
TODO(riju) : Use event.timestamp here.

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensor.cpp (right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensor.cpp:119:
m_hasEventListener = false;
On 2016/05/06 17:05:41, timvolodine wrote:
> could this go to the Sensor superclass?

I could add the that to Sensor superclass, 
But then Sensor class should inherit PlatformEventController whose primary job
is to be a Base controller class for registering controllers with a dispatcher.

Concrete sensors should have their own Dispatchers, hence this design.

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensor.h (right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensor.h:66:
Member<AmbientLightSensorReading> m_lightReading;
On 2016/05/06 17:05:41, timvolodine wrote:
> does this mean we have two readings now? one from Sensor an this one

We can make SensorReading and SensorOptions private in Sensor to restrict
access?

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensorDispatcher.cpp
(right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensorDispatcher.cpp:62:
Platform::current()->serviceRegistry()->connectToRemoteService(mojo::GetProxy(&m_lightSensor));
On 2016/05/06 17:05:41, timvolodine wrote:
> does this mean that for for each blink instance we would have a separate mojo
> service? i.e. multiple registrations for 'light sensor' on the platform side.
if
> so, are there any issues with that?

Isn't it the case that even for multiple blink instance of concrete sensors,
there is 1 dispatcher per sensor type ?
I think there is 1 interface_ptr, i.e m_lightSensor for all concrete sensor of
ALS, and if that is bound (connection exists), then we skip registration.

[timvolodine, 2016-05-17 17:46:18]

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensor.h (right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensor.h:66:
Member<AmbientLightSensorReading> m_lightReading;
On 2016/05/16 16:58:51, riju_ wrote:
> On 2016/05/06 17:05:41, timvolodine wrote:
> > does this mean we have two readings now? one from Sensor an this one
> 
> We can make SensorReading and SensorOptions private in Sensor to restrict
> access?

I think the idea would be to use the member of Sensor to store lightReading,
otherwise what's the point of having a member in Sensor?

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/sensor/AmbientLightSensorDispatcher.cpp
(right):

https://codereview.chromium.org/1892083002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/sensor/AmbientLightSensorDispatcher.cpp:62:
Platform::current()->serviceRegistry()->connectToRemoteService(mojo::GetProxy(&m_lightSensor));
On 2016/05/16 16:58:51, riju_ wrote:
> On 2016/05/06 17:05:41, timvolodine wrote:
> > does this mean that for for each blink instance we would have a separate
mojo
> > service? i.e. multiple registrations for 'light sensor' on the platform
side.
> if
> > so, are there any issues with that?
> 
> Isn't it the case that even for multiple blink instance of concrete sensors,
> there is 1 dispatcher per sensor type ?
> I think there is 1 interface_ptr, i.e m_lightSensor for all concrete sensor of
> ALS, and if that is bound (connection exists), then we skip registration.

yeah sorry I wasn't clear, I meant there are multiple instances of mojo
implementations (i.e. sensorImpl in your case), but it looks like they all go to
a singleton SensorService anyway so you would not register multiple times on the
platform side. (Registering multiple times on platform side would probably work
as well, was just checking if there would be any issues with that). 

Looking ahead though looks like creating two ALS sensors in javascript with
different frequencies would be non-trivial to implement..

[riju_, 2016-05-19 13:25:30]

Patchset #4 (id:80001) has been deleted

[riju_, 2016-05-26 08:56:05]

Patchset #5 (id:120001) has been deleted

[riju_, 2016-05-26 11:24:33]

Patchset #5 (id:140001) has been deleted