implement a nsITelephonyService for Simulator

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
5 years ago
4 years ago

People

(Reporter: vicamo, Unassigned)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:3.0?)

Details

(Whiteboard: [dependency: marketplace-partners])

Attachments

(7 attachments, 5 obsolete attachments)

860.48 KB, image/png
Details
16.89 KB, patch
Details | Diff | Splinter Review
1.41 KB, patch
khuey
: review+
Details | Diff | Splinter Review
75.45 KB, patch
hsinyi
: feedback+
Details | Diff | Splinter Review
38.81 KB, patch
khuey
: review+
Details | Diff | Splinter Review
11.53 KB, patch
vicamo
: review+
Details | Diff | Splinter Review
18.37 KB, patch
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #989926 +++

Please see bug 989926 comment 40 for detailed thoughts.
Call holding and resume are working. Conference should be working I guess. Working on CDMA.

Again, working branch is in https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator

To have a try, build both b2g-desktop and firefox nightly from the branch HEAD.

  $ cd ${B2G_DESKTOP}
  $ git clone https://git.mozilla.org/releases/gaia.git gaia
  $ ln -s b2g/config/mozconfigs/macosx64_gecko/nightly .mozconfig
  $ nice -n 20 ./mach build

  $ cd ${FIREFOX_NIGHTLY}
  $ ln -s browser/config/mozconfigs/macosx64/debug .mozconfig
  # If you have MacOSX10.5 SDK, use browser/config/mozconfigs/macosx-universal/nightly instead
  $ nice -n 20 ./mach build
  $ ./mach run

Use the just built b2g-desktop as the source of gecko runtime as shown in https://developer.mozilla.org/en-US/Firefox_OS/Running_custom_builds_in_the_App_Manager .
From bug 989926 comment 51, I should also separate patches by the frontend/backend. Also list necessary/handy elements to be shown.
Vicamo, as mentioned in bug 989926, the directory `browser/devtools/app-manager/` will disappear soon. You want to build a WebIDE panel (or a deck) to control the RIL actor.

If you want, you can take care of the actor, and I will make sure someone in my team builds the UI.
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #4)
> Vicamo, as mentioned in bug 989926, the directory
> `browser/devtools/app-manager/` will disappear soon. You want to build a
> WebIDE panel (or a deck) to control the RIL actor.
> 
> If you want, you can take care of the actor, and I will make sure someone in
> my team builds the UI.

Thank you for the great support. I had a quick scan in WebIDE UI bits. Really not as simple as what AppManager has.

I'm having a review of the call management in the back-end side. Found a few more bugs in B2G emulator. After this, I'll have a detailed description about the elements in need, and action flows.
Here is a patch that provides a UI placeholder for the telephony tools.

Just add your code to telephony.(xhtml|js).

Let me know if that works for you.
This patch sets _PLATFORM_WITH_RIL when building with FXOS_SIMULATOR turned on. However, FXOS_SIMULATOR itself is an environment variable, therefore I'm not really sure if this is a good idea.  And what's its relation to MOZ_MULET? Should I use MOZ_MULET instead?

In the following patches I created dom/telephony/simulator and dom/system/simulator. Shall I use dom/telephony/mulet, dom/system/mulet instead?
Attachment #8456949 - Flags: feedback?(lissyx+mozillians)
In this patch I copied dom/system/gonk/{RILContentHelper.js,RadioInterfaceLayer.manifest} to dom/system/simulator.  We may still need a fake RadioInterfaceLayer to simulate radio on/off though.

Most bits relying on RadioInterfaceLayer has been removed.  ICC is always the one we have on emulator, so applies to mobileconnection.
Attachment #8456951 - Flags: review?(htsai)
Assignee: nobody → vyang
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #8)
> Created attachment 8456951 [details] [diff] [review]
> part 2/5: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail
> backends

