Closed Bug 699235 (b2g-telephony) Opened 13 years ago Closed 12 years ago

[meta] B2G Telephony

Categories

(Core :: DOM: Device Interfaces, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
blocking-kilimanjaro +
blocking-basecamp +

People

(Reporter: qdot, Assigned: philikon)

References

Details

Attachments

(3 obsolete files)

Create a stack to handle sending/receiving/parsing Android Parcels from/to the RIL IO thread, as well as maintaining the state of the phone stack (radio state, call list, etc...), and communicating with the telephony DOM in the main thread.
Remember that a good portion of this is similarly implemented in the mock-ril, distributed with B2G. http://android.git.linaro.org/gitweb?p=platform/hardware/ril.git;a=tree;f=mock-ril/src/js;h=2039ad66530a3a26c2ec928d241b19c72adf67c3;hb=master Note that it's pretty heavily protobuf'd/v8'd, but the general idea is there.
Depends on: 698621
Work will probably happen at http://www.github.com/kmachulis-mozilla/mozilla-central Will land patches through this bug.
Assignee: nobody → kmachulis
Depends on: 699242
Depends on: 699256
Until we figure out where exactly this should go in m-c, I'm building the JS framework on github at http://www.github.com/kmachulis-mozilla/b2g-js-ril Ideally, we want the JS RIL stack to be a copy of the minimalist structure we've already build in python (since that's the language jaoo/ferjm/I are familiar with). This python file is now in the b2g-js-ril repo at https://github.com/kmachulis-mozilla/b2g-js-ril/blob/master/python/dialer.py
Attached patch First pass at JS RIL manager (obsolete) (deleted) — Splinter Review
Checking in first (non-working) patch for JS RIL, to see if basic structure is ok. This was written as part of http://www.github.com/kmachulis-mozilla/b2g-js-ril and isn't actually ready to be merged into m-c yet, but the patch does apply and puts the js files in dom/telephony/worker-js Some questions that still need to be answered: - What should PostMessage send to the main thread? - The assumption is that nothing in the worker thread should block, ever. Does that still hold true? - RILManager used to call a parcel's callbacks after it was done constructing the parcel. There's now an explicit function to call the callbacks (currently commented out), which we can call at some other time if we want to keep the receive loop running (since that's dictated by whatever is sending in data from the radio). Do we want to keep it this way, or was the old way working (since, once again, callbacks shouldn't ever block)?
Attachment #575626 - Flags: review+
Attachment #575626 - Flags: review+
Attachment #575626 - Attachment is patch: true
Comment on attachment 575626 [details] [diff] [review] First pass at JS RIL manager >+/** >+ * Base implementation. >+ */ >+function RILParcel(data) { Wrapping every parcel into this class creates an allocation. >+ let arg; >+ [this.response_type, arg] = new Int32Array(this.buffer, 0, 2); This allocates another 2 objects for every parcel. >+ /** >+ * Turn a string into an array buffer of 16-bit chars >+ */ >+ strToByteArray: function(s) { >+ var buf = ArrayBuffer(s.length * 2); >+ var uint16Buf = Uint16Array(buf); >+ var i = 0; >+ for(i = 0; i < s.length; ++i) >+ { >+ uint16Buf[i] = s.charCodeAt(i); >+ } >+ return buf; >+ }, >+ /** >+ * Turn an array buffer of 16-bit chars into a string >+ */ >+ byteArrayToStr: function (start, len) { >+ var st = ""; >+ for each (x in Uint16Array(this.buffer, start, len)) st += String.fromCharCode(x); >+ return st; This yields horrible performance due to the way iteration works on arrays. >+ // TODO: Actually unpack data correctly >+ let pn_str = this.strToByteArray(this.data[0]); >+ this.buffer = new ArrayBuffer(4 + pn_str.byteLength + 4 + 4 + 4); >+ Uint32Array(this.buffer, 0, 1)[0] = pn_str.byteLength / 2; >+ (Uint8Array(this.buffer, 4, pn_str.byteLength)).set(Uint8Array(pn_str, 0, pn_str.byteLength)); >+ //Uint32Array(this.buffer, ).set([0,0,0]); Again globs of object constructions. Kyle, this new code is a really bad idea. The previous version was carefully constructed to avoid object allocation as much as possible. This code is going to be pretty GC happy which is probably a bad idea for code that has real-time requirements.
(In reply to Andreas Gal :gal from comment #6) > >+ let arg; > >+ [this.response_type, arg] = new Int32Array(this.buffer, 0, 2); > > This allocates another 2 objects for every parcel. A lot of these types of allocations can be avoided by getting a (native) DataView for the buffer and only looking at that. Then one can read any types of values via the DataView and avoid having to create lots of different typed array objects. > >+ /** > >+ * Turn an array buffer of 16-bit chars into a string > >+ */ > >+ byteArrayToStr: function (start, len) { > >+ var st = ""; > >+ for each (x in Uint16Array(this.buffer, start, len)) st += String.fromCharCode(x); > >+ return st; > > This yields horrible performance due to the way iteration works on arrays. It would be lovely if typed arrays (or the DataView) had a native call for getting their data as a string. Or of course we could convert all data into 16 bit strings before we hand it to JS in the first place, and then in the JS code we just pick the values from those strings and substring when we want actual strings. No idea whether this would be better or not. I bet the performance, GC, and real-time characteristics will probably depend on the types, frequency, and size of parcels (which I have no idea about.)
Well we will get a DataView soon, but even with that, this code is _way_ complicated and over-abstracted. I am going to take a stab at squeezing out some if it.
There seems to be two data types in those packets. Int32 and strings. Write two functions that (bitshift) read those from a bytearray or string using an index you supply, and two functions to write this out. DataView is really not helpful here. Don't over-abstract. Make it simple and brief. This code should be 100 lines or less, plus constants.
(In reply to Andreas Gal :gal from comment #9) > There seems to be two data types in those packets. Int32 and strings. Oh, having seen Uint8 and what not in there I figured we needed all sorts of different types. Yeah, that makes things a lot easier. Probably easiest would be working with strings where each char is constructed from 2 bytes. That's what the RIL strings seem to look like and we'd avoid tedious string mechanics in JS.
Well, uh, damn. Quite the "Welcome to Javascript". So, one thing about the "real-time"ness and the GC. I really don't think we're sending/receiving nearly enough packets that the allocations I'm incurring would lock the GC. There's an interesting question of whether that could cause a DDOS attack against the GC somehow if we did that, but, really, we're getting maybe 100-200 packets a minute under normal use, shouldn't have more that 10-20 outstanding at any time. Nothing is being streamed. The RIL protocol isn't even made for that.
Ok the more I think about this, the more the disconnect is startin to make sense. I've been in implementations of this in languages that either have easier memory overlays (c), native overlays (pbufs via v8 in mockril) or, uh, whatever excuse java is. Was planning on cleaning up te dataview stuff (I realize how gross the uint8view indexing was), but I guess I can remove the parcel class completely and just parse packets and change the state of the phone object as we go (by adding more functions to the array weve already got going in parcel.js) should be much lighter weight.
Attached file lightweight impl (WIP) (obsolete) (deleted) —
Kyle and I paired on a more lightweight implementation yesterday. Here's the WIP so far. This is somewhat inspired by Andreas's prototype, except that we use a TypedArray ringbuffer to even avoid new string allocations. We're now working on writing tests for it and hooking it up to the socket so we can have the phone dialing again.
Attachment #575626 - Attachment is obsolete: true
Attachment #576230 - Flags: feedback?(gal)
Comment on attachment 576230 [details] lightweight impl (WIP) Make the ringbuffer grow dynamically every time the tail catches the head. Should be pretty straight forward. Just memcpy and update the constants. Very nice otherwise.
Attachment #576230 - Flags: feedback?(gal) → feedback+
Any updates here?
(In reply to Andreas Gal :gal from comment #15) > Any updates here? Kyle and I are revving quickly on GitHub: http://github.com/kmachulis-mozilla/b2g-js-ril and my fork of that.
I'm not sure if this comment fits in here. Anyway, since we have been quiet for a while, I just wanted to keep you posted about what jaoo and I are currently doing regarding the RIL stuff. We've been working a bit on figuring out how the 3G works and how we could set the connection up (I've made a comment on github about that), we've also been trying to see how to receive calls and SMS. We wasn't able to make any successful test regarding 3G or calls reception yet, even without the rild2 patch, on a fresh B2G build. We are wondering if there is a problem within the libril propietary library itself. Maybe it is similar to the github issue #56 related to the mediaserver. Some sort of differences between versions issue. jaoo is probably starting to work on this. SMS reception is working fine without the rild2 patch, so while we figure out what is going on with the issues on the proprietary libril over the B2G build, I am probably starting to prototype a PDU JS parser if none of you is already working on that. I've seen that Philip wrote a TODO about that on the b2g-js-ril repo.
(In reply to Fernando Jiménez from comment #17) > SMS reception is working fine without the rild2 patch, so while we figure > out what is going on with the issues on the proprietary libril over the B2G > build, I am probably starting to prototype a PDU JS parser if none of you is > already working on that. I've seen that Philip wrote a TODO about that on > the b2g-js-ril repo. That's a great idea! Feel free to work a fork of the b2g-js-ril repo, we'll be happy to integrate your work.
Attached patch Patch for implementing full B2G RIL Stack (obsolete) (deleted) — Splinter Review
This is everything. Dials the phone. Hooray. (Still causes phone to heat up due to RIL Thread not sleeping, but signals didn't fix it on android. Need to talk to cjones.)
Attachment #576230 - Attachment is obsolete: true
Prior attachment is against current head of http://github.com/cgjones/mozilla-central, hash da62104d4d28196374c3978066dfc98feaba86ca
Comment on attachment 578482 [details] [diff] [review] Patch for implementing full B2G RIL Stack Let's make this a tracking bug. I have split this up and applied to m-c, will attach patches to individual bugs.
Attachment #578482 - Attachment is obsolete: true
Summary: Create JS RIL Stack for B2G Telephony → [meta] Create JS RIL Stack for B2G Telephony
Depends on: 699222
Depends on: 707629
Depends on: 707883
Morphing this bug further to include all aspects that are necessary for telephony.
Summary: [meta] Create JS RIL Stack for B2G Telephony → [meta] B2G Telephony
Depends on: 708446
Depends on: 708528
Depends on: 708926
Blocks: b2g-sms
Alias: b2g-telephony
Depends on: 709565
Depends on: 709862
Depends on: 709915
Depends on: 710092
Blocks: b2g-ril
No longer blocks: b2g-sms
No longer depends on: 708926
No longer depends on: 710092
No longer depends on: 709915
Component: DOM → DOM: Device Interfaces
QA Contact: general → device-interfaces
Depends on: webtelephony
Depends on: 712942
Depends on: 712944
With pretty close to m-c tip, I see the following on my (real) b2g phone when I call it I/Gecko ( 2587): -*- TelephonyWorker component: Received message: {"type":"callstatechange","callState":"incoming","callIndex":1,"number":"15106845513","name":null} but I don't see any incoming-call UI pop up in gaia. Are we delivering "incoming" events through the DOM telephony api yet? (If so, this is a gaia bug.)
(In reply to Chris Jones [:cjones] [:warhammer] from comment #23) > Are we delivering "incoming" events through the DOM telephony api yet? Yes we are.
Depends on: 714968
Depends on: 714973
Depends on: 716709
Depends on: 719331
Depends on: 720811
Depends on: 720892
No longer depends on: 720892
Depends on: 721266
Depends on: 722841
No longer depends on: 722841
Depends on: 725599
Depends on: 727795
No longer depends on: 727795
Depends on: 727803
Depends on: 727795
No longer depends on: 727803
No longer depends on: 727795
No longer depends on: 731786
Assignee: kyle → philipp
Depends on: 735165
Depends on: 735167
Depends on: 735170
No longer depends on: 735170
Depends on: 740230
Depends on: 746496
Depends on: 746886
Depends on: 749794
No longer depends on: 748187
Depends on: 757773
No longer blocks: b2g-demo-phone
Whiteboard: [b2g:blocking+]
blocking-basecamp: --- → +
blocking-kilimanjaro: --- → +
Whiteboard: [b2g:blocking+]
No longer depends on: 746886
No longer depends on: 751550
Depends on: 766273
Initial implementation is basically complete. Breaking dependency on the one non-implementation related bug, and closing this.
Status: NEW → RESOLVED
Closed: 12 years ago
No longer depends on: 719331
Resolution: --- → FIXED
Depends on: 719331
Depends on: 1008471
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: