1206289 - Additional TTY mode support

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Carol Yang [:cyang]]

Hi Hsinyi/Bevis,

Sorry we only caught this now but looks like the original TTY mode support in bug 1185438 isn't exactly what we need. These are the additional support we need:

1) To query/set TTY mode, RIL requests made so there should be callbacks for them once the response to these requests are received
2) Again, because of the RIL requests being made, the APIs need to take in clientId like the other APIs in nsITelephonyService
3) Some TTY behavior depends on whether the headset is plugged in or not. In particular, nsITelephonyAudioService.idl needs to supply something similar to WiredHeadsetManager [1] for:
- isWiredHeadsetOn() which provides the status of whether a headset is plugged in or not
- onHeadsetPluggedInChanged which would notify registered listeners when a headset is plugged/unplugged
- setParameters() to allow tty_mode to be set with the audio manager

[1] https://www.codeaurora.org/cgit/quic/la/platform/packages/services/Telecomm/tree/src/com/android/server/telecom/WiredHeadsetManager.java?h=LA.AF.1.1.1

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Carol Yang [:cyang] from comment #0)
> Hi Hsinyi/Bevis,
> 
> Sorry we only caught this now but looks like the original TTY mode support
> in bug 1185438 isn't exactly what we need. These are the additional support
> we need:
> 
> 1) To query/set TTY mode, RIL requests made so there should be callbacks for
> them once the response to these requests are received
> 2) Again, because of the RIL requests being made, the APIs need to take in
> clientId like the other APIs in nsITelephonyService
> 3) Some TTY behavior depends on whether the headset is plugged in or not. In
> particular, nsITelephonyAudioService.idl needs to supply something similar
> to WiredHeadsetManager [1] for:
> - isWiredHeadsetOn() which provides the status of whether a headset is
> plugged in or not
> - onHeadsetPluggedInChanged which would notify registered listeners when a
> headset is plugged/unplugged
> - setParameters() to allow tty_mode to be set with the audio manager
> 
> [1]
> https://www.codeaurora.org/cgit/quic/la/platform/packages/services/Telecomm/
> tree/src/com/android/server/telecom/WiredHeadsetManager.java?h=LA.AF.1.1.1

Hi Carol,

As we discussed earlier in bug 1185438, Mozilla is expected to provide setter/getter API of the TTY Mode preference in Web API.
The underlying implementation of the TTY mode is expected to be done by the chip vendors because the corresponding implementation might be various with different chip solution.

Hence, with this getter/setter APIs, the TelephonyService replace by CAF could check the current value of the TTY mode preference and monitor the change of the TTY mode preference with additional information of current headset status to apply the TTY mode to multiple modems and audio respectively.

The only problem I saw currently is how to be informed of the HEADSET status change in gecko.
I'll follow-up this item and feedback it soon, so set the NI on myself.


--

BTW, here is the implementation in AOSP for your reference:
1. A predefined user preference for TTYMode (Off, Full, HCO, VCO) and default set to Off.
2. TtyManager in CallManager: (currentTtyMode to Off by default)
   http://androidxref.com/5.1.1_r6/xref/packages/services/Telecomm/src/com/android/server/telecom/TtyManager.java#72
    - Listen to user preference change & headset plugged in change:
    - if wired headset is plugged, defined newTtyMode to the preferred one. Set to Off, Otherwise.
        - if currentTtyMode != newTtyMode, set currentTtyMode to newTtyMode and
            - notify tty_mode to AudioManager via AudioManager.setParameters.
            - notify tty_mode to TtyManager in each phone instance to set TTY mode properly.
              http://androidxref.com/5.1.1_r6/xref/packages/services/Telephony/src/com/android/services/telephony/TtyManager.java#updateTtyMode
3. TtyManager in PhoneGlobals:
    - Listen to user preference change to update UI TTY Mode to each phone. (IMS Phone currently. This is weird!)
    - Update TTY Mode notify from TtyManager in CallManager to each modem/phone instances.
