InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.7k stars 923 forks source link

FR: Improved notification category support #337

Closed pfeerick closed 3 years ago

pfeerick commented 3 years ago

GATT allows for multiple categories of notifications, but currently InfiniTime only specifically considers active (phone) calls to be different to a simple alerts.

This feature request is for support to be added for as many of these as seems warranted - I don't see any reason 0 - 9 could not be fully implemented, with the main difference being an icon that would be part of a future resource library shown to make each notification unique and make it easier to identify the source of the message... i.e. I just got an app notification and an email, and couldn't easily identify the source of the notification. Some notification categories may also warrant different notification screen layouts... but that is wandering into the territory of a complete re-write.

Description Value
Simple Alert 0
Email 1
News 2
Call 3
Missed Call 4
SMS/MMS 5
Voice Mail 6
Schedule 7
High Prioritized Alert 8
Instant Message 9
Reserved for Future Use 10–250
Defined by Service Specification 251–255
Avamander commented 3 years ago

Mi Bands and Pebbles use a similar scheme but the end result is that quite often there's an icon missing that people would like for their apps. Thus companion apps have to deal with the app->icon mapping constantly and often use the wrong icons that approximately match, which is relatively annoying and sometimes confusing for the user.

I would propose that first, we avoid any overlapping generic categories. Have non-overlapping categories and icons for (instant) message, e-mail, call, calendar and blank/unknown.

Secondly, the rest of the icons could be loaded from the filesystem based on some kind of icon UID that can be generated from the applications name (maybe md5(app_name.lower())[:4] or similar?).

This would:

Downsides:

pfeerick commented 3 years ago

That's why I am only asking for the GATT Bluetooth specification category icons... not for anything fancy like application specific ones (requiring non-standard companion support to operate). Especially as that goes into the realms of discussion stuff that isn't remotely possible yet, such as filesystems and user configurable icons... that would be a natural extension way down the track.

Much better to have smaller, simpler issues, that can easily be resolved. Otherwise we get hundreds of issues that are that complex that only JF is the one who can do anything about them, rather than be a PR other people can pick up and implement.

0 and 3 are already supported in some fashion... it's the other eight categories that need supporting in some way... I'm proposing that in the first instance it simply be a small category icon, before heading into separate screen layouts as appropriate for each.

This is the purpose of the dropdown under 'message/caller' on the gadgetbridge debug screen, isn't it... to set app icons? InfiniTime is nowhere near ready for that level of fancyness... ;)

Avamander commented 3 years ago

That's why I am only asking for the GATT Bluetooth specification category icons...

Which part of the specification?

not for anything fancy like application specific ones (requiring non-standard companion support to operate).

I haven't seen app notification support that doesn't require a specific implementation companion-side.

Much better to have smaller, simpler issues, that can easily be resolved.

I have to disagree, this inherently isn't a simple topic if the goal is to do it well. Half-baked implementations and backwards compatibility are really no fun.

pfeerick commented 3 years ago

That would be the Alerts Categories, which is used by ANY and ALL companion apps that send notifications, including Gadgetbridge.

https://www.bluetooth.com/specifications/specs/gatt-specification-supplement-4/

3.142 New Alert, octet 1 is the category. The categories are specified at 3.2 Alert Category ID.

I have to disagree, this inherently isn't a simple topic

No, it is an inherently simple topic if you don't add irrelevant noise like app icons to the mix. As I said right from the start, InfiniTime already supports two of the categories, I am only proposing that the remainder be supported also. You are the one who then added the complexity of application icons, user configuration-able icons, UUIDs, etc. All of which which have absolutely nothing to do with the GATT New Alert Categories. 🤦

I'm not saying I don't agree that those would be nice, but that belongs in a separate issue, specifically asking for app icon support.

Avamander commented 3 years ago

No, it is an inherently simple topic if you don't add irrelevant noise like app icons to the mix. All of which which have absolutely nothing to do with the GATT New Alert Categories. facepalm

"It's a simple topic if we totally ignore what people want", I guess then yeah. It's the thing you're missing a bit, the specification is at best a suggestion that should be followed if there isn't anything better, it's not something that must be followed.

Nothing very smartwatch-y be achieved by following it strictly, it's just too narrow and doesn't represent all real use cases. I think this has been demonstrated multiple times already, with services InfiniTime already has.

Which is used by ANY and ALL companion apps that send notifications

To an extent and it's still a custom implementation everywhere for InfiniTime.

