882985 - [B2G] [CDMA] Support receiving CDMA info record

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Anshul]

Add UI support to display CDMA info record as defined in C.S0005 section 3.7.5.1 and extended Display Info Rec as defined in C.S0005 section 3.7.5.16

Comment 1

[Ken Chang[:kenkai]]

(In reply to Anshul from comment #0)
> Add UI support to display CDMA info record as defined in C.S0005 section
> 3.7.5.1 and extended Display Info Rec as defined in C.S0005 section 3.7.5.16

Hi Anshul, do you have detail information to let us know how to show the UI for this bug?

Comment 2

[Anshul]

Ken, I think Android just shows a dialog box on the UI with the information. FFOS already has similar capabilities to display message box as part of cell broadcast https://github.com/mozilla-b2g/gaia/pull/10140. We would need something similar.

Comment 3

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

So the preferred API looks like cell broadcast, not using system message?

Comment 4

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Vicamo suggests to add a |oncdmainforecreceived| in mobile connection.

Comment 5

[Anshul]

(In reply to Chuck Lee [:chucklee] from comment #4)
> Vicamo suggests to add a |oncdmainforecreceived| in mobile connection.
That sounds good. I am assuming Gaia on receiving this event would display a dialog box if the cdma info received is of type RIL_CDMA_DISPLAY_INFO_REC

Comment 6

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Anshul from comment #5)
> (In reply to Chuck Lee [:chucklee] from comment #4)
> > Vicamo suggests to add a |oncdmainforecreceived| in mobile connection.
> That sounds good. I am assuming Gaia on receiving this event would display a
> dialog box if the cdma info received is of type RIL_CDMA_DISPLAY_INFO_REC

Based on the ril code [1], there might be a set of information records in one incoming Parcel.
So I think of parsing it into an object for the onreceived function, let application decide which to use.
Or its better to trigger onreceived for each single record?

[1] https://git.mozilla.org/?p=b2g/platform_hardware_ril.git;a=blob;f=libril/ril.cpp;h=59bfdff4c156edb0f8b08f3d817c71db9bb4fedd;hb=HEAD#l1827

Comment 7

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

By the way, we are lack of reference test data. We have no way to confirm if we handled the parcel correctly in RIL now.

Comment 8

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0001. Handle CDMA information record event from RIL.

Handle CDMA information record parcel from RIL.

The parcel format is referenced from the source code of RIL, but no test environment nor test data to confirm if its right.

Comment 9

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0002. Add callback event and system message for CDMA information record reception.

Add |mozMobileConnection.oncdmainforecreceived| per comment 5 and system message "cdma-info-rec-received"

Comment 10

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0003. Fire event to DOM on reception.

Comment 11

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0004. Test case.

Simulate CDMA information reception on reception of SMS.
Just for testing the message path from RIL to Gaia, since there's no way to test real reception.

Comment 12

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Test - Test event reception using ui_test

Comment 13

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Handle CDMA information record event from RIL. V2

Rebase.

Comment 14

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Send system message on receiving CDMA information record.

Since we don't know the use case of CDMA info record, just send a system message containing these information.

Comment 15

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Test Only - Send CDMA info record system message from gecko.

We can't find a way to receive real CDMA info record message, nor simulate it in emulator.
For test purpose, we triggered a CDMA info record system message while receiving a SMS.

Comment 16

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Test Only - Receive and dump CDMA info record system message in Gaia.

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 800587 [details] [diff] [review]
0001. Handle CDMA information record event from RIL. V2

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

::: dom/system/gonk/ril_worker.js
@@ +6245,5 @@
> +        }
> +
> +        /*
> +         * I can't tell for source code of ril that if extended display is
> +         * decoded by modem or not. If so, followings just work.

Please remove all the commented lines here.  We have mechanism, quirks, to deal with modem ability differences.  File a follow-up some day when that's necessary.

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 800589 [details] [diff] [review]
0002. Send system message on receiving CDMA information record.

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

Please merge the two patches into one.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1163,5 @@
>          this.handleExitEmergencyCbMode(message);
>          break;
> +      case "cdma-info-rec-received":
> +        if (DEBUG) this.debug("cdma-info-rec-received: " + JSON.stringify(message));
> +        this.handleCdmaInfoRecReceived(message);

Just inline |gSystemMessenger.broadcastMessage|.

Comment 19

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Receive CDMA information record and send system message.

Address reviewer's comments.

Comment 20

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=e0bd4cb8697e

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 801477 [details] [diff] [review]
Receive CDMA information record and send system message.

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

::: dom/system/gonk/ril_worker.js
@@ +6231,5 @@
> +        // Based on spec, headerByte must be 0x80 now
> +        record.extendedDisplay.indicator = (headerByte >> 7);
> +        record.extendedDisplay.type = (headerByte & 0x7F);
> +
> +        while (display > 0) {

|display| is undefined here.

@@ +6232,5 @@
> +        record.extendedDisplay.indicator = (headerByte >> 7);
> +        record.extendedDisplay.type = (headerByte & 0x7F);
> +
> +        while (display > 0) {
> +          let display = {};

Then it's re-initialized to another type.  Please don't do this.  A variable would be better not re-initialize throughout the scope it lives in.  If you really have to, only re-initialize it to something of the same type.

@@ +6240,5 @@
> +              display.tag !== INFO_REC_EXTENDED_DISPLAY_SKIP) {
> +            record.content = Buf.readString();
> +          }
> +
> +          record.extendedDisplay.push(display);

|extendedDisplay| is an object.  What's its |push| method means?

Comment 22

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Receive CDMA information record and send system message. V2

Fix errors indicated in comment 21.

Comment 23

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Test case for CDMA information record decoder.

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 801605 [details] [diff] [review]
0001. Receive CDMA information record and send system message. V2

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

Thank you :)

Comment 25

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 801607 [details] [diff] [review]
0002. Test case for CDMA information record decoder.

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

Since we have bug 912442 and bug 912909 right now, please have a test run on tbpl before I will check in this. :)

Comment 26

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=44229946d71d

Comment 27

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Receive CDMA information record and send system message. V2.1

Add comment for new decoder.

Comment 28

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Receive CDMA information record and send system message. V2.2

Fix syntax error.

Comment 29

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=dd1288631990

Comment 30

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/cccdb7f084a4
https://hg.mozilla.org/integration/b2g-inbound/rev/3e66d6106662

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cccdb7f084a4
https://hg.mozilla.org/mozilla-central/rev/3e66d6106662