Skip to content

Conversation

@langleyd
Copy link
Member

@langleyd langleyd commented Aug 4, 2021

Old behaviour

On receiving of push notifications we do a background sync to fill in missing information in the push content.
Previously we would update the notification with a call to notify but with the sound disabled in an attempt to update the notification and avoid a second notify sound.
Depending on on the time it would take to do the background sync you would sometimes see different behaviours:

  • The first sound would play, the sync would complete after it had finished and the the push would be updated silently.
  • The first sound would start to play, the sync would complete during it playing and the, second call to notify would cut off the sound playing.
  • The sync would complete before the sound played, the second call to notify would in effect silence the sound.

New behaviour

I am removing the modification of the notification noisy property and as per this guide using the setOnlyAlertOnce property on all group notifications other than the summary notification. So that any sounds on notifications are not silenced(or double fired) by updates. Child notifications continue to be rolled up into a single summary notification here.

The behaviour also appears different across android versions. As reported in the issue it appears to affect versions 8+ but in those below it does not seem to cut off the notification on update.

I tested this on 7.0 and 8+ and is working across both.
I'll look for somebody else to test this on API < 24 as I don't have an old device and it's difficult to test push on the emulator for versions that old.
I'll also wait for this to be reviewed by @bmarty when he is available as I think he has most context on this code.

Fixes #3243

@langleyd langleyd requested a review from bmarty August 4, 2021 11:02
@langleyd langleyd self-assigned this Aug 4, 2021
@DoM1niC
Copy link

DoM1niC commented Aug 4, 2021

Ringing on incoming Call only do a simple Notification Sound? I think in v1 there was ringing ????

@LinAGKar
Copy link
Contributor

LinAGKar commented Aug 11, 2021

Ringing on incoming Call only do a simple Notification Sound? I think in v1 there was ringing ????

Something I've noticed is that if you have "Sound" set to something in the ringing notification category (which I had, although I don't remember changing it), it will just play that sound, but if you set it to none, it will play the phone's normal ringtone.

@langleyd langleyd changed the title Fix missing sounds on notifications. Fix missing sounds on room notifications (messages, invitations, etc). Aug 11, 2021
@langleyd
Copy link
Member Author

langleyd commented Aug 11, 2021

Ringing on incoming Call only do a simple Notification Sound? I think in v1 there was ringing ????

@DoM1niC @LinAGKar This PR is just related to room notifications like message notifications not having a sound (I 've updated the title to make that clearer). If you are seeing other notification issues (like calling) mind creating a new Issue with steps to reproduce?

@LinAGKar
Copy link
Contributor

LinAGKar commented Aug 11, 2021

For something actually related to this pull request, I'm wondering if setOnlyAlertOnce() also affects smart watch notifications (in which case this PR might also solve #1673/#2270), or if it just affects the alerts on-device.

@bahur142
Copy link

bahur142 commented Aug 18, 2021

Hi.
I can confirm that this PR works on devices with Android 8.1 and above. The sound is back again on Wi-Fi !
On Android devices below 8.1, the notification sound plays twice for a single message. But it is better than noting.

Edit:
There is one side effect after two-three days of testing. Half of the time, the phone did not wake-up nor shows missed call when someone calls me. I have to open the app to show the missed call.
Before to apply the PR, there was no such issue. Unfortunately I have to revert it.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@bmarty
Copy link
Member

bmarty commented Aug 23, 2021

@bahur142 thanks for your test.

There is one side effect after two-three days of testing. Half of the time, the phone did not wake-up nor shows missed call when someone calls me. I have to open the app to show the missed call.
Before to apply the PR, there was no such issue. Unfortunately I have to revert it.

Let's merge the PR and see if the regression you are describing can be reproduced by others.

@bmarty bmarty merged commit ee0b87b into develop Aug 23, 2021
@bmarty bmarty deleted the feature/dla/fix_missing_notification_noise branch August 23, 2021 13:04
@LinAGKar
Copy link
Contributor

Doesn't seem to work perfectly. Now with 1.2.2 I sometimes get double sounds, on Android 11 (which is an improvement over missed notifications, but still).

@binarydad
Copy link

I know this is an old request, but given the current state of the code, would anyone know of a (hopefully) quick way to only have the alert sound happen once? I'm trying to build an alternate version that simply plays the sound for each DM, as my kids and I use it, and they constantly miss notifications from messages.

My original post is here: #5161

I am not an Android developer, so I know very little about it (so far), but am merely trying to change the behavior of the audible alerts from just the first alert (setOnlyAlertOnce) to an alert for EVERY DM.

I'm able to get sounds for every DM currently, but as mentioned in this thread, the sounds are doubled for older devices. Would anyone have a solution? Many thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No sound notifications (noisy) when using Wi-Fi

7 participants