in a separate issue, specifically asking for app icon support.

That's the thing, there's no point in implementing what the people at Bluetooth SIG imagined notifications would be like when you're soon going to redesign the entire thing because it's simply shit in so many ways. Sorry for the harsh opinion, but it simply isn't good enough to be the ground truth. I have not even mentioned notification dismissal and updates... which are also something we will need, if not now, in the future, it's simply not avoidable if good user experience is a goal.

pfeerick commented 3 years ago

There's not much point deleting your edits since the email update shows what you wrote. 😆

"It's a simple topic if we totally ignore what people want", I guess then yeah. It's the thing you're missing a bit, the specification is at best a suggestion that should be followed if there isn't anything better, it's not something that must be followed.

No, the thing you don't get is that you either follow it or you don't... you don't go half-assed about it, else you cause problems because it isn't compliant with the standard it claims to implement.

To an extent and it's still a custom implementation everywhere for InfiniTime.

No, and yes. The GATT New Alert specification for this is being followed perfectly... if I open up NRFConnect, and send a new alert... shock horror, it works... who would have thunk it... following the rules would work! What has required custom implementation is InfiniTime hasn't taken into when deciding to split the string into a title and a message strings that don't have a title and effectively chopping the first character of the message off. This is possibly where using one of the reserved for custom implementation categories could have been better, and defining a spec for the string. That or simply first checking for that custom separator, and using it if present, or displaying the string as is, thus preserving 100% compatibility out of the box for all companion apps.

That's the thing, there's no point in implementing what the people at Bluetooth SIG imagined notifications would be like when you're soon going to redesign the entire thing because it's simply shit in so many ways. Sorry for the harsh opinion, but it simply isn't good enough to be the ground truth. I have not even mentioned notification dismissal and updates... which are also something we will need, if not now, in the future, it's simply not avoidable if good user experience is a goal.

On this I entirely agree. Don't say it's harsh, say it's honest opinion. Remove the emotion from it, to avoid it sounding like your Moses on the hill :-P

Keep in mind it would require creating yet another custom service and accompanying characteristics, defining all the fields, keeping in mind what access companion apps on iOS, Android, Linux, MacOS and Windows will have to do things like dismiss/delete notifications, and if they are even willing to add support for the one oddball watch. Then you might reconsider redesigning it was the shit idea, because it's simply unobtainable. If you want to support just one companion app, with one niche OS, sure, there will be a way to do it... just give your companion app admin powers... but that's not real world, now is it? We're not Apple, where you can control the whole ecosystem and in doing so write your own standards.

