881142 - WebTelephony: remove Busy state

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

When we dial someone busy, we will hear busy tone from the network. But there is NO signal for 'busy' state transition accompanying that voice tone from the carrier. The call state remains alerting until user hangs it up, the call is redirected/forwarded, or the carrier releases it.

"Busy" information can be obtained by querying the carrier about the fail cause of the last call, but at the moment, the call has been already released at an earlier time. This is what we have done to transfer the call state to 'busy' in B2G, but we shall re-think about it, IMHO.

Removing busy state but providing a reason seems appropriate, if we do want to provide this 'busy' information.

Related discussion in W3C sysapps:
[1] http://lists.w3.org/Archives/Public/public-sysapps/2012Nov/0011.html
[2] http://lists.w3.org/Archives/Public/public-sysapps/2012Nov/0020.html

Comment 1

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

Dialer listens to busy event in [1] for playing busy tone. 
CC some gaia devs. to catch their attention and see whether the proposal in comment #0 sounds good to them. More ideas are welcomed! :)


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.js#L68

Comment 2

[Szu-Yu Chen [:aknow] (inactive)]

Proposal:
1. remove busy state.
2. carry the disconnect cause in the call event 'disconnected'.

if dialer need the busy information, it can do something like:

call.ondisconnected = function (evt) {
  if (evt.cause == busy) {
    busy();
  }
};

Comment 3

[Szu-Yu Chen [:aknow] (inactive)]

Another proposal.

'busy' should be one of the call error.
We notify it through call's error event.

call.onerror = function errorCB(evt) {
  if (evt.call.error.name == 'BusyError') busy();
};

I prefer this solution because except normal clearance, any other disconnect cause should be regarded as a call error.

Comment 4

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: WIP: remove ril call state busy

Modification based on Comment 3.

1. Remove call state busy in RIL, but still keep busy state in telephony for backward compatible.
  => tested. got busy tone
2. Fire error event 'BusyError'
  => app://communications.gaiamobile.org/dialer/js/telephony_helper.js:66 in errorCB: Unexpected error:  BusyError

future work: Keep going and remove the busy state in telephony after we figure out how we like to get notified of busy in dialer

Comment 5

[Etienne Segonzac (:etienne)]

Cc'ing Beatriz who originally reported bug 857951.

Comment 6

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

(In reply to Szu-Yu Chen [:aknow] from comment #3)
> Another proposal.
> 
> 'busy' should be one of the call error.
> We notify it through call's error event.
> 
> call.onerror = function errorCB(evt) {
>   if (evt.call.error.name == 'BusyError') busy();
> };
> 
> I prefer this solution because except normal clearance, any other disconnect
> cause should be regarded as a call error.

This sounds great to me as well :)

Comment 7

[Beatriz Rodríguez [:brg]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> (In reply to Szu-Yu Chen [:aknow] from comment #3)
> > I prefer this solution because except normal clearance, any other disconnect
> > cause should be regarded as a call error.
> 
> This sounds great to me as well :)
Will this proposal add the possibility to manage other disconnection reasons such as: No response(when called did not pick up the call) or Rejected (when called party rejects the incoming call)?

Comment 8

[Germán Toro del Valle (:gtorodelvalle)]

(In reply to Szu-Yu Chen [:aknow] from comment #3)
> Another proposal.
> 
> 'busy' should be one of the call error.
> We notify it through call's error event.
> 
> call.onerror = function errorCB(evt) {
>   if (evt.call.error.name == 'BusyError') busy();
> };
> 
> I prefer this solution because except normal clearance, any other disconnect
> cause should be regarded as a call error.

Hi guys! I definitely consider more intuitive the comment 2 option also by :aknow ;-) Is the busy state really an error? :-)

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Beatriz Rodríguez [:brg] from comment #7)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> > (In reply to Szu-Yu Chen [:aknow] from comment #3)
> > > I prefer this solution because except normal clearance, any other disconnect
> > > cause should be regarded as a call error.
> > 
> > This sounds great to me as well :)
> Will this proposal add the possibility to manage other disconnection reasons
> such as: No response(when called did not pick up the call) or Rejected (when
> called party rejects the incoming call)?

contains following errors
Ref: 3GPP TS 24.008: Cause information element values