When MOZ_B2G_RIL is enabled on FxOS simulator build, we need this patch so that we can boot to homescreen without being stopped by various exceptions.
After discussion with Hsin-Yi this afternoon, I removed those incomplete CDMA lines. Conference/hold/resume for GSM calls are supported at the time being.
Attachment #8456955 - Flags: review?(htsai)
In this patch I try to hook up that nsISimulatorTelephonyService and RDP.

So far I defined a dictionary type "voicecall" to represent each voice call, two events "voicecall-state-changed" and "voicecall-error" to notify client side voice call state changes. Would you give me some opinions?
Attachment #8456964 - Flags: review?(paul)
Posted patch [Deprecated] AppManager UI (obsolete) — Splinter Review
This patch add a "Voice Calls" panel to AppManager. However that's going to be replaced by WebIDE so I'm not going to request reviews for it. Just for further reference and a handy tool before we really have something landed in WebIDE.
Comment on attachment 8456964 [details] [diff] [review]
part 4/5: Add devtools RDP support for telephony service

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

Can you add a comment at the top of the file explaining what RIL means and what this file does? If you're not familiar with B2G, this file is a bit cryptic.

::: toolkit/devtools/server/actors/ril.js
@@ +8,5 @@
> +
> +Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> +
> +const events = require("sdk/event/core");
> +const {on, once, off, emit} = events;

I don't think any of these are used.

@@ +45,5 @@
> +  QueryInterface: XPCOMUtils.generateQI([Ci.nsITelephonyListener]),
> +
> +  _actor: null,
> +  _callsEnumerating: null,
> +  calls: null,

Maybe you want to make `.calls` a readonly getter.
Attachment #8456964 - Flags: review?(paul) → review+
Blocks: 1040014
Comment on attachment 8456949 [details] [diff] [review]
bug-1038606-1-simulator-voice-calls.patch

Please help clarify what should I do for a new simulator back-end.

We have MOZ_B2G, FXOS_SIMULATOR, MOZ_MULET. What's relationship between them and which is more appropriate for guarding simulator components?
Attachment #8456949 - Flags: feedback?(etienne)
Attachment #8456949 - Flags: feedback?(21)
Alexandre will know.
Flags: needinfo?(lissyx+mozillians)
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #15)
> Comment on attachment 8456949 [details] [diff] [review]
> bug-1038606-1-simulator-voice-calls.patch
> 
> Please help clarify what should I do for a new simulator back-end.
> 
> We have MOZ_B2G, FXOS_SIMULATOR, MOZ_MULET. What's relationship between them
> and which is more appropriate for guarding simulator components?

We tend to use MOZ_MULET for stuff that is really problematic between B2G Desktop and Browser, as far as I can remember. I think (but not 100% sure) we can define this ordering: MOZ_B2G < MOZ_MULET < FXOS_SIMULATOR.
Flags: needinfo?(lissyx+mozillians)
Comment on attachment 8456949 [details] [diff] [review]
bug-1038606-1-simulator-voice-calls.patch

I would say that any flag that targets B2G Desktop would be fine :)
Attachment #8456949 - Flags: feedback?(lissyx+mozillians)
Attachment #8456949 - Flags: feedback?(etienne)
Comment on attachment 8456955 [details] [diff] [review]
part 3/5: implement TelephonyService for FxOS simulator

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

Looks really good! I think we still need to invite build peers for package-manifest.in and moz.build.
Attachment #8456955 - Flags: review?(htsai) → review+
Comment on attachment 8456951 [details] [diff] [review]
part 2/5: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends

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

As offline discussion, all cpmm-related code should be removed. Thank you!

