1113466 - [Network Alert] Support CDMA CMAS Alert

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Network Alerts, defect)

Description

[Steve Chung [:steveck]]

We didn't check if the CB message from CDMA is CMAS or not for now. If we need to suooirt the CMAS in CDMA like GSM, we will need to check the range[1] of CDMA service category[2] and check it in both system and network alert(if this fixing need to land before bug 1093922).

[1] http://androidxref.com/4.4.4_r1/xref/frameworks/opt/telephony/src/java/com/android/internal/telephony/cdma/sms/BearerData.java#1880
[2] http://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozCellBroadcastMessage.webidl#72

Comment 1

[Julien Wajsberg [:julienw]]

Carol, do we need to fix this for v2.2 too? Reading bug 1091751 it seems like we should otherwise CMAS in CDMA is broken, right?

Comment 2

[Carol Yang [:cyang]]

(In reply to Julien Wajsberg [:julienw] from comment #1)
> Carol, do we need to fix this for v2.2 too? Reading bug 1091751 it seems
> like we should otherwise CMAS in CDMA is broken, right?

I think at this point, CDMA CMAS is technically "not broken" in the sense that it would display, just not through Network Alert app. So I guess your call on whether this makes it into v2.2?

Comment 3

[Julien Wajsberg [:julienw]]

If it's displayed through the System app, we won't have the dedicated signal as implemented in bug 1099100.

My main misunderstanding is about bug 1091751 comment 22, where it's said that that patch would break CMAS in CDMA.

Moreoever I think v2.2 is supposed to ship on a CDMA device.

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Comment 5

[Steve Chung [:steveck]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Hi Bevis, I think there's no complicate logic for CDMA service category, but it would be great if you can give any feedback if something is missing or incorrect.

Hi Carol, this patch add CDMA CMAS checking logic in network alert and system, could you please help verify:
- GSM alerts works fine without regression
- CDMA CMAS message is displayed correctly by network-alerts app instead of system, and
- no duplicated notification for CDMA CMAS
- works correctly for Presidential alert

Thanks!

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Hi Steve,

Per bug 1091960 comment 2 and and confirmation of bug 1091960 comment 9 with Carol, please help identify a CDMA CBS message by checking whether "cdmaServiceCategory" is null instead. ('cdmaServiceCategory' is nullable in MozCellBroadcastMessage.webidl and will only be available when this CBS message is delivered from CDMA network.)

Note: MessageId '0' is a valid id in GSM network.

Thanks!

Comment 7

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:

Comment 8

[Carol Yang [:cyang]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Was having some trouble using this patch as-is:

1) We should be passing in message rather than message.messageId when calling isEmergencyAlert

@@ -65,11 +67,14 @@
    * information like Identifier and body for displaying attention screen.
    */
   function onCellbroadcast(message) {
-    if (!isEmergencyAlert(message.messageId)) {
+    if (!isEmergencyAlert(message)) {

2) What Bevis pointed out is definitely a problem I ran into. Because messageId can be 0 and valid, the check that's currently there needs to be reversed to checking cdmaServiceCategory first.

On that note though, I think there are enough differences between GSM/CDMA messages that we should have two different notification pipes. Since we're not doing this for v2.2, I was wondering if we can consider maybe splitting this up in master:

void notifyGsmMessageReceived(in unsigned long aServiceId,
                              in unsigned long aGsmGeographicalScope,
                              in unsigned short aMessageCode,
                              in unsigned short aMessageId,
                              in DOMString aLanguage,
                              in DOMString aBody,
                              in unsigned long aMessageClass,
                              in DOMTimeStamp aTimestamp,
                              in boolean aHasEtwsInfo,
                              in unsigned long aEtwsWarningType,
                              in boolean aEtwsEmergencyUserAlert,
                              in boolean aEtwsPopup);


void notifyCdmaMessageReceived(in unsigned long aServiceId,
                               in DOMString aLanguage,
                               in DOMString aBody,
                               in DOMTimeStamp aTimestamp,
                               in unsigned long aServiceCategory,
                               in boolean aHasCmasInfo,
                               in unsigned long aCategory,
                               in unsigned long aResponseType,
                               in unsigned long aSeverity,
                               in unsigned long aUrgency,
                               in unsigned long aCertainty);

A few things to note about this proposal:
- notifyGsmMessageReceived:
-- This hasn't changed much from the original notifyMessageReceived. It just has aCdmaServiceCategory removed.
- notifyCdmaMessageReceived
-- This should have something similar to aHasEtwsInfo for CMAS info. The last 5 parameters (starting at aCategory and ending at aCertainty) are only sent/included if aHasCmasInfo=true
-- Removed several fields that were specific to GSM

Bevis/Steve, what do you think?

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Carol Yang [:cyang] from comment #8)
> A few things to note about this proposal:
> - notifyGsmMessageReceived:
> -- This hasn't changed much from the original notifyMessageReceived. It just
> has aCdmaServiceCategory removed.
> - notifyCdmaMessageReceived
> -- This should have something similar to aHasEtwsInfo for CMAS info. The
> last 5 parameters (starting at aCategory and ending at aCertainty) are only
> sent/included if aHasCmasInfo=true
> -- Removed several fields that were specific to GSM
> 
I've created bug 1156146 for further discussion of the API changes including the Web API changes.

We could focus on the bug fixing in this bug and API change discussion in bug 1156146 instead. :)

Comment 10

[Steve Chung [:steveck]]

Hi Carol, thanks for finding out the bug and I'll update the patch with test later.

I agree with you that we should have different API for GSM/CDMA separately if the set of message properties is different, and I'll add comment in bug 1156146 directly.

Comment 11

[Steve Chung [:steveck]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Hi Carol, patch updated and we'll need your help to verify CDMA CMAS on real device, hope it works fine after this patch applied.

Hi Julien/Kevin, it's the patch that need to filter out the CDMA CMAS for network-alerts specific notification and need some modification in both sides. You can reference CDMA CMAS document here[1] or Bug 1091960 comment 8 in short, thanks!

[1]http://www.3gpp2.org/public_html/specs/C.R1001-G_v1.0_Param_Administration.pdf, chapter 9.3.3 Commercial Mobile

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

I can't really test but this looks good.

I have some nits and some questions for Bevis before finishing the review. Please request review once the nits are fixed and hopefully Bevis would have answered by then :)

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

I don't know this spec very well, and it seems Julien is on top of it - so I think it would be better if he handled this one. I'll take a quick look at the pull request though, thanks!

Comment 14

[Carol Yang [:cyang]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Hi Steve,

Just to be thorough, I thought this might be a good enough list of tests to run to cover most cases:

1) Inject GSM Presidential Alert
2) Inject GSM Extreme Alert
3) Inject GSM regular CBS message
4) Inject CDMA Presidential Alert
5) Inject CDMA Extreme Alert
6) Inject CDMA regular CBS message

