Last Comment Bug 734324 - implement device motion - rotation rate and acceleration
: implement device motion - rotation rate and acceleration
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Geolocation (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Doug Turner (:dougt)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-08 22:11 PST by Doug Turner (:dougt)
Modified: 2012-03-14 13:39 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v.1 (35.92 KB, patch)
2012-03-08 22:11 PST, Doug Turner (:dougt)
josh: review+
Details | Diff | Splinter Review

Description Doug Turner (:dougt) 2012-03-08 22:11:43 PST
Created attachment 604308 [details] [diff] [review]
patch v.1

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.
Comment 1 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-03-11 21:30:12 PDT
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.
Comment 3 Matt Brubeck (:mbrubeck) 2012-03-14 10:09:13 PDT
https://hg.mozilla.org/mozilla-central/rev/fdb8a781820a

Note You need to log in before you can comment on or make changes to this bug.