Closed Bug 882984 Opened 7 years ago Closed 6 years ago

[B2G] [CDMA] Update nsIDOMMozMobileCellInfo to also report base station id, base station latitude, base station longitude, system id, network id

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: anshulj, Assigned: edgar)

References

Details

(Whiteboard: [fixed-in-birch])

Attachments

(5 files, 8 obsolete files)

2.56 KB, patch
edgar
: review+
Details | Diff | Splinter Review
8.75 KB, patch
edgar
: review+
Details | Diff | Splinter Review
1.56 KB, patch
edgar
: review+
Details | Diff | Splinter Review
1.37 KB, patch
edgar
: review+
Details | Diff | Splinter Review
3.59 KB, patch
edgar
: review+
Details | Diff | Splinter Review
This information would be needed for any clients like GPS.
Assignee: nobody → echen
Component: General → DOM: Device Interfaces
OS: All → Gonk (Firefox OS)
Product: Boot2Gecko → Core
Attachment #771190 - Attachment description: Part 1: Expose CDMA registration information to mozMobileCellInfo → Part 1: Expose CDMA registration information to mozMobileCellInfo, v1
Attached patch Part 3: Fix xpcshell tests, v1 (obsolete) — Splinter Review
Blocks: 874446
Blocks: 890822
Attachment #771190 - Attachment is obsolete: true
Comment on attachment 777666 [details] [diff] [review]
Part 1: Expose CDMA registration information to mozMobileCellInfo, v2

I expose cdma registration information to mozMobileCellInfo and also define the value for each field when the information is unavailable. Could you give some feedback, hsinyi
Attachment #777666 - Flags: feedback?
Attachment #777666 - Flags: feedback? → feedback?(htsai)
Comment on attachment 777666 [details] [diff] [review]
Part 1: Expose CDMA registration information to mozMobileCellInfo, v2

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

Looks good!
Attachment #777666 - Flags: feedback?(htsai) → feedback+
Attached patch Part 4: Fix xpcshell tests, v1 (obsolete) — Splinter Review
Attachment #771192 - Attachment is obsolete: true
Attachment #777666 - Flags: review?(htsai)
Attachment #778296 - Flags: review?(htsai)
Attachment #778300 - Flags: review?(htsai)
Attachment #778301 - Flags: review?(htsai)
Comment on attachment 777666 [details] [diff] [review]
Part 1: Expose CDMA registration information to mozMobileCellInfo, v2

Hi Mounir:

This bug is for exposing cdma registration information which would be needed for GPS clients to MobileCellInfo. Please help to review the interface changes.

Thanks in advance.
Attachment #777666 - Flags: superreview?(mounir)
Comment on attachment 778298 [details] [diff] [review]
Part 3: GPS changes for MobileCellInfo, v1

Kan-Ru, we change the the type of gsmLocationAreaCode and gsmCellId in this bug, and it is needed to change some codes in GPS as well, could you help to review this? Thanks in advance.
Attachment #778298 - Flags: review?(kchen)
Comment on attachment 778298 [details] [diff] [review]
Part 3: GPS changes for MobileCellInfo, v1

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

r=me with following addressed.

::: dom/system/gonk/GonkGPSGeolocationProvider.cpp
@@ +484,5 @@
> +        int32_t lac;
> +        int64_t cid;
> +
> +        cell->GetGsmLocationAreaCode(&lac);
> +        if (lac >= 0x0 && lac <= 0xffff) {

Comment the magic number 0xffff. Or use a constant like MAX_GSM_LAC to hold that.

@@ +489,5 @@
> +          location.u.cellID.lac = lac;
> +        }
> +
> +        cell->GetGsmCellId(&cid);
> +        if (cid >= 0x0 && cid <= 0xffffffff) {

Ditto.
Attachment #778298 - Flags: review?(kchen) → review+
Comment on attachment 778296 [details] [diff] [review]
Part 2: RIL implementation for exposing CDMA registration information, v2

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

Looks great! Thank you :)
Attachment #778296 - Flags: review?(htsai) → review+
Comment on attachment 778300 [details] [diff] [review]
Part 4: Fix xpcshell tests, v1

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

Nice!!!
Attachment #778300 - Flags: review?(htsai) → review+
Comment on attachment 778301 [details] [diff] [review]
Part 5: Marionette tests for cdma registration information, v1

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

Great!

Would you please file another follow-up bug for cdma marionette tests? Thank you.
Attachment #778301 - Flags: review?(htsai) → review+
Comment on attachment 777666 [details] [diff] [review]
Part 1: Expose CDMA registration information to mozMobileCellInfo, v2

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

Looks good!
Attachment #777666 - Flags: review?(htsai) → review+
Blocks: 896396
Comment on attachment 777666 [details] [diff] [review]
Part 1: Expose CDMA registration information to mozMobileCellInfo, v2

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

Hsin-Yi's review should be enough here.
Attachment #777666 - Flags: superreview?(mounir)
(In reply to Mounir Lamouri (:mounir) from comment #18)
> Comment on attachment 777666 [details] [diff] [review]
> Part 1: Expose CDMA registration information to mozMobileCellInfo, v2
> 
> Review of attachment 777666 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hsin-Yi's review should be enough here.

Got it.
Thanks, Mounir!
Add r=hsinyi after r+.
Attachment #777666 - Attachment is obsolete: true
Attachment #780172 - Flags: review+
1). Rebase.
2). Add r=hsinyi after r+.
Attachment #778296 - Attachment is obsolete: true
Attachment #780173 - Flags: review+
1). Address review comment #13.
2). Add r=kchen after r+.
Attachment #778298 - Attachment is obsolete: true
Attachment #780174 - Flags: review+
Add r=hsinyi after r+.
Attachment #778300 - Attachment is obsolete: true
Attachment #780175 - Flags: review+
1). Rebase.
2). Add r=hsinyi after r+.
Attachment #778301 - Attachment is obsolete: true
Attachment #780179 - Flags: review+
You need to log in before you can comment on or make changes to this bug.