Index: content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java |
diff --git a/content/public/android/java/src/org/chromium/content/browser/DeviceOrientation.java b/content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java |
similarity index 48% |
rename from content/public/android/java/src/org/chromium/content/browser/DeviceOrientation.java |
rename to content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java |
index a77cd28b27ab6901f6a12f7a2e4e012b3e7c8f71..6a09ee016f4788b6e9bb12aca09ed35ac8fea4d3 100644 |
--- a/content/public/android/java/src/org/chromium/content/browser/DeviceOrientation.java |
+++ b/content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java |
@@ -1,4 +1,4 @@ |
-// Copyright (c) 2012 The Chromium Authors. All rights reserved. |
+// Copyright (c) 2013 The Chromium Authors. All rights reserved. |
// Use of this source code is governed by a BSD-style license that can be |
// found in the LICENSE file. |
@@ -12,17 +12,23 @@ import android.hardware.SensorManager; |
import android.os.Handler; |
import android.os.Looper; |
+import com.google.common.collect.ImmutableMap; |
+import com.google.common.collect.ImmutableSet; |
+import com.google.common.collect.Sets; |
+ |
import org.chromium.base.CalledByNative; |
import org.chromium.base.JNINamespace; |
import org.chromium.base.WeakContext; |
import java.util.List; |
+import java.util.Map; |
+import java.util.Set; |
/** |
- * Android implementation of the DeviceOrientation API. |
+ * Android implementation of the device motion and orientation API. |
Peter Beverloo
2013/03/12 17:11:22
nit: s/API/APIs/.
timvolodine
2013/03/13 12:32:24
Done.
|
*/ |
@JNINamespace("content") |
-class DeviceOrientation implements SensorEventListener { |
+class DeviceMotionAndOrientation implements SensorEventListener { |
// These fields are lazily initialized by getHandler(). |
private Thread mThread; |
@@ -44,14 +50,56 @@ class DeviceOrientation implements SensorEventListener { |
// The geomagnetic vector expressed in the body frame. |
private float[] mMagneticFieldVector; |
+ // The linear acceleration vector expressed in the body frame. |
+ private float[] mAccelerationVector; |
+ |
+ // Gyroscope |
Peter Beverloo
2013/03/12 17:11:22
"The angular speed at which the device is rotating
timvolodine
2013/03/13 12:32:24
Done.
|
+ private float[] mGyroVector; |
Peter Beverloo
2013/03/12 17:11:22
Let's spell out the full name: mGyroscopeVector.
timvolodine
2013/03/13 12:32:24
Done.
|
+ |
// Lazily initialized when registering for notifications. |
private SensorManager mSensorManager; |
// The only instance of that class and its associated lock. |
- private static DeviceOrientation sSingleton; |
+ private static DeviceMotionAndOrientation sSingleton; |
private static Object sSingletonLock = new Object(); |
- private DeviceOrientation() { |
+ /** |
+ * constants for using in JNI calls, also see |
+ * content/browser/device_orientation/data_fetcher_impl_android.cc |
+ * @see #start |
+ * @see #stop |
+ */ |
+ private static enum EventType { |
Miguel Garcia
2013/03/12 17:00:26
what happened to my comment about not using an enu
Peter Beverloo
2013/03/12 17:11:22
Please use int constants instead of enums (enums i
timvolodine
2013/03/13 12:32:24
sorry seem to have missed that while uploading new
timvolodine
2013/03/13 12:32:24
Done.
|
+ DEVICE_ORIENTATION(0), |
+ DEVICE_MOTION(1); |
Miguel Garcia
2013/03/12 17:00:26
test?
timvolodine
2013/03/13 12:32:24
hmm not sure, is this N/A anymore (?)
|
+ |
+ public int value; |
+ private EventType(int value) { |
+ this.value = value; |
+ } |
+ } |
+ |
+ private static ImmutableSet<Integer> DEVICE_ORIENTATION_SENSORS = |
+ ImmutableSet.of(Sensor.TYPE_ACCELEROMETER, |
+ Sensor.TYPE_MAGNETIC_FIELD); |
+ |
+ private static ImmutableSet<Integer> DEVICE_MOTION_SENSORS = |
+ ImmutableSet.of(Sensor.TYPE_ACCELEROMETER, |
+ Sensor.TYPE_LINEAR_ACCELERATION, |
+ Sensor.TYPE_GYROSCOPE); |
+ |
+ private Set<Integer> mActiveSensors = Sets.newHashSet(); |
+ |
+ private static Map<Integer, ImmutableSet<Integer>> SENSORS_FOR_EVENT = ImmutableMap.of( |
+ EventType.DEVICE_ORIENTATION.value, DEVICE_ORIENTATION_SENSORS, |
+ EventType.DEVICE_MOTION.value, DEVICE_MOTION_SENSORS); |
Peter Beverloo
2013/03/12 17:11:22
For line 82 to 95: I think this is much too verbos
timvolodine
2013/03/13 12:32:24
Done.
|
+ |
+ // keep an array of active event types for speed because it is used in @see # |
Miguel Garcia
2013/03/12 17:00:26
@see # ?
where is it used?
Peter Beverloo
2013/03/12 17:11:22
nit: Grammar. You seem to be missing a reference h
timvolodine
2013/03/13 12:32:24
Done.
timvolodine
2013/03/13 12:32:24
Done.
|
+ private boolean[] mIsSpecEventActive = new boolean[EventType.values().length]; |
Peter Beverloo
2013/03/12 17:11:22
As I mentioned in the native code review, please d
timvolodine
2013/03/13 12:32:24
Done.
|
+ |
+ private DeviceMotionAndOrientation() { |
+ mIsSpecEventActive[EventType.DEVICE_ORIENTATION.value] = false; |
+ mIsSpecEventActive[EventType.DEVICE_MOTION.value] = false; |
} |
Miguel Garcia
2013/03/12 17:00:26
can you do this in a for loop so we don't forget t
Peter Beverloo
2013/03/12 17:11:22
I think we should just have two separate members.
timvolodine
2013/03/13 12:32:24
Done.
timvolodine
2013/03/13 12:32:24
Done.
|
/** |
@@ -61,33 +109,53 @@ class DeviceOrientation implements SensorEventListener { |
* @param nativePtr Value to pass to nativeGotOrientation() for each event. |
* @param rateInMilliseconds Requested callback rate in milliseconds. The |
* actual rate may be higher. Unwanted events should be ignored. |
+ * @param specEventType type of event to listen to, can either 0 for device |
Peter Beverloo
2013/03/12 17:11:22
nit: grammar (capital). s/can either/can be either
timvolodine
2013/03/13 12:32:24
Done.
|
+ * orientation, or 1 for device motion. |
* @return True on success. |
*/ |
@CalledByNative |
- public boolean start(int nativePtr, int rateInMilliseconds) { |
+ public boolean start(int nativePtr, int eventType, int rateInMilliseconds) { |
+ Set<Integer> sensorsForEvent = SENSORS_FOR_EVENT.get(eventType); |
+ if (sensorsForEvent == null) |
+ return false; |
+ |
synchronized (mNativePtrLock) { |
- stop(); |
- if (registerForSensors(rateInMilliseconds)) { |
+ Set<Integer> sensorsToActivate = Sets.newHashSet(sensorsForEvent); |
+ sensorsToActivate.removeAll(mActiveSensors); |
+ boolean success = registerForSensors(sensorsToActivate, rateInMilliseconds); |
+ if (success) { |
mNativePtr = nativePtr; |
- return true; |
+ mIsSpecEventActive[eventType] = true; |
} |
- return false; |
+ return success; |
} |
} |
/** |
- * Stop listening for sensor events. Always succeeds. |
+ * Stop listening for particular sensor events. Always succeeds. |
* |
+ * @param specEventType type of event to listen to, can either 0 for device |
Peter Beverloo
2013/03/12 17:11:22
nit: dito to line 112.
timvolodine
2013/03/13 12:32:24
Done.
|
+ * orientation, or 1 for device motion. |
* We strictly guarantee that nativeGotOrientation() will not be called |
* after this method returns. |
*/ |
@CalledByNative |
- public void stop() { |
+ public void stop(int eventType) { |
+ if (SENSORS_FOR_EVENT.get(eventType) == null) |
+ return; |
+ Set<Integer> sensorsToRemainActive = Sets.newHashSet(); |
+ |
synchronized (mNativePtrLock) { |
- if (mNativePtr != 0) { |
- mNativePtr = 0; |
- unregisterForSensors(); |
+ for (EventType event : EventType.values()) { |
+ if (event.value != eventType && mIsSpecEventActive[event.value]) { |
+ sensorsToRemainActive.addAll(SENSORS_FOR_EVENT.get(event.value)); |
+ } |
} |
+ Set<Integer> sensorsToDeactivate = Sets.newHashSet(mActiveSensors); |
+ sensorsToDeactivate.removeAll(sensorsToRemainActive); |
+ unregisterForSensors(sensorsToDeactivate); |
+ mNativePtr = 0; |
+ mIsSpecEventActive[eventType] = false; |
} |
} |
@@ -98,12 +166,39 @@ class DeviceOrientation implements SensorEventListener { |
@Override |
public void onSensorChanged(SensorEvent event) { |
+ boolean dispatchDeviceOrientation = |
Miguel Garcia
2013/03/12 17:00:26
I would change this a bit so that the switch just
timvolodine
2013/03/13 12:32:24
Done.
|
+ mIsSpecEventActive[EventType.DEVICE_ORIENTATION.value]; |
+ boolean dispatchDeviceMotion = |
+ mIsSpecEventActive[EventType.DEVICE_MOTION.value]; |
Peter Beverloo
2013/03/12 17:11:22
When we have two separate booleans for the events,
timvolodine
2013/03/13 12:32:24
Done.
|
+ |
switch (event.sensor.getType()) { |
case Sensor.TYPE_ACCELEROMETER: |
if (mGravityVector == null) { |
mGravityVector = new float[3]; |
} |
System.arraycopy(event.values, 0, mGravityVector, 0, mGravityVector.length); |
+ if (dispatchDeviceMotion) |
Peter Beverloo
2013/03/12 17:11:22
nit: Please include brackets for readability. The
timvolodine
2013/03/13 12:32:24
Done.
|
+ gotAccelerationIncludingGravity(mGravityVector[0], mGravityVector[1], |
+ mGravityVector[2]); |
+ break; |
+ case Sensor.TYPE_LINEAR_ACCELERATION: |
+ if (mAccelerationVector == null) { |
Peter Beverloo
2013/03/12 17:11:22
We're not using mAccelerationVector anywhere outsi
timvolodine
2013/03/13 12:32:24
Done.
|
+ mAccelerationVector = new float[3]; |
+ } |
+ System.arraycopy(event.values, 0, mAccelerationVector, 0, |
+ mAccelerationVector.length); |
+ if (dispatchDeviceMotion) |
Peter Beverloo
2013/03/12 17:11:22
Why do we have this if statement here? We'll only
timvolodine
2013/03/13 12:32:24
Done.
|
+ gotAcceleration(mAccelerationVector[0], mAccelerationVector[1], |
+ mAccelerationVector[2]); |
+ break; |
Miguel Garcia
2013/03/12 17:00:26
extra blank line
timvolodine
2013/03/13 12:32:24
Done.
|
+ |
+ case Sensor.TYPE_GYROSCOPE: |
Peter Beverloo
2013/03/12 17:11:22
This whole block: dito to the comments on line 185
timvolodine
2013/03/13 12:32:24
Done.
|
+ if (mGyroVector == null) { |
+ mGyroVector = new float[3]; |
+ } |
+ System.arraycopy(event.values, 0, mGyroVector, 0, mGyroVector.length); |
+ if (dispatchDeviceMotion) |
+ gotRotationRate(mGyroVector[0], mGyroVector[1], mGyroVector[2]); |
break; |
case Sensor.TYPE_MAGNETIC_FIELD: |
@@ -119,7 +214,8 @@ class DeviceOrientation implements SensorEventListener { |
return; |
} |
- getOrientationUsingGetRotationMatrix(); |
+ if (dispatchDeviceOrientation) |
Miguel Garcia
2013/03/12 17:00:26
I think the style guide says to use brackets on al
Peter Beverloo
2013/03/12 17:11:22
nit: dito to the comment on line 180.
timvolodine
2013/03/13 12:32:24
Done.
timvolodine
2013/03/13 12:32:24
Done.
|
+ getOrientationUsingGetRotationMatrix(); |
} |
void getOrientationUsingGetRotationMatrix() { |
@@ -163,15 +259,6 @@ class DeviceOrientation implements SensorEventListener { |
gotOrientation(alpha, beta, gamma); |
} |
- boolean registerForSensors(int rateInMilliseconds) { |
- if (registerForSensorType(Sensor.TYPE_ACCELEROMETER, rateInMilliseconds) |
- && registerForSensorType(Sensor.TYPE_MAGNETIC_FIELD, rateInMilliseconds)) { |
- return true; |
- } |
- unregisterForSensors(); |
- return false; |
- } |
- |
private SensorManager getSensorManager() { |
if (mSensorManager != null) { |
return mSensorManager; |
@@ -180,12 +267,28 @@ class DeviceOrientation implements SensorEventListener { |
return mSensorManager; |
} |
- void unregisterForSensors() { |
- SensorManager sensorManager = getSensorManager(); |
- if (sensorManager == null) { |
- return; |
+ private boolean registerForSensors(Iterable<Integer> sensorTypes, int rateInMilliseconds) { |
+ for (Integer sensorType : sensorTypes) { |
+ if (!mActiveSensors.contains(sensorType)) { |
+ if (!registerForSensorType(sensorType, rateInMilliseconds)) { |
+ // restore the previous state upon failure |
+ unregisterForSensors(sensorTypes); |
Peter Beverloo
2013/03/12 17:11:22
Won't this fail if device orientation registered f
timvolodine
2013/03/13 12:32:24
this method is always called from start() with a l
|
+ return false; |
+ } |
+ } |
+ mActiveSensors.add(sensorType); |
+ } |
+ return true; |
+ } |
+ |
+ private void unregisterForSensors(Iterable<Integer> sensorTypes) { |
+ for (Integer sensorType : sensorTypes) { |
+ if (mActiveSensors.contains(sensorType)) { |
+ getSensorManager().unregisterListener(this, |
+ getSensorManager().getDefaultSensor(sensorType)); |
+ mActiveSensors.remove(sensorType); |
+ } |
} |
- sensorManager.unregisterListener(this); |
} |
boolean registerForSensorType(int type, int rateInMilliseconds) { |
@@ -193,17 +296,14 @@ class DeviceOrientation implements SensorEventListener { |
if (sensorManager == null) { |
return false; |
} |
- List<Sensor> sensors = sensorManager.getSensorList(type); |
- if (sensors.isEmpty()) { |
+ Sensor defaultSensor = sensorManager.getDefaultSensor(type); |
Peter Beverloo
2013/03/12 17:11:22
I looked into doing this change myself, but I'm a
timvolodine
2013/03/13 12:32:24
ok, so should I revert to the previous solution wi
Peter Beverloo
2013/03/13 14:02:47
I do think we should revert until we properly unde
Peter Beverloo
2013/03/13 14:02:47
Can you file an issue to track looking in to switc
timvolodine
2013/03/13 15:29:49
Done.
timvolodine
2013/03/13 15:29:49
will do
|
+ if (defaultSensor == null) { |
return false; |
} |
final int rateInMicroseconds = 1000 * rateInMilliseconds; |
- // We want to err on the side of getting more events than we need. |
- final int requestedRate = rateInMicroseconds / 2; |
- |
- // TODO(steveblock): Consider handling multiple sensors. |
- return sensorManager.registerListener(this, sensors.get(0), requestedRate, getHandler()); |
+ return sensorManager.registerListener(this, defaultSensor, rateInMicroseconds, |
+ getHandler()); |
} |
void gotOrientation(double alpha, double beta, double gamma) { |
@@ -214,6 +314,30 @@ class DeviceOrientation implements SensorEventListener { |
} |
} |
+ void gotAcceleration(double x, double y, double z) { |
Miguel Garcia
2013/03/12 17:00:26
I wonder if we could not simplify this by having o
Miguel Garcia
2013/03/12 17:00:26
can these methods be private or public if meant to
Peter Beverloo
2013/03/12 17:11:22
We cannot be sure in regards to which sensors we c
Peter Beverloo
2013/03/12 17:11:22
They should be private.
timvolodine
2013/03/13 12:32:24
Done.
timvolodine
2013/03/13 12:32:24
Done.
timvolodine
2013/03/13 12:32:24
Done.
timvolodine
2013/03/13 12:32:24
Done.
|
+ synchronized (mNativePtrLock) { |
+ if (mNativePtr != 0) { |
+ nativeGotAcceleration(mNativePtr, x, y, z); |
+ } |
+ } |
+ } |
+ |
+ void gotAccelerationIncludingGravity(double x, double y, double z) { |
+ synchronized (mNativePtrLock) { |
+ if (mNativePtr != 0) { |
+ nativeGotAccelerationIncludingGravity(mNativePtr, x, y, z); |
+ } |
+ } |
+ } |
+ |
+ void gotRotationRate(double alpha, double beta, double gamma) { |
+ synchronized (mNativePtrLock) { |
+ if (mNativePtr != 0) { |
+ nativeGotRotationRate(mNativePtr, alpha, beta, gamma); |
+ } |
+ } |
+ } |
+ |
private Handler getHandler() { |
synchronized (mHandlerLock) { |
// If we don't have a background thread, start it now. |
@@ -251,19 +375,42 @@ class DeviceOrientation implements SensorEventListener { |
} |
@CalledByNative |
- private static DeviceOrientation getInstance() { |
+ private static DeviceMotionAndOrientation getInstance() { |
synchronized (sSingletonLock) { |
if (sSingleton == null) { |
- sSingleton = new DeviceOrientation(); |
+ sSingleton = new DeviceMotionAndOrientation(); |
} |
return sSingleton; |
} |
} |
/** |
- * See chrome/browser/device_orientation/data_fetcher_impl_android.cc |
+ * Native JNI calls, |
+ * see content/browser/device_orientation/data_fetcher_impl_android.cc |
*/ |
private native void nativeGotOrientation( |
int nativeDataFetcherImplAndroid, |
double alpha, double beta, double gamma); |
-} |
+ |
+ /** |
+ * linear acceleration w/o gravity |
Peter Beverloo
2013/03/12 17:11:22
nit: Please use proper grammar and spell out "with
timvolodine
2013/03/13 12:32:24
Done.
|
+ */ |
+ private native void nativeGotAcceleration( |
+ int nativeDataFetcherImplAndroid, |
+ double x, double y, double z); |
+ |
+ /** |
+ * acceleration including gravity |
Peter Beverloo
2013/03/12 17:11:22
nit: dito.
timvolodine
2013/03/13 12:32:24
Done.
|
+ */ |
+ private native void nativeGotAccelerationIncludingGravity( |
+ int nativeDataFetcherImplAndroid, |
+ double x, double y, double z); |
+ |
+ /** |
+ * gyroscope |
Peter Beverloo
2013/03/12 17:11:22
nit: dito.
timvolodine
2013/03/13 12:32:24
Done.
|
+ */ |
+ private native void nativeGotRotationRate( |
+ int nativeDataFetcherImplAndroid, |
+ double alpha, double beta, double gamma); |
+ |
+} |