4. CallStateMonitor for PhoneGlobals:
    - Listen to the TTY Mode changed from remote party and display a dialog to inform the user to change the tty mode accordingly. (Only available from IMS currently)

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

The change of HEADSET could be observed by adding an observer for the topic of "headphones-status-changed"[1].

If a wrapper of this is required to have a clear interface between MOZ-RIL and CAF-RIL.
We could define a listener for this in nsITelephonyAudioService.

[1] http://mxr.mozilla.org/mozilla-b2g37_v2_2/source/dom/system/gonk/AudioManager.cpp#406

Comment 3

[Carol Yang [:cyang]]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #1)
> (In reply to Carol Yang [:cyang] from comment #0)
> > Hi Hsinyi/Bevis,
> > 
> > Sorry we only caught this now but looks like the original TTY mode support
> > in bug 1185438 isn't exactly what we need. These are the additional support
> > we need:
> > 
> > 1) To query/set TTY mode, RIL requests made so there should be callbacks for
> > them once the response to these requests are received
> > 2) Again, because of the RIL requests being made, the APIs need to take in
> > clientId like the other APIs in nsITelephonyService
> > 3) Some TTY behavior depends on whether the headset is plugged in or not. In
> > particular, nsITelephonyAudioService.idl needs to supply something similar
> > to WiredHeadsetManager [1] for:
> > - isWiredHeadsetOn() which provides the status of whether a headset is
> > plugged in or not
> > - onHeadsetPluggedInChanged which would notify registered listeners when a
> > headset is plugged/unplugged
> > - setParameters() to allow tty_mode to be set with the audio manager
> > 
> > [1]
> > https://www.codeaurora.org/cgit/quic/la/platform/packages/services/Telecomm/
> > tree/src/com/android/server/telecom/WiredHeadsetManager.java?h=LA.AF.1.1.1
> 
> Hi Carol,
> 
> As we discussed earlier in bug 1185438, Mozilla is expected to provide
> setter/getter API of the TTY Mode preference in Web API.
> The underlying implementation of the TTY mode is expected to be done by the
> chip vendors because the corresponding implementation might be various with
> different chip solution.

Agreed that implementation will be done by chip vendors. I think what we didn't take into consideration was that the setting/getting of TTY mode are asynchronous calls. In particular, the call here will result in a RIL request sent down to modem. This is why we need a CB after receiving the response from those requests. For the same reason, this is why we need a clientId.

http://androidxref.com/5.1.1_r6/xref/packages/services/Telephony/src/com/android/services/telephony/TtyManager.java#92

> 
> Hence, with this getter/setter APIs, the TelephonyService replace by CAF
> could check the current value of the TTY mode preference and monitor the
> change of the TTY mode preference with additional information of current
> headset status to apply the TTY mode to multiple modems and audio
> respectively.
> 
> The only problem I saw currently is how to be informed of the HEADSET status
> change in gecko.
> I'll follow-up this item and feedback it soon, so set the NI on myself.
> 
> 
> --
> 
> BTW, here is the implementation in AOSP for your reference:
> 1. A predefined user preference for TTYMode (Off, Full, HCO, VCO) and
> default set to Off.
> 2. TtyManager in CallManager: (currentTtyMode to Off by default)
>   
> http://androidxref.com/5.1.1_r6/xref/packages/services/Telecomm/src/com/
> android/server/telecom/TtyManager.java#72
>     - Listen to user preference change & headset plugged in change:
>     - if wired headset is plugged, defined newTtyMode to the preferred one.
> Set to Off, Otherwise.
>         - if currentTtyMode != newTtyMode, set currentTtyMode to newTtyMode
> and
>             - notify tty_mode to AudioManager via AudioManager.setParameters.
>             - notify tty_mode to TtyManager in each phone instance to set
> TTY mode properly.
>              
> http://androidxref.com/5.1.1_r6/xref/packages/services/Telephony/src/com/
> android/services/telephony/TtyManager.java#updateTtyMode
> 3. TtyManager in PhoneGlobals:
>     - Listen to user preference change to update UI TTY Mode to each phone.
> (IMS Phone currently. This is weird!)
>     - Update TTY Mode notify from TtyManager in CallManager to each
> modem/phone instances.
> 4. CallStateMonitor for PhoneGlobals:
>     - Listen to the TTY Mode changed from remote party and display a dialog
> to inform the user to change the tty mode accordingly. (Only available from
> IMS currently)