Lets say for the sake of discussion it was just Linux and Android (via GadgetBridge) ... as those are the more achievable goals... on Linux, you can tap into dbus, so can probably do whatever you need to with notifications (but I really haven't dug into that yet). What is the situation with GadgetBridge... it has permissions to see all notifications... can it dismiss them? Is there a framework in place already by which connected devices can dismiss a notification? And if GadgetBridge can dismiss notifications, what is the liklehood of it being yanked from it in the near future?

Avamander commented 3 years ago

No, the thing you don't get is that you either follow it or you don't...

Claiming to fully compliant has never been the goal.

else you cause problems because it isn't compliant with the standard it claims to implement.

To whom? :smile:

What has required custom implementation is ...

"Oh no, noncompliance, don't implement!"

if I open up NRFConnect, and send a new alert... shock horror, it works... who would have thunk it... following the rules would work!

Quoting myself from earlier, it works if we totally ignore what people want in the end.

thus preserving 100% compatibility out of the box for all companion apps.

InfiniTime exposes a version string, any companions that don't employ defensive coding mechanisms and misbehave, that's only self-inflicted pain. An ossified protocols is an anti-goal.

if they are even willing to add support for the one oddball watch.

Avoiding features for the sake of not having to write any companion code will end up with the watch being featureless.

If you want to support just one companion app, with one niche OS, sure

Limitations of some specific OSs are none of our concern. If $OS can't do "proper notifications", erm, so what? In the end you're going to have features that are inaccessible to some, that's usually also a choice they made. In the end majority of the userbase has an OS that can utilize the feature and that is what matters.

We also aren't going to remove the navigation app because there isn't yet a companion for each and every map app out there, these things follow not predate.

(Also keep in mind that no OS is any smarter with a fully standards-compliant services and device. The absolute best they can do is display battery status any maybe do mouse-keyboard without custom software. That is not the feature set we want to limit ourselves to.)

Can it dismiss them? Is there a framework in place already by which connected devices can dismiss a notification? And if GadgetBridge can dismiss notifications, what is the liklehood of it being yanked from it in the near future?

Irrelevant but yes, some devices have more features than the PineTime, thus a lot of features exist in GB that could be adapted for our use.

TL;DR: Implementing these categories is a waste of time and the service has to be refactored anyways.

pfeerick commented 3 years ago

TL;DR I see from this discussion there is clearly no point pursuing this, as any desire to trivially fix any incompatibilities or finish implementing standards are going to be met with ridicule and sarcasm. I see no reason at this point in time to continue to contribute to this project, to continue working on the companion app I was working on, or to financially support it.

No, the thing you don't get is that you either follow it or you don't...

Claiming to fully compliant has never been the goal.

So you're claiming you know what the goal is, and that there is only one?

else you cause problems because it isn't compliant with the standard it claims to implement.

To whom? 😄

To the companion app authors who you are asking support you in order for the firmware to be more functional. If you want to write your own companion app, that's fine. Add to your workload if you wish.

What has required custom implementation is ...

"Oh no, noncompliance, don't implement!"

Sarcasm is not helping your argument. It's actually quite amusing you're trying to be sarcastic about a compatibility issue, given your prior comments of backward compatibility... this is exactly what wasn't considered here, and is quite simple to fix.

if I open up NRFConnect, and send a new alert... shock horror, it works... who would have thunk it... following the rules would work!

Quoting myself from earlier, it works if we totally ignore what people want in the end.

No, it does not. You are clearly not reading what I wrote about implementing the alerts as intended, and using the custom categories which were reserved of exactly this purpose in the specification.

thus preserving 100% compatibility out of the box for all companion apps.

InfiniTime exposes a version string, any companions that don't employ defensive coding mechanisms and misbehave, that's only self-inflicted pain. An ossified protocols is an anti-goal.

Defensive, eh? Interesting choice of word. And heaven forbid we have documented APIs and protocols which we stick to... I don't know how OSS software got to this point without those!

if they are even willing to add support for the one oddball watch.

Avoiding features for the sake of not having to write any companion code will end up with the watch being featureless.

Nothing was said about avoiding features, if you actually read what was said, it was that features should be implemented in accordance with the guidelines and standards - there are mechanisms in place to allow for expansion beyond the template spec provides - use them! It's documented for a reason!

If you want to support just one companion app, with one niche OS, sure

Limitations of some specific OSs are none of our concern. If $OS can't do "proper notifications", erm, so what? In the end you're going to have features that are inaccessible to some, that's usually also a choice they made. In the end majority of the userbase has an OS that can utilize the feature and that is what matters.

We also aren't going to remove the navigation app because there isn't yet a companion for each and every map app out there, these things follow not predate.

You're bringing up removal of features not me. You also exemplify my point once again, as a custom service and characteristics were implemented, which the GATT specification specifically allows for and expects .

Can it dismiss them? Is there a framework in place already by which connected devices can dismiss a notification? And if GadgetBridge can dismiss notifications, what is the liklehood of it being yanked from it in the near future?

Irrelevant but yes, some devices have more features than the PineTime, thus a lot of features exist in GB that could be adapted for our use.

TL;DR: Implementing these categories is a waste of time and the service has to be refactored anyways.

No, it is not irrelevant... only a moron would implement client-side deletion of notifications if it's not possible for any companion app to do so! Before any feature such as that (which I will remind you is nothing to do with this issue/FR, but you brought it up) is implemented, you need to be realistic as to is the time spend on implementing worth it if it can't be used! You survey all the OSes that you wish to target, and see what level of support they provide. Then you make an informed decision. This is why I asked if a) GadgetBridge supported it, as Android will be the biggest and most easily supportable phone OS, with Linux/Pinephone being the other achievable goal and b) since Google/Android is tightening down on privacy, is there anything to suggest that dismissal of notifications will be removed as a capability, to prevent bad actors from dismissing them. As if that is the case, there goes the biggest and main reason to implement such a feature.

Avamander commented 3 years ago

So you're claiming you know what the goal is, and that there is only one? And heaven forbid we have documented APIs and protocols which we stick to... I don't know how OSS software got to this point without those! No, it is not irrelevant... only a moron would implement

Let's recap:

Anything you specifically want to refute?

I see no reason at this point in time to [...]

Such a reaction because of what, Bluetooth GATT specification? Just eesh.