Closed Bug 800533 Opened 7 years ago Closed 7 years ago

[unagi][orientation] unagi device has some orientation issues

Categories

(Firefox OS Graveyard :: General, defect, P1, critical)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-basecamp:+, firefox18 verified, firefox19 verified)

VERIFIED FIXED
blocking-basecamp +
Tracking Status
firefox18 --- verified
firefox19 --- verified

People

(Reporter: marcia, Assigned: gal)

Details

(Keywords: unagi, Whiteboard: [dogfooding-blocker])

Attachments

(1 file, 1 obsolete file)

unagi device using:

b2g revision="ca1f327d5acc198bb4be62fa51db2c039032c9ce
gaia" revision="58e2582ba479e4334fddfc2e759b474e74b7f52d
gecko" revision="e0fb02b979b3109057ceeedd2011516522d4ad29
gonk-misc"revision="8c3562dbbc63813fbb2c433a8714904d6cb17ae8

STR:
1. No exact STR, but it happens on the phone intermittently. Have seen it when loading a web page

Actual
1. Screen moves into all orientations on its own in a pulsating fashion.
STR was straight from reboot, no network connection, no setup, etc.
Keywords: steps-wanted
blocking-basecamp: --- → ?
launching :
Gallery, FM Radio, Marketplace, Email, and Video all have this same problem.
Turns out that this bug only occurs when you're offline and portrait mode.  online the orientation is fine.
I saw this last night when testing email. I got in several states where the phone just went crazy. I have a video and will upload it shortly. This is very bad when it happens.

BTW, for me it did not happen during launch. It happened later when I had added a second account. Last evening I was testing with this build:

## Environment :
Unagi phone, build 2012-10-11 us
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 0d6d050bc37d3167cc82a1885fd7660456bb0f4e
* "gaia" revision= 2667536e3b06e46dd193aa6d76ba08dad2d867be 
* "releases-mozilla-central" revision= c0ea0728044b2a5f9556ca988635140963cff037
* "gonk-misc" revision= 6ec34aa3d66331054de37eabc594ad923654b27c
Summary: [orientation] unagi device has some orientation issues → [unagi][orientation] unagi device has some orientation issues
This happens to me every time I open the browser, rendering it unusable.  I'm tempted to consider this a dogfooding blocker since it will significantly reduce our testers' ability to test many of the features of the phone - the orientation flips around so much that you can't a) stop it or b) still try to do anything useful.

Jonas - can we get someone assigned to looking into this?
Severity: normal → critical
Priority: -- → P1
I was under the impression that this mostly happened when you actually were rotating the device?

If this is happening constantly for the browser app, then I agree it's a dogfood blocker, but is that really the case?

cc'ing Dougt and mwu to see if either of them have any idea what's wrong with the gyroscope since that's where this problem is likely originating.
I think that's happening with any app that doesn't lock the orientation. When switching back to the homescreen, the orientation changes stop.
The screen rotation is depend on orientation sensor (virtual one combined by magnetic and acceleration sensors). 
1. May I know this bug is happened on which device?
2. Does Otoro have this bug too?

If I have device to reproduce it, I can check it first.
Ah, makes sense if this happens on any app which doesn't lock orientation.

Marco: I think this is only happening on the unagi device. Would you be able to get hold of one of those?
Can someone do me a favor for mailing me the device model name?
Thanks very much.
Whiteboard: [dogfooding-blocker]
Severity: critical → blocker
(In reply to Marco Chen [:mchen] from comment #11)
> Can someone do me a favor for mailing me the device model name?
> Thanks very much.

Marco, I've emailed you, and a few others, the specs of the Unagi.
Marco, assuming you're going to get a device soon, I'm assigning this to you :)  Fabrice said he could help if you don't get a device soon.
Assignee: nobody → mchen
Logging pitch and roll at http://mxr.mozilla.org/mozilla-central/source/widget/gonk/OrientationObserver.cpp#206 with the browser app opened while *not* moving my phone I get the following:

