DuT doesn't show "busy line" screen or play "busy line" tone

RESOLVED FIXED in 1.0.1 IOT1 (10may)

Status

P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: brg, Assigned: gtorodelvalle)

Tracking

unspecified
1.0.1 IOT1 (10may)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.1 fixed)

Details

(Whiteboard: IOT, Spain, Ikura [status: needs new patch][madrid])

Attachments

(2 attachments)

Comment hidden (empty)
(Reporter)

Comment 1

5 years ago
When calling to a busy line, DuT doesn't show "busy line" screen and doesn't play "busy line" tone. It just finish the call and go back to previous screen (call log, contact...). Same occured when rejecting call in the other side.

- Steps to Reproduce:
    
1.- Call to a busy line --> check the screen and tones
2.- Call to a phone number and reject the call --> check the screen and tones
    
- Actual Result:
    
DuT doesn't show "busy line" screen and doesn't play "busy line" tone

- Expected Result (correct behaviour wished):
    
It may show the "busy line" screen and play the corresponding tone 

Buildid "20130321070205", device: ikura
blocking-b2g: --- → tef?

Updated

5 years ago
blocking-b2g: tef? → tef+
DuT stands for 'Device Under Test'.
It seems that the dialer app doesn't handle the 'onbusy' events. This is not only a bug for Ikura devices but also for all devices.
Assignee: nobody → gtorodelvalle
José Antonio is absolutely right :-O

Ayman, could you please provide me with the concrete text you would like to show the user to notify him that the line is busy? I guess this informative message would be shown to the user using the Confirm BB: http://buildingfirefoxos.com/building-blocks/confirm/ including only one button. Please, provide me also with the text of that button and if it should be a "normal" one or a "danger" (red) one. Thank you very much!
Flags: needinfo?(aymanmaat)
Germán, is there any way we can avoid requiring late strings for this solution?
Flags: needinfo?(gtorodelvalle)
We may just close the attention screen (as it is already done right now) while playing the "busy line" tones for some time (2.5 seconds or so since it seems they are 0.5s long tones (http://www.tech-faq.com/frequencies-of-the-telephone-tones.html)) before actually hanging the call.

Another option may be to show some kind of icon in the outgoing call attention screen instead of the timer shown now when the call is established to somehow notify the user that the other side is busy, as well as playing the "busy line" tones of course, until the user clicks on the hang up button of the outgoing call attention screen to close it and actually hang up the call.

I hope I made myself clear ;-)
Flags: needinfo?(gtorodelvalle)
Requesting info from Dani regarding comment 6. Just to help you keep track of this ;-) Thanks!
Flags: needinfo?(dcoloma)
Changing prio to P1, as this is a certification blocker.
Priority: -- → P1
Is the busy tone or the icon/text (please no ;) the blocker here?  Or is it both?
Flags: needinfo?(dpv)
Adding some colleagues in CC
Hi, 

We are going to check if it'd be enough implementing just the busy tone. I hope to have an answer today. 

Thanks!
David
Flags: needinfo?(dpv)

Updated

5 years ago
Whiteboard: IOT, Spain, Ikura
Hi again, 

The busy tone for sure is a blocker. We are trying the carrier to accept not to include the text for the call ending. 

I hope to have some news tomorrow... 

Thanks!
David
Hi guys! Don't know if it helps but using Android 4.2.2, no busy line tones are played. The outgoing call screen just shows a "Finalized call" message (hard to see, BTW) and it disappears after a couple of seconds ;-)
(In reply to gtorodelvalle from comment #13)
> Hi guys! Don't know if it helps but using Android 4.2.2, no busy line tones
> are played. The outgoing call screen just shows a "Finalized call" message
> (hard to see, BTW) and it disappears after a couple of seconds ;-)

I tested the same on Kiss II device (unagi) running Android ICS and busy tones were played.
ni?(dpv) while he gets information from comment 12.
Flags: needinfo?(dpv)
We have manage to agree that for certifying 1.0.1 we only need the tone (as we are already string frozen), however we need to open a follow up bug to display the busy message for 1.1
Flags: needinfo?(dpv)
Flags: needinfo?(dcoloma)
Will we have country-specific tones?

http://en.wikipedia.org/wiki/Busy_signal
German, when do you estimate to have a fix for this?
Whiteboard: IOT, Spain, Ikura → IOT, Spain, Ikura [status: needs ETA]
Hi! I would say today EOB ;-) Basically, due to the testing.
BTW, I am leaning the solution on Etienne's patch for https://bugzilla.mozilla.org/show_bug.cgi?id=859354 where the tone playing library has been refactored. So basically, both bugs will be solved at the same time, or at least that's the intention ;-)
(In reply to gtorodelvalle from comment #20)
> So basically, both bugs will be solved at the
> same time, or at least that's the intention ;-)

Sounds like a good plan :)
Depends on: 859354
Created attachment 738149 [details] [diff] [review]
Proposed patch.

This patch is dependent on the patch proposed by Etienne for https://bugzilla.mozilla.org/show_bug.cgi?id=859354 which can be checked here: https://github.com/mozilla-b2g/gaia/pull/9151

This patch MUST NOT be merged before https://github.com/mozilla-b2g/gaia/pull/9151 lands.
Attachment #738149 - Flags: review?(etienne)
Etienne, if it helps you can find the branch I created from your branch bug-859354-call-waiting-alert here: https://github.com/gtorodelvalle/gaia/tree/dialer-bug-857951-busy-tone-from-etiennesegonzac-bug-859354-call-waiting-alert

The commit including this patch is this one: https://github.com/gtorodelvalle/gaia/commit/9da6c4b2317dcffffd1b852cce6482e00dc4536e