Comment 4

[Carol Yang [:cyang]]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #2)
> The change of HEADSET could be observed by adding an observer for the topic
> of "headphones-status-changed"[1].
> 
> If a wrapper of this is required to have a clear interface between MOZ-RIL
> and CAF-RIL.
> We could define a listener for this in nsITelephonyAudioService.
> 
> [1]
> http://mxr.mozilla.org/mozilla-b2g37_v2_2/source/dom/system/gonk/
> AudioManager.cpp#406

Thanks Bevis. Aside from observing headset change, there should be a wrapper to allow us to query the headset status as well.

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Carol Yang [:cyang] from comment #3)
> Agreed that implementation will be done by chip vendors. I think what we
> didn't take into consideration was that the setting/getting of TTY mode are
> asynchronous calls. In particular, the call here will result in a RIL
> request sent down to modem. This is why we need a CB after receiving the
> response from those requests. For the same reason, this is why we need a
> clientId.
> 
> http://androidxref.com/5.1.1_r6/xref/packages/services/Telephony/src/com/
> android/services/telephony/TtyManager.java#92
> 

The behavior in my mind is actually the same to what have done in AOSP:
1. The getter/setter of TTY mode is as simple as what we have done for mute/speaker.
2. The implementation will try to apply the TTY mode when all the conditions are fufilled, including:
   a. detecting the HEADSET status.
   b. detecting the change of TTY preference.
   b. apply the TTY mode to the Audio and all the modems(depends on how many clientIds available including the ims client).
In AOSP, 
1. the telecom.TtyManager will detect the change of TTY preference:
http://androidxref.com/5.1.1_r6/xref/packages/services/Telecomm/src/com/android/server/telecom/TtyManager.java#117
2. Update the tty mode to audio and ask each telephony.TtyManager bound to dedicated Phone instance to set the TTY mode to the protocol.
http://androidxref.com/5.1.1_r6/xref/packages/services/Telephony/src/com/android/services/telephony/TtyManager.java#93
And there is no need to report the error to the app layer if there is something wrong in audio or in protocol.
http://androidxref.com/5.1.1_r6/xref/packages/services/Telephony/src/com/android/services/telephony/TtyManager.java#52


Hence, based on these assumption, 
We have done step 1) in bug 1185438, and expect step 2) to be done by CAF.
Is there anything that we misunderstood?


In addition, I'll check how to get current headset status in gecko, so keep NI on me.

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #5)
> The behavior in my mind is actually the same to what have done in AOSP:
> 1. The getter/setter of TTY mode is as simple as what we have done for
> mute/speaker.
> 2. The implementation will try to apply the TTY mode when all the conditions
> are fufilled, including:
>    a. detecting the HEADSET status.
>    b. detecting the change of TTY preference.
>    c. apply the TTY mode to the Audio and all the modems(depends on how many
> clientIds available including the ims client).

In addition, based on this design, we don't have to expose the |clientId| from API perspective, IMO.

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Carol Yang [:cyang] from comment #4)
> Thanks Bevis. Aside from observing headset change, there should be a wrapper
> to allow us to query the headset status as well.

I'll expose the listener and the getter in TelephonyAudioService for the headset status in this bug. :)

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #7)
> (In reply to Carol Yang [:cyang] from comment #4)
> > Thanks Bevis. Aside from observing headset change, there should be a wrapper
> > to allow us to query the headset status as well.
> 
> I'll expose the listener and the getter in TelephonyAudioService for the
> headset status in this bug. :)

