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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Anshul]

This information would be needed for any clients like GPS.

Comment 1

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA registration information to mozMobileCellInfo, v1

Comment 2

[Edgar Chen [:edgar]]

Attachment: Part 2: RIL implementation for exposing CDMA registration information, v1

Comment 3

[Edgar Chen [:edgar]]

Attachment: Part 3: Fix xpcshell tests, v1

Comment 4

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA registration information to mozMobileCellInfo, v2

Comment 5

[Edgar Chen [:edgar]]

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

Comment 6

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

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!

Comment 7

[Edgar Chen [:edgar]]

Attachment: Part 2: RIL implementation for exposing CDMA registration information, v2

Comment 8

[Edgar Chen [:edgar]]

Attachment: Part 3: GPS changes for MobileCellInfo, v1

Comment 9

[Edgar Chen [:edgar]]

Attachment: Part 4: Fix xpcshell tests, v1

Comment 10

[Edgar Chen [:edgar]]

Attachment: Part 5: Marionette tests for cdma registration information, v1

Comment 11

[Edgar Chen [:edgar]]

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.

Comment 12

[Edgar Chen [:edgar]]

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.

Comment 13

[Kan-Ru Chen [:kanru] (UTC+9)]

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.

Comment 14

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

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 :)

Comment 15

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

Comment on attachment 778300 [details] [diff] [review]
Part 4: Fix xpcshell tests, v1

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

Nice!!!

Comment 16

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

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.

Comment 17

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

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!

Comment 18

[Mounir Lamouri (:mounir)]

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.

Comment 19

[Edgar Chen [:edgar]]

(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!

Comment 20

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA registration information to mozMobileCellInfo, v2, r=hsinyi

Add r=hsinyi after r+.

Comment 21

[Edgar Chen [:edgar]]

Attachment: Part 2: RIL implementation for exposing CDMA registration information, v3, r=hsinyi

1). Rebase.
2). Add r=hsinyi after r+.

Comment 22

[Edgar Chen [:edgar]]

Attachment: Part 3: GPS changes for MobileCellInfo, v2, r=kchen

1). Address review comment #13.
2). Add r=kchen after r+.

Comment 23

[Edgar Chen [:edgar]]

Attachment: Part 4: Fix xpcshell tests, v1, r=hsinyi

Add r=hsinyi after r+.

Comment 24

[Edgar Chen [:edgar]]

Attachment: Part 5: Marionette tests for cdma registration information, v2, r=hsinyi

1). Rebase.
2). Add r=hsinyi after r+.

Comment 25

[Edgar Chen [:edgar]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=7f5224ad19eb

Comment 26

[Edgar Chen [:edgar]]

https://hg.mozilla.org/projects/birch/rev/65d019d4162c
https://hg.mozilla.org/projects/birch/rev/59298610ee48
https://hg.mozilla.org/projects/birch/rev/1a6b938e93fd
https://hg.mozilla.org/projects/birch/rev/734ab44f2204
https://hg.mozilla.org/projects/birch/rev/db22940c0cba

Comment 27

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/65d019d4162c
https://hg.mozilla.org/mozilla-central/rev/59298610ee48
https://hg.mozilla.org/mozilla-central/rev/1a6b938e93fd
https://hg.mozilla.org/mozilla-central/rev/734ab44f2204
https://hg.mozilla.org/mozilla-central/rev/db22940c0cba