Closed Bug 1099100 Opened 5 years ago Closed 5 years ago

CMAS shall use a dedicated alert signal

Categories

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

defect
Not set

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S10 (17apr)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: olof.wickstrom, Assigned: julienw)

References

()

Details

(Whiteboard: [caf priority: p2][CR 812387])

Attachments

(4 files)

• For Netherlands the specification, attached, refers to 3GPP TS 22.268  where it is stated: “The PWS-UE [the phone] shall support a dedicated alerting indication (audio attention signal and a dedicated vibration cadence) and be distinct from any other device alerts and restricted to use for Warning Notification purposes.”
• For Chile and Peru the notification tone must be according to the attached standard (J-STD-100) and the tone must be different from the regular call-in tone.

So in short – it is not OK to use the standard Firefox OS notification for CMAS alerts
Yes, you're right, the sound and vibration sequence we use is not correct. We need to have different tones and vibration sequences.
Component: Gaia → Gaia::Network Alerts
Duplicate of this bug: 1147199
(Requested in the dupe)
blocking-b2g: --- → 2.2?
Whiteboard: [CR 812387]
Whiteboard: [CR 812387] → [caf priority: p2][CR 812387]
Julien,
Thanks for confirming this. Will you own fixing this for FxOS 2.2 FC?

Francisco,
If Julien won't be owning this please assign another owner. This issue blocks FxOS 2.2's FC planned for April 6th, about 1 week from today.

Thanks,
Mike
Flags: needinfo?(francisco)
Flags: needinfo?(felash)
I can do this.
Assignee: nobody → felash
Flags: needinfo?(felash)
Thanks for taking care of this Julien, Mike, this is not a blocker yet, who is triaging this bugs?
Flags: needinfo?(francisco)
This patch should follow the pattern defined in the attachment.

The specified noise is really ugly, I wonder if we should have a dedicated channel for this instead of reusing the "notification" channel. Also I wonder if it's too loud.

Carol, maybe you could help checking whether the patch works and give your advice on this? Thanks !
Flags: needinfo?(cyang)
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> Thanks for taking care of this Julien, Mike, this is not a blocker yet, who
> is triaging this bugs?

Hi Francisco,

CAF and I triage all bugs Bug 1063044, the CAF-v2.2-FC-metabug, depends on weekly. Wesley Huang and others triage all 2.2? bugs [1], weekly.

Mike

[1] http://goo.gl/p9SoAS
(In reply to Julien Wajsberg [:julienw] from comment #9)
> This patch should follow the pattern defined in the attachment.
> 
> The specified noise is really ugly, I wonder if we should have a dedicated
> channel for this instead of reusing the "notification" channel. Also I
> wonder if it's too loud.
> 
> Carol, maybe you could help checking whether the patch works and give your
> advice on this? Thanks !

Hi Julien,

After integrating your patch, I still don't hear any alerts. I checked that audio.volume.notification is non-zero. Also, I do see that ringtone() gets called in notify.js. Let me know if there are certain logs I can turn on to help you debug.

Thanks,
Carol
Flags: needinfo?(cyang)
[Tracking Requested - why for this release]:

[Blocking Requested - why for this release]:

triage: this is new feature, and landing such patch one week before FC is too risky. 
Let's not block on it.
blocking-b2g: 2.2? → 3.0?
tracking-b2g: --- → +
blocking-b2g: 3.0? → ---
feature-b2g: --- → 3.0?
Carol, I made it work in the emulator with this patch. You need to rehash the application's permission because I add a permission in this patch. There are several ways to do this:

* "make reset-gaia" reinstalls everything, including permissions
* push the app using WebIDE or sole's tool: https://github.com/sole/push-app
* use my hacky way; with [1], run "addpref rehash-manifest"

[1] https://github.com/julienw/config-files/blob/master/addpref

The sound is IMO too loud so I'll put it quieter (I'm comparing with the normal SMS notification to have it quite similar in loudness).

Also with this patch the sound is played again when the user displays the message again, I think this is incorrect and I'll patch this as well.
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

This looks ready to me :)
Attachment #8585613 - Flags: review?(schung)
(In reply to Julien Wajsberg [:julienw] from comment #13)
> Carol, I made it work in the emulator with this patch. You need to rehash
> the application's permission because I add a permission in this patch. There
> are several ways to do this:
> 
> * "make reset-gaia" reinstalls everything, including permissions
> * push the app using WebIDE or sole's tool: https://github.com/sole/push-app
> * use my hacky way; with [1], run "addpref rehash-manifest"
> 
> [1] https://github.com/julienw/config-files/blob/master/addpref
> 
> The sound is IMO too loud so I'll put it quieter (I'm comparing with the
> normal SMS notification to have it quite similar in loudness).
> 
> Also with this patch the sound is played again when the user displays the
> message again, I think this is incorrect and I'll patch this as well.

I tried using [1] but still not getting any sounds. Also, I tried pulling in your change and creating a clean build. That ought to fix the rehash issue but still doesn't.
This is weird, because I could get the sound with the emulator.

The emulator is very slow so maybe there is a race condition.
I'll try something tomorrow (waiting for visibilitychange event like for vibration) and ask you to try it again.
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

Removing review request for now.
Attachment #8585613 - Flags: review?(schung)
Command to simulate a CMAS Presidential Alert with the emulator:

