Closed
Bug 1029721
Opened 10 years ago
Closed 10 years ago
CDMA call waiting call is not logged in call history
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: anshulj, Assigned: thills)
References
()
Details
(Whiteboard: [planned-sprint])
Attachments
(2 files, 6 obsolete files)
STR: 1. On a CDMA device, make an outgoing call 2. When the call in step 1 is connected place a call to this device from another device (incoming call) 3. Accept the cdma call waiting call. 4. End both the calls. 5. Go to the Call log Expected: Both the outgoing and the incoming call should be shown Observed: Only the outgoing call is available in the call log screen.
Comment 1•10 years ago
|
||
Wilfred - Are there any production devices shipping in 2.0 with CDMA support?
Flags: needinfo?(wmathanaraj)
(In reply to Jason Smith [:jsmith] from comment #1) > Wilfred - Are there any production devices shipping in 2.0 with CDMA support? This issue is already reproducible on 1.4. The version 1.4 is FFOS's first official CDMA release but no one seems to have noticed this bug.
Comment 3•10 years ago
|
||
I dont think we have any 1.4 product with CDMA out there - but 2.0 may be the first one with Madai. We can not do any CDMA testing in Europe - given there is no CDMA network. Sounds like a bug we should fix.
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(wmathanaraj)
Comment 5•10 years ago
|
||
I'm dealing with a few 1.4/1.3T+ blockers but once I'm done with them I'll try to fix this bug if no one else is available. Realistically that's going to happen sometimes in the middle of next week.
Comment 6•10 years ago
|
||
The call log is populated by the telephony-call-ended system message. And in the case of CDMA call waiting, we never actually see 2 call objects, we have only 1 call with an optional secondNumber property, which isn't even bubbled through the system message [1] (note that that probably would not help since by the time the call ends the secondNumber property should be empty). This looks like a feature request requiring gecko and gaia work, we might want to reconsider the blockiness. [1] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyService.js#730
Comment 7•10 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #6) > The call log is populated by the telephony-call-ended system message. Now that you make me think about it this bug might be unfixable because of how CDMA message works: AFAIK there is no message from the network about the call ending - ever. You get a message indicating that there's an incoming call and you can accept it or not but irrespective of if you've replied the second call or not you don't get anything back from the network. Hsin-Yi can you confirm this and maybe give us some more details? Would there be a way to introduce a telephony-call-ended message for CDMA's second call? Maybe using a timeout or something?
Flags: needinfo?(htsai)
Comment 9•10 years ago
|
||
(In reply to Doug Sherk (:drs) from comment #8) > See comment 6 and comment 7. Wilfred - Can you triage this based on the new info provided in comment 6 & 7?
Flags: needinfo?(wmathanaraj)
Comment 10•10 years ago
|
||
(In reply to Gabriele Svelto [:gsvelto] from comment #7) > (In reply to Etienne Segonzac (:etienne) from comment #6) > > The call log is populated by the telephony-call-ended system message. > > Now that you make me think about it this bug might be unfixable because of > how CDMA message works: AFAIK there is no message from the network about the > call ending - ever. Correct. It's hardly possible in reference phones. In customized phones, gecko can get particular messages from operator to know the exact state of the incoming call, but not in reference design. > You get a message indicating that there's an incoming > call and you can accept it or not but irrespective of if you've replied the > second call or not you don't get anything back from the network. Hsin-Yi can > you confirm this and maybe give us some more details? Would there be a way > to introduce a telephony-call-ended message for CDMA's second call? Maybe > using a timeout or something? If it's really needed in 2.0+, then for short-term and for only this issue, I think we can introduce one more attribute like "secondNumber" in the telephony-call-ended message, which is sent when the *only connection* ends. Or, we can send one more "telephony-call-ended" message along with that for the 1st call. We have no idea when the 2nd call is actually ended. We only know for sure when the 1st call ends, so does the 2nd. Is it okay for gaia? For long-term, Gabriele might have known that, I've been thinking about moving the current cdma call waiting API to what we have for gsm. Current API works well on answering and switching calls, but as it provides less information, we've seen some problems: 1) When user rejects the second call, the API and TelephonyService have no idea. That also leads other components monitoring TelephonyService, e.g. BT, to some trouble. If we change the API, then when user rejects the cdma incoming call, BT module as well as TelephonyService knows that. And it's TelephonyService's responsibility to make sure API's behaviour, like CDMA 3-way call. 2) The call log is another example. Gecko could do the best guess to provide the most precise information such as call duration if modem doesn't provide customized message. 3) Not being able to provide more call states. If partners want to know specific states of the second call as they require in cdma 3-way call (it's possible with customization), the current API isn't flexible enough. I would be happier to see this bug being viewed as a new feature request, so that we could review and work on the long-term proposal properly.
Flags: needinfo?(htsai)
Comment 11•10 years ago
|
||
This is a clear cert blocker in the 2.0 timeframe for CDMA, so this needs to stay as a blocker for 2.0.
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(wmathanaraj)
Comment 12•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #10) > If it's really needed in 2.0+, then for short-term and for only this issue, > I think we can introduce one more attribute like "secondNumber" in the > telephony-call-ended message, which is sent when the *only connection* ends. > > Or, we can send one more "telephony-call-ended" message along with that for > the 1st call. We have no idea when the 2nd call is actually ended. We only > know for sure when the 1st call ends, so does the 2nd. > > Is it okay for gaia? Both are fine, the second one has the advantage of requiring no gaia work at all :)
Comment 13•10 years ago
|
||
Moving to RIL. Is this something that Tamara could work on with mentorship?
Component: Gaia::Dialer → RIL
Whiteboard: [planned-sprint]
Comment 14•10 years ago
|
||
(In reply to Doug Sherk (:drs) from comment #13) > Moving to RIL. Is this something that Tamara could work on with mentorship? I think this is good for her. I could be the mentor, of course :)
Comment 15•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #14) > (In reply to Doug Sherk (:drs) from comment #13) > > Moving to RIL. Is this something that Tamara could work on with mentorship? > > I think this is good for her. I could be the mentor, of course :) Hey Tamara, Are you interested in taking this bug? I will be happy to be the mentor. :)
Flags: needinfo?(thills)
Comment 16•10 years ago
|
||
We need 2.0+ to be cleared before end of sprint6.
Target Milestone: --- → 2.0 S6 (18july)
Assignee | ||
Comment 17•10 years ago
|
||
Hi Hsin-Yi, I would like to work on this. Will start taking look through comment #10. In the meantime, would be good to know what kind of env I would need in order to repro this as I don't have a CDMA phone and I'm not sure about how to set it up for emulation. Thanks!
Assignee: nobody → thills
Flags: needinfo?(htsai)
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 18•10 years ago
|
||
(In reply to Tamara Hills [:thills] from comment #17) > Hi Hsin-Yi, I would like to work on this. Will start taking look through > comment #10. In the meantime, would be good to know what kind of env I > would need in order to repro this as I don't have a CDMA phone and I'm not > sure about how to set it up for emulation. > > Thanks! Hi Tamara, The CDMA phone I use for Fxos development is wasabi. I am not sure how you could get that. We don't have many in fact. Sadly, emulator for cdma call waiting support isn't finished so you are not able to rely on the emulator. I think you could still study the code and provide your patch. I could test it for you.
Flags: needinfo?(htsai)
Assignee | ||
Comment 19•10 years ago
|
||
Thanks Hsin-Yi, We are trying to get a few wasabi devices. I will try and look through the code for this one to get started.
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(thills)
Assignee | ||
Comment 20•10 years ago
|
||
Hi Hsin-Yi, I was reading through the code and it looks like there is this parent/child relationship for the CDMA call and the secondNumber is now a secondID in TelephonyCall now pointing to the index of the second incoming call. I'm trying to find a spot to grab that second number and put it into the System Message for the telephony-call-ended event. I'm seeing some info in the IPCCdmaWaitingCallData structure, but not sure how I can access. Can I access the second number from the secondId? I'm thinking what would help (since I don't have a device yet) is either a class diagram or a debug trace of the steps to repro. I'll keep looking and maybe have some clearer questions for you. :) Thanks, -tamara
Flags: needinfo?(htsai)
Assignee | ||
Comment 21•10 years ago
|
||
Hi Hsin-Yi, Would something like the attached work to get the second number up to Gaia on a CDMA call hangup? Wondering if the data is still available at this point.
Comment 22•10 years ago
|
||
Comment on attachment 8453494 [details] [diff] [review] 1029721 Review of attachment 8453494 [details] [diff] [review]: ----------------------------------------------------------------- Hi Tamara, CDMA telephony API isn't firmed I would say. :( So, we have two different designs for Cdma call waiting and Cdma 3-way calling. The code you touched is the part of cdma 3way calling, not cdma call waiting. To resolve this issue, you will need to create a call object on TelephonyService in [1]. And then do what we need when receiving |notifyCallDisconnected| for the 1st call. [1] dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyService.js?from=TelephonyService.js#878
Comment 23•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #22) > Comment on attachment 8453494 [details] [diff] [review] > 1029721 > > Review of attachment 8453494 [details] [diff] [review]: > ----------------------------------------------------------------- > > Hi Tamara, > Okay, maybe I should ask this first: Which is the solution on comment 12 you are going to take? Are you going to send an extended system message including secondNumber or send a system message per call? Seems the former? Just want to confirm! > CDMA telephony API isn't firmed I would say. :( So, we have two different > designs for Cdma call waiting and Cdma 3-way calling. The code you touched > is the part of cdma 3way calling, not cdma call waiting. To resolve this > issue, you will need to create a call object on TelephonyService in [1]. And > then do what we need when receiving |notifyCallDisconnected| for the 1st > call. > > [1] > dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyService. > js?from=TelephonyService.js#878
Flags: needinfo?(htsai)
Assignee | ||
Comment 24•10 years ago
|
||
Hi Hsin-Yi, Yes, that was the approach I was looking at.
Flags: needinfo?(htsai)
Comment 25•10 years ago
|
||
Comment on attachment 8453494 [details] [diff] [review] 1029721 Review of attachment 8453494 [details] [diff] [review]: ----------------------------------------------------------------- Tamara, Based on your goal comment 24, below is my comments in detail. Thank you! ::: dom/telephony/gonk/TelephonyService.js @@ +730,5 @@ > duration: duration, > direction: aCall.isOutgoing ? "outgoing" : "incoming" > }; > + > + let secondCall = this._currentCalls[aClientId][CDMA_SECOND_CALL_INDEX]; Let's not do this way. If doing so, then in cdma 3way calling scenario, gaia will receive two telephony-call-ended system messages both with 'secondNumber' attribute. Due to the difference b/w the design of cdma 3way calling and cdma call waiting, I'd suggest in this bug we still keep the implementation for these two separate. We need a new variable '_cdmaWaitingCallNumber,' assign a valid value in notifyCdmaCallWaiting(), and properly clear it in notifyCallDisconnected().
Updated•10 years ago
|
Flags: needinfo?(htsai)
Assignee | ||
Comment 26•10 years ago
|
||
Hi Hsin-Yi, Is the attached in the right direction? I wasn't sure if you meant a global in TelephonyService.js is ok or not. I had another question about the calling number presentation (the privacy bits). It looks like Gecko notifies Gaia what it can present for an incoming call, but the telephony-call-ended event does not look like it includes those flags about what can be presented. I couldn't find a place in Gaia where this is done (but I might be missing something). Thank you, -tamara
Attachment #8454265 -
Flags: feedback?(htsai)
Flags: needinfo?(htsai)
Comment 27•10 years ago
|
||
Comment on attachment 8454265 [details] [diff] [review] WIP - Gecko only Review of attachment 8454265 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this is what I was talking about. It looks really good!
Attachment #8454265 -
Flags: feedback?(htsai) → feedback+
Comment 28•10 years ago
|
||
(In reply to Tamara Hills [:thills] from comment #26) > Created attachment 8454265 [details] [diff] [review] > WIP - Gecko only > > Hi Hsin-Yi, > > Is the attached in the right direction? I wasn't sure if you meant a global > in TelephonyService.js is ok or not. > It looks great, thank you. > I had another question about the calling number presentation (the privacy > bits). It looks like Gecko notifies Gaia what it can present for an > incoming call, but the telephony-call-ended event does not look like it > includes those flags about what can be presented. I couldn't find a place > in Gaia where this is done (but I might be missing something). > Oh, that's because the Gaia part isn't ready yet, see Bug 980598. IIRC, our partner is working on the gaia but. Regarding telephony-call-ended system messages, I am not aware of a feature request about the call id presentation on call log. The information is only exposed to TelephonyCall now. > Thank you, > > -tamara
Flags: needinfo?(htsai)
Assignee | ||
Comment 29•10 years ago
|
||
Attachment #8453494 -
Attachment is obsolete: true
Attachment #8454265 -
Attachment is obsolete: true
Assignee | ||
Comment 30•10 years ago
|
||
Assignee | ||
Comment 31•10 years ago
|
||
Hi Anthony, Do you think you can take a look at the callEnded event changes I made to handle this blocker? I've tested some of the paths, but don't have a device to test the true CDMA (yet).
Attachment #8454633 -
Attachment is obsolete: true
Attachment #8455338 -
Flags: review?(anthony)
Assignee | ||
Comment 32•10 years ago
|
||
Hi Anshul, Do you think you'd be able to help test this patch? It's a two-part patch (both gaia and gecko). I don't have my device yet, but will be getting one soon. Thanks, -tamara
Flags: needinfo?(anshulj)
Reporter | ||
Comment 33•10 years ago
|
||
Tamara, we don't have the corresponding gecko changes done yet so won't be able to verify the patch just yet.
Flags: needinfo?(anshulj)
Comment 34•10 years ago
|
||
(In reply to Anshul from comment #33) > Tamara, we don't have the corresponding gecko changes done yet so won't be > able to verify the patch just yet. Anshul can you please help with the bug # where the gecko changes are happening ?
Flags: needinfo?(anshulj)
Reporter | ||
Comment 35•10 years ago
|
||
Bhavana, I was talking about the proprietary changes that need to happen on our side.
Flags: needinfo?(anshulj)
Comment 36•10 years ago
|
||
Tarama, do you need any help in Gecko patch? Since this week is the last week of sprint 6 of 2.0, we are supposed to fix all blockers for 2.0 before this week....:-(.
Comment 37•10 years ago
|
||
I could help verify the patches with wasabi.
Assignee | ||
Comment 38•10 years ago
|
||
(In reply to Ken Chang[:ken] from comment #36) > Tarama, do you need any help in Gecko patch? Since this week is the last > week of sprint 6 of 2.0, we are supposed to fix all blockers for 2.0 before > this week....:-(. Hi Ken, I'm not sure what Gecko patch is being referred to here. The gecko patch for this bug is attached. It's a two part patch. One gecko and one gaia patch. If there is something else required, then I'm not aware of that. We need to get it tested on Wasabi. I'm getting a device, but won't get it in time to test this. It sounds like Hsin-Yi can help test on the Wasabi.
Assignee | ||
Comment 39•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #37) > I could help verify the patches with wasabi. Hi Hsin-Yi, Yes, that would be great if you can help test. Let me know if you have any question. thanks -tamara
Comment 40•10 years ago
|
||
Comment on attachment 8454634 [details] [diff] [review] Gecko Portion of Patch Review of attachment 8454634 [details] [diff] [review]: ----------------------------------------------------------------- Tamara's patches (gecko + gaia) work really good. There's a call log for the cdma waiting call. :) ::: dom/telephony/gonk/TelephonyService.js @@ +876,5 @@ > this.notifyCallDisconnected(aClientId, call); > } > + > + let presentNumber = aCall.numberPresentation != null ? > + aCall.numberPresentation : nsITelephonyService.CALL_PRESENTATION_ALLOWED; I think we are fine to just have |this._cdmaCallWaitingNumber = aCall.number|. AFAIK, modem/rild would take care of the number string based on the value of number presentation.
Assignee | ||
Comment 41•10 years ago
|
||
Hi Hsin-Yi, thanks for testing that out. As per your comment, I removed some of the checks for that CLID blocking (as it should be handled by RILD/modem). Do you mind testing it again just to be sure? Thanks, -tamara
Attachment #8454634 -
Attachment is obsolete: true
Attachment #8456276 -
Flags: feedback?(htsai)
Comment 42•10 years ago
|
||
Comment on attachment 8456276 [details] [diff] [review] Updated Gecko Patch Review of attachment 8456276 [details] [diff] [review]: ----------------------------------------------------------------- It works well, thank you! ::: dom/telephony/gonk/TelephonyService.js @@ +108,5 @@ > this._numClients = gRadioInterfaceLayer.numRadioInterfaces; > this._listeners = []; > this._currentCalls = {}; > > + this._cdmaWaitingCallNumber = null; You use _cdmaCallWaitingNumber elsewhere. I am fine with either name as long as we use it consistently.
Attachment #8456276 -
Flags: feedback?(htsai) → feedback+
Assignee | ||
Comment 43•10 years ago
|
||
Hi Hsin-Yi, Good catch. Thanks. Do you mind retesting? Thanks, -tamara
Attachment #8456276 -
Attachment is obsolete: true
Attachment #8456687 -
Flags: feedback?(htsai)
Comment 44•10 years ago
|
||
Comment on attachment 8456687 [details] [diff] [review] Updated Gecko Patch -- _cdmaCallWaitingNumber Review of attachment 8456687 [details] [diff] [review]: ----------------------------------------------------------------- Looks really nice so I gave r+ directly :P
Attachment #8456687 -
Flags: feedback?(htsai) → review+
Assignee | ||
Comment 45•10 years ago
|
||
Anthony, This pull request has changes in dialer.js as well as new tests added in dialer_test.js. Would like your feedback on this. Thank you, -tamara
Attachment #8455338 -
Attachment is obsolete: true
Attachment #8455338 -
Flags: review?(anthony)
Attachment #8456752 -
Flags: feedback?(anthony)
Comment 46•10 years ago
|
||
Comment on attachment 8456752 [details]
Gaia Pull Request for feedback
Redirecting to Gabriele since he has more experience with CDMA.
Tamara: You'll need new unit tests for this.
Attachment #8456752 -
Flags: feedback?(anthony) → feedback?(gsvelto)
Comment 47•10 years ago
|
||
Oops, you did add unit tests, I just didn't refresh the github page.
Comment 48•10 years ago
|
||
Comment on attachment 8456752 [details]
Gaia Pull Request for feedback
f- because the code handling the high-priority wakelock is incorrect. The rest looks good to me (including the tests). Once you fix the high-priority wakelock issue it would be nice if you could add a test that ensures that:
- in case the secondNumber field is present the wake-lock is unlocked only once
- the wakelock is not unlocked after the first call is stored in the DB but only after both are
Attachment #8456752 -
Flags: feedback?(gsvelto) → feedback-
Assignee | ||
Comment 49•10 years ago
|
||
Need checkin in the Gecko portion only. r+ for the Gecko portion is in the Comment 44.
Keywords: checkin-needed
Comment 50•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/88ad2e93bce6 Great job, Tamara! leave-open for the gaia part.
Keywords: checkin-needed → leave-open
Assignee | ||
Comment 51•10 years ago
|
||
Comment on attachment 8456752 [details]
Gaia Pull Request for feedback
Hi Gabriele,
I spent bunch of time with Anthony on this today. We were able to do it without the promises.
Would you be able to take look?
Thanks,
-tamara
Attachment #8456752 -
Flags: review?(gsvelto)
Comment 52•10 years ago
|
||
Comment on attachment 8456752 [details]
Gaia Pull Request for feedback
LGTM except for a change in where the high-priority wakelock is released. Feel free to land once that's been addressed and covered in the unit-tests as I described on GitHub.
Attachment #8456752 -
Flags: review?(gsvelto) → review+
Assignee | ||
Comment 53•10 years ago
|
||
Hi Hsin-Yi, The Gaia portion has been updated and we have the review done. If you get a chance can you try out on the device? https://github.com/mozilla-b2g/gaia/pull/21638/ Thank you! -tamara
Flags: needinfo?(htsai)
Assignee | ||
Comment 54•10 years ago
|
||
(In reply to Gabriele Svelto [:gsvelto] from comment #52) > Comment on attachment 8456752 [details] > Gaia Pull Request for feedback > > LGTM except for a change in where the high-priority wakelock is released. > Feel free to land once that's been addressed and covered in the unit-tests > as I described on GitHub. I addressed the wakelock call and we Updated tests to make sure that the lock is released in teh cdma case only after the second call has completed.
Comment 56•10 years ago
|
||
Gecko patch was landed, I changed the component to Dialer since next step is to get gaia patch landed as well. Hsinyi will play with the Gaia patch and provide feedback.
Component: RIL → Gaia::Dialer
Comment 57•10 years ago
|
||
(In reply to Tamara Hills [:thills] from comment #53) > Hi Hsin-Yi, > > The Gaia portion has been updated and we have the review done. > > If you get a chance can you try out on the device? > > https://github.com/mozilla-b2g/gaia/pull/21638/ > > Thank you! > > -tamara Work like a charm!
Flags: needinfo?(htsai)
Comment 58•10 years ago
|
||
Tamara is on a plane so I merged it for her. https://github.com/mozilla-b2g/gaia/commit/858cb715524502bcaa15b5e5060c6ad67eec890e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 59•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/7d1d5ae5d8a6 v2.0: https://github.com/mozilla-b2g/gaia/commit/bd2303de0be9bad0f5a3a1339c5a48c808c90e3a
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
Keywords: leave-open
Comment 60•10 years ago
|
||
This will need a new test case to be covered.
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Comment 61•10 years ago
|
||
Test case has been added to moztrap: https://moztrap.mozilla.org/manage/case/14386/
QA Whiteboard: [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Flags: in-moztrap+
You need to log in
before you can comment on or make changes to this bug.
Description
•