993107 - [B2G][NFC] In NFC usage, _reconfigureTimeout does not cancel dimming

Status: RESOLVED DUPLICATE of bug 1028776

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Garner Lee]

As per comment: https://bugzilla.mozilla.org/show_bug.cgi?id=975268#c17

ScreenManager's _reconfigScreenTimeout doesn't cancel and rebrighten the screen if dimming is already taking place.

In this bug, we can decide if this is correct UX design intent, or it needs to be fixed as inconsistent.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

If we simply want to stop screen from dimming when shrinking UI is shown, the code can simply require a wake lock from Gecko without talking to ScreenManager directly.

Siddartha, is this something you could handle? Thanks!

Comment 2

[psiddh]

We need to look at this

Comment 3

[psiddh]

Oops submitted the previous comment without completing it.

We should look at these usecases closely from the overall Nfc stack perspective. (Dimming of screen in Shrinking UI in one such scenario). 
- Other similar scenarios include screen dimming or turning off after you hold a tag in device's near field
- Similar issues may occur during P2P scenarios as well.

Maybe an optimal solution is to hold a wake lock that keeps the 'cpu on' through a 'timed partial wake lock'.
It will still allow the screen to dim and turn itself off.
 
This lock can be held between 'tech-discovered' (Create a wake lock when 'tech-discovered') & 'tech-lost' (release the wake lock when 'tech-lost') during an ongoing nfc session. Creation and releasing of wakelocks can be done either in Nfc.js (Gecko) or even in nfcd.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Siddartha, do you need UX involvement on defining the behavior? If so please needinfo them.

Comment 5

[psiddh]

I did some testing today on Android devices.
I think the current implementation on FxOS builds is fairly consistent with its Android counterpart.
Even on Android Nexus-4, when you hold a tag (or) a peer device the screen dims and turns itself off when the time-out occurs. Here is the scenario that I tested
1) Launch browser on two  different Android phones res
2) Bring them closer. Observe that UI shrinks on both android phones
3) Wait for screen to dim and time out (OR) proactively click h/w power button. Continue holding both phones together.
4) Then turn on the screen by clicking power button and unlock
5) Observe that both UIs on respective phones shrink again.

I tested the same scenario + (Passive Tag one) also with two FxOS builds and the behavior seems to be consistent to the one explained above. Note that I repeated the same experiment with one FxOS build and one Android phone and the behavior still remains the same.

In short,  the question is do we really want the screen from NOT dimming (and turning itself off) when you hold a tag / peer device in the device's near field? At-least Android seems to be turning the screen off. Also in terms of UX behaviour current NFC behavior seems to be in alignment with Android.

If we agree to keep the behavior as-is with that of Android, then I guess there is nothing much to do with the bug (Pls ignore my comment#3 in that case), unless I am missing something here.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

It looks like we do need UX to make the call here.

According to comment 5 we should amend the spec and close this bug as invalid if we want to keep the current behavior (If I am reading Siddartha correctly).

Comment 7

[Juwei Huang[:juwei]]

We should cancel dimming when NFC is pairing.
The reason I suggest so is that sometimes when people try to tap their phones back to back together, it may get a little bit busy for two people pushing their phones together and trying to rub the phone together in order to find where to activate NFC (since the sensor would be placed in different place of different phones). In such a long process, if the users set up their phone to screen timeout less than 10 seconds, it would be very bothersome for them to be interrupted by dimming screen.
And I also think it's no harm to let users do the pairing process until they do finish NFC file transfer, right?

If there's any other concerns, let me know :)

Comment 8

[Garner Lee]

To be clear, undimming is possible, it's just in a different timer (restore screen brightness according to local variable in screen manager), and animation. I removed it as per review comments pending UX design.

It's in the top few lines in screen manager:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/screen_manager.js#L387

If that's fine, the obvious thing to do is re-introduce a new function that is basically the same, but does undimming, and resetting a timer. For the "10 second" issue where neither user knows where the NFC antenna is, can we have a different "NFC timer class" for example?

Perhaps an ability to register timers that revert to the standard system timer after once use?

Wake lock:
A wake lock is a bad idea due to a NFC keyboard we had on hand to test it out (a wake lock will drain the battery due to the display).

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

What's the status here

Comment 10

[Garner Lee]

No update yet.

What may be of use is I noticed the phone in general (Flame) seems to time out at places, earlier than expected, not just with NFC. A more general solution/fix that ties it more directly into user initiated hardware events at a lower level that merely reports to Gaia's screen manager is worth discussing.

I would propose something like "screen-keep-alive", origin: "nfc", "application switch", etc. so the screen manager can still ignore by some kind of power profile/policy.

Comment 11

[Krzysztof Mioduszewski[:tauzen]]

From the original description this bug is a duplicate of Bug 1028776, which does not occur any more on Flame. Yoshi can we close this bug?

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Great, but usually we need to confirm this with reporter.

Comment 13

[Krzysztof Mioduszewski[:tauzen]]

Hi Garner, please take a look at comment 11. Are you ok to close this bug as a duplicate of Bug 1028776?

Comment 14

[Garner Lee]

Fixed as Dup. Yes, the fix just used the original screenOn solution, so it should work (and now already checked in).