861725 - B2G Emulator: support call forwarding

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[Edgar Chen [:edgar]]

Enhance emulator to support call forwarding.
- RIL_REQUEST_QUERY_CALL_FORWARD_STATUS [1]
- RIL_REQUEST_SET_CALL_FORWARD [2]
- AT+CCFC [3]

[1] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1676
[2] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1692
[3] Please see TS 27.007 session 7.11

Thanks

Comment 1

[Edgar Chen [:edgar]]

hardware/ril working branch:
https://github.com/EdgarChen/platform_hardware_ril/commits/bug_861725

Comment 2

[Edgar Chen [:edgar]]

external/qemu working branch:
https://github.com/EdgarChen/platform_external_qemu/commits/bug_861725

Comment 3

[Edgar Chen [:edgar]]

Attachment: platform_hardware_ril: PR #37

Comment 4

[Edgar Chen [:edgar]]

Attachment: platform_external_qemu: PR #85

Comment 5

[Edgar Chen [:edgar]]

Attachment: Part 1: Disable callforwarding test cases to wait emualtor changes, v1

The changes of reference-ril and qemu break the current callforwarding test cases, so we have to disable those test first.

Comment 6

[Edgar Chen [:edgar]]

Attachment: Part 2: Revise the call forwarding test cases, v1

Comment 7

[Edgar Chen [:edgar]]

Landing sequence:
1). Gecko part 1
2). qemu and hardware/ril
3). Gecko part 2

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8435472 [details] [diff] [review]
Part 1: Disable callforwarding test cases to wait emualtor changes, v1

Hi Vicamo, could you help to review this? Thank you

Comment 9

[Edgar Chen [:edgar]]

Comment on attachment 8424721 [details] [review]
platform_hardware_ril: PR #37

Hi Vicamo, could you help to review this? Thank you

Comment 10

[Edgar Chen [:edgar]]

Comment on attachment 8424722 [details] [review]
platform_external_qemu: PR #85

Hi Vicamo, could you help to review this? Thank you

Comment 11

[Edgar Chen [:edgar]]

Comment on attachment 8435524 [details] [diff] [review]
Part 2: Revise the call forwarding test cases, v1

Hi Vicamo, could you help to review this? Thank you

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8435524 [details] [diff] [review]
Part 2: Revise the call forwarding test cases, v1

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

::: dom/mobileconnection/tests/marionette/test_mobile_call_forwarding.js
@@ +36,5 @@
> +    is(aEvent.timeSeconds, aOptions.timeSeconds, "check timeSeconds");
> +    is(aEvent.serviceClass, aOptions.serviceClass, "check serviceClass");
> +  }));
> +  // Check DOMRequest's result.
> +  promises.push(setCallForwardingOption(aOptions).then(() => {}, (aError) => {

nit: simply place a null for the onresolve handler.

@@ +40,5 @@
> +  promises.push(setCallForwardingOption(aOptions).then(() => {}, (aError) => {
> +    ok(false, "got '" + aError.name + "' error");
> +  }));
> +
> +  return Promise.all(promises);

Please check validity after |Promise.all()| resolves.  Something like:

  return Promise.all()
                .then(function(aResults) {
    // aResults[0] should be an DOMEvent for "cfstatechange".
    ...
  });

@@ +84,5 @@
> +      reason: reason,
> +      action: MozMobileConnection.CALL_FORWARD_ACTION_ERASURE
> +    };
> +    promise =
> +      promise.then(setCallForwardingOption(options).then(() => {}, () => {}));

nit: to prevent rejects, you can simply assign onreject handler only, which makes |.then(null, true)| in this case.

@@ +99,5 @@
> +                     .then(() => testGetCallForwardingOption(data.reason, data));
> +  }
> +  // reset call forwarding settings.
> +  return promise.then(() => clearAllCallForwardingSettings(),
> +                      () => clearAllCallForwardingSettings());

nit: can be also rewritten as:

  return promise.then(null, true)
                .then(() => clearAllCallForwardingSettings());

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 8424722 [details] [review]
platform_external_qemu: PR #85

Thank you for your reivew, Vicamo. Will send review request again once comments on github addressed. :)

Comment 14

[Edgar Chen [:edgar]]

