875677 - [Buri][BT][PTS][HFP] Bluetooth certification test case TC_AG_TWC_BV_05_I failed (Needs to follow HFP 1.6 not HFP 1.5)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect, P1)

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.111
 Firefox os  v1.0.1
 Mozilla build ID:20130518070208
 
 
 +++ This bug was initially created as a clone of Bug #457697 +++
 
 DEFECT DESCRIPTION:
 The 3 HFP cases  failed
  REPRODUCING PROCEDURES:
 TC_AG_TCA_BV_01_I :first prompt close the SCO connection between IUT and
 PTS,when I press speaker icon,the call will auto end itself.
 TC_AG_TCA_BV_04_I:PTS show first prompt is :close the SCO,but have no any
 prompt to have a call before,so can not do this case.
 TC_AG_TWC_BV_05_I:The second time IUT dial a call(TSPX_Second phone number),it
 will auto end,then the case end.
 
  EXPECTED BEHAVIOUR:
 The all supported HFP cases pass
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 5/5
  For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #457697 description ++++++++++
 
 
 
 CONTACT INFO (Name,Phone number):
 
  DEFECT DESCRIPTION:
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Clone from brother

Comment 2

[sync-1]

Attachment: sniffer log

Clone from brother

Comment 3

[sync-1]

Clone from brother

Comment 4

[sync-1]

Attachment: kernel log

Clone from brother

Comment 5

[sync-1]

Clone from brother

Comment 6

[sync-1]

Attachment: Test plan file

Clone from brother

Comment 7

[sync-1]

Clone from brother

Comment 8

[sync-1]

Attachment: logcat

Clone from brother

Comment 9

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

For TC_AG_TWC_BV_05_I. Please refer to Bug 875677 and discussion in Bug 828804.

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Shawn Huang from comment #9)
> For TC_AG_TWC_BV_05_I. Please refer to Bug 875677 and discussion in Bug
> 828804.

typo: Please refer to Bug  875686.

Comment 11

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

TC_AG_TCA_BV_04_I:PTS show first prompt is :close the SCO,but have no any
 prompt to have a call before,so can not do this case.
Go to Settings and disconnect bluetooth device, PTS can pass.

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

TC_AG_TCA_BV_01_I:
I cannot reproduce this bug. And press speaker icon shall not end the call. Can you provide more information? From sniffer, i can only see verdict as none.

Comment 13

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Please feedback question on Comment 11 and 12. Especially, Comment 12, I also try your buri. And I cannot reproduce this bug.

Comment 14

[qingluo.wang]