cbs pdu C0021112011154741914AFA7C76B9058FEBEBB41E6371EA4AEB7E173D0DB5E9683E8E832881DD6E741E4F7B9D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D100
(obviously on one line)
Carol, I tried on a Flame; I simulated receiving a CMAS message using a SMS and it works for me (I get the sound and the vibration). I don't know what's broken on your side :/
With this patch, and after rehashing manifests, you can send a message "4370:message" to the phone and it should trigger the same behavior than receiving a CMAS message.
Wesley, since the fix is already being worked on, I am requesting 2.2 again?
blocking-b2g: --- → 2.2?
Flags: needinfo?(whuang)
Hi Anshul,

Very likely won't take risk to get this uplifted as we're very close to FC.
But let's keep the nom for now.
Triagers: the fix is really not risky.

Carol: I tried on master only, maybe there is a problem on 2.2? What is the version you test on? I'll check this today.
OK, it actually doesn't work on v2.2.
I get this error:

04-02 14:20:25.932  2506  2506 W Network Alerts: [JavaScript Error: "TypeError: Float32Array.from is not a function" {file: "app://network-alerts.gaiamobile.org/js/attention/notify.js" line: 58}]

So I'll try to change this.
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

Carol, this updated patch is working on v2.2 for me. I don't know for earlier versions because I use some new ES6 syntax, so I hope you don't need it in older versions :)

Please tell me if this looks right from your point of view.

Steve, Oleg, asking review from both of you but only 1 review is enough :) Just the first one take it !

Thanks !
Attachment #8585613 - Flags: review?(schung)
Attachment #8585613 - Flags: review?(azasypkin)
Attachment #8585613 - Flags: feedback?(cyang)
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

Overall looks good to me! Just few nits and unit test suggestion.

One note: when my Flame receives 4 or 5 messages in a row all that sounds mix into some awful cracking low-frequency sound :)
Attachment #8585613 - Flags: review?(azasypkin) → review+
(In reply to Julien Wajsberg [:julienw] from comment #26)
> Comment on attachment 8585613 [details] [review]
> [gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master
> 
> Carol, this updated patch is working on v2.2 for me. I don't know for
> earlier versions because I use some new ES6 syntax, so I hope you don't need
> it in older versions :)
> 
> Please tell me if this looks right from your point of view.
> 
> Steve, Oleg, asking review from both of you but only 1 review is enough :)
> Just the first one take it !
> 
> Thanks !

Hi Julien,

Sorry I've been busy with a few other issues. Yes, I was trying on v2.2 and not master. Thanks for getting to the bottom of this. I will verify this tomorrow and let you know.
(In reply to Oleg Zasypkin [:azasypkin] from comment #27)
> Comment on attachment 8585613 [details] [review]
> [gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master
> 
> Overall looks good to me! Just few nits and unit test suggestion.
> 
> One note: when my Flame receives 4 or 5 messages in a row all that sounds
> mix into some awful cracking low-frequency sound :)

Humm maybe we should stop the previous one if we have another message. I'll see if it's enough to use a visibility change event.
Pushed an untested unfinished new version whose goal is to stop the previous attention sound/vibration if there is a new one.
Will finish next week.
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

Hi Julien,

I hear the alert now with this patch.

Thanks!
Attachment #8585613 - Flags: feedback?(cyang) → feedback+
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

Sorry for the late review. I only have a small question that whether we should stop the notification signal easily, but the patch still looks great.
Attachment #8585613 - Flags: review?(schung) → review+
hey Alive, here I have a situation where:
* the application launches an attention screen
* then it launches a second attention screen
=> I'd like to be notified in the first attention screen

My first try has been to use a "visibilitychange" event but it does not seem to get it when the second attention screen starts.

Do you know if this should work?

I'm trying on master but I'd need this to work in v2.2 as well.

Thanks!
Flags: needinfo?(alive)
Blocks: 1151979
I filed bug 1151979 for the issue of stopping a alert if another one is starting. I think we hsould use BroadcastChannel instead of "visibilitychange" event because as Steve pointed out we could have this event for other reasons.

Keeping the NI for alive because there could be a bug in the System app, but I'll land the changes without the last commit.
Keywords: checkin-needed
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1026685
[User impact] if declined: No sound/vibration alert when a PWS/CMAS alert is received by the device
[Testing completed]: yes (unit, manual, partner)
[Risk to taking this patch] (and alternatives if risky): low, because the new code is called after the old one.
[String changes made]: none
Attachment #8585613 - Flags: approval-gaia-v2.2?
http://docs.taskcluster.net/tools/task-graph-inspector/#X8k9Yo4KRh2kUim8VTKBLQ

The pull request failed to pass integration tests. It could not be landed, please try again.
Manually merged because the failures are unrelated to this patch.
master: 6935169fc9f2727c9e7eaa0560ca05737698fcb6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
blocking-b2g: 2.2? → 2.2+
Comment on attachment 8585613 [details] [review]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master

Given Julien's risk assessment I am approving this for 2.2 uplift due to the partner request here.
Attachment #8585613 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
WOW, I don't know that we could open two attention window in the same app now :/
It's not strange you will not get visibilitychange because I don't know we have this use case.

To support multiple attention window we will need another bug to implement it, and I hope it's not blocking :/ Could you just use BroadcastChannel (or, at worst, localStorage change event) to notify the 1st iframe in 2nd iframe?
Flags: needinfo?(alive)
Flags: needinfo?(whuang)
feature-b2g: 3.0? → ---
tracking-b2g: + → ---
You need to log in before you can comment on or make changes to this bug.