1000485 - [meta] Streamline nsITelephonyService API

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: RIL, defect)

Description

[Michael Schwartz [:m4]]

nsITelephonyProvider has redundant entries:

# boolean isActive is redundant with unsigned short callState
# conferenceCallStateChanged is redundant as it is just a reflection of the state for calls with conf=True

Comment 1

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

(In reply to Michael Schwartz [:m4] from comment #0)
> nsITelephonyProvider has redundant entries:
> 
> # boolean isActive is redundant with unsigned short callState

Agree we could think about removing this.

> # conferenceCallStateChanged is redundant as it is just a reflection of the
> state for calls with conf=True

Not exactly. This is used to notify that every conference participant has been visited. So that TelephonyDOM could know when is the correct time to dispatch conference state change event to gaia.

Comment 2

[Michael Schwartz [:m4]]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #1)
> (In reply to Michael Schwartz [:m4] from comment #0)
> > # conferenceCallStateChanged is redundant as it is just a reflection of the
> > state for calls with conf=True
> 
> Not exactly. This is used to notify that every conference participant has
> been visited. So that TelephonyDOM could know when is the correct time to
> dispatch conference state change event to gaia.

Ok, thanks; good to know.  If that's the case then it should be called conferenceCallStateChanged without a state parameter, right?

Also, the API is ambiguous about the usage of CallError ie. does it replace the CallStateChanged(DISCONNECTED) notification?  Seems like we should remove the DISCONNECTED state and swap out the corresponding callStateChanged notification or callError for callDisconnected(DisconnectCause).

Comment 3

[Anshul]

Hsin-Yi, in addition to what M4 suggested, am wondering if we could also look into simplifying the number of messages exchanged between DOM and RIL. 

For example, currently if the state of two calls have changed (switch active and holding call), we end up sending two call state change notification for each of the call. But instead if we change the API to simply notify to DOM that the state of calls changed as a single API call and then gecko calls EnumerateCalls, which already exists, to get the states of all the calls in one go, the number of messages could be reduced. Let me know what you think?

Comment 4

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

Hi Michael and Anshul,

Thanks for the suggestions! I am overwhelmed by the blockers these days. Let me get back to you later. :)

Also loop BT dev. into CC, since BT is one consumer of nsITelephonyProvider.

Comment 5

[Michael Schwartz [:m4]]

Just adding to the list so we don't forget.  Seems like there is a requirement that calls is the DISCONNECTED state cannot have conf=True or the dialer gets stuck.  If that's the case, that should be incorporated as well.

Comment 6

[Michael Schwartz [:m4]]

Looks like the secondCall attribute of the telephony-call-ended message doesn't work well for cases where multiple calls are received.  For instance, we receive a CW call, they drop out, and then someone else calls.  Currently only the last call would be logged.

Comment 7

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

(In reply to Michael Schwartz [:m4] from comment #2)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #1)
> > (In reply to Michael Schwartz [:m4] from comment #0)
> > > # conferenceCallStateChanged is redundant as it is just a reflection of the
> > > state for calls with conf=True
> > 
> > Not exactly. This is used to notify that every conference participant has
> > been visited. So that TelephonyDOM could know when is the correct time to
> > dispatch conference state change event to gaia.
> 
> Ok, thanks; good to know.  If that's the case then it should be called
> conferenceCallStateChanged without a state parameter, right?
> 

IMO having a state parameter makes this interface more friendly. Otherwise, how could the API user know the conference state easily? 

> Also, the API is ambiguous about the usage of CallError ie. does it replace
> the CallStateChanged(DISCONNECTED) notification?  Seems like we should
> remove the DISCONNECTED state and swap out the corresponding
> callStateChanged notification or callError for
> callDisconnected(DisconnectCause).

Agree on this. "CallError" API is ambiguous and confusing. We already filed Bug 1043165 - [B2G] refine TelephonyCall.onerror API for the enhancement.

Comment 8

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

(In reply to Anshul from comment #3)
> Hsin-Yi, in addition to what M4 suggested, am wondering if we could also
> look into simplifying the number of messages exchanged between DOM and RIL. 
> 

Good idea. It sounds a nice improvement to me :) Thanks Anshul.

> For example, currently if the state of two calls have changed (switch active
> and holding call), we end up sending two call state change notification for
> each of the call. But instead if we change the API to simply notify to DOM
> that the state of calls changed as a single API call and then gecko calls
> EnumerateCalls, which already exists, to get the states of all the calls in
> one go, the number of messages could be reduced. Let me know what you think?

Comment 9

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

(In reply to Michael Schwartz [:m4] from comment #6)
> Looks like the secondCall attribute of the telephony-call-ended message
> doesn't work well for cases where multiple calls are received.  For
> instance, we receive a CW call, they drop out, and then someone else calls. 
> Currently only the last call would be logged.

You are right, current implementation is still limited... However, I am not sure how this bug is related to the cdma logging issue. At least from the proposals above I don't see how any of them could solve this issue. I believe we need to review the cdma implementation and its webapi again. How about keeping discussing the cdma issue on bug 1036305? Thank you!

Comment 10

[Anshul]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #9)
> (In reply to Michael Schwartz [:m4] from comment #6)
> > Looks like the secondCall attribute of the telephony-call-ended message
> > doesn't work well for cases where multiple calls are received.  For
> > instance, we receive a CW call, they drop out, and then someone else calls. 
> > Currently only the last call would be logged.
> 
> You are right, current implementation is still limited... However, I am not
> sure how this bug is related to the cdma logging issue. At least from the
> proposals above I don't see how any of them could solve this issue. I
> believe we need to review the cdma implementation and its webapi again. How
> about keeping discussing the cdma issue on bug 1036305? Thank you!
Hsin-Yi, M4 and I are simply using this bug to provide feedback on all the API related issues that we encounter. Think of this as a metabug for all such issues :).
Thanks for filing separate bugs for each of the issues that you agree on.

Comment 11

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

(In reply to Anshul from comment #10)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #9)
> > (In reply to Michael Schwartz [:m4] from comment #6)
> > > Looks like the secondCall attribute of the telephony-call-ended message
> > > doesn't work well for cases where multiple calls are received.  For
> > > instance, we receive a CW call, they drop out, and then someone else calls. 
> > > Currently only the last call would be logged.
> > 
> > You are right, current implementation is still limited... However, I am not
> > sure how this bug is related to the cdma logging issue. At least from the
> > proposals above I don't see how any of them could solve this issue. I
> > believe we need to review the cdma implementation and its webapi again. How
> > about keeping discussing the cdma issue on bug 1036305? Thank you!
> Hsin-Yi, M4 and I are simply using this bug to provide feedback on all the
> API related issues that we encounter. Think of this as a metabug for all
> such issues :).

Oh, it's good to know. :) If it helps, feel free to change the bug title a bit to meet your needs; otherwise, I would still view it as a bug targeting on streamlining nsITelephonyService :P


> Thanks for filing separate bugs for each of the issues that you agree on.

Comment 12

[BMO Automation]

Firefox OS is not being worked on