Add support for dev input jack events

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: m1, Assigned: m1)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

GonkSwitch.cpp currently uses a uevent [1] to detect headset/headphone jack insertion/removal, yet not all kernels support this.

An alternative is to use the SW_HEADPHONE_INSERT / SW_MICROPHONE_INSERT input events, yet not all kernels support this and some known kernels running B2G have buggy implementations of these two events.

So we need Gecko support for both, but the dev input events should probably be disabled by default and enabled only by a device system property for now.  

[1] http://dxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSwitch.cpp#l34
(Assignee)

Comment 1

5 years ago
Created attachment 778140 [details] [diff] [review]
v0

The nsAppShell->GonkSwitch linkage is a little gross, f welcome!
Attachment #778140 - Flags: feedback?(mwu)

Comment 2

5 years ago
See also bug 835211 which I believe was also trying to do the same thing.

Updated

5 years ago
Duplicate of this bug: 835211
(Assignee)

Comment 4

5 years ago
The SwitchEventRunnable stuff in https://bug835211.bugzilla.mozilla.org/attachment.cgi?id=707388 looks nicer than the hal_impl::NotifyInputSwitch() stuff in attachment 778140 [details] [diff] [review], I'll probably play with that in a day or two unless you'd like to give it a go Alan?   We should be able to make this a patch only to nsAppShell.

Comment 5

5 years ago
(In reply to Michael Vines [:m1] [:evilmachines] from comment #4)
Oh, I was a little confused about https://bugzilla.mozilla.org/show_bug.cgi?id=835211#c9, now I think I know what you mean and attempt to do. Feel free to take it :)
(Assignee)

Comment 6

5 years ago
Created attachment 779453 [details] [diff] [review]
v1
Attachment #778140 - Attachment is obsolete: true
Attachment #778140 - Flags: feedback?(mwu)
Attachment #779453 - Flags: review?(mwu)
Attachment #779453 - Flags: feedback?(ahuang)

Comment 7

5 years ago
Comment on attachment 779453 [details] [diff] [review]
v1

Hello Randy,
As I know in bug 893516, you've been working on headphone/headset states in AudioManager and gonk. Can you give some feedback for this patch, too? Many thanks.
Attachment #779453 - Flags: feedback?(ahuang) → feedback?(rlin)
Comment on attachment 779453 [details] [diff] [review]
v1

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

This patch looks good to me.
Attachment #779453 - Flags: feedback?(rlin) → feedback+

Comment 9

5 years ago
Comment on attachment 779453 [details] [diff] [review]
v1

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

This is definitely the nicest looking implementation for this I've seen so far. Wish we could just pick either kernel events or input events though.

::: widget/gonk/nsAppShell.cpp
@@ +80,5 @@
>  static int epollfd = 0;
>  static int signalfds[2] = {0};
> +static bool sDevInputAudioJack;
> +static int32_t sHeadphoneState = AKEY_STATE_UNKNOWN;
> +static int32_t sMicrophoneState = AKEY_STATE_UNKNOWN;

I would prefer initializing these in InitInputDevices, or leaving them at 0 if possible.

@@ +720,5 @@
>  
> +    if (sDevInputAudioJack) {
> +        sHeadphoneState  = mReader->getSwitchState(-1, AINPUT_SOURCE_SWITCH, SW_HEADPHONE_INSERT);
> +        sMicrophoneState = mReader->getSwitchState(-1, AINPUT_SOURCE_SWITCH, SW_MICROPHONE_INSERT);
> +        updateHeadphoneSwitch();

Does this let us recover after a crash, or does it just grab events that came in during boot?

@@ +745,5 @@
>  {
> +    char value[PROPERTY_VALUE_MAX];
> +    property_get("ro.moz.devinputjack", value, "0");
> +    if (!strcmp(value, "1")) {
> +        sDevInputAudioJack = true;

sDevInputAudioJack = !strcmp(value, "1");
Attachment #779453 - Flags: review?(mwu) → review+
(Assignee)

Comment 10

5 years ago
Created attachment 782942 [details] [diff] [review]
v2.patch

Review comments addressed, carrying forward r+
Attachment #779453 - Attachment is obsolete: true
Attachment #782942 - Flags: review+
(Assignee)

Comment 11

5 years ago
(In reply to Michael Wu [:mwu] from comment #9)
> I would prefer initializing these in InitInputDevices, or leaving them at 0
> if possible.

Moved to InitInputDevices().  Sadly AKEY_STATE_UNKNOWN != 0

> Does this let us recover after a crash, or does it just grab events that
> came in during boot?

Yes!
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9262cbb74ed5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 893516
You need to log in before you can comment on or make changes to this bug.