"BadNumberError";            
"BusyError";                 <= called is busy
"NoUserRespondingError";     <= network cannot reach the called
"UserAlertingNoAnswerError"; <= called ring, user no answer
"CallRejectedError";         <= user reject
"NumberChangedError";        
"PreEmptionError";           
"DestinationOutOfOrderError";
"InvalidNumberFormatError";  
"FacilityRejectedError";     
"CongestionError";           
"NetworkOutOfOrderError";    
"NetworkTempFailureError";   
"IncomingCallExceededError"; 
"BarredError";               
"FDNBlockedError";           
"SubscriberUnknownError";    
"DeviceNotAcceptedError";    
"UnspecifiedError";

Comment 10

[Beatriz Rodríguez [:brg]]

(In reply to Szu-Yu Chen [:aknow] from comment #9)
> (In reply to Beatriz Rodríguez [:brg] from comment #7)
> > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> > > (In reply to Szu-Yu Chen [:aknow] from comment #3)
> > > > I prefer this solution because except normal clearance, any other disconnect
> > > > cause should be regarded as a call error.
> > > 
> > > This sounds great to me as well :)
> > Will this proposal add the possibility to manage other disconnection reasons
> > such as: No response(when called did not pick up the call) or Rejected (when
> > called party rejects the incoming call)?
> 
> contains following errors
> Ref: 3GPP TS 24.008: Cause information element values
> 
> "BadNumberError";            
> "BusyError";                 <= called is busy
> "NoUserRespondingError";     <= network cannot reach the called
> "UserAlertingNoAnswerError"; <= called ring, user no answer
> "CallRejectedError";         <= user reject
> "NumberChangedError";        
> "PreEmptionError";           
> "DestinationOutOfOrderError";
> "InvalidNumberFormatError";  
> "FacilityRejectedError";     
> "CongestionError";           
> "NetworkOutOfOrderError";    
> "NetworkTempFailureError";   
> "IncomingCallExceededError"; 
> "BarredError";               
> "FDNBlockedError";           
> "SubscriberUnknownError";    
> "DeviceNotAcceptedError";    
> "UnspecifiedError";

Thanks :aknow, if we can handle all the disconnection reasons, IMHO it is the best approach. Shall we file a new bug for this or it is already somewhere there?

Comment 11

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Beatriz Rodríguez [:brg] from comment #10)
> Thanks :aknow, if we can handle all the disconnection reasons, IMHO it is
> the best approach. Shall we file a new bug for this or it is already
> somewhere there?

It's already existed in current code except "BusyError".

The purpose of this bug is to remove the busy state and get it back as one of the error cause. Thus we can handle busy/reject/no response... all in call.onerror().

Comment 12

[Beatriz Rodríguez [:brg]]

(In reply to Szu-Yu Chen [:aknow] from comment #11)
> The purpose of this bug is to remove the busy state and get it back as one
> of the error cause. Thus we can handle busy/reject/no response... all in
> call.onerror().
Thanks for the clarification. I got confused because they are disconnection causes/reasons but not all of them are "errors". But now I got it!

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I like the solution of removing the busy state, but instead signaling through the error state that the error was due to the callee being busy.

Comment 14

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Remove call state busy

Comment 15

[Szu-Yu Chen [:aknow] (inactive)]

File a bug for dialer modification - Bug 888799
Dialer should remove the reference to busy state.

Comment 16

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

Comment on attachment 769554 [details] [diff] [review]
Remove call state busy

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

Don't we need to modify Telephony.cpp and nsIDOMTelephony.idl accordingly?

Comment 17

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #16)
> Comment on attachment 769554 [details] [diff] [review]
> Remove call state busy
> 
> Review of attachment 769554 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Don't we need to modify Telephony.cpp and nsIDOMTelephony.idl accordingly?

Sure. I just forgot this part. Thanks for the reminder.
I will add another patch later.

Comment 18

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1: Remove call state busy (idl)

Comment 19

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Remove call state busy (telephony dom)

Comment 20

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 3: Remove call state busy (bluetooth dom)

Hi Gina,

CALL_STATE_BUSY is removed.
I add a new parameter in hfp->HandleCallStateChanged() to pass the error code with busy information. Please help me check whether the modifications and new param order are appropriate. It's welcome to suggest a better way for this issue because I'm not familiar with this part. Thanks.

Comment 21

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 4: Remove call state busy (RIL)

Comment 22

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 5: Modify test_outgoing_busy for removing call state busy

Comment 23

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 769619 [details] [diff] [review]
Part 1: Remove call state busy (idl)

I'm not an sr! -> sicking.

Comment 24

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

Comment on attachment 769619 [details] [diff] [review]
Part 1: Remove call state busy (idl)

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

Looks good to me!

Comment 25

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

Comment on attachment 769629 [details] [diff] [review]
Part 5: Modify test_outgoing_busy for removing call state busy

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

Thank you :)

Comment 26

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

Comment on attachment 769626 [details] [diff] [review]
Part 3: Remove call state busy (bluetooth dom)

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

Thanks for your help, aknow. I had no problem with this patch, but I'd like to test it before landing, and btain is helping on this. I'll r+ after getting his feedback.

Comment 27

[Ben Tian (inactive)]

aknow,

I've verified patch part 3 works correctly. Still one minor change is required: patch part 3 conflicts on the latest m-c as following, probably due to our recent change on BluetoothTelephonyListener.cpp.

--
dom/bluetooth/BluetoothTelephonyListener.cpp.rej:

--- dom/bluetooth/BluetoothTelephonyListener.cpp
+++ dom/bluetooth/BluetoothTelephonyListener.cpp
@@ -75,7 +75,7 @@
   // via setting CALL_STATE_DISCONNECTED
   hfp->HandleCallStateChanged(aCallIndex,
                               nsITelephonyProvider::CALL_STATE_DISCONNECTED,
-                              EmptyString(), false, true);
+                              aError, EmptyString(), false, true);
   NS_WARNING("Reset the call state due to call transition ends abnormally");
   NS_WARNING(NS_ConvertUTF16toUTF8(aError).get());
   return NS_OK;
--

Also is dialer app aware of BUSY state removal? Dialer app didn't play busy tone when I was verifying.

Comment 28

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

Comment on attachment 769626 [details] [diff] [review]
Part 3: Remove call state busy (bluetooth dom)

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

Please update patch based on comment 27. Would like to review the patch again after revisioning.

Comment 29

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 3#2: Remove call state busy (bluetooth dom)

Comment 30

[Szu-Yu Chen [:aknow] (inactive)]

Hi Ben,

> Also is dialer app aware of BUSY state removal? Dialer app didn't play busy
> tone when I was verifying.

Bug 888799 (not yet ready now), this is the bug for dialer modification.

Comment 31

[Szu-Yu Chen [:aknow] (inactive)]

Comment on attachment 771255 [details] [diff] [review]
Part 3#2: Remove call state busy (bluetooth dom)

rebase done

Comment 32

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

Comment on attachment 771255 [details] [diff] [review]
Part 3#2: Remove call state busy (bluetooth dom)

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

r+ with nits addressed. Thanks!

::: dom/bluetooth/BluetoothHfpManager.h
@@ +86,5 @@
>     * @param aSend A boolean indicates whether we need to notify headset or not
>     */
>    void HandleCallStateChanged(uint32_t aCallIndex, uint16_t aCallState,
> +                              const nsAString& aError, const nsAString& aNumber,
> +                              const bool aIsOutgoing, bool aSend);

Please help to remove the "const" for aIsOutgoing.

Comment 33

[Szu-Yu Chen [:aknow] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=4aab565f3329

Comment 34

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 1: Remove call state busy (idl). r=hsinyi, sr=jonas

Comment 35

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 2: Remove call state busy (telephony dom). r=hsinyi,bent

Comment 36

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 3: Remove call state busy (bluetooth dom). r=gyeh

Comment 37

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 4: Remove call state busy (RIL). r=hsinyi

Comment 38

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 5: Modify test_outgoing_busy for removing call state busy. r=hsinyi

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/projects/birch/rev/7cfd5e50d336
https://hg.mozilla.org/projects/birch/rev/fdd7e0128a06
https://hg.mozilla.org/projects/birch/rev/a786390a0e95
https://hg.mozilla.org/projects/birch/rev/4f2be38defdb
https://hg.mozilla.org/projects/birch/rev/e8268cbaf2d2

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7cfd5e50d336
https://hg.mozilla.org/mozilla-central/rev/fdd7e0128a06
https://hg.mozilla.org/mozilla-central/rev/a786390a0e95
https://hg.mozilla.org/mozilla-central/rev/4f2be38defdb
https://hg.mozilla.org/mozilla-central/rev/e8268cbaf2d2