openhab / openhab-android

openHAB client for Android
https://play.google.com/store/apps/details?id=org.openhab.habdroid
Eclipse Public License 2.0
598 stars 316 forks source link

Push Notifications improvements for OH 4.0 #3193

Closed digitaldan closed 2 months ago

digitaldan commented 1 year ago

Hi @maniac103 and @mueller-ma, I would also like to invite @weakfl and @timbms from the IOS project https://github.com/openhab/openhab-ios/ to this discussion.

I would like to propose enhancing our push notifications across both mobile platforms. While i'm sure there is a very long list of stuff we could do, i was thinking about starting of with 2 new features and see what people think.

The first feature would be to let users add in a UI navigation target , so that when the notification is clicked on the users device, the app can open to a specific page. I was thinking about name spacing this, so you might have: sitemap:main/00000100 or webui:/page/cameras

The user would have this as an option when generating the notification from rules, for example: actions.NotificationAction.sendNotification(email, message, path);

The second feature i would propose is adding a media-url property, so users could attach an image or even a video URL to the notification. This URL would need to be accessible by the device, either a public URL, VPN or proxy, or through myopenHAB, but is a common pattern for media rich notifications. In rules this may look like: actions.NotificationAction.sendNotificationWithMedia(email, message, media); actions.NotificationAction.sendNotificationWithMedia(email, message, path, media);

I would welcome feedback on the naming conventions in the addon for these, i have not put much thought there.

Both of these would require work on the native devices to respond to these new payload fields. I am happy to work on the binding as well as myopenHAB, and can pitch in if needed on the IOS client.

wdyt?

cinemarene commented 1 year ago

Another feature I have always missed is that it would be possible to use the Android notification categories. Important events should be heard even when the phone is on silent. The category is then configured accordingly in the system settings. actions.NotificationAction.sendNotificationWithMedia(email, message, path, category, media);

maniac103 commented 1 year ago

Another feature I have always missed is that it would be possible to use the Android notification categories.

That already is supported. The Android app (ab?)uses the 'severity' field of the notification as category.

cinemarene commented 1 year ago

That already is supported. The Android app (ab?)uses the 'severity' field of the notification as category.

Good to know, I didn't realise that. I have been using OpenHAB since version 2.5. How embarrassing.

mueller-ma commented 1 year ago

I would like to propose enhancing our push notifications across both mobile platforms. While i'm sure there is a very long list of stuff we could do, i was thinking about starting of with 2 new features and see what people think.

First of all: Good to see some enhancements here 👍 I'd say that we can implement new features even if it won't be supported on both platforms.

Both new features seem useful to me. 👍

sitemap:main/00000100

These IDs will change when the Sitemap is edited. Maybe Sitemaps should be enhanced, so subpages can be tagged with user-chooses tags. These tags could be used in other places, e.g. #3140.

actions.NotificationAction.sendNotification(email, message, path);

IMO we should use a builder here. Otherwise there are too many combinations of function names and parameters.

Something like

actions.NotificationAction.sendNotification(
    actions.NotificationAction.Notification("foo@bar") // Or no mail address here to create a broadcast
        .setIsLogOnly(boolean)
        .setIcon(string)
        .setPriority(string)
        .setMedia(string)
        .setWhatEverIsAddedInTheFuture()
)

I'd add another feature as well: Notification actions (https://developer.android.com/develop/ui/views/notifications#Actions) have been requested several time, e.g. here: https://github.com/openhab/openhab-cloud/issues/160

digitaldan commented 1 year ago

These IDs will change when the Sitemap is edited. Maybe Sitemaps should be enhanced, so subpages can be tagged with user-chooses tags.

Yeah, that had crossed my mind, but i did not have a good solution for it. Tags sounds like a reasonable solution.

IMO we should use a builder here

👍

i'd add another feature as well: Notification actions

I think that sounds useful as well, and probably is not that much more work on top of rich/media notifications . This can actually be done on IOS as well i think. It would be nice to try and keep similar(ish) features if we can. The builder pattern might be useful here as well, i could see something like: .addUserAction(itemName, label, state) You could call this multiple times to add buttons as needed.

I am working right now to clean up the cloud notification architecture, and have it able to pass generic payloads so new features can be added without changing the cloud code. I'm also thinking about moving IOS to fcm in an effort to reduce complexity and have one way of doing things.

If we can agree on these features (and then prevent further scope creep), i think that sounds like a good place to start and get the ball rolling.

maniac103 commented 1 year ago

i could see something like: .addUserAction(itemName, label, state)

What might be (equally? even more?) useful is triggering rules (basically as equivalent to trigger channels). Not sure what the best way to accomplish that would be, though. A workaround to achieve that with your proposal would be a proxy item with expire profile, but that certainly is less user friendly.

cinemarene commented 1 year ago

I have one more thing that I miss. There is the possibility of withdrawing notifications. For example, you get a message about an event, and when you have done what the notification tells you to do, the message disappears without you having to take action. Of course, the system then has to know what the notification ID is that comes back from Firebase.

An example from my life: I am asked by OpenHAB to open a window because the humidity is very high. I receive the message, take note of it, go to the window and open it. The message disappears from my smartphone.

mueller-ma commented 1 year ago

The Android app (ab?)uses the 'severity' field of the notification as category.

There is the possibility of withdrawing notifications.

One solution could be to rename "severity" to "tag" or "category" and add an action that hides all notifications of one "tag"/"category".

digitaldan commented 1 year ago

Just FYI i am still working on cleaning up the server side ( running into a few fcm hiccups), once thats done, i'll circle back here to continue our discussion. Hopefully that will be soon,.

florian-h05 commented 1 year ago

@digitaldan Any news?

digitaldan commented 1 year ago

So, i ran into an issue with our legacy GCM/FCM account on Google, and paused for a while while i looked to shore up our cloud service. Thats been running nicely, so your ping is a good reminder to pick this up again. I will need to probably discuss some options with @mueller-ma and @maniac103 about our FCM account on google going forward. I'll see if i can dig into that this weekend.

florian-h05 commented 1 year ago

openHAB 4.0 is out now, so this is hpefully something for 4.1? @digitaldan Have you made any progress?

tkarle commented 10 months ago

I am also interested in this. Any news on this development?

digitaldan commented 2 months ago

So we finally have our notification system in a good state across both mobile platforms, the next release of the IOS app will have all users on Firebase which will be nice from a code maintenance pov.

@mueller-ma couple of questions as i am looking back over this:

For the navigation, for the mainui i am thinking to mirror the existing command UI functionality, https://next.openhab.org/docs/mainui/about.html#ui-command-item

For the Sitemap UI, it sounds like some progress was made to target sitemaps by id? I quickly looked over https://github.com/openhab/openhab-android/issues/3140 , but maybe you can briefly summarize what the end result of that is? Would be nice to get this into the IOS client as well for consitancy.

regarding the Notification Actions, would the "action" data (pending intent i guess in android) use the above mainui/sitemap navigation string ? Would we want to extend that to support sending commands ? so like:

command:my_item:ON

mueller-ma commented 2 months ago

I implemented the Sitemap navigation via a command item: https://next.openhab.org/docs/apps/android.html#ui-command-item It probably needs some more documentation: #3704 I will open an issue for iOS when the docs are done.

For actions it should be possible to define several actions. Each action should have a label and "data". The data should be at least:

What about my builder idea from https://github.com/openhab/openhab-android/issues/3193#issuecomment-1419123109? With that we could add functions addCommandAction(label, item, state) for better UX.

digitaldan commented 2 months ago

I like the builder pattern idea, that will probably be the next thing i look to tackle (unless someone else would like to!)

I actully have never really used "actions" on my phone before (you have to long press them) , so spent the last few days playing around with it. Whats Interesting is all of Apple's docs say you need to pre-register actions when the app is started, they don't support dynamically creating them from say the message payload data, which was a little but of a challenge. Home Assistant's IOS grabs those actions from the users server when the app launches, so they need to be predefined in a config file, and then can be managed in the IOS settings later. This seems like a less then ideal user experience. Fortunately, i was able to use a "NotificationService" extention on our app to intercept the message before the main app is called, and dynamically register actions then, this surprisingly worked!

Now that i have that working, and have actually used actions, I'm excited to see this feature.

One other thought, this extension i had to implement is commonly used to add media attachments to the message, so an image, short video etc.... Basically the payload would contain say an "attachment" key with a URL pointing to the file, we download this in the background and attach it to the message. I would love this feature as well. It looks like Android can do something similar with the NotificationCompat.BigPictureStyle option, so we could at least have cross platform compatibility to include images, which is a good start. .

This is a very long way i saying i think it would be nice to have a .setAttachmentURL(string) feature, that contains a link to some media. If the platform supports that file type, then it can display it, so if its an image, both IOS and Android would be able to use it.

