Closed
Bug 1113466
Opened 10 years ago
Closed 10 years ago
[Network Alert] Support CDMA CMAS Alert
Categories
(Firefox OS Graveyard :: Gaia::Network Alerts, defect)
Firefox OS Graveyard
Gaia::Network Alerts
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: steveck, Assigned: steveck)
References
Details
(Keywords: feature)
Attachments
(1 file)
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•10 years ago
|
||
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?
Flags: needinfo?(cyang)
Comment 2•10 years ago
|
||
(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?
Flags: needinfo?(cyang)
Comment 3•10 years ago
|
||
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•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8591524 [details]
[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!
Attachment #8591524 -
Flags: feedback?(cyang)
Attachment #8591524 -
Flags: feedback?(btseng)
Comment 6•10 years ago
|
||
Comment on attachment 8591524 [details]
[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!
Attachment #8591524 -
Flags: feedback?(btseng)
Updated•10 years ago
|
tracking-b2g:
backlog → ---
Keywords: feature
Comment 8•10 years ago
|
||
Comment on attachment 8591524 [details]
[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?
Flags: needinfo?(schung)
Flags: needinfo?(btseng)
Attachment #8591524 -
Flags: feedback?(cyang) → feedback-
Comment 9•10 years ago
|
||
(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. :)
Flags: needinfo?(btseng)
Assignee | ||
Comment 10•10 years ago
|
||
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.
Flags: needinfo?(schung)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8591524 [details]
[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
Attachment #8591524 -
Flags: review?(kgrandon)
Attachment #8591524 -
Flags: review?(felash)
Attachment #8591524 -
Flags: feedback?(cyang)
Attachment #8591524 -
Flags: feedback-
Comment 12•10 years ago
|
||
Comment on attachment 8591524 [details]
[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 :)
Attachment #8591524 -
Flags: review?(felash) → review-
Comment 13•10 years ago
|
||
Comment on attachment 8591524 [details]
[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!
Attachment #8591524 -
Flags: review?(kgrandon)
Comment 14•10 years ago
|
||
Comment on attachment 8591524 [details]
[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?
Attachment #8591524 -
Flags: feedback?(cyang)
Comment 15•10 years ago
|
||
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
Flags: needinfo?(cyang)
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8591524 [details]
[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.
Attachment #8591524 -
Flags: review?(kgrandon)
Attachment #8591524 -
Flags: review?(felash)
Attachment #8591524 -
Flags: review-
Attachment #8591524 -
Flags: feedback?(cyang)
Comment 17•10 years ago
|
||
Comment on attachment 8591524 [details]
[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.
Attachment #8591524 -
Flags: review?(felash) → review-
Comment 18•10 years ago
|
||
Comment on attachment 8591524 [details]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master
Letting Julien take this as in Comment 13.
Attachment #8591524 -
Flags: review?(kgrandon)
Comment 19•10 years ago
|
||
(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
Flags: needinfo?(cyang)
Updated•10 years ago
|
Attachment #8591524 -
Flags: feedback?(cyang) → feedback+
Assignee | ||
Comment 20•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #17)
> Comment on attachment 8591524 [details]
> [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.
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8591524 [details]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master
Patch updated with suggestion addressed, thanks!
Attachment #8591524 -
Flags: review- → review?(felash)
Comment 22•10 years ago
|
||
Comment on attachment 8591524 [details]
[gaia] steveck-chung:network-alerts-for-CDMA > mozilla-b2g:master
r=me
thanks !
Attachment #8591524 -
Flags: review?(felash) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 23•10 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/5172fa02b28fee0a9943b9789dfd532ed83f6b9f
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•