Closed
Bug 1099100
Opened 10 years ago
Closed 10 years ago
CMAS shall use a dedicated alert signal
Categories
(Firefox OS Graveyard :: Gaia::Network Alerts, defect)
Firefox OS Graveyard
Gaia::Network Alerts
Tracking
(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)
People
(Reporter: olof.wickstrom, Assigned: julienw)
References
()
Details
(Whiteboard: [caf priority: p2][CR 812387])
Attachments
(4 files)
(deleted),
application/pdf
|
Details | |
(deleted),
application/pdf
|
Details | |
(deleted),
text/x-github-pull-request
|
azasypkin
:
review+
steveck
:
review+
cyang
:
feedback+
bajaj
:
approval-gaia-v2.2+
|
Details |
(deleted),
patch
|
Details | Diff | Splinter Review |
• 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
Reporter | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
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
Assignee | ||
Updated•10 years ago
|
Blocks: cmas-application
Updated•10 years ago
|
Whiteboard: [CR 812387]
Updated•10 years ago
|
Whiteboard: [CR 812387] → [caf priority: p2][CR 812387]
Updated•10 years ago
|
Blocks: CAF-v2.2-metabug
Comment 5•10 years ago
|
||
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)
Comment 7•10 years ago
|
||
Thanks for taking care of this Julien, Mike, this is not a blocker yet, who is triaging this bugs?
Flags: needinfo?(francisco)
Comment 8•10 years ago
|
||
Assignee | ||
Comment 9•10 years ago
|
||
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)
Comment 10•10 years ago
|
||
(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
Comment 11•10 years ago
|
||
(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)
Comment 12•10 years ago
|
||
[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:
--- → +
Updated•10 years ago
|
blocking-b2g: 3.0? → ---
feature-b2g: --- → 3.0?
Assignee | ||
Comment 13•10 years ago
|
||
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.
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8585613 [details]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master
This looks ready to me :)
Attachment #8585613 -
Flags: review?(schung)
Comment 15•10 years ago
|
||
(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.
Assignee | ||
Comment 16•10 years ago
|
||
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.
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8585613 [details]
[gaia] julienw:1099100-dedicated-alert-tone > mozilla-b2g:master
Removing review request for now.
Attachment #8585613 -
Flags: review?(schung)
Assignee | ||
Comment 18•10 years ago
|
||
Command to simulate a CMAS Presidential Alert with the emulator:
cbs pdu C0021112011154741914AFA7C76B9058FEBEBB41E6371EA4AEB7E173D0DB5E9683E8E832881DD6E741E4F7B9D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D100
Assignee | ||
Comment 19•10 years ago
|
||
(obviously on one line)
Assignee | ||
Comment 20•10 years ago
|
||
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 :/
Assignee | ||
Comment 21•10 years ago
|
||
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.
Comment 22•10 years ago
|
||
Wesley, since the fix is already being worked on, I am requesting 2.2 again?
blocking-b2g: --- → 2.2?
Flags: needinfo?(whuang)
Comment 23•10 years ago
|
||
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.
Assignee | ||
Comment 24•10 years ago
|
||
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.
Assignee | ||
Comment 25•10 years ago
|
||
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.
Assignee | ||
Comment 26•10 years ago
|
||
Comment on attachment 8585613 [details]
[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 27•10 years ago
|
||
Comment on attachment 8585613 [details]
[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+
Comment 28•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #26)
> Comment on attachment 8585613 [details]
> [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.
Assignee | ||
Comment 29•10 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin] from comment #27)
> Comment on attachment 8585613 [details]
> [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.
Assignee | ||
Comment 30•10 years ago
|
||
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 31•10 years ago
|
||
Comment on attachment 8585613 [details]
[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 32•10 years ago
|
||
Comment on attachment 8585613 [details]
[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+
Assignee | ||
Comment 33•10 years ago
|
||
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)
Assignee | ||
Comment 34•10 years ago
|
||
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.
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 35•10 years ago
|
||
Comment on attachment 8585613 [details]
[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?
Comment 36•10 years ago
|
||
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.
Assignee | ||
Comment 37•10 years ago
|
||
Manually merged because the failures are unrelated to this patch.
master: 6935169fc9f2727c9e7eaa0560ca05737698fcb6
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
blocking-b2g: 2.2? → 2.2+
Comment 38•10 years ago
|
||
Comment on attachment 8585613 [details]
[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+
Comment 39•10 years ago
|
||
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)
Updated•10 years ago
|
Flags: needinfo?(whuang)
Updated•10 years ago
|
feature-b2g: 3.0? → ---
tracking-b2g:
+ → ---
Comment 40•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•