(In reply to Shawn Huang from comment #13)
> Please feedback question on Comment 11 and 12. Especially, Comment 12, I
> also try your buri. And I cannot reproduce this bug.

Hi Shawn,

It's the reporter's default,TC_AG_TCA_BV_01_I and TC_AG_TCA_BV_04_I can pass now,about TC_AG_TWC_BV_05_I, maybe we need to wait for the patch of 875686,and I'll nominate 875686 as 'tef?'.

Thanks for your support.

Comment 15

[Alex Keybl [:akeybl]]

Can somebody explain the exact user scenarios that are impacted here?

Comment 16

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hi Cheng-An, XIONG,
TC_AG_TCA_BV_04_I: 
We found it's a valid bug. Patch is in Bug 875719. And this patch needs to go into v1.0.1. Although tester can pass this case by a tricky way to operate as Comment 11 depicted. But that's not a correct to do it. The impact of the bug mentioned in Bug 875719. Please refer it.

Comment 17

[qingluo.wang]

Test case : TC_AG_TWC_BV_05_I started
        - SDP Service record for PTS: 'Handsfree HF' successfully registered
        - The IUT claims support for the following eSCO LMP packet types: EV3,
2-EV3, 
        - AT: SPP connect succeeded
        - AT: Service Level Connection established
        - AT: post SLC command sequence complete
        - AT: RING
        - AT: Received +CLIP: "02161460666",129
        - HCI: Audio Connection enabled
        - AT: Incoming call established
        - MTC: Call established by tester
        - unexpected second call completion (second outgoing call)
        - FATAL ERROR (AT): (callsetup=2) received when (call=1), (callheld=1),
and there is no second outgoing call process.
        - FATAL ERROR (AT): callsetup=0 received when there is a held call, and
active call, but no previously initiated CHLD action.
        - HCI: Audio Connection disabled
        - AT: SPP disconnect succeeded
        - MTC: Test case ended
Final Verdict : Inconclusive
TC_AG_TCA_BV_05_I 的問題是符合HFP 1.5 Spec.測試結果是在此處：
  +CIEV: 3,1 (callheld=1)
  +CIEV: 4,2  (callsetup=2)
判定fail. 應該是PTS測試工具的問題

Comment 18

[Eric Chou [:ericchou] [:echou]]

> TC_AG_TCA_BV_05_I 的問題是符合HFP 1.5 Spec.測試結果是在此處：
>   +CIEV: 3,1 (callheld=1)
>   +CIEV: 4,2  (callsetup=2)
> 判定fail. 應該是PTS測試工具的問題

Let me translate it to English:

The implementation follows HFP 1.5 spec correctly, but the test case TC_AG_TCA_BV_05_I still fails. Therefore we think it should be a issue of PTS.

Comment 19

[Daniel Coloma:dcoloma]

What is the conclusion here? Do we need to change anything in our implementation to pass BT certification?

Comment 20

[sync-1]

Comment from Mozilla:What is the conclusion here? Do we need to change anything in our implementation to pass BT certification?

Comment 21

[Eric Chou [:ericchou] [:echou]]

Two of these three test cases could be passed with current implementation per comment 14.

About test case TC_AG_TWC_BV_05_I, we followed HFP spec 1.5 and implementation has been done for a long time. However, our partners told us that our code couldn't pass Bluetooth certification. 

After investigation, we finally came up with a conclusion: it's a known issue of HFP 1.5, and it has been revised in HFP 1.6. There is an official errata which records the whole problem and the discussion. (https://www.bluetooth.org/errata/errata_view.cfm?errata_id=2459)

We should fix this, and I'll provide a patch today. Nominate as tef+ since it blocks Bluetooth certification.

Comment 22

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: v1: Modify the logic of sending 'callheld' indicator to fit HFP 1.6

The behavior of handling held calls is different between HFP 1.5 and HFP 1.6. In order to pass certification, we have to follow the mechanism defined in HFP 1.6.

Comment 23

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 756420 [details] [diff] [review]
patch 1: v1: Modify the logic of sending 'callheld' indicator to fit HFP 1.6

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

The patch seems reasonable to me. Thanks for your effort.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +1343,5 @@
>            UpdateCIND(CINDType::CALLSETUP, CallSetupState::NO_CALLSETUP, aSend);
> +
> +          if (FindFirstCall(nsIRadioInterfaceLayer::CALL_STATE_HELD)) {
> +            UpdateCIND(CINDType::CALLHELD, CallHeldState::ONHOLD_ACTIVE, aSend);
> +          }

I would suggest to combine the logic here with the following case of CALL_STATE_HELD becausewe will call FindFirstCall(CALL_STATE_HELD) in both cases. And, we also handle held calls separately whenever there's a disconnected call.

Comment 24

[Eric Chou [:ericchou] [:echou]]

Attachment: (v1.0.1) patch 1: final: Modify the logic of sending 'callheld' indicator to fit HFP 1.6, r=gyeh

* advice taken

Comment 25

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Please let me know the final patch is upload, i will verify it again.

Comment 26

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

batch update on tef+ milestones. partner to make a final on 6/3 Asia time. TEF+ needs to be resolved by 6/3 to be in the final build. thanks

Comment 27

[Eric Chou [:ericchou] [:echou]]

Attachment: (b2g18) patch 1: final: Modify the logic of sending 'callheld' indicator to fit HFP 1.6, r=gyeh

* b2g18-specific patch

Comment 28

[Eric Chou [:ericchou] [:echou]]

Attachment: (m-c) patch 1: final: Modify the logic of sending 'callheld' indicator to fit HFP 1.6, r=gyeh

* m-c patch

Comment 30

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/projects/birch/rev/a2d4ffa8ef69

Comment 31

[Eric Chou [:ericchou] [:echou]]

Attachment: (b2g18) patch 1: final: Modify the logic of sending 'callheld' indicator to fit HFP 1.6, r=gyeh

* updated b2g18-speficic patch

Comment 32

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/mozilla-central/rev/736965f66934

Comment 33

[Eric Chou [:ericchou] [:echou]]

backed out due to build break: 

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

Comment 34

[Eric Chou [:ericchou] [:echou]]

Root cause: applied the previous version of patch.

I'll wait for m-c becoming all green and apply patch again.

Comment 35

[Eric Chou [:ericchou] [:echou]]

Resend to m-c:

https://hg.mozilla.org/mozilla-central/rev/6edffc15e2dc

Comment 36

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/4785b1353fd7
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/42555e1e72fa

Comment 37

[Ryan VanderMeulen [:RyanVM]]

(In reply to Eric Chou [:ericchou] [:echou] (6/25 ~ 28 @ Shanghai, GSMA MAE) from comment #33)
> backed out due to build break: 
> 
>   https://hg.mozilla.org/mozilla-central/rev/d87eaee61930

Why exactly did you land this on m-c separately if it was already on birch? It would have been merged over eventually anyway and would have saved the bustage and churn...

Comment 38

[Eric Chou [:ericchou] [:echou]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #37)
> (In reply to Eric Chou [:ericchou] [:echou] (6/25 ~ 28 @ Shanghai, GSMA MAE)
> from comment #33)
> > backed out due to build break: 
> > 
> >   https://hg.mozilla.org/mozilla-central/rev/d87eaee61930
> 
> Why exactly did you land this on m-c separately if it was already on birch?
> It would have been merged over eventually anyway and would have saved the
> bustage and churn...

Sorry, I don't understatnd the whole process very well. So, if I want to get the patch which has been already in birch merged into b2g18 and v1_0_1, I don't need to care about whether it's in m-c or not, right?

Comment 39

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/9632a9a4981c

Comment 40

[Ryan VanderMeulen [:RyanVM]]

(In reply to Eric Chou [:ericchou] [:echou] (6/25 ~ 28 @ Shanghai, GSMA MAE) from comment #38)
> Sorry, I don't understatnd the whole process very well. So, if I want to get
> the patch which has been already in birch merged into b2g18 and v1_0_1, I
> don't need to care about whether it's in m-c or not, right?

Normally you would land on birch and birch would then be merged to m-c. The uplifts to b2g18* are typically after it lands on m-c, but we've bent the rules in the past for urgent ones (like this one must have been if it couldn't even wait until after the weekend was over).

Comment 41

[Eric Chou [:ericchou] [:echou]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #40)
> (In reply to Eric Chou [:ericchou] [:echou] (6/25 ~ 28 @ Shanghai, GSMA MAE)
> from comment #38)
> > Sorry, I don't understatnd the whole process very well. So, if I want to get
> > the patch which has been already in birch merged into b2g18 and v1_0_1, I
> > don't need to care about whether it's in m-c or not, right?
> 
> Normally you would land on birch and birch would then be merged to m-c. The
> uplifts to b2g18* are typically after it lands on m-c, but we've bent the
> rules in the past for urgent ones (like this one must have been if it
> couldn't even wait until after the weekend was over).

Oh, I know the whole birch->m-c->b2g18* process. In fact, the urgent case you mentioned is exactly where I was, and that's why I was trying to merge these two bugs into v1_0_1 in a rush. So, my question is, if I have to make the patch which has been on birch already land on b2g18*, all I have to do is check it into b2g18* but not check into m-c. Is that correct?

Comment 42

[Ryan VanderMeulen [:RyanVM]]

If it's green on birch, you can just land directly on b2g18* in a pinch, yes. Keep in mind that you'll need to watch your push per the tree rules (especially important in this case since birch has reduced builds/tests compared to m-c and b2g18*). Please leave the m-c landing for birch merges only. Also, I'm handling the v1.1hd landings by merging b2g18 over, so you don't need to worry about landing there directly.

Comment 43

[Eric Chou [:ericchou] [:echou]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #42)
> If it's green on birch, you can just land directly on b2g18* in a pinch,
> yes. Keep in mind that you'll need to watch your push per the tree rules
> (especially important in this case since birch has reduced builds/tests
> compared to m-c and b2g18*). Please leave the m-c landing for birch merges
> only. Also, I'm handling the v1.1hd landings by merging b2g18 over, so you
> don't need to worry about landing there directly.

Very clear. Thank you, Ryan.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

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