BTW,
In addition to the headset, is there anything else required to be exposed from AudioManager?
For example, in AOSP, the telecom.TtyManager will set additional parameters "tty_mode" into AudioManager:
http://androidxref.com/5.1.1_r6/xref/packages/services/Telecomm/src/com/android/server/telecom/TtyManager.java#89 

Currently, we expect this could be done by CAF internally.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #8)
> (In reply to Bevis Tseng[:bevistseng][:btseng] from comment #7)
> > (In reply to Carol Yang [:cyang] from comment #4)
> > > Thanks Bevis. Aside from observing headset change, there should be a wrapper
> > > to allow us to query the headset status as well.
> > 
> > I'll expose the listener and the getter in TelephonyAudioService for the
> > headset status in this bug. :)
> 
> BTW,
> In addition to the headset, is there anything else required to be exposed
> from AudioManager?
> For example, in AOSP, the telecom.TtyManager will set additional parameters
> "tty_mode" into AudioManager:
> http://androidxref.com/5.1.1_r6/xref/packages/services/Telecomm/src/com/
> android/server/telecom/TtyManager.java#89 
> 
> Currently, we expect this could be done by CAF internally.

You have mentioned that you need an API to set ttymode to audio system in comment 0.

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 1: Expose Headset State and TTY Mode from AudioManager.

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 2: Add Wrapper APIs in TelephonyAudioService.

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 3: Add TTY Mode Support in RIL Worker.

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 4: Add TTY Mode Support in TelephonyService.

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Carol,

The attached patches are WIP for your reference of 
1. How the headset/ttymode related APIs to be exposed from TelephonyAudioService. (Part2)
2. How MOZ-RIL adopt these APIs to support TTY in TelephonyService. (Part3)

Please let us know if this could meet your need for the TTY implementation in CAF-RIL.

Thanks!

Comment 15

[Carol Yang [:cyang]]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #6)
> (In reply to Bevis Tseng[:bevistseng][:btseng] from comment #5)
> > The behavior in my mind is actually the same to what have done in AOSP:
> > 1. The getter/setter of TTY mode is as simple as what we have done for
> > mute/speaker.

So I did notice that in TelephonyAudioService, prefs is used to store the TTY mode. Will this persist over device reboot?

> > 2. The implementation will try to apply the TTY mode when all the conditions
> > are fufilled, including:
> >    a. detecting the HEADSET status.
> >    b. detecting the change of TTY preference.
> >    c. apply the TTY mode to the Audio and all the modems(depends on how many
> > clientIds available including the ims client).
> 
> In addition, based on this design, we don't have to expose the |clientId|
> from API perspective, IMO.

I'm still verifying if down at the modem level, if setting TTY mode is subscription agnostic. If this is something that is going to be an issue, I'll bring it up with you later.

I see your point regarding no need to report response for query/set so agree that no CB is needed.

As for your WIP patch, I think you are providing what we need in TelephonyAudioService.

Thanks!

Comment 16

[Bevis Tseng[:bevis][:btseng](Exited)]

Additional APIs from TelephonyAudioService is required per comment 0 and comment 15.

Comment 17

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Carol Yang [:cyang] from comment #15)
> So I did notice that in TelephonyAudioService, prefs is used to store the
> TTY mode. Will this persist over device reboot?
Yes, the preference is persistent over device reboot.
> 
> I'm still verifying if down at the modem level, if setting TTY mode is
> subscription agnostic. If this is something that is going to be an issue,
> I'll bring it up with you later.
Here is my test result in flame to both clientIds:
I got with positive response from both clients when applying the TTY mode no matter there is a SIM inserted or not.

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v2) Part 4: Add TTY Mode Support in TelephonyService.

Fix the problem that blocking the request to ril_worker.

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=902fb484e5f4

Comment 20

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Carol,

I have one problem regarding AudioSystem::SetParameter("tty_mode=tty_xxx");

In patch Part1 & Part2, I have defined a Wrapper to set expected TTY mode to the AudioSystem.
However, when testing, I always get an ERROR in Flame said:
> E/audio_a2dp_hw(  206): adev_set_parameters: ERROR: set param called even when stream out is null
Even the parameter I set is correct:
> D/audio_hw_primary(  206): adev_set_parameters: enter: tty_mode=tty_off

The error seems to be reported from the audio driver which we are not able to access.
We are not pretty sure if there is any limitation on Flame for the TTY support in Audio part.

Would you mind to see if you are able to apply the |tty_mode| parameter into AudioSystem with the patches of Part1 & Part2?
If the result is positive, then we can help to land this patch into 2.2r safely. :)

