Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow TTS notifications to also show as normal notifications #3947

Open
Suxsem opened this issue Oct 17, 2023 · 8 comments
Open

Allow TTS notifications to also show as normal notifications #3947

Suxsem opened this issue Oct 17, 2023 · 8 comments
Labels
enhancement New feature or request notifications

Comments

@Suxsem
Copy link
Contributor

Suxsem commented Oct 17, 2023

Is your feature request related to a problem? Please describe.
When using Text-to-speech (TTS) notifications, only the voice is played, no actual notification is displayed

Describe the solution you'd like
I would like an option to also use the normal behavior (with all the nice features we have, such as url, image, ecc...) with TTS notifications.
Ideally, the app should not check for the message being TTS. If the tts_text data attribute is set, just speak the given message. To still allow voice-only notifications a new message HIDDEN (or something like that) could be used.

Describe alternatives you've considered, if any
Use two notifications, one for TTS and one for text. But this would mean two notifications used against the 500 rate limit and general greater general overhead for the server and the phone.

Additional context
I have tested on Android

@Suxsem Suxsem added the enhancement New feature or request label Oct 17, 2023
@dshokouhi
Copy link
Member

Ideally, the app should not check for the message being TTS. If the tts_text data attribute is set, just speak the given message. To still allow voice-only notifications a new message HIDDEN (or something like that) could be used.

Lets not make this a breaking change. Might be best to introduce a new parameter that gets checked while TTS is being processed to also post the notification. I am not sure how well this is going to work when combined with alarm stream modifications though so special considerations need to be done there. In fact the notification ping may not happen at all if the device is speaking.

@Suxsem
Copy link
Contributor Author

Suxsem commented Oct 17, 2023

I understand your concern about the breaking change. I think it would be cleaner to use the parameter to avoid the notification but if well documented using the parameter to post the notification could also be fine.
About the notification sound (ping), I think it's ok to not have the notification sound if the device is speaking: just the speaking alone would be a "sound" indication that a new notification arrived.

In fact, the whole point is to avoid loosing the notification if away from the phone. Immagine being away while a new TTS "House on fire" notification arrives. When checking the phone after being away there is no indication that something happened. This is a serious issue in my opinion. What do you think?

@dshokouhi
Copy link
Member

In fact, the whole point is to avoid loosing the notification if away from the phone. Immagine being away while a new TTS "House on fire" notification arrives. When checking the phone after being away there is no indication that something happened. This is a serious issue in my opinion. What do you think?

While I agree it can be serious keep 1 thing in mind. Its up to the user to decide to how they want to be notified of things and even more so on the user to properly test these things to make sure it works to their own expectations. Users are not stuck currently as they can send a separate notification or even use persistent notification to get around the rate limit issue. In fact personally speaking anything as serious as your scenario I would highly recommend using 2 different notification services. I personally do that with my alarm send one to the phone and one to discord as a backup measure. If your phone is off you may not receive the notification from the app, but other services like discord will keep that notification for you.

@Suxsem
Copy link
Contributor Author

Suxsem commented Oct 17, 2023

Sure, this is a great advice! Still, the focus here is on native (FCM in case of Android) notifications, I think they could be greatly improved by a (relatively) simple change and in fact, this could be the only way to prevent the two hits to the rate limit without using other notification services or using a persistent connection that unfortunately greatly impact the power consumption of the companion app. So I still think this worth the effort.

@masi
Copy link

masi commented Jan 20, 2024

One of the biggest issue IMHO of native notifications is that the app has no proper backlog of notifications. Critical notifications should clearly visible within the app even the user has dismissed the native Android notification.

As @Suxsem writes "house on fire" is very important.

Perhaps critical notifications can get an extra allowance after the rate limit has been reached. Then you don't have to worry if you send one or more critical notifications.

@dshokouhi
Copy link
Member

One of the biggest issue IMHO of native notifications is that the app has no proper backlog of notifications.

Settings > companion app > notification history.

@masi
Copy link

masi commented Jan 21, 2024

One of the biggest issue IMHO of native notifications is that the app has no proper backlog of notifications.

Settings > companion app > notification history.

Thank you.

Sorry to say, but with proper I meant a backlog that is

  • not limited to 25 (too few, needs IMHO to be higher or configurable)
  • is not "hidden" in a very long configuration page (IMHO cramping all app related infos on one page does not make a good user experience)
  • highlights critical notifications

IMHO the last critical notifications should be prominent within the app even when the user has dismissed the native notification.

I can cope with the history, but I'm sure my wife won't be able to that. If she dismisses "house on fire" accidentally without reading the notification will be lost for her.

@dshokouhi
Copy link
Member

  • not limited to 25 (too few, needs IMHO to be higher or configurable)

There is a drop down to change it to 50 and 100 and also a search bar. History is kept indefinitely, we just dont show it all by default

  • is not "hidden" in a very long configuration page (IMHO cramping all app related infos on one page does not make a good user experience)

There is an existing feature request, lets not combine multiple issues into one issue

  • highlights critical notifications

use search

IMHO the last critical notifications should be prominent within the app even when the user has dismissed the native notification.

make the notification non-dismissable, track the notification being cleared. Use exsiting app features to do what you want. Not every user wants the same thing. There are features you can use to do what you want

None of what you are mentioning are part of this issue, my comment was to merely tell you that you did not explore the app or its features long enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request notifications
Projects
None yet
Development

No branches or pull requests

3 participants