Closed Bug 870756 Opened 7 years ago Closed 7 years ago

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

Categories

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

ARM
Gonk (Firefox OS)

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

RESOLVED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: evanxd)

Details

(Whiteboard: [TD-26579])

Attachments

(1 file)

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: sasikala.paruchuri8@gmail.com
Whiteboard: [TD-26579]
Hi Vicamo, I don't see our RIL codes would attempt to block class-0 SMS. Is this one a potential bug?
tracking-b2g18: --- → ?
Target Milestone: --- → 1.1 QE2
blocking-b2g: --- → leo?
Priority: -- → P3
Whiteboard: [TD-26579] → [TD-26579], u=fx-os-user c=may-20-31 p=1
Whiteboard: [TD-26579], u=fx-os-user c=may-20-31 p=1 → [TD-26579], u=fx-os-user c=scravag-sprint-may-20-31 p=1
ni? dcoloma,since it's likely that carriers send class 0 messages, is it a cert blocker for tef?
tracking-b2g18: ? → ---
Flags: needinfo?(dcoloma)
Two quick questions: 1) is it commercial ril? 2) what's the TPDU of that class-0 message? Binary typed or text especially.
Flags: needinfo?(leo.bugzilla.gaia)
Component: Gaia::SMS → DOM: Device Interfaces
Product: Boot2Gecko → Core
Not a blocker for us
Flags: needinfo?(dcoloma)
Hello Vicamo,

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

Thanks,
Leo
Flags: needinfo?(leo.bugzilla.gaia)
Then in my opinion, there is little MozRIL can do.
Assignee: nobody → mvines
Whiteboard: [TD-26579], u=fx-os-user c=scravag-sprint-may-20-31 p=1 → [TD-26579]
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
Assignee: mvines → anshulj
Basd on comment 4 & 6 this doesn't look like a blocker for us.
blocking-b2g: leo? → -
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 ?
Flags: needinfo?(leo.bugzilla.gaia)
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
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
Flags: needinfo?(leo.bugzilla.gaia)
Shall we change the component affected by this bug accordingly?
Nominating to Leo
blocking-b2g: - → leo?
Component: DOM: Device Interfaces → Gaia::SMS
Product: Core → Boot2Gecko
blocking-b2g: leo? → leo+
Assignee: anshulj → nobody
Assignee: nobody → evanxd
Hi Steve,

Please help me review the patch.

Thanks. :)
Attachment #757305 - Flags: review?(schung)
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.
Attachment #757305 - Flags: review?(schung)
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. :)
Attachment #757305 - Flags: review?(schung)
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.
Attachment #757305 - Flags: review?(schung)
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?
Attachment #757305 - Flags: review?(schung)
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.
Attachment #757305 - Flags: review?(schung)
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. :)
Attachment #757305 - Flags: review?(schung)
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. :)
Attachment #757305 - Flags: review?(waldron.rick)
Target Milestone: 1.1 QE2 (6jun) → 1.1 QE3 (24jun)
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.
Hi Steve,

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

Thanks. :)
Hi Even, the patch looks good now except the conflict problem. I'll r+ once you fix it, thanks.
Comment on attachment 757305 [details]
Point to GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10157

Looks good , r=me
Attachment #757305 - Flags: review?(schung) → review+
Attachment #757305 - Flags: review?(waldron.rick)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Uplifted e2f19420fa6a26c4287588701efaec09a750dba1 to:
v1-train: 231cd0d6933486aeb7eef1f2739aafd2db653eb6
1.1hd: 231cd0d6933486aeb7eef1f2739aafd2db653eb6
You need to log in before you can comment on or make changes to this bug.