I/Gecko   (  110): Orientation pitch=-152.459991 roll=465.359985
I/Gecko   (  110): Orientation pitch=-54.484375 roll=2.187500
I/Gecko   (  110): Orientation pitch=-152.699997 roll=465.299988
I/Gecko   (  110): Orientation pitch=-54.718750 roll=2.125000
I/Gecko   (  110): Orientation pitch=-153.239990 roll=465.299988
I/Gecko   (  110): Orientation pitch=-54.718750 roll=2.125000
I/Gecko   (  110): Orientation pitch=-153.239990 roll=465.119995
I/Gecko   (  110): Orientation pitch=-54.656250 roll=2.187500
I/Gecko   (  110): Orientation pitch=-152.819992 roll=465.239990
I/Gecko   (  110): Orientation pitch=-54.328125 roll=2.187500
I/Gecko   (  110): Orientation pitch=-152.220001 roll=464.459991
I/Gecko   (  110): Orientation pitch=-54.546875 roll=2.296875
I/Gecko   (  110): Orientation pitch=-152.099991 roll=464.519989
I/Gecko   (  110): Orientation pitch=-54.593750 roll=2.093750
I/Gecko   (  110): Orientation pitch=-152.099991 roll=464.160004
I/Gecko   (  110): Orientation pitch=-54.859375 roll=1.937500
I/Gecko   (  110): Orientation pitch=-151.860001 roll=464.099976
I/Gecko   (  110): Orientation pitch=-54.750000 roll=2.093750
I/Gecko   (  110): Orientation pitch=-152.099991 roll=463.979980
I/Gecko   (  110): Orientation pitch=-54.671875 roll=2.125000
FWIW, I've been able to stabilize things by holding a magnet next to the phone. I guess it's some kind of problem in the sensor hal.
Attached patch patch (obsolete) — Splinter Review
This is more a workaround than a proper fix, but this makes the crazyness go away. Orientation changes are slower with this patch, so we'll want to back it out if we can get the low level sensor hal fixed.
Assignee: mchen → fabrice
Attachment #671601 - Flags: review?(mwu)
Attachment #671601 - Flags: feedback?
Jonas, would you have everyone on all teams drop what they're doing to fix this?
This looks like a very bad hack. The bug seems trivial here. We are oscillating around boundary values. E.g. the sensor returns 44 46 44 46 44 46 and we do > 45 on these. Instead, what we have to do is once we flip to 90 degrees (> 45), we have to stick to that until we substantially drop below 45 degrees. Does this make sense?
What I see is one out of two values is right, and the other is fully wrong.

When moving the phone, we get series like:
40
-120
41
-120
42
-120
43
-120

etc. It's not just oscillating around a boundary, at least not always.
Ok, so looks like every other value is wrong.
Is always the first value right? Maybe we should always read values twice? I bet you thats what android does thats why the vendor never saw this bug.
Just wondering, could it also have something to do with needing a delay to read the next value? ie, it's registering a wrong value because the sensor isn't ready in time for a reading?
Ok, the problem is that android doesn't use the ORIENTATION sensor type any more. There is a new ROTATION_VECTOR sensor that android uses instead, and the unagi has buggy support for the deprecated ORIENTATION sensor. We need to switch what sensor we use here.

