[B2G] [CDMA] Support receiving CDMA info record

RESOLVED FIXED in Firefox 26

Status

Firefox OS
RIL
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Anshul, Assigned: chucklee)

Tracking

unspecified
1.2 FC (16sep)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:koi+, firefox26 fixed)

Details

(Whiteboard: [ETA:8/9], [FT:RIL], [Sprint:2])

Attachments

(4 attachments, 10 obsolete attachments)

1.32 KB, patch
Details | Diff | Splinter Review
1.16 KB, patch
Details | Diff | Splinter Review
4.49 KB, patch
vicamo
: review+
Details | Diff | Splinter Review
10.73 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
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

5 years ago
(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?
(Reporter)

Comment 2

5 years ago
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.
So the preferred API looks like cell broadcast, not using system message?

Updated

5 years ago
blocking-b2g: --- → koi+

Updated

5 years ago
Assignee: nobody → chulee

Updated

5 years ago
Blocks: 890826

Updated

5 years ago
Blocks: 891755
Summary: [B2G] [CDMA] Add UI support to display CDMA info record → [B2G] [CDMA] Support receiving CDMA info record
Vicamo suggests to add a |oncdmainforecreceived| in mobile connection.
(Reporter)

Comment 5

5 years ago
(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
(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
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.
Created attachment 776268 [details] [diff] [review]
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.
Attachment #776268 - Flags: feedback?(vyang)
Created attachment 776269 [details] [diff] [review]
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"
Attachment #776269 - Flags: feedback?(vyang)
Created attachment 776270 [details] [diff] [review]
WIP - 0003. Fire event to DOM on reception.
Attachment #776270 - Flags: feedback?(vyang)
Created attachment 776276 [details] [diff] [review]
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.
Created attachment 776914 [details] [diff] [review]
Test - Test event reception using ui_test

Updated

5 years ago
Whiteboard: [ETA:8/9]

Updated

5 years ago
Whiteboard: [ETA:8/9] → [ETA:8/9], [FT:RIL], [Sprint:2]
Created attachment 800587 [details] [diff] [review]
0001. Handle CDMA information record event from RIL. V2

Rebase.
Attachment #776268 - Attachment is obsolete: true
Attachment #776268 - Flags: feedback?(vyang)
Attachment #800587 - Flags: review?(vyang)
Created attachment 800589 [details] [diff] [review]
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.
Attachment #776269 - Attachment is obsolete: true
Attachment #776270 - Attachment is obsolete: true
Attachment #776269 - Flags: feedback?(vyang)
Attachment #776270 - Flags: feedback?(vyang)
Attachment #800589 - Flags: review?(vyang)
Created attachment 800590 [details] [diff] [review]
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.
Attachment #776276 - Attachment is obsolete: true
Created attachment 800592 [details] [diff] [review]
Test Only - Receive and dump CDMA info record system message in Gaia.
Attachment #776914 - Attachment is obsolete: true
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.
Attachment #800587 - Flags: review?(vyang) → review+
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|.
Attachment #800589 - Flags: review?(vyang)
Created attachment 801477 [details] [diff] [review]
Receive CDMA information record and send system message.

Address reviewer's comments.
Attachment #800587 - Attachment is obsolete: true
Attachment #800589 - Attachment is obsolete: true
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?
Attachment #801477 - Flags: review-
Created attachment 801605 [details] [diff] [review]
0001. Receive CDMA information record and send system message. V2

Fix errors indicated in comment 21.
Attachment #801477 - Attachment is obsolete: true
Attachment #801605 - Flags: review?(vyang)
Created attachment 801607 [details] [diff] [review]
0002. Test case for CDMA information record decoder.
Attachment #801607 - Flags: review?(vyang)

Updated

5 years ago
Component: General → RIL
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 :)
Attachment #801605 - Flags: review?(vyang) → review+
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. :)
Attachment #801607 - Flags: review?(vyang) → review+
Created attachment 802425 [details] [diff] [review]
0001. Receive CDMA information record and send system message. V2.1

Add comment for new decoder.
Attachment #801605 - Attachment is obsolete: true
Created attachment 802431 [details] [diff] [review]
0001. Receive CDMA information record and send system message. V2.2

Fix syntax error.
Attachment #802425 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/cccdb7f084a4
https://hg.mozilla.org/mozilla-central/rev/3e66d6106662
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 1.2 FC (16sep)
status-firefox26: --- → fixed
You need to log in before you can comment on or make changes to this bug.