870756 - [SMS] No Notification sound when SMS class 0(popup) is recived

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect, P3)

Description

[Leo]

1. Title : There is no notification sound when SMS class 0(popup) is received
2. Precondition : SMS app should be working
3. Tester's Action : Receive class 0 SMS
4. Detailed Symptom (ENG.) : No notification sound for class-0 message 
5. Expected : There should be notification sound when class-0 sms is received
6.Reproducibility: Y
1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced on v1-train and Gaia master
8.Gaia Revision: 61d7ab244db3e2174b22bfa6de3e3d69136b4904
9.Personal email id: [e-mail]

Comment 1

[Gene Lian (I already quit Mozilla)]

Hi Vicamo, I don't see our RIL codes would attempt to block class-0 SMS. Is this one a potential bug?

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

ni? dcoloma,since it's likely that carriers send class 0 messages, is it a cert blocker for tef?

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Two quick questions: 1) is it commercial ril? 2) what's the TPDU of that class-0 message? Binary typed or text especially.

Comment 4

[Daniel Coloma:dcoloma]

Not a blocker for us

Comment 5

[Leo]

Hello Vicamo,

1.We are using commercial ril.
2.The TPDU for class-0 message is "text".

Thanks,
Leo

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

Then in my opinion, there is little MozRIL can do.

Comment 7

[Anshul]

Please provide the complete android logs as per instructions below. Please also provide the complete class 0 PDU for the SMS.

 adb logcat -b radio -b main -v time

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Basd on comment 4 & 6 this doesn't look like a blocker for us.

Comment 9

[Beatriz Rodríguez [:brg]]

We think this is important for 1.1 version.
Does commercial RIL need any support from other parts to have this in leo?

Can the reporter provide the log requested in comment 7 ?

Comment 10

[Anshul]

I am still waiting for logs on this issue from the reporter. Please also enable the following in

hardware/ril/libril/ril.cpp and get me the android main logs and the radio logs.

//uncomment this block to enable logging from this file.
-#if 0
#define LOG_NDEBUG 0
#define LOG_NDDEBUG 0
#define LOG_NIDEBUG 0
-#endif

Comment 11

[Leo]

Hi,

1.The class-0 messages are handled in the application side.
2.In the current scenario when we receive a class-0 messages we are not showing this message in the notification bar and we are silently receiving the messages with out sound.
3.As per our understanding this is a problem with the implementation of class-0 message

We have done some workaround for this defect in the following way:
MessageManager.deleteMessage(message.id, function() {
  app.launch();
  NotificationHelper.send(null, null, null, null);
  alert(messageBody);
  releaseWakeLock();
});

Added NotificationHelper.send(null, null, null, null) as we are not showing any notification for class-0 messages(in notification bar) we are passing with null.
In notification.js inside addNotification(),we are checking the condition if (detail.icon || detail.title || detail.text) { add the notification for the message },So that for class-0 messages as we are passing null in NotificationHelper.send() the condition of adding notification will not be executed and plays the sound.

I dont think this is a problem with either commercial ril or mozRIL.

Thanks,
Leo

Comment 12

[Beatriz Rodríguez [:brg]]

Shall we change the component affected by this bug accordingly?
Nominating to Leo

Comment 13

[Evan Tseng [:evanxd]]

Attachment: Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Steve,

Please help me review the patch.

Thanks. :)

Comment 14

[Steve Chung [:steveck]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Evan, I left a comment about the ringtone volume. I'll confirm that later, but could you also check 0 volume on your device? Thanks.

Comment 15

[Evan Tseng [:evanxd]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Steve,

I updated the code.
Please review the patch.

Thanks. :)

Comment 16

[Steve Chung [:steveck]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Evan, I update the comments, please take a look. If you are available for adding the class-0 message unit test and sound/vibrate test case, it would be great.

Comment 17

[Evan Tseng [:evanxd]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Steve,

I updated the code for your comments.
And I added two unit tests for testing the play ringtone and vibrate function.

I think we could create a new bug for adding test cases of receiving class-0 message.
How do you think?

Comment 18

[Steve Chung [:steveck]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

I left some comment about your test case, please take a look, thanks.

Comment 19

[Evan Tseng [:evanxd]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Steve,

I updated the code.
Please help me review the patch.
Thanks. :)

Comment 20

[Evan Tseng [:evanxd]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Hi Rick,

I updated the code for your comments.
Please help me review the patch.

Thanks. :)

Comment 21

[Steve Chung [:steveck]]

Hi Evan, could you please fix the conflict and the move the function to IIFE scope(Thanks Rick, it's a nice refactor step)? The pact is pretty good now and we can close this one in today.

Comment 22

[Evan Tseng [:evanxd]]

Hi Steve,

I updated the code in GitHub.
And please help me review the patch.

Thanks. :)

Comment 23

[Steve Chung [:steveck]]

Hi Even, the patch looks good now except the conflict problem. I'll r+ once you fix it, thanks.

Comment 24

[Steve Chung [:steveck]]

Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Looks good , r=me

Comment 25

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted e2f19420fa6a26c4287588701efaec09a750dba1 to:
v1-train: 231cd0d6933486aeb7eef1f2739aafd2db653eb6

Comment 26

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: 231cd0d6933486aeb7eef1f2739aafd2db653eb6