Thanks!

Comment 21

[Carol Yang [:cyang]]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #20)
> Hi Carol,
> 
> I have one problem regarding AudioSystem::SetParameter("tty_mode=tty_xxx");
> 
> In patch Part1 & Part2, I have defined a Wrapper to set expected TTY mode to
> the AudioSystem.
> However, when testing, I always get an ERROR in Flame said:
> > E/audio_a2dp_hw(  206): adev_set_parameters: ERROR: set param called even when stream out is null
> Even the parameter I set is correct:
> > D/audio_hw_primary(  206): adev_set_parameters: enter: tty_mode=tty_off
> 
> The error seems to be reported from the audio driver which we are not able
> to access.
> We are not pretty sure if there is any limitation on Flame for the TTY
> support in Audio part.
> 
> Would you mind to see if you are able to apply the |tty_mode| parameter into
> AudioSystem with the patches of Part1 & Part2?
> If the result is positive, then we can help to land this patch into 2.2r
> safely. :)
> 
> Thanks!

I checked with our Audio team and the response is that this error can be ignored.

Comment 22

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=548d4a9fff96

Comment 23

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v3) Part 1: Expose Headset State and TTY Mode from AudioManager.

Hi Alaster,

May I have your review for exposing the headset state and the setter of TTY Mode to audio system?

Thanks!

Comment 24

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v3) Part 2: Add Wrapper APIs in TelephonyAudioService.

Hi Hsinyi,

In this patch,
1. Add wrapper APIs of headset/tty mode from AudioManager.
2. More straight-forward implementation of getter/setter for TTY preference without "nsPref:changed" observer involved to ensure that the the preference is updated after set.

May I have your review for these change?

Thanks!

Comment 25

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v3) Part 3: Add TTY Mode Support in RIL Worker.

Hi Hsinyi,

May I have your review for the API of setting TTY in ril_worker.js?

BTW, the TTY related constants added in ril_consts.js is not used but just for reference from the one defined in ril.h in AOSP.

Thanks!

Comment 26

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v3) Part 4: Add TTY Mode Support in TelephonyService.

To have the support of applying preferred TTY mode in TelephonyService.

Comment 27

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v3) Part 5: Add Test Case for Headset Detection.

Add Test case to verify headset listener in TelephonyAudioService.

Comment 28

[Alastor Wu [:alwu]]

Comment on attachment 8667715 [details] [diff] [review]
(v3) Part 1: Expose Headset State and TTY Mode from AudioManager.

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

LGTM, thanks!

::: dom/system/gonk/nsIAudioManager.idl
@@ +42,5 @@
> +  const unsigned short TTY_MODE_OFF = 0;
> +  const unsigned short TTY_MODE_FULL = 1;
> +  const unsigned short TTY_MODE_HCO = 2;
> +  const unsigned short TTY_MODE_VCO = 3;
> +

Could you add the comment to shortly describe what is TTY?