I ran all 6 tests on the following settings configurations:

Configuration 1:
"Cell Broadcast" = on
"Emergency Alert" = on

For both GSM and CDMA, Presidential/Extreme alerts are displayed by Network Alerts app. Also for both, regular CBS message is displayed by system.


Configuration 2:
"Cell Broadcast" = on
"Emergency Alert" = off

For both GSM and CDMA, Presidential alert is displayed by Network Alerts app. Extreme alert is not displayed at all as expected. Regular CBS message is displayed by system.


Configuration 3:
"Cell Broadcast" = off
"Emergency Alert" = off

For both GSM and CDMA, Presidential alert is displayed by Network Alerts app. Extreme alert is not displayed at all as expected. Regular CBS message is displayed by system. This last part seems incorrect? If "Cell Broadcast" was toggled off under Message settings, I shouldn't expect to see any CBS messages at all here, correct?

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Carol,

It seems that the attribute of "messageId" was not defined & documented clearly in [1][2] that this attribute is only expected to be available in GSM CBS message but always be set in current design.

Can you tell us what the value of "messageId" you assign when receiving a CDMA CBS message?

Thanks!

[1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozCellBroadcastMessage.webidl#43
[2] https://dxr.mozilla.org/mozilla-central/source/dom/cellbroadcast/interfaces/nsICellbroadcastMessenger.idl#21-22

Comment 16

[Steve Chung [:steveck]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Patch updated per suggestions. I found that system CB seems didn't check the displayed option before showing the notification. Maybe that's why the normal CB notification is still displayed.

Comment 17

[Julien Wajsberg [:julienw]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Some more simple nits resulting from the conversation with Bevis.

Also I think the issue with disabling CBS should move in a separate bug because it should be 2.2+ if it's also missing in 2.2.

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Letting Julien take this as in Comment 13.

Comment 19

[Carol Yang [:cyang]]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #15)
> Hi Carol,
> 
> It seems that the attribute of "messageId" was not defined & documented
> clearly in [1][2] that this attribute is only expected to be available in
> GSM CBS message but always be set in current design.
> 
> Can you tell us what the value of "messageId" you assign when receiving a
> CDMA CBS message?

Hey Bevis, it's 0 here.

> 
> Thanks!
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/dom/webidl/
> MozCellBroadcastMessage.webidl#43
> [2]
> https://dxr.mozilla.org/mozilla-central/source/dom/cellbroadcast/interfaces/
> nsICellbroadcastMessenger.idl#21-22

Comment 20

[Steve Chung [:steveck]]

(In reply to Julien Wajsberg [:julienw] from comment #17)
> Comment on attachment 8591524 [details] [review]
> [gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master
>
> Also I think the issue with disabling CBS should move in a separate bug
> because it should be 2.2+ if it's also missing in 2.2.

You are right that we should separate this into another bug since thie problem might be more severe. I created bug 1157190 for disabling CB.

Comment 21

[Steve Chung [:steveck]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

Patch updated with suggestion addressed, thanks!

Comment 22

[Julien Wajsberg [:julienw]]

Comment on attachment 8591524 [details] [review]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master

r=me

thanks !

Comment 23

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/5172fa02b28fee0a9943b9789dfd532ed83f6b9f