978814 - [DSDS] RIL should re-use currently in-use SIM cards

Status: RESOLVED FIXED

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

Description

[Doug Sherk (:drs) (inactive)]

From an email thread on this:

> What we’re saying is that, if a call is in progress, all call buttons will not
> respect the preferred SIM setting (SIM1, SIM2, always ask). Instead, call
> buttons will display (and use) the SIM used by the call in progress. And call
> log will go to the Dialer instead of dialing directly (to inform the user).

Comment 1

[Doug Sherk (:drs) (inactive)]

Blocking the meta bug for now.

Comment 2

[Anthony Ricaud (:rik)]

In CallButton, we should have the UI and the event handlers look at mozTelephony.active.

Something like:
if(mozTelephony.active) {
  cardIndex = mozTelephony.active.serviceId;
}

Comment 3

[Anthony Ricaud (:rik)]

In terms of architecture, we should be careful about CallButton in SMS. SMS doesn't have the permission for mozTelephony so either we need to do the mozTelephony things outside of CallButton or CallButton needs to check the existence of mozTelephony.

Comment 4

[Preeti Raghunath(:Preeti)]

Wilfred,

Please review if this would be a blocker

Comment 5

[Doug Sherk (:drs) (inactive)]

This can potentially be pretty bad if not done. For example, emergency calls might not be placed, and if the user tries to make a call on an inactive SIM when there's already a call in progress, the active call will be lost (no audio sent or received) but still be active. There's some Gecko work going on to prevent this state from happening in bug 984919, but at the very least we should handle the new error message that they're creating in Gaia.

Comment 6

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

(In reply to Anthony Ricaud (:rik) from comment #2)
> In CallButton, we should have the UI and the event handlers look at
> mozTelephony.active.
> 
> Something like:
> if(mozTelephony.active) {
>   cardIndex = mozTelephony.active.serviceId;
> }

Kindly note that there could be no mozTelephony.active but there's still a call. This condition isn't strong enough.

Comment 7

[Anthony Ricaud (:rik)]

Yeah, it was a last note before leaving on week-end. We talked about that IRL and if (mozTelephony.calls.length && mozTelephony.calls[0].serviceId) sounds like the right test. I'm not sure about this if there is a conference call though.

Comment 8

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

(In reply to Anthony Ricaud (:rik) from comment #7)
> Yeah, it was a last note before leaving on week-end. We talked about that
> IRL and if (mozTelephony.calls.length && mozTelephony.calls[0].serviceId)
> sounds like the right test. I'm not sure about this if there is a conference
> call though.
Then you need this:
mozTelephony.conferenceGroup.calls.length

Comment 9

[Doug Sherk (:drs) (inactive)]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17332

Comment 10

[Preeti Raghunath(:Preeti)]

Moving to 1.4+ for pretty bad broken experience as described in comment 5.

Comment 11

[Anthony Ricaud (:rik)]

Comment on attachment 8393601 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17332

Can you move the checks in a function TelephonyHelper.getInUseCardIndex for MultiSIMActionButton and SimPicker?
The code would read better I believe without needing to put comments. The logic would be a bit DRY-er.

Comment 12

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8393601 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17332

(In reply to Anthony Ricaud (:rik) from comment #11)
> Comment on attachment 8393601 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17332
> 
> Can you move the checks in a function TelephonyHelper.getInUseCardIndex for
> MultiSIMActionButton and SimPicker?
> The code would read better I believe without needing to put comments. The
> logic would be a bit DRY-er.

Updated PR. I chose to put it in SimPicker instead since it didn't make sense to me to put it in TelephonyHelper (circular dependency and weird SoC). I also realized that I forgot to rename SimPicker.show, so I did that too.

Comment 13

[Peter Dolanjski [:pdol]]

Commenting on Wilfred's behalf while he is out:
Blocking is the right decision here based on the user impact described.

Comment 14

[Doug Sherk (:drs) (inactive)]

After Anthony and I talked in person, we came to the conclusion that sticking this logic in TelephonyHelper would be best since we're going to eventually be moving it into shared/ as part of breaking up the comm apps. I updated the PR with this change.

Comment 15

[Joe Cheng [:jcheng] (please needinfo)]

set target milestone to 1.4 S4 for now

Comment 16

[Anthony Ricaud (:rik)]

Comment on attachment 8393601 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17332

r+ with the changes mentioned on Github.

Comment 17

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/d66964e80e55dfc7de8096b40151bdc347d9e5fb

Comment 18

[Ryan VanderMeulen [:RyanVM]]

This has conflicts on 1.4.

Comment 19

[Doug Sherk (:drs) (inactive)]

v1.4: https://github.com/mozilla-b2g/gaia/commit/a2a88d0638594a6510f878d2c5e99a6ead7520ad