digitaldan commented 2 months ago

I just had deja-vu and realized i already had this media attachment idea, and in fact its in my first post, doh ! 🤦

mueller-ma commented 2 months ago

Good to hear you found a workaround. Without UX would have been worse!

Attachments are a good idea, but on Android apps are woken up for a short time only when a notification arrives that may not be long enough to download an image. Sometimes the icons are missing for notifications, but let's see how it works.

Maybe we should create an issue template at the openhab-cloud repo to track every improvement to notifications. That should be similar to https://github.com/openhab/openhab-core/blob/main/.github/ISSUE_TEMPLATE/sitemap_change.md with a checklist for cloud, cloud binding, Android and iOS.

digitaldan commented 2 months ago

Android apps are woken up for a short time only when a notification arrives that may not be long enough to download an image.

IOS also has a similar limitation, i'm not sure if its longer or shorter then Android, but if you can't do your background work fast enough, you get a second callback saying time is up and to return what you have.

Maybe we should create an issue template at the openhab-cloud repo to track every improvement to notifications.

Sure , its probably a good common place to track things, i can set that up.

I have much of this working in a PR, https://github.com/openhab/openhab-ios/pull/764 , dynamics actions, sending commands, navigating the main ui, attaching media, and so on. I have small TODO list there i can migrate to our central one.

One thing I need to talk to @florian-h05 and @ghys about is to expose something in the MainUI to allow us to send navigation commands to the webapp. I have a quick hack on my system that works nicely, but did not give it enough thought yet regarding security or extensibility .

digitaldan commented 2 months ago

So i also have the cloud service working at this point, so progress.

I was just looking at the cloud binding code, specifically with how actions and rules are defined and exposed in openHAB. I don't think a builder pattern is going to work, actions expect to be simple functions, if they have a return value, its the result of executing the action logic, although most actions i'm looking at don't return anything. They also take simple arguments, string, numbers, etc.... There is already a lot of GUI code in the UI built around this as well (screenshots below). Parameters are optional, so we could make a single action with all options. I'll play around with it, i actually have not touched that code before, so just getting up to speed how actions are actually implemented in the core.

So just FYI these are the additional fields i have been working with in the messaging payload, naming is hard, so if there are any suggestions, i'm open to it:

attachment-url : String // URL of a image or video to attach navigate : String // uses the item command format, used if the user clicks on the notification actions : Array [ title: String, action: String ] // action buttons (long press on IOS), title of button, and then action (navigate, command)

some screen shots of how the main UI renders actions

image image
mueller-ma commented 2 months ago

attachment-url : String // URL of a image or video to attach

Maybe media-attachment-url?

navigate : String // uses the item command format, used if the user clicks on the notification

So the values here would be e.g. navigate:/location? The prefix navigate: might be redundant here as the property is only for navigations. The addon action could accept /location as well and add navigate: on the fly!?

digitaldan commented 2 months ago

The prefix navigate: might be redundant here as the property is only for navigations.

Sorry, was not very clear on that. I was intending it to start with either mainui:..... or sitemap:.... so in the case of the main ui it might be mainui:navigate:/page/kitchen or mainui:popup:camera

I'm just reading #3709 , I was actually assuming for sitemap navigation, it would use the native sitemap renderer, not the web version.

For mainui navigation I'm currently using the JS bridge to send the navigation requests to the webapp instead of controlling the native WebView, that way things like popups and popovers work as well as navigation history.

mueller-ma commented 2 months ago

The native Sitemap renderer is used. It's just the url for Basic UI for two reasons:

  1. No further prefix is required: link.startsWith("/basicui/app") is enough to know the UI that needs to be opened: https://github.com/openhab/openhab-android/blob/36736ef650c85b1422f10c747491534110b53132/mobile/src/main/java/org/openhab/habdroid/ui/MainActivity.kt#L812
  2. IMO it's better UX as the user only has to copy+paste the URL from the browser to the rule that sends the command.

It has one downside: The command item listener only works when a Sitemap is open. As soon as Main UI gets opened, any new command isn't handled. The user needs to configure the same Item in Main UI as well. Maybe we need to extend the JS interface, so Main UI can query the command Item from the app.

digitaldan commented 2 months ago

@mueller-ma i have setup our todo tracker here https://github.com/openhab/openhab-addons/issues/16934 , lets move the conversation to it. Also i propose using ui over navigate for UI related commands, and took your suggestions for on-click and media-attachment-url