::: dom/system/simulator/RILContentHelper.js
@@ +930,4 @@
>        return request;
>      }
>  
>      cpmm.sendAsyncMessage("RIL:GetCallForwardingOptions", {

There's no corresponding 'ppmm'' to handle this. We should clean the cpmm part.
Attachment #8456951 - Flags: review?(htsai)
Attachment #8456949 - Flags: feedback?(21)
We don't need ipc/unixsocket for b2g simulator. Actually, that doesn't compile on desktop.

Rebase only.
Attachment #8456949 - Attachment is obsolete: true
Attachment #8465986 - Flags: review?(khuey)
Address comment 20. All cpmm/ppmm related parts are removed. Use dispatchFireRequestSuccess/Error instead.
Attachment #8456951 - Attachment is obsolete: true
Attachment #8465991 - Flags: review?(htsai)
1. Rebase
2. Document nsISimulatorTelephonyService even more.
3. handle empty string failcause
Attachment #8456955 - Attachment is obsolete: true
Attachment #8465994 - Flags: review?(khuey)
Attachment #8465991 - Attachment description: part 2/5: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends : v2 → part 2/4: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends : v2
Attachment #8465994 - Attachment description: part 3/5: implement TelephonyService for FxOS simulator : v2 → part 3/4: implement TelephonyService for FxOS simulator : v2
Address comment 13. Document the new ril actors module more. Note that 'emit' is referenced because we do emit two events "voicecall-state-changed" and "voicecall-error".
Attachment #8456964 - Attachment is obsolete: true
Attachment #8465995 - Flags: review+
Attachment #8465995 - Attachment description: part 4/5: Add devtools RDP support for telephony service : v2 → part 4/4: Add devtools RDP support for telephony service : v2
Add 'voicecalls-store.js' to EXTRA_JS_MODULES after bug 1043957.
Attachment #8456968 - Attachment is obsolete: true
Comment on attachment 8465991 [details] [diff] [review]
part 2/4: FxOS simulator icc/mobileconnection/cellbroadcast/voicemail backends : v2

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

Hi Vicamo,

Bug 843452 is review granted and should be landed in a day or two. Can you provide a patch rebased on that and with "mobileconnection" part addressed accordingly? Other than that, the whole structure looks good to me. Thank you, and sorry for the inconvenience.
Attachment #8465991 - Flags: review?(htsai) → feedback+
Depends on: 843452
Comment on attachment 8465994 [details] [diff] [review]
part 3/4: implement TelephonyService for FxOS simulator : v2

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

r=me on the build and xpcom glue.

Somebody else needs to look at the telephony service.

::: dom/telephony/simulator/TelephonyService.js
@@ +1,4 @@
> +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */

I'm not qualified to review this file.

::: dom/telephony/simulator/nsISimulatorTelephonyService.idl
@@ +5,5 @@
> +
> +#include "nsITelephonyService.idl"
> +
> +[scriptable, uuid(08dc6bb6-cd78-433c-8530-7c0eb8783b7f)]
> +interface nsISimulatorTelephonyService : nsITelephonyService

Whoever is reviewing the implementation (hsinyi?) should also review this interface.
Attachment #8465994 - Flags: review?(khuey) → review+
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #28)
> Comment on attachment 8465994 [details] [diff] [review]
> part 3/4: implement TelephonyService for FxOS simulator : v2
> 
> Review of attachment 8465994 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me on the build and xpcom glue.
> 
> Somebody else needs to look at the telephony service.
> 
> ::: dom/telephony/simulator/TelephonyService.js
> @@ +1,4 @@
> > +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> > + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> > + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> I'm not qualified to review this file.
> 
> ::: dom/telephony/simulator/nsISimulatorTelephonyService.idl
> @@ +5,5 @@
> > +
> > +#include "nsITelephonyService.idl"
> > +
> > +[scriptable, uuid(08dc6bb6-cd78-433c-8530-7c0eb8783b7f)]
> > +interface nsISimulatorTelephonyService : nsITelephonyService
> 
> Whoever is reviewing the implementation (hsinyi?) should also review this
> interface.

Yup, it's review granted from me, see comment 19.
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #15)
> Comment on attachment 8456949 [details] [diff] [review]
> bug-1038606-1-simulator-voice-calls.patch
> 
> Please help clarify what should I do for a new simulator back-end.
> 
> We have MOZ_B2G, FXOS_SIMULATOR, MOZ_MULET. What's relationship between them
> and which is more appropriate for guarding simulator components?

Just keep using FXOS_SIMULATOR, I'll figure out later what to do on mulet.
(Mulet may end up implying FXOS_SIMULATOR)
Waiting for bug 843452 being landed to m-c.
Depends on: 1063304
Depends on: 1064231
Hi,

To which repo/branch these patches apply? I tried m-c and b2g-inbound but none applies. I also tried the github repository [1] but I could not build it (seems old code?).

[1] https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator
Flags: needinfo?(vyang)
(In reply to Wander Lairson Costa [:wcosta] from comment #32)
> Hi,
> 
> To which repo/branch these patches apply? I tried m-c and b2g-inbound but
> none applies. I also tried the github repository [1] but I could not build
> it (seems old code?).
> 
> [1] https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator

Yes, as you have seen we're re-organizing RIL interfaces a lot in order to meet 2.2FL interface freezing requirement and that breaks the patches here. Am I blocking any urgent task here?
Flags: needinfo?(vyang)
Bug 889737 is going to unify sendMMI() and dial().
Depends on: 889737
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #33)
> (In reply to Wander Lairson Costa [:wcosta] from comment #32)
> > Hi,
> > 
> > To which repo/branch these patches apply? I tried m-c and b2g-inbound but
> > none applies. I also tried the github repository [1] but I could not build
> > it (seems old code?).
> > 
> > [1] https://github.com/vicamo/b2g_mozilla-central/tree/experimental/simulator
> 
> Yes, as you have seen we're re-organizing RIL interfaces a lot in order to
> meet 2.2FL interface freezing requirement and that breaks the patches here.
> Am I blocking any urgent task here?

No worries, nothing urgent here. Is there a (meta) bug/wiki page/repo for these changes?
Flags: needinfo?(vyang)
Whiteboard: [dependency: marketplace-partners]
Hi,

I updated the patches to build on top of b2g-inbound again [1], but still struggling to make it run on desktop.

[1] https://github.com/walac/gecko-dev/tree/bugz/1038606
Hi Vivien,
Looks like Vicamo is no longer actively working on this.
Do you have mitigation plan in mind?
Flags: needinfo?(21)
[Tracking Requested - why for this release]:
tracking-b2g: --- → ?
Wesley, Would your patch still "work" on top of current master https://github.com/walac/gecko-dev/tree/bugz/1038606 ?
I'm willing to help if there is anything wrong on the devtools/RDP/simulator side.
But I'm afraid I won't be of much help if there is something wrong on the RIL/telephony side.

What were your issues?
(In reply to Alexandre Poirot [:ochameau] from comment #39)
> Wesley, Would your patch still "work" on top of current master
> https://github.com/walac/gecko-dev/tree/bugz/1038606 ?
> I'm willing to help if there is anything wrong on the devtools/RDP/simulator
> side.
> But I'm afraid I won't be of much help if there is something wrong on the
> RIL/telephony side.
> 
> What were your issues?

I am not working on this anymore (at least for now). But basically simulator was not even starting, I just got to the point to build the code, nothing more.
Can we get this prioritized? Who should take this decision?
blocking-b2g: --- → 2.2?
tracking-b2g: ? → ---
Flags: needinfo?(wcosta)
Flags: needinfo?(vicamo)
Flags: needinfo?(anygregor)
Flags: needinfo?(21)
Wilfred, Peter, how should we proceed with developer tools improvement? Should we add this to the product roadmap or should engineering prioritize this internally?
Flags: needinfo?(wmathanaraj)
Flags: needinfo?(pdolanjski)
Flags: needinfo?(anygregor)
(In reply to Alexandre LISSY :gerard-majax from comment #41)
> Can we get this prioritized? Who should take this decision?

afaik, nobody is leading this currently. I began to work on it, but had to move to other stuff.
Flags: needinfo?(wcosta)
i would like to make it part of planning  - it is critical part.
Flags: needinfo?(wmathanaraj)
We can work on the integration in the devtools/WebIDE (see that with :jryans and :ednapiranha). But we need to understand what the roadmap is for the platform bit.
(In reply to Gregor Wagner [:gwagner] from comment #42)
> Wilfred, Peter, how should we proceed with developer tools improvement?
> Should we add this to the product roadmap or should engineering prioritize
> this internally?

What area of the product will need work to support this?
Flags: needinfo?(pdolanjski) → needinfo?(anygregor)
Per comment 37, and hope you don't mind, Vicamo ;)
Assignee: vicamo → nobody
Assignee: nobody → wcosta
Flags: needinfo?(anygregor)
Is there are rough time frame for the platform work here?  WebIDE team is happy to help on the UI side once the platform work moves along.
Flags: needinfo?(wcosta)
Hi,

In Portland we came to an agreement to use the approach implemented at Bug 989926 (maybe we should reopen that bug). I started to work on it this week by updating :gerard-majax's patches [1], but I am still working on TC stuff [2]. Sorry it is taking so much time to get it done :(

[1] https://github.com/walac/gecko-dev/tree/b2g-desktop-ril
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1080265
Flags: needinfo?(wcosta)
Any update here?
This has stalled a bit (my fault here) I have asked wcosta to wrap up this prior to finalizing work here https://bugzilla.mozilla.org/show_bug.cgi?id=1119387
ni myself to triage it in telephony platform team.
Flags: needinfo?(whuang)
triage: this is more like a feature so let's not block on it.
I redirect it to feature-b2g:3.0? for future planning. 
Maybe James' team will look into this.
blocking-b2g: 2.2? → ---
feature-b2g: --- → 3.0?
Flags: needinfo?(whuang)
(In reply to Wander Lairson Costa [:wcosta] from comment #49)
> Hi,
> 
> In Portland we came to an agreement to use the approach implemented at Bug
> 989926 (maybe we should reopen that bug). I started to work on it this week
> by updating :gerard-majax's patches [1], but I am still working on TC stuff
> [2]. Sorry it is taking so much time to get it done :(
> 
> [1] https://github.com/walac/gecko-dev/tree/b2g-desktop-ril
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1080265

Rebased at https://github.com/lissyx/mozilla-central/commits/libril-bug989926-split_rebase_gecko38
Depends on: 1136729
(In reply to Alexandre LISSY :gerard-majax from comment #54)
> (In reply to Wander Lairson Costa [:wcosta] from comment #49)
> > Hi,
> > 
> > In Portland we came to an agreement to use the approach implemented at Bug
> > 989926 (maybe we should reopen that bug). I started to work on it this week
> > by updating :gerard-majax's patches [1], but I am still working on TC stuff
> > [2]. Sorry it is taking so much time to get it done :(
> > 
> > [1] https://github.com/walac/gecko-dev/tree/b2g-desktop-ril
> > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1080265
> 
> Rebased at
> https://github.com/lissyx/mozilla-central/commits/libril-bug989926-
> split_rebase_gecko38

It's building and mostly starting, but someone will have to see the RIL-related changes that landed in between, I'm getting errors related to gMobileConnectionService for example ...
Depends on: 1137151
Depends on: 1137155
Assignee: wcosta → nobody
Should we RESOLVED:WONTFIX this?
Flags: needinfo?(htsai)
Flags: needinfo?(anygregor)
(In reply to Alexandre LISSY :gerard-majax from comment #56)
> Should we RESOLVED:WONTFIX this
My team don't have a plan for this, and we'd want to make emu-x86 feasible. So, WONTFIX makes sense to me.
Flags: needinfo?(htsai)
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(anygregor)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.