Closed Bug 1021478 Opened 7 years ago Closed 7 years ago

Set outgoing calls "Always ask" with two sim card inside, use bluetooth headset to dial, there will be no network message

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
major

Tracking

(blocking-b2g:2.0M+, b2g-v1.3T wontfix, b2g-v1.4 fixed, b2g-v2.0 wontfix, b2g-v2.0M verified, b2g-v2.1 verified)

RESOLVED FIXED
2.1 S1 (1aug)
blocking-b2g 2.0M+
Tracking Status
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- wontfix
b2g-v2.0M --- verified
b2g-v2.1 --- verified

People

(Reporter: wei.gao, Assigned: drs)

References

()

Details

Attachments

(2 files, 14 obsolete files)

9.78 KB, patch
rik
: review+
Details | Diff | Splinter Review
8.28 MB, video/mp4
Details
OS version
---------------------------------------------
FireFoxOS v1.4

Reproduce steps:
---------------------------------------------
1) With two sim cards inside
2) Settings > SIM manager > Outgoing calls > Always ask
3) Just dial a phone using any card
4) Connect bluetooth headset, and double click button of bluetooth headset to dial
5) There will be no network message

Expected result:
---------------------------------------------
Dial normal
Attached patch bluetooth_dial.patch (obsolete) — Splinter Review
I think the cause is when set outgoing calls "Always ask", the value of 'ril.telephony.defaultServiceId' is -1.

When dial via bluetooth headset, navigator.mozMobileConnections[-1] is undefined, so there will be no network message.

There is one patch about this, and it will work normal.

Please help to review.
Thanks a lot.
Flags: needinfo?(ehung)
Comment on attachment 8435487 [details] [diff] [review]
bluetooth_dial.patch

Hi Etienne
Can you help to review this patch?

Thank you very much.
Attachment #8435487 - Flags: review?(etienne)
Flags: needinfo?(ehung)
IMHO SimSettingsHelper should return the original value and let the app display the corresponding UI.
Comment on attachment 8435487 [details] [diff] [review]
bluetooth_dial.patch

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

Forwarding to Doug.
Attachment #8435487 - Flags: review?(etienne) → review?(drs+bugzilla)
Comment on attachment 8435487 [details] [diff] [review]
bluetooth_dial.patch