http://androidxref.com/4.0.4/xref/frameworks/base/services/sensorservice/SensorService.cpp
(In reply to Andreas Gal :gal from comment #23)
> Ok, the problem is that android doesn't use the ORIENTATION sensor type any
> more. There is a new ROTATION_VECTOR sensor that android uses instead, and
> the unagi has buggy support for the deprecated ORIENTATION sensor. We need
> to switch what sensor we use here.
> 
> http://androidxref.com/4.0.4/xref/frameworks/base/services/sensorservice/
> SensorService.cpp

That doesn't work for now since we fail to find the sensor when trying to activate it at
http://mxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSensor.cpp#211
Sorry, critical is indeed enough. I had misunderstood the decisions from the last triage regarding how to mark dogfood blockers.
Severity: blocker → critical
the sensors we have are:
#define SENSOR_TYPE_ACCELEROMETER 1
#define SENSOR_TYPE_MAGNETIC_FIELD 2
#define SENSOR_TYPE_ORIENTATION 3
#define SENSOR_TYPE_LIGHT 5
#define SENSOR_TYPE_PROXIMITY 8
We will go back to the hack that ignores one out of two sensor reads. Lame.
I think we have to unblock here. The driver is simply insane. We need vendor help. mwu, can you reach out? We will try a hack to dampen the values, but we shouldn't wait on that.
Hi all,

As my experience, this bug might be related to calibration issue. Most sensor vendors need to do calibration for correcting the output of orientation sensor. Ex: The orientation sensor didn't output any value on ST sensors if calibration is not done.

My suggestion is
1. Verify this issue on unagi with Android version.
   (Note you need test with e-compass app because screen orientation is depend on acceleration sensor in Android not orientation.)
2. The normal calibration behavior is "figure-8" in the air.
3. Use the workaround first.
4. Refer to Bug 788975

I will contact with the chip partner for figuring out this issue. But I think it need time.
Ok, from playing with the values, I am pretty sure that the vendor driver is reading the events partially from the kernel event queue. The Z value is left in the queue, and is read as the next X value. The android orientation code is extremely robust and reacts less to the bad values because it uses different thresholds after a rotation whereas we just use the same boundary value.
I am pretty confident that #29 is wrong. We are seeing crazy values coming out of the sensor HAL. This is not a calibration issue.
Actually this might be a bug in our code.

E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.603339 z=9.720459
E/GonkSensor( 1702): i=0 type=2 x=8.280000 y=31.500000 z=-39.660000
E/GonkSensor( 1702): i=1 type=3 x=332.921875 y=3.500000 z=7.015625
E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.593762 z=9.720459
E/GonkSensor( 1702): i=0 type=2 x=8.220000 y=31.559999 z=-39.719997
E/GonkSensor( 1702): i=1 type=3 x=333.093750 y=3.421875 z=7.015625
E/GonkSensor( 1702): i=0 type=1 x=1.197101 y=-0.593762 z=9.720459
E/GonkSensor( 1702): i=0 type=1 x=1.197101 y=-0.593762 z=9.730036
E/GonkSensor( 1702): i=0 type=1 x=1.197101 y=-0.584185 z=9.730036
E/GonkSensor( 1702): i=0 type=2 x=8.220000 y=31.559999 z=-39.899998
E/GonkSensor( 1702): i=1 type=3 x=333.265625 y=3.328125 z=6.921875
E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.603339 z=9.720459
E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.603339 z=9.730036
E/GonkSensor( 1702): i=0 type=2 x=8.639999 y=31.619999 z=-40.320000
E/GonkSensor( 1702): i=1 type=3 x=333.375000 y=3.500000 z=7.015625

We are co-mingling some sensor types together. Only type 1 looks sane:

E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.603339 z=9.720459
E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.593762 z=9.720459
E/GonkSensor( 1702): i=0 type=1 x=1.197101 y=-0.593762 z=9.720459
E/GonkSensor( 1702): i=0 type=1 x=1.197101 y=-0.593762 z=9.730036
E/GonkSensor( 1702): i=0 type=1 x=1.197101 y=-0.584185 z=9.730036
E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.603339 z=9.720459
E/GonkSensor( 1702): i=0 type=1 x=1.206678 y=-0.603339 z=9.730036
Yep, we don't have support for the magnetic field sensor (type 2) and mix that in with orientation. Patch coming up.
Attached patch patchSplinter Review
Assignee: fabrice → gal
Attachment #671601 - Attachment is obsolete: true
Attachment #671601 - Flags: review?(mwu)
Attachment #671601 - Flags: feedback?
Attachment #671723 - Flags: review?(jones.chris.g)
I am no the right owner for this. Please anyone steal and land.
The attached patch fixes the orientation problems.
Attachment #671723 - Flags: review?(jones.chris.g) → review+
Once this lands on Aurora we'll want some QA on this to verify that we're in the clear as far as dogfooding blocker goes (ie: all default apps should be able to open and be interacted with without freaking out on orientation).
Keywords: qawanted, verifyme
Hi Andreas, 

It's great to hear this good news and I think Taipei still need to have at least one unagi device.

And I checked your patch. This patch is used to filter out the magnetic event first then wait for supporting later. 
But as I knew the event belonged to magnetic will be set to SENSOR_UNKNOWN at http://mxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSensor.cpp#114 . So the later calling on NotifySensorChange() will not send it because no one listened to SENSOR_UNKNOWN. According this reason there is no necessary to filter out magnetic event.
 
(another bug is at here http://mxr.mozilla.org/mozilla-central/source/hal/Hal.cpp#491 . Because SENSOR_UNKNOWN is -1 which will cause system crash when using gSensorObservers[-1])

So I guess the code at http://mxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSensor.cpp#115 is the place which changed SENSOR_UNKNOWN back to SENSOR_ORIENTATION. I didn't have device so maybe Andreas can help to check this. Thanks.
Marco, we will have some Unagi soon.
Marco, feel free to do a follow up patch for this. I will close this bug for now so we can get dogfooding started.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Verified fixed - I am no longer seeing these orientation issues on my device.
Keywords: qawanted, verifyme
Status: RESOLVED → VERIFIED
QA Contact: mozillamarcia.knous
You need to log in before you can comment on or make changes to this bug.