1162426 - B2G RIL: need a way to know current telephony state

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Jessica Jong [:jessica] (inactive)]

For bug 1156231 and bug 931670, we'd like to have an easy way to know the current telephony state, e.g. whether there is an active call. A simple way may be to be notified about call state changes, e.g. any call started / all calls ended, and we can track the state ourselves. The design is up to you :)

Comment 1

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (WIP) Provide TelephonyUtils

Comment 2

[Szu-Yu Chen [:aknow] (inactive)]

My idea is to provide a TelephonyUtils that wrap the underlying TelephonyService.
Currently, 3 APIs in my mind:

- boolean TelephonyUtils.isActive([optional] aClientId)
- boolean TelephonyUtils.hasNoCalls([optional] aClientId)
- Promise TelephonyUtils.waitForNoCalls([optional] aClientId)

Without passing aClientId, the action works on the entire system, i.e., the union of sim1, sim2, ...

Any ideas for the APIs?  What utilities we want?  Naming?  Architecture?
You may refer to attachment 8603182 [details] [diff] [review] which contains the implementation and an xpcshell test for demonstration.

Comment 3

[Jessica Jong [:jessica] (inactive)]

(In reply to Szu-Yu Chen [:aknow] from comment #2)
> My idea is to provide a TelephonyUtils that wrap the underlying
> TelephonyService.
> Currently, 3 APIs in my mind:
> 
> - boolean TelephonyUtils.isActive([optional] aClientId)
> - boolean TelephonyUtils.hasNoCalls([optional] aClientId)
> - Promise TelephonyUtils.waitForNoCalls([optional] aClientId)
> 
> Without passing aClientId, the action works on the entire system, i.e., the
> union of sim1, sim2, ...
> 
> Any ideas for the APIs?  What utilities we want?  Naming?  Architecture?
> You may refer to attachment 8603182 [details] [diff] [review] which contains
> the implementation and an xpcshell test for demonstration.

Thanks Aknow for the quick patch, the APIs work for me, just a comment about the naming.
'isActive' sounds a little vague to me, will 'isAnyActiveCall' be better? or any other suggestion?

Comment 4

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1: Provide TelephonyUtils

APIs in this version:
- boolean TelephonyUtils.hasAnyCalls([optional] aClientId)
- boolean TelephonyUtils.hasConnectedCalls([optional] aClientId)
- Promise TelephonyUtils.waitForNoCalls([optional] aClientId)

Comment 5

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Test case

Comment 6

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:

Comment 7

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

Comment on attachment 8604012 [details] [diff] [review]
Part 2: Test case

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

The fundamental structure looks good but we have some failures to fix.

::: dom/telephony/test/xpcshell/test_TelephonyUtils.js
@@ +49,5 @@
> +  // Hold the call.
> +
> +  TelephonyService.notifyCurrentCalls(0, {
> +    0: {
> +      state: RIL.CALL_STATE_HELD,

Should be CALL_STATE_HOLDING

@@ +69,5 @@
> +  let p = TelephonyUtils.waitForNoCalls();
> +  p.then(() => run_next_test(), () => ok(false, "promise should not reject"));
> +
> +  // Override _sendToRilWorker to avoid calling into RadioInterfaceLayer.
> +  TelephonyService._sendToRilWorker = function(aClientId, message, options,

This doesn't work.  I got the error 

 5:32.21 LOG: Thread-1 ERROR NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot modify properties of a WrappedNative
test_oneCall@test_TelephonyUtils.js:73:3
_run_next_test@/data/local/tests/xpcshell/head.js:1447:11
do_execute_soon/<.run@/data/local/tests/xpcshell/head.js:653:9
_do_main@/data/local/tests/xpcshell/head.js:207:5
_execute_test@/data/local/tests/xpcshell/head.js:513:5
@-e:1:1

Comment 8

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

Comment on attachment 8604011 [details] [diff] [review]
Part 1: Provide TelephonyUtils

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

Looks pretty good!

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

> @@ +69,5 @@
> > +  let p = TelephonyUtils.waitForNoCalls();
> > +  p.then(() => run_next_test(), () => ok(false, "promise should not reject"));
> > +
> > +  // Override _sendToRilWorker to avoid calling into RadioInterfaceLayer.
> > +  TelephonyService._sendToRilWorker = function(aClientId, message, options,
> 
> This doesn't work.  I got the error 
> 
>  5:32.21 LOG: Thread-1 ERROR NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot
> modify properties of a WrappedNative
> test_oneCall@test_TelephonyUtils.js:73:3
> _run_next_test@/data/local/tests/xpcshell/head.js:1447:11
> do_execute_soon/<.run@/data/local/tests/xpcshell/head.js:653:9
> _do_main@/data/local/tests/xpcshell/head.js:207:5
> _execute_test@/data/local/tests/xpcshell/head.js:513:5
> @-e:1:1

Hsinyi,

I didn't get this error when I ran the xpcshell-test on emulator.
Please let me know your test step and environment. I'd like to reproduce it.

Comment 10

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

(In reply to Szu-Yu Chen [:aknow] from comment #9)
> > @@ +69,5 @@
> > > +  let p = TelephonyUtils.waitForNoCalls();
> > > +  p.then(() => run_next_test(), () => ok(false, "promise should not reject"));
> > > +
> > > +  // Override _sendToRilWorker to avoid calling into RadioInterfaceLayer.
> > > +  TelephonyService._sendToRilWorker = function(aClientId, message, options,
> > 
> > This doesn't work.  I got the error 
> > 
> >  5:32.21 LOG: Thread-1 ERROR NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot
> > modify properties of a WrappedNative
> > test_oneCall@test_TelephonyUtils.js:73:3
> > _run_next_test@/data/local/tests/xpcshell/head.js:1447:11
> > do_execute_soon/<.run@/data/local/tests/xpcshell/head.js:653:9
> > _do_main@/data/local/tests/xpcshell/head.js:207:5
> > _execute_test@/data/local/tests/xpcshell/head.js:513:5
> > @-e:1:1
> 
> Hsinyi,
> 
> I didn't get this error when I ran the xpcshell-test on emulator.
> Please let me know your test step and environment. I'd like to reproduce it.

I was running on emu-kk build with gecko a week older (I thought I have used the latest one...). 
I'll try it again upon the newest build.

Comment 11

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #10)
> (In reply to Szu-Yu Chen [:aknow] from comment #9)
> > > @@ +69,5 @@
> > > > +  let p = TelephonyUtils.waitForNoCalls();
> > > > +  p.then(() => run_next_test(), () => ok(false, "promise should not reject"));
> > > > +
> > > > +  // Override _sendToRilWorker to avoid calling into RadioInterfaceLayer.
> > > > +  TelephonyService._sendToRilWorker = function(aClientId, message, options,
> > > 
> > > This doesn't work.  I got the error 
> > > 
> > >  5:32.21 LOG: Thread-1 ERROR NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot
> > > modify properties of a WrappedNative
> > > test_oneCall@test_TelephonyUtils.js:73:3
> > > _run_next_test@/data/local/tests/xpcshell/head.js:1447:11
> > > do_execute_soon/<.run@/data/local/tests/xpcshell/head.js:653:9
> > > _do_main@/data/local/tests/xpcshell/head.js:207:5
> > > _execute_test@/data/local/tests/xpcshell/head.js:513:5
> > > @-e:1:1
> > 
> > Hsinyi,
> > 
> > I didn't get this error when I ran the xpcshell-test on emulator.
> > Please let me know your test step and environment. I'd like to reproduce it.
> 
> I was running on emu-kk build with gecko a week older (I thought I have used
> the latest one...). 
> I'll try it again upon the newest build.

The command I use is "./mach xpchsell-test dom/telephony/tests/xpcshell/test_TelephonyUtils.js

Comment 12

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1#2: Provide TelephonyUtils

Found a wrong |this|. Request review again.

Comment 13

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2#2: Test case

For some reason, I change it to a marionette test. So it's totally different from previous version.

Comment 14

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

Comment on attachment 8608642 [details] [diff] [review]
Part 2#2: Test case

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

Thank you.

Comment 15

[Szu-Yu Chen [:aknow] (inactive)]

try looks good
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0b2c48fd152

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/22504f6f9c78
https://hg.mozilla.org/integration/b2g-inbound/rev/15c06e854079

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/22504f6f9c78
https://hg.mozilla.org/mozilla-central/rev/15c06e854079