(In reply to Arthur Chen [:arthurcc] from comment #3)
> IMHO SimSettingsHelper should return the original value and let the app
> display the corresponding UI.

I agree with this. Knowing that it's set to "always ask" is useful information, and without this we're actually liable to break anything that uses this function.
Attachment #8435487 - Flags: review?(drs+bugzilla) → review-
> I agree with this. Knowing that it's set to "always ask" is useful information,
> and without this we're actually liable to break anything that uses this function.

Hi Doug

I think that's right. So I modify the code so as to show the "always ask".
Can you help to check this patch if it can be granted.

Thank you very much.
Flags: needinfo?(drs+bugzilla)
My understanding here is that the case you're handling is when the user hits the call button from a Bluetooth headset. Here, there's no easy way for the user to pick which SIM to call from. I think this really needs UX review. This is a problem that we have everywhere from v1.4 onward, so I think it's best if we deal with this properly, even if we create an interim bad solution before doing something better.

My opinion, acknowledging that I'm designing in a vacuum, is that (for now) we should just arbitrarily make the call from SIM 1. If the user has the device in their hands, then they wouldn't be using the Bluetooth call button to begin with. Or if they were, then they could end the call as soon as they see that it's being made from the wrong SIM. So I think it makes the most sense to assume that the user has the device in their pocket and doesn't want to have to take it out to make the call.

Another alternative is to have a second "make calls from" SIM setting for Bluetooth devices, where "Always ask" isn't an option. I actually like this more than the former option, but it would involve a lot more work. We could do this as a followup, though.

Carrie, what do you think?
Flags: needinfo?(drs+bugzilla) → needinfo?(cawang)
(In reply to Doug Sherk (:drs) from comment #7)
> 
> My opinion, acknowledging that I'm designing in a vacuum, is that (for now)
> we should just arbitrarily make the call from SIM 1. If the user has the
> device in their hands, then they wouldn't be using the Bluetooth call button
> to begin with. Or if they were, then they could end the call as soon as they
> see that it's being made from the wrong SIM. So I think it makes the most
> sense to assume that the user has the device in their pocket and doesn't
> want to have to take it out to make the call.
> 

Hi Doug and Carrie

I agree with you, let users to choose a sim is not a good user experience.
By the way, can't we designed to use the last sim to dial the last call number when make a call from a bluetooth headset?
Is that be granted?

Thanks.
Hi, 

I wonder if it's double-click, it should be calling back to the most recent call of the history. In this case, we should use the SIM that the user used for dialing the call last time. Some BT devices can choose calls from call history and we can still adopt the rule here - always use the SIM that users used for that call log last time. 
If users want to make call to other contacts which are not in the call log, they shall use voice command and I think we don't have this function yet on our OS. So probably we can exclude this use case for now.
Does this make sense to you guys? Thanks!
Flags: needinfo?(cawang)
Hi Doug and Carrie

I think this patch can realize our idea, when dial a number from bluetooth headset, we should dial the last call number through the sim used last time instead of letting users to choose a sim card.

Can this be granted?
Thanks.
Attachment #8435487 - Attachment is obsolete: true
Hi,

I recommit a patch about this.
Please help to review.
Thanks.
Attachment #8441139 - Attachment is obsolete: true
Comment on attachment 8441223 [details] [diff] [review]
bug1021478_dial_the_last_callnumber_through_the_sim_used_last_time.patch

(In reply to Carrie Wang [:carrie] from comment #9)
> Hi, 
> 
> I wonder if it's double-click, it should be calling back to the most recent
> call of the history. In this case, we should use the SIM that the user used
> for dialing the call last time. Some BT devices can choose calls from call
> history and we can still adopt the rule here - always use the SIM that users
> used for that call log last time. 
> If users want to make call to other contacts which are not in the call log,
> they shall use voice command and I think we don't have this function yet on
> our OS. So probably we can exclude this use case for now.
> Does this make sense to you guys? Thanks!

Carrie, I agree with this but I'm not sure if it's possible to get this done in time, or that it's safe enough, to land on v1.4. Would you be OK with using my suggestion for landing on v1.4, and your method for v2.0 onward?

(In reply to wei.gao from comment #11)
> Created attachment 8441223 [details] [diff] [review]
> bug1021478_dial_the_last_callnumber_through_the_sim_used_last_time.patch
> 
> Hi,
> 
> I recommit a patch about this.
> Please help to review.
> Thanks.

Sorry, I think I missed this. To help me find it, please set the review flag next time. We need Carrie's reply before I can review this, though. Thanks!
Attachment #8441223 - Flags: review?(drs+bugzilla)
Flags: needinfo?(cawang)
(In reply to Doug Sherk (:drs) from comment #12)
> To help me find it, please set the review flag next time. 

Sorry, I get it, I will do so in the future.
I would have concerns on using SIM 1 as default SIM in this case. Since users select always ask as their preference, we can't really predict which SIM will be their "expected default SIM" and this might cause confusion and even unexpected cost. However, I'll leave this time frame issue to EPM and if we really can't make it, I'd prefer still popping up the options for users to select the SIM, because this is all about cost and users do really care. Thanks!
Flags: needinfo?(cawang) → needinfo?(whuang)
This isn't a release blocker so just put into backlog.
Fix can be cherry picked from master if partner needs it.
blocking-b2g: --- → backlog
Flags: needinfo?(whuang)
Comment on attachment 8441223 [details] [diff] [review]
bug1021478_dial_the_last_callnumber_through_the_sim_used_last_time.patch

(In reply to Carrie Wang [:carrie] from comment #14)
> I would have concerns on using SIM 1 as default SIM in this case. Since
> users select always ask as their preference, we can't really predict which
> SIM will be their "expected default SIM" and this might cause confusion and
> even unexpected cost. However, I'll leave this time frame issue to EPM and
> if we really can't make it, I'd prefer still popping up the options for
> users to select the SIM, because this is all about cost and users do really
> care. Thanks!

(In reply to Wesley Huang [:wesley_huang] from comment #15)
> This isn't a release blocker so just put into backlog.
> Fix can be cherry picked from master if partner needs it.

OK, thanks. In that case, this patch isn't the correct fix, so I have to r- it. 

Wei, let me know if you need help implementing it.
Attachment #8441223 - Flags: review?(drs+bugzilla) → review-
(In reply to Carrie Wang [:carrie] from comment #9)

> I wonder if it's double-click, it should be calling back to the most recent
> call of the history. In this case, we should use the SIM that the user used
> for dialing the call last time. Some BT devices can choose calls from call
> history and we can still adopt the rule here - always use the SIM that users
> used for that call log last time. 

(In reply to Carrie Wang [:carrie] from comment #14)
> I would have concerns on using SIM 1 as default SIM in this case. Since
> users select always ask as their preference, we can't really predict which
> SIM will be their "expected default SIM" and this might cause confusion and
> even unexpected cost. 

Ok, but now there still show no network message when setting outgoing calls "Always ask" with two sim card inside, and using bluetooth headset to dial.

I am confused about the strategy we desided at last. Is our decision to use the sim1 as default?
I wonder can this patch be granted.
Thanks.
Attachment #8444887 - Flags: review?(drs+bugzilla)
Comment on attachment 8444887 [details] [diff] [review]
Bug319336_dial_the_last_callnumber_through_sim1.patch

(In reply to Wei Gao (Spreadtrum) from comment #17)
> I am confused about the strategy we desided at last. Is our decision to use
> the sim1 as default?
> I wonder can this patch be granted.
> Thanks.

Wei, we decided that the call should be placed on the SIM that the contact being called was last in a call with. So for example, if you call me on my SIM2, then we hang up, and then I hit the bluetooth call button, the call should be placed on SIM2.

I'm giving your patch feedback+ because I think it's better than what we currently have, and if a partner wants to cherry-pick it, it's better than nothing. But if we want to land this on the Mozilla Gaia repo, we'll have to have the better fix.
Attachment #8444887 - Flags: review?(drs+bugzilla) → feedback+
(In reply to Doug Sherk (:drs) from comment #18)
> Wei, we decided that the call should be placed on the SIM that the contact
> being called was last in a call with. So for example, if you call me on my
> SIM2, then we hang up, and then I hit the bluetooth call button, the call
> should be placed on SIM2.

Hi Doug

Thanks for your attention.
I agree with you when the user set with "Always ask". But if the user set "Outgoing calls" with "Sim1" or "Sim2", when the user call a number through bluetooth headset, which sim should be used should depend on the setting in any case I think.
How do you think about it?

Thanks.
Flags: needinfo?(drs+bugzilla)
(In reply to Wei Gao (Spreadtrum) from comment #19)
> Hi Doug
> 
> Thanks for your attention.
> I agree with you when the user set with "Always ask". But if the user set
> "Outgoing calls" with "Sim1" or "Sim2", when the user call a number through
> bluetooth headset, which sim should be used should depend on the setting in
> any case I think.
> How do you think about it?
> 
> Thanks.

That's a great question. My knee-jerk answer based on my understanding of how our users use dual-SIM devices would be that it should call on the SIM you have set in your settings, but I think it's best if Carrie decides that.

Also, I'm sorry for taking so long to reply. It's not really an excuse but I got swamped with other things. I'll be more responsive from now on.
Flags: needinfo?(drs+bugzilla) → needinfo?(cawang)
(In reply to Doug Sherk (:drs) from comment #20)
> Also, I'm sorry for taking so long to reply. It's not really an excuse but I
> got swamped with other things. I'll be more responsive from now on.

That's nothing, it is no big deal.
No matter how, I should thank you for your kindly help.
Yes, if user has set SIM1/ SIM2 as primary SIM for outgoing calls in Settings, we should apply their settings even when they use BT headset to make calls. Thanks!
Flags: needinfo?(cawang)
Attached patch dial_via_bluetooth_headset.patch (obsolete) — Splinter Review
(In reply to Carrie Wang [:carrie] from comment #22)
> Yes, if user has set SIM1/ SIM2 as primary SIM for outgoing calls in
> Settings, we should apply their settings even when they use BT headset to
> make calls. Thanks!

Hi Doug

Can this patch be granted? 
Please help to review it.
Thanks.
Attachment #8441223 - Attachment is obsolete: true
Attachment #8444887 - Attachment is obsolete: true
Attachment #8450039 - Flags: review?(drs+bugzilla)
Attachment #8450039 - Flags: review?(drs+bugzilla)
Attached patch dial_via_bluetooth_headset.patch (obsolete) — Splinter Review
I am sorry for there is a little problem in that patch. 
I commit it again.
Please help to review.
Thanks a great.
Attachment #8450039 - Attachment is obsolete: true
Attachment #8450041 - Flags: review?(drs+bugzilla)
Comment on attachment 8450041 [details] [diff] [review]
dial_via_bluetooth_headset.patch

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

(In reply to Wei Gao (Spreadtrum) from comment #24)
> Created attachment 8450041 [details] [diff] [review]
> dial_via_bluetooth_headset.patch
> 
> I am sorry for there is a little problem in that patch. 
> I commit it again.
> Please help to review.
> Thanks a great.

I would strongly recommend adding tests for this functionality to dialer_test.js.
Attachment #8450041 - Flags: review?(drs+bugzilla) → review-
(In reply to Doug Sherk (:drs) from comment #25)
> I would strongly recommend adding tests for this functionality to
> dialer_test.js.

As for v1.4, there is little tests and many mock file is not added in. I wonder I should add tests in v1.4 or master?
Thanks.
Flags: needinfo?(drs+bugzilla)
(In reply to Wei Gao (Spreadtrum) from comment #26)
> (In reply to Doug Sherk (:drs) from comment #25)
> > I would strongly recommend adding tests for this functionality to
> > dialer_test.js.
> 
> As for v1.4, there is little tests and many mock file is not added in. I
> wonder I should add tests in v1.4 or master?
> Thanks.

I just checked, and everything you need should be on both branches. The patch might be slightly different when uplifted to v1.4, but I don't think it'll be significant. Is there anything in particular that you noticed was lacking that you need?
Flags: needinfo?(drs+bugzilla)
Attached patch dial_via_bluetooth_headset.patch (obsolete) — Splinter Review
Hi Doug

As I know litter about how to write test file, maybe there will be many questions in my patch. Could you help to correct me?
Thanks a great.
Attachment #8450041 - Attachment is obsolete: true
Flags: needinfo?(drs+bugzilla)
Comment on attachment 8454890 [details] [diff] [review]
dial_via_bluetooth_headset.patch

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

This is actually good overall. There are just some small mechanical mistakes within the tests that I think will be easy to fix.

::: apps/communications/dialer/js/dialer.js
@@ +358,5 @@
>        if (result && (typeof result === 'object') && result.number) {
> +        var settings = navigator.mozSettings;
> +        var getServiceId = settings.createLock().get('ril.telephony.defaultServiceId');
> +        getServiceId.onsuccess = function getDefaultServiceId() {
> +          var ServiceId = getServiceId.result['ril.telephony.defaultServiceId'] || 0;

s/ServiceId/serviceId/

@@ +359,5 @@
> +        var settings = navigator.mozSettings;
> +        var getServiceId = settings.createLock().get('ril.telephony.defaultServiceId');
> +        getServiceId.onsuccess = function getDefaultServiceId() {
> +          var ServiceId = getServiceId.result['ril.telephony.defaultServiceId'] || 0;
> +          if(ServiceId == -1) {

Space between if and (

@@ +363,5 @@
> +          if(ServiceId == -1) {
> +            // dial the last call number via the sim used last time
> +            CallHandler.call(result.number, result.serviceId);
> +          }
> +          else {

} else {

::: apps/communications/dialer/test/unit/dialer_test.js
@@ +460,5 @@
> +        MockNavigatorSettings.mSettings['ril.telephony.defaultServiceId'] = -1;
> +        getSpy.yield({number: '424242', serviceId: 0});
> +        sinon.assert.calledWith(callSpy, '424242', 0);
> +        getSpy.yield({number: '424242', serviceId: 1});
> +        sinon.assert.calledWith(callSpy, '424242', 1);

I would write this as:

```js
suite('> Dialing last entry in the call log', function() {
  var getSpy;
  var callSpy;

  setup(function() {
    getSpy = this.sinon.stub(MockCallLogDBManager, 'getGroupAtPosition');
    callSpy = this.sinon.stub(CallHandler, 'call');

    sendCommand('BLDN');
    sinon.assert.calledWith(getSpy, 1, 'lastEntryDate', true);
  });

  [0, 1].forEach(function(serviceId) {
    test('should dial on correct SIM', function() {
      // use serviceId var
      // ...
    });
  });

  test('should use last serviceId when set to always ask', function() {
    // check if it calls on 0 and 1
    // ...
  });
});
```
Flags: needinfo?(drs+bugzilla)
Attached patch dial_via_bluetooth_headset.patch (obsolete) — Splinter Review
Hi Doug

Thanks for your suggestion. That's very helpful for me.
I modified the patch, but there still some mistakes. I am confused about it.
Could you help me again?
Thanks a great.

Error: expected getGroupAtPosition to be called with arguments 1, lastEntryDate, true
Attachment #8454890 - Attachment is obsolete: true
Flags: needinfo?(drs+bugzilla)
(In reply to Wei Gao (Spreadtrum) from comment #30)
> Created attachment 8455089 [details] [diff] [review]
> dial_via_bluetooth_headset.patch
> 
> Hi Doug
> 
> Thanks for your suggestion. That's very helpful for me.
> I modified the patch, but there still some mistakes. I am confused about it.
> Could you help me again?
> Thanks a great.

This is good progress, don't worry.

> Error: expected getGroupAtPosition to be called with arguments 1,
> lastEntryDate, true

Where are you getting this error?
Flags: needinfo?(drs+bugzilla)
Attached image unit-test.png (obsolete) —
Dear Doug

When I run "./bin/gaia-test" to make a unit test, this error will show in terminal.
I don't know what's the matter.
Is there anything wrong about my operation?
Flags: needinfo?(drs+bugzilla)
Hi Wei,

test('> Dialing a specific recent entry', function() {

should be

suite('> Dialing a specific recent entry', function() {

Same for '> Dialing the last recent entry'
Flags: needinfo?(drs+bugzilla)
Attached patch dial_via_bluetooth_headset.patch (obsolete) — Splinter Review
(In reply to Doug Sherk (:drs) from comment #33)
> test('> Dialing a specific recent entry', function() {
> 
> should be
> 
> suite('> Dialing a specific recent entry', function() {
> 
> Same for '> Dialing the last recent entry'

Thanks for your suggestion.
That's so correct. But there still has some errors in unit-test.
I think "MockNavigatorSettings.mSettings['ril.telephony.defaultServiceId'] = serviceId;" is not good for this.

Error: expected call to be called with arguments 424242, 0
Error: expected call to be called with arguments 424242, 1
Error: expected call to be called with arguments 424242, 0
Error: expected call to be called with arguments 333, 0
Error: expected call to be called with arguments 333, 1
Error: expected call to be called with arguments 333, 0

I am so sorry to trouble you, but I don't know how to write correctly.
By the way, is there some study materials of unit-test file.
Thanks for your kindly help.
Attachment #8455089 - Attachment is obsolete: true
Attachment #8455919 - Attachment is obsolete: true
Flags: needinfo?(drs+bugzilla)
Ok, here it is fixed up. This patch can be cherry-picked if a partner needs it.
Attachment #8439646 - Attachment is obsolete: true
Attachment #8458480 - Attachment is obsolete: true
Flags: needinfo?(drs+bugzilla)
(In reply to Doug Sherk (:drs) from comment #35)
> Created attachment 8460334 [details] [diff] [review]
> Use last used SIM or default call SIM when calling with Bluetooth.
> 
> Ok, here it is fixed up. This patch can be cherry-picked if a partner needs
> it.

Oh, Thanks for your kindly help.
Ok, could I commit a PR for this patch, then it can be landed on master/v1.4/v1.3t ?
Thanks a lot.
Flags: needinfo?(drs+bugzilla)
Since this is closer to, if not the, correct solution, I think we should explore landing this again. I'm not sure if it'll get approval to land on 1.4 though. Partners might have to cherry-pick it themselves.

Carrie, here's what this patch implements. When the user hits the Bluetooth call button:
* If the outgoing call SIM is set to "always ask", we get the last-used SIM for the last-called contact and call back to them on it.
* If the outgoing call SIM is set to anything else, we use that SIM and call the last-called contact with it.
Does this seem reasonable to you? What should we do if the call log is empty? My suggestion would be to just ignore the button press, or open the keypad.

This isn't ready to land as it needs a proper review. Since I've written a lot of it, I can't review it anymore. I'd also like to do some cleanup on it before we take it back into review.

Some notes for myself when I come back to this:
* Can we do something cleaner than using setTimeout?
* Maybe we should assume the outgoing call SIM is -1/ALWAYS_ASK if it's not set.
* We need to fix the teardown logic.
* The CallHandler.call() function call can be consolidated a bit.
* We don't need |var settings|.
Flags: needinfo?(drs+bugzilla) → needinfo?(cawang)
Hi Doug, 

Yes, it looks fine to me. 
In addition, if the call log is empty, we just ignore the click, because users may not look at the screen while using BT headset. Thanks!
Flags: needinfo?(cawang)
Thanks Carrie. I'm taking this.
Assignee: nobody → drs+bugzilla
Status: NEW → ASSIGNED
Target Milestone: --- → 2.1 S1 (1aug)
Summary: [1.4] Set outgoing calls "Always ask" with two sim card inside, use bluetooth headset to dial, there will be no network message → Set outgoing calls "Always ask" with two sim card inside, use bluetooth headset to dial, there will be no network message
Comment on attachment 8462799 [details] [diff] [review]
Call on the last used SIM for this contact/number when hitting the Bluetooth call button.

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

feedback+ for the general implementation.

Can you also take a look and see if we can safely remove the shim from bug 982163 after landing this?

::: apps/communications/dialer/js/dialer.js
@@ +380,5 @@
>      CallLogDBManager.getGroupAtPosition(position, 'lastEntryDate', true, null,
>      function(result) {
>        if (result && (typeof result === 'object') && result.number) {
> +        var request = navigator.mozSettings.createLock().get(
> +          'ril.telephony.defaultServiceId');

We should load sim_settings_helper here. We are going lazy load it anyway in CallHandler.call. That will avoid the duplication of ALWAYS_ASK_OPTION_VALUE.

::: apps/communications/dialer/test/unit/dialer_test.js
@@ +488,5 @@
>          });
>        }
>  
>        test('> Dialing a specific number', function() {
> +        var callStub = this.sinon.stub(CallHandler, 'call');

Why all the spy -> stub changes? We're just watching the usage of CallHandler.call, we're not observing the return values.
Attachment #8462799 - Flags: review?(anthony) → feedback+
Blocks: 982163
The SimSettingsHelper.ALWAYS_ASK_OPTION_VALUE being loaded for the test is actually `undefined` because it's not included in the mock. Do you have any suggestions here to avoid code duplication?

Updated PR.
Attachment #8462799 - Attachment is obsolete: true
Attachment #8463762 - Flags: review?(anthony)
Comment on attachment 8463762 [details] [diff] [review]
Call on the last used SIM for this contact/number when hitting the Bluetooth call button.

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

::: apps/communications/dialer/js/dialer.js
@@ +379,5 @@
>        if (result && (typeof result === 'object') && result.number) {
> +        var request = navigator.mozSettings.createLock().get(
> +          'ril.telephony.defaultServiceId');
> +        request.onsuccess = function() {
> +          LazyLoader.load(['/shared/js/sim_settings_helper.js'], function() {

Sorry, my comment wasn't precise enough. We should load sim_settings_helper before and use SimSettingsHelper.getCardIndexFrom('outgoingCall', f() {}) instead of querying mozSettings directly.

::: apps/communications/dialer/test/unit/dialer_test.js
@@ +494,5 @@
> +      var getStub;
> +      var callSpy;
> +
> +      setup(function() {
> +        getStub = this.sinon.stub(MockCallLogDBManager, 'getGroupAtPosition');

getStub is hard to read later, please use a more meaningful variable name.

@@ +506,5 @@
>  
> +      suite('> Dialing the last recent entry', function() {
> +        setup(function() {
> +          sendCommand('BLDN');
> +          sinon.assert.calledWith(getStub, 1, 'lastEntryDate', true);

Move this assert in a test.

@@ +510,5 @@
> +          sinon.assert.calledWith(getStub, 1, 'lastEntryDate', true);
> +        });
> +
> +        [0, 1].forEach(function(serviceId) {
> +          test('should dial on correct SIM ' + serviceId, function(done) {

'should dial on user preferred SIM n'

@@ +524,3 @@
>  
> +        [0, 1].forEach(function(serviceId) {
> +          test('should use last serviceId ' + serviceId + ' if set always ask',

'should use serviceId (n) of last call'

@@ +542,5 @@
> +          sinon.assert.calledWith(getStub, 3, 'lastEntryDate', true);
> +        });
> +
> +        [0, 1].forEach(function(serviceId) {
> +          test('should dial on correct SIM ' + serviceId, function(done) {

'should dial on user preferred SIM n'

@@ +556,3 @@
>  
> +        [0, 1].forEach(function(serviceId) {
> +          test('should use last serviceId ' + serviceId + ' when always ask',

'should use serviceId (n) of 3rd last call'
Attachment #8463762 - Flags: review?(anthony) → review-
Comment on attachment 8464130 [details] [diff] [review]
Call on the last used SIM for this contact/number when hitting the Bluetooth call button.

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

I forgot to catch this earlier: please remove all the setTimeout, they are not needed. To do before landing.
Attachment #8464130 - Flags: review?(anthony) → review+
(In reply to Anthony Ricaud (:rik) from comment #45)
> I forgot to catch this earlier: please remove all the setTimeout, they are
> not needed. To do before landing.

Thanks, nice catch.

https://github.com/mozilla-b2g/gaia/commit/5e28fb653ee50f749cf450decc9d75c2ee5a02d1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
[Blocking Requested - why for this release]:
blocking-b2g: backlog → 1.4?
[Blocking Requested - why for this release]:

I think v1.4 also need this patch.
Can it be merged into v1.4?
Thanks.
Hi Doug,if no high risk or other concern on 1.4,Could you please kindly help provide 1.4 rebase patch for this? Thanks!
Flags: needinfo?(drs+bugzilla)
blocking-b2g: 1.4? → 1.4+
v1.4: https://github.com/mozilla-b2g/gaia/commit/3feb37ee2ed2319c9e556728723a5517dc1663ea

Note that per the recent policy change, you will need to explicitly request v2.0 approval if you want it to land there as well.
Flags: needinfo?(drs+bugzilla)
Needs to be covered by new test case.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Test case added in moztrap:

https://moztrap.mozilla.org/manage/case/14349/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Flags: in-moztrap+
Blocks: Woodduck
blocking-b2g: 1.4+ → 2.0M?
blocking-b2g: 2.0M? → 2.0M+
Attached video Verify video
This problem is verified not to happen on Woodduck2.0 and Flame 2.1.
See attachment: Verify_video.MP4
Occurrence rate: 0/5
According to the Comment#37, we have verified successfully with another case:
When the outgoing call SIM is set to SIM 1 (or SIM2), double click bluetooth headset button, it will use SIM1 (or SIM2) and call the last-called contact successfully.
	
Woodduck 2.0 build:
Gaia-Rev        afa87cffbd3cd9e2070b26d45dd556a9324bd4d5
Gecko-Rev       911e6cd6aecf8d37d42c203e162847b78a68a8d8
Build-ID        20141224050313
Version         32.0

Flame 2.1 build:
Gaia-Rev        17c7ad2e4919a994f0844239b483116090412dee
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/39dfb662c82a
Build-ID        20141223001203
Version         34.0
Blocks: 1104407
See Also: → 1104407
Duplicate of this bug: 1104407
You need to log in before you can comment on or make changes to this bug.