Closed Bug 734324 Opened 12 years ago Closed 12 years ago

implement device motion - rotation rate and acceleration

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: dougt, Assigned: dougt)

Details

Attachments

(1 file)

Attached patch patch v.1Splinter Review
This patch does a few things:

1) uses TYPE_LINEAR_ACCELERATION to provide acceleration data without gravity.  An alternative is to use a low pass filter, but I suppose if Android provides this type, we should use it

2) uses TYPE_GYROSCOPE to provide rate of rotation

3) removal of the ACCELERATION_EVENT event.  Instead, we can use one event type and just use the mFlags to determine what kind of sensor the event is about.
Attachment #604308 - Flags: review?(josh)
Comment on attachment 604308 [details] [diff] [review]
patch v.1

Review of attachment 604308 [details] [diff] [review]:
-----------------------------------------------------------------

This looks ok to me. You may want to have someone else more familiar with the Java side look things over as well; I gave it the same level as scrutiny as the rest, but it's my first time seeing it.

::: dom/system/nsDeviceMotion.cpp
@@ +247,5 @@
>        nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(mWindowListeners[i]);
> +      if (type == nsIDeviceMotionData::TYPE_ACCELERATION || 
> +          type == nsIDeviceMotionData::TYPE_LINEAR_ACCELERATION || 
> +          type == nsIDeviceMotionData::TYPE_GYROSCOPE )
> +          FireDOMMotionEvent(domdoc, target, type, x, y, z);

2-space indent.

::: mobile/android/base/GeckoAppShell.java
@@ +106,5 @@
>      static public final int WPL_STATE_STOP = 0x00000010;
>      static public final int WPL_STATE_IS_DOCUMENT = 0x00020000;
>      static public final int WPL_STATE_IS_NETWORK = 0x00040000;
>  
> +    static public final int DEFAULT_SENSOR_POLL_RATE = 100;

This field isn't used.

::: mobile/android/base/GeckoEvent.java
@@ +299,5 @@
>              event.mY = s.values[1];
>              event.mZ = s.values[2];
>              break;
> +
> +        case 10 /*Sensor.TYPE_LINEAR_ACCELEROMETER*/ :

What's the story behind this?

@@ +325,4 @@
>              break;
>  
>          case Sensor.TYPE_PROXIMITY:
> +            // XXX maybe we can get rid of this event.  is

File a bug, add a number.

::: widget/android/AndroidBridge.cpp
@@ +312,5 @@
>  AndroidBridge::EnableDeviceMotion(bool aEnable)
>  {
>      ALOG_BRIDGE("AndroidBridge::EnableDeviceMotion");
>  
> +    // todo, we probably can make this finer grain based on

File a bug, add a number.

::: xpcom/system/nsIDeviceMotion.idl
@@ +37,5 @@
>  #include "nsISupports.idl"
>  
>  interface nsIDOMWindow;
>  
> +[scriptable, uuid(C9CC22EB-F8A8-4025-825E-5F0C0FF9A8EC)]

No vtable changes, so no uuid update necessary.
Attachment #604308 - Flags: review?(josh) → review+
https://hg.mozilla.org/mozilla-central/rev/fdb8a781820a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.