Try with qemu/reference-ril changes: https://tbpl.mozilla.org/?tree=Try&rev=830e50b8a448

Comment 15

[Edgar Chen [:edgar]]

Attachment: Part 2: Revise the call forwarding test cases, v2, r=vicamo

Address nit in comment #12.

Comment 16

[Edgar Chen [:edgar]]

Comment on attachment 8424722 [details] [review]
platform_external_qemu: PR #85

Need to address comments on github, cancel the review first.

Comment 17

[Edgar Chen [:edgar]]

Comment on attachment 8424722 [details] [review]
platform_external_qemu: PR #85

Already updated patches on github, thank you.

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8424722 [details] [review]
platform_external_qemu: PR #85

Thank you :)

Comment 19

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #7)
> Landing sequence:
> 1). Gecko part 1
> 2). qemu and hardware/ril
> 3). Gecko part 2

Gecko part1:
https://hg.mozilla.org/integration/b2g-inbound/rev/974b969d02c5

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/974b969d02c5

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

https://github.com/mozilla-b2g/platform_external_qemu/commit/eb0c93761bb9919567257d19bf25fa433cda3c00

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

https://github.com/mozilla-b2g/platform_hardware_ril/commit/cd88d860656c31c7da7bb310d6a160d0011b0961

Comment 23

[Edgar Chen [:edgar]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> https://github.com/mozilla-b2g/platform_external_qemu/commit/
> eb0c93761bb9919567257d19bf25fa433cda3c00

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #22)
> https://github.com/mozilla-b2g/platform_hardware_ril/commit/
> cd88d860656c31c7da7bb310d6a160d0011b0961

http://hg.mozilla.org/integration/b2g-inbound/rev/9ec91eeb5299

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Edgar Chen [:edgar][:echen] from comment #23)
> http://hg.mozilla.org/integration/b2g-inbound/rev/9ec91eeb5299

http://hg.mozilla.org/mozilla-central/rev/c482c28b35b6

Comment 25

[Edgar Chen [:edgar]]

Ah, just found some typo in https://github.com/mozilla-b2g/platform_external_qemu/blob/eb0c93761bb9919567257d19bf25fa433cda3c00/telephony/android_modem.c#L2629-L2635. :(

It should be

if (reason == A_CALL_FORWARDING_REASON_ALL ||
    reason == A_CALL_FORWARDING_REASON_ALL_CONDITIONAL) {
 ....
}

Comment 26

[Edgar Chen [:edgar]]

Attachment: (follow-up) platform_external_qemu: PR #99

(In reply to Edgar Chen [:edgar][:echen] from comment #25)
> Ah, just found some typo in
> https://github.com/mozilla-b2g/platform_external_qemu/blob/
> eb0c93761bb9919567257d19bf25fa433cda3c00/telephony/android_modem.c#L2629-
> L2635. :(
> 
> It should be
> 
> if (reason == A_CALL_FORWARDING_REASON_ALL ||
>     reason == A_CALL_FORWARDING_REASON_ALL_CONDITIONAL) {
>  ....
> }

PR for this.

Comment 27

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8441205 [details] [review]
(follow-up) platform_external_qemu: PR #99

https://github.com/mozilla-b2g/platform_external_qemu/commit/3326b51017252e09ccdd715dec6c5e12a7d1ecfe

Comment 28

[Edgar Chen [:edgar]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #27)
> Comment on attachment 8441205 [details] [review]
> (follow-up) platform_external_qemu: PR #99
> 
> https://github.com/mozilla-b2g/platform_external_qemu/commit/
> 3326b51017252e09ccdd715dec6c5e12a7d1ecfe

Thank you, Vicamo. :)

http://hg.mozilla.org/integration/b2g-inbound/rev/b5ec958db6d3

Comment 29

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #28)
> http://hg.mozilla.org/integration/b2g-inbound/rev/b5ec958db6d3

http://hg.mozilla.org/mozilla-central/rev/b5ec958db6d3

Comment 30

[Edgar Chen [:edgar]]

Try result for part2: https://tbpl.mozilla.org/?tree=Try&rev=5026f7446843

Comment 31

[Edgar Chen [:edgar]]

Gecko part2:
https://hg.mozilla.org/integration/b2g-inbound/rev/fe0359fb57db

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fe0359fb57db