Both include obviously the updates made in the patch I just attached.
Whiteboard: IOT, Spain, Ikura [status: needs ETA] → IOT, Spain, Ikura [status: needs review etienne]

Updated

5 years ago
Whiteboard: IOT, Spain, Ikura [status: needs review etienne] → IOT, Spain, Ikura [status: needs review etienne][madrid]
Comment on attachment 738149 [details] [diff] [review]
Proposed patch.

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

To sum up:
- we need to add this small test, happy to help

And we should be able to cover all the cases with the |busyNotificationLock|
- false when we start
- true when we start notifying
- false once we're done (before the exitCallScreen call)
- false whenever somebody presses end (before we check if we need to close)

::: apps/communications/dialer/js/handled_call.js
@@ +181,5 @@
>  };
>  
> +HandledCall.prototype.busy = function hc_busy() {
> +  OnCallHandler.notifyBusyLine();
> +};

Can you add a quick test to |handled_call_test.js|?
Should be pretty straightforward, ping me if needed.

::: apps/communications/dialer/js/oncall.js
@@ +153,5 @@
>    var closing = false;
>    var animating = false;
>    var ringing = false;
> +  var notifying = false;
> +  var ignoreNotifying = false;

|var busyNotificationLock = false|;

@@ +447,4 @@
>    }
>  
>    function exitCallScreen(animate) {
> +    if (closing || (notifying && !ignoreNotifying)) {

if (closing || busyNotificationLock)

@@ +612,1 @@
>  

-- in the end() function we release the lock
|busyNotificationLock = false;|

@@ +612,4 @@
>  
>      // If not we're rejecting the last incoming call
>      if (!handledCalls.length) {
> +      ignoreNotifying = true;

not needed anymore (I think)

@@ +617,5 @@
>        return;
>      }
>  
>      var lastCallIndex = handledCalls.length - 1;
> +    ignoreNotifying = true;

not needed either

@@ +647,4 @@
>      postToMainWindow(message);
>    }
>  
> +  function notifyBusyLine() {

|busyNotificiationLock = true|

::: apps/communications/dialer/oncall.html
@@ +11,4 @@
>      <!-- Localization -->
>      <script defer type="text/javascript" src="/shared/js/lazy_loader.js"></script>
>      <script defer type="application/javascript" src="/dialer/js/lazy_l10n.js"></script>
> +    <link rel="resource" type="audio/ogg" href="/shared/resources/media/notifications/notifier_bap.opus"/>

probably a merge issue, I don't think we need this anymore.
Attachment #738149 - Flags: review?(etienne)
Whiteboard: IOT, Spain, Ikura [status: needs review etienne][madrid] → IOT, Spain, Ikura [status: needs new patch][madrid]
Very nice comments as always, Etienne ;-)

You can find the updated local commit at https://github.com/gtorodelvalle/gaia/commit/cd640703da956b54491cb4e21fa563cd30c19c66
Ups! Give me a sec :-O Solving Lint issues! :-)
(In reply to gtorodelvalle from comment #27)
> The right local commit (Lint issues solved ;-)):
> https://github.com/gtorodelvalle/gaia/commit/
> 75e39551b4a2b02971bad809ca4127eefa7a5d28

Looking good and all tests green!
Created attachment 738617 [details]
Associated PR.
Attachment #738617 - Flags: review?(etienne)
Comment on attachment 738617 [details]
Associated PR.

\o/
Attachment #738617 - Flags: review?(etienne) → review+
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
status-b2g18: --- → affected
status-b2g18-v1.0.1: --- → affected

Updated

5 years ago
Target Milestone: --- → Madrid (19apr)
Uplifted 060ceed93b781cf43a4454ffccabf48ef06c681e to:
v1-train: 31875d104c1c90e06db21e0596c620171ef08f44
v1.0.1: 89d509e1c196022b426a51d1d44ba11b3fd1751d
status-b2g18: affected → fixed
status-b2g18-v1.0.1: affected → fixed
Hi guys! This patch is highly dependent on the one for bug 859354 which it seems has not already been landed either in v1.0.1 or v1-train and consequently this patch will not work in both branches :-(
Flags: needinfo?(aymanmaat)
I have tested this issue in the following version
  build ID:20130426165700
  build information:
  gaia commit:
  e25b349 Merge pull request #9273 from gasolin/issue-863126
  gecko commit:
  6dfd179 Bug 849757 - Part 4: xpcshell tests. r=vicamo, a=tef+
  AU:V1.01.00.01.019.085
and it is still failing. 
Since the patch should be already uplifted to the v1.0.1 branch, I reopen the bug
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
qawanted - is this fixed on any branch in that case?

Hopefully we can get this fixed ASAP.
status-b2g18: fixed → ?
status-b2g18-v1.0.1: fixed → ?
Keywords: qawanted
Target Milestone: 1.0.1 Madrid (19apr) → 1.0.1 IOT1 (10may)
Not to be entire stickler on process, but can we please open a new bug and mark it as a dependency on this bug? I'd rather not reopen bugs with patches that have already landed.

Feel free to put qawanted again though on the followup bug.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
status-b2g18: ? → fixed
status-b2g18-v1.0.1: ? → fixed
Keywords: qawanted
Resolution: --- → FIXED
(In reply to Jason Smith [:jsmith] from comment #36)
> Not to be entire stickler on process, but can we please open a new bug and
> mark it as a dependency on this bug? I'd rather not reopen bugs with patches
> that have already landed.
> 
> Feel free to put qawanted again though on the followup bug.

No problem at all. Sorry about that. I'll open a new bug
And please include the proper dependencies amongst bugs for easier tracking ;-) Thanks!
Blocks: 867065
Depends on: 872989

Updated

5 years ago
See Also: → bug 883619
You need to log in before you can comment on or make changes to this bug.