Comment 29

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Alastor Wu [:alwu][OOO from 10/3-10/11] from comment #28)
> Comment on attachment 8667715 [details] [diff] [review]
> (v3) Part 1: Expose Headset State and TTY Mode from AudioManager.
> 
> Review of attachment 8667715 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM, thanks!
> 
> ::: dom/system/gonk/nsIAudioManager.idl
> @@ +42,5 @@
> > +  const unsigned short TTY_MODE_OFF = 0;
> > +  const unsigned short TTY_MODE_FULL = 1;
> > +  const unsigned short TTY_MODE_HCO = 2;
> > +  const unsigned short TTY_MODE_VCO = 3;
> > +
> 
> Could you add the comment to shortly describe what is TTY?

Sure! :)

Comment 30

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v3) Part 4: Add TTY Mode Support in TelephonyService.

Sorry for uploading wrong patch of Part 4.

Comment 31

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8668768 [details] [diff] [review]
(v3) Part 4: Add TTY Mode Support in TelephonyService.

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

r=me with comment addressed, thank you!

::: dom/telephony/gonk/TelephonyService.js
@@ +178,5 @@
>  
>    this._cdmaCallWaitingNumber = null;
>  
> +  this._headsetState = gAudioService.headsetState;
> +  gAudioService.registerListener(this);

Did you forget to call unregisterListener() when shutting down?

Comment 32

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8667717 [details] [diff] [review]
(v3) Part 2: Add Wrapper APIs in TelephonyAudioService.

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

Thank you, Bevis :)

::: dom/telephony/gonk/TelephonyAudioService.js
@@ +78,5 @@
>    QueryInterface: XPCOMUtils.generateQI([Ci.nsITelephonyAudioService,
>                                           Ci.nsIObserver]),
>  
> +  _shutdown: function() {
> +    Services.prefs.removeObserver(kPrefRilDebuggingEnabled, this);

I think this is not necessary according to [1].
[1] https://dxr.mozilla.org/mozilla-central/source/modules/libpref/nsPrefBranch.cpp?from=nsPrefBranch.cpp&case=true#662

Comment 33

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8668768 [details] [diff] [review]
(v3) Part 4: Add TTY Mode Support in TelephonyService.

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

::: dom/telephony/gonk/TelephonyService.js
@@ +178,5 @@
>  
>    this._cdmaCallWaitingNumber = null;
>  
> +  this._headsetState = gAudioService.headsetState;
> +  gAudioService.registerListener(this);

My bad, I'll address this in next update.

Thanks!

Comment 34

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8667717 [details] [diff] [review]
(v3) Part 2: Add Wrapper APIs in TelephonyAudioService.

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

::: dom/telephony/gonk/TelephonyAudioService.js
@@ +78,5 @@
>    QueryInterface: XPCOMUtils.generateQI([Ci.nsITelephonyAudioService,
>                                           Ci.nsIObserver]),
>  
> +  _shutdown: function() {
> +    Services.prefs.removeObserver(kPrefRilDebuggingEnabled, this);

Cool, glad to know this!

Thanks!

Comment 35

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8667721 [details] [diff] [review]
(v3) Part 5: Add Test Case for Headset Detection.

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

Test is the best!

Comment 36

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=86bf4f254263

Comment 37

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5680848a349

Comment 38

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 1: Expose Headset State and TTY Mode from AudioManager. r=alwu,a=2.2r+

upload final patch of part 1.

Comment 39

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 2: Add Wrapper APIs in TelephonyAudioService. r=htsai,a=2.2r+

upload final patch of part 2 to address comment 32.

Comment 40

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 3: Add TTY Mode Support in RIL Worker. r=htsai,a=2.2r+

upload final patch of part 3.

Comment 41

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 4: Add TTY Mode Support in TelephonyService. r=htsai,a=2.2r+

upload final patch of part 4 to address comment 31.

Comment 42

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 5: Add Test Case for Headset Detection. r=htsai,a=2.2r+

upload final patch of part 5.

Comment 43

[Bevis Tseng[:bevis][:btseng](Exited)]

see try server result in comment 36.

Comment 44

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/2c5ac35a9f61
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/695914e0ed1d
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/9aacca580eda
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/4852b5eb9b83
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/b8bc3d446c8f

Comment 45

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Patched already landed in v2.2r.