Closed Bug 978814 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect
Not set

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S4 (28mar)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: drs, Assigned: drs)

References

Details

(Whiteboard: [cr 639080])

Attachments

(1 file)

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).
Blocking the meta bug for now.
Blocks: b2g-dsds-1.4
See Also: → 926345
In CallButton, we should have the UI and the event handlers look at mozTelephony.active.

Something like:
if(mozTelephony.active) {
  cardIndex = mozTelephony.active.serviceId;
}
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.
Assignee: nobody → drs+bugzilla
blocking-b2g: --- → 1.4?
Wilfred,

Please review if this would be a blocker
Flags: needinfo?(wmathanaraj)
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.
See Also: → 984919
(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.
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.
(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
Moving to 1.4+ for pretty bad broken experience as described in comment 5.
blocking-b2g: 1.4? → 1.4+
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.
Attachment #8393601 - Flags: review?(anthony) → review-
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.
Attachment #8393601 - Flags: review- → review?(anthony)
Blocks: 985888
Commenting on Wilfred's behalf while he is out:
Blocking is the right decision here based on the user impact described.
Flags: needinfo?(wmathanaraj)
Component: Gaia → Gaia::Dialer
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.
set target milestone to 1.4 S4 for now
Target Milestone: --- → 1.4 S4 (28mar)
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.
Attachment #8393601 - Flags: review?(anthony) → review+
https://github.com/mozilla-b2g/gaia/commit/d66964e80e55dfc7de8096b40151bdc347d9e5fb
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
This has conflicts on 1.4.
Flags: needinfo?(drs+bugzilla)
Whiteboard: [cr 639080]
You need to log in before you can comment on or make changes to this bug.