Refresh using Server Sent Events

[lolodomo]

I would like to discuss about the move to SSE to refresh the app.

Unfortunately, SSE client is not part of the Android SDK ? So should we use an existing library or try to implement a SSE client in our app ? What do you thing about this library: https://github.com/tylerjroach/eventsource-android ? It is apparetnly not updated since a long time.

[lolodomo]

I found oksse https://github.com/heremaps/oksse which is an extension to OkHttp. The license is Apache 2.0. Is it compatible with our own license ?

[lolodomo]

Maybe an option could be a Webview using EventSource ? I am not source if the data received by the Webview could then be handled by the app.

[lolodomo]

After studying the way the long polling is implemented, I understand why some updated events can be lost. The app is not always listening for changes, when handlimg a previous change.

[digitaldan]

Yes, the current impl of long polling is not good. The apache license is fine for us, I would strongly encourage using an existing framework. I have not played with either, but https://github.com/tylerjroach/eventsource-android was updated in November. Would LOVE to see SSE in the app!

[lolodomo]

I would like to use oksse as it is an extension of okhttp and we are already relying on okhttp in the app. I will cut the work in 2 steps:

1. first step: subscribing to SSE to be notified of all changes in the current page
2. second step: update the UI according to received SSE messages

First step is relatively clear for me. Second step is not as clear for me, I will have to study how to trigger in the app the refresh of only a particular widget.

[lolodomo]

@kaikreuzer @resetnow : I have a question for you about sitemap SSE. In the Basic UI, it looks like we are handling a unique SSE subscription and when the user changes page, the subscription is updated to be linked to the new page. The Android app can display (in landcape mode) two levels of page, the parent page at left and the child page at right. Does it mean I need to manage two different sitemap SSE subscriptions ?

[vlad-ivanov-name]

From the current architecture I would think you need two of them. One of alternate solutions is to extend SSE events with ability to track nested pages but I think it's too complex.

[lolodomo]

Ok. What is the result of calling GET rest/sitemaps/events/{subscriptionid} without setting the parameters sitemap and page (they are optional) ?

[lolodomo]

@digitaldan @mueller-ma : the refresh granularity of the app is the (full) fragment and not the widget in a fragment. Can you confirm that it is something we cannot change without rewritting the app ?

@kaikreuzer @resetnow : can you explain how to release a subscription id ? Is it a problem (for the server) to request several subscription id ? The easiest solution in the Android app would be to have one subscription id for each fragment (which means one subscrition id for each tree level in the sitemap).

[vlad-ivanov-name]

how to release a subscription id ?

They are garbage-collected after a certain timeout

Is it a problem (for the server)

Not really, it's just like if two people opened BasicUI on two devices

[digitaldan]

Can you confirm that it is something we cannot change without rewritting the app ?

I don't think we need to rewrite the whole thing... but I have not given it the proper time to come up with a high level proposal .

[lolodomo]

I will trigger as it is now a refresh of the page/fragment as soon as an update event is received but that is not an optimized solution. Imagine you have an image in your page, it will be reloaded each time another field in the page is changed.

[lolodomo]

I was finally able to establish SSE connections using OkSse. I am now waiting for this fix on the server side: https://github.com/eclipse/smarthome/pull/4810 Then the last step should be not too much difficult.

[lolodomo]

@resetnow I encounter a new major problem: the widget ids returned by the sitemap REST API and the ones provided by the sitemap SSE events are different !

Basic UI servlet has an access to itemUIRegistry to create and store these widget ids used later by the sitemap SSE. But a standard UI using the REST sitemap API will not get these ids. Of course the app could try to recreate them the same way as in the itemUIRegistry but in this case we have another problem: the sitemap REST API doesn't return the widgets that are currently hidden.

I don't find a valid solution with what we have now !

Why not using in the sitemap REST API the widget ids from the itemUIRegistry (and so the same as in the sitemap SSE) ? This should make things terribly easier ? WDYT ?

[vlad-ivanov-name]

I have seen similar problem before, but with basicui — there was an edge case where widget ID generated with itemUiRegistry was lost. Yes, it's a valid concern and I think a unified solution would serve better here, but I would say it's not up to me — we need to hear more opinions.

[lolodomo]

I opened a new issue in the ESH framework: https://github.com/eclipse/smarthome/issues/4819

[lolodomo]

I am happy: using the fix for the widget ids ( https://github.com/eclipse/smarthome/pull/4820 ), I was finally able to have something working in the app (refreshing the pages from SSE messages). I am now first waiting for the merge of my 2 PR in the ESH framework. In parallel, I will improve my code to handle limit cases like for example when a subscription id is no more available. And of course, I will test more in deepth.

[lolodomo]

The PR https://github.com/eclipse/smarthome/pull/4820 is now merged and so the problem with widget ID will be solved. I am still waiting for the merge of the PR https://github.com/eclipse/smarthome/pull/4810 Then we will be able to move to SSE in the Android app. I will have to find a way to enable SSE only when connecting to an OH server including these 2 fixes.

[lolodomo]

I have a last thing I would like to improve. My current behaviour when handling a SSE message is first update the widget in widgetList and then call notifyDataSetChanged to update the UI. In case several displayed data are updated in a very short time as for example when a weather binding polls for new data, the Android app will receive several SSE messages, one for each updated item. I would like to call notifyDataSetChanged only one time. So I need to schedule a delayed call to notifyDataSetChanged by for example 50 ms and reschedule it in case I received a new SSE message in the 50 ms frame. @mueller-ma : what would be the right pattern to be used in Android ? What ssheduler ?

[maniac103]

@lolodomo Right pattern would be using android.os.Handler

[lolodomo]

My required server fixes are now all merged and available in the recent openHAB snapshots. So I will be able to restart testing and finish my PR this weekend.

But as there are a lot of PRs that refactor a lot of code and as my PR was developed on a branch from mid-December, I should probably wait for some merge of these PRs and then rebase my own PR and test again.

[lolodomo]

The PRs I should probably wait for a merge are: 556 562 638 640 and 650.

[maniac103]

@lolodomo Once #762 is merged, this diff should get you started :-) (it's what you'll need to call from the onMessage callback in oksse)

diff --git a/mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java b/mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java
index b07f2643..4e814081 100644
--- a/mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java
+++ b/mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java
@@ -271,6 +271,14 @@ public abstract class OpenHABWidget implements Parcelable {
         }
     }

+    public static OpenHABWidget updateFromEvent(OpenHABWidget source, JSONObject eventPayload)
+            throws JSONException {
+        return source.toBuilder()
+                .label(eventPayload.optString("label", source.label()))
+                .item(OpenHABItem.fromJson(eventPayload.getJSONObject("item")))
+                .build();
+    }
+
     private static String determineOH2IconPath(OpenHABItem item, Type type, String icon,
             String iconFormat, boolean hasMappings) {
         String itemState = item != null ? item.state() : null;
diff --git a/mobile/src/main/java/org/openhab/habdroid/ui/activity/PageConnectionHolderFragment.java b/mobile/src/main/java/org/openhab/habdroid/ui/activity/PageConnectionHolderFragment.java
index 919fd2bc..a55cfdde 100644
--- a/mobile/src/main/java/org/openhab/habdroid/ui/activity/PageConnectionHolderFragment.java
+++ b/mobile/src/main/java/org/openhab/habdroid/ui/activity/PageConnectionHolderFragment.java
@@ -344,5 +344,25 @@ public class PageConnectionHolderFragment extends Fragment {
                 return false;
             }
         }
+
+        private void updateWidgetFromSseEvent(String payload) {
+            if (mLastWidgetList == null) {
+                return;
+            }
+            try {
+                JSONObject object = new JSONObject(payload);
+                String widgetId = object.getString("widgetId");
+                for (int i = 0; i < mLastWidgetList.size(); i++) {
+                    OpenHABWidget widget = mLastWidgetList.get(i);
+                    if (widgetId.equals(widget.id())) {
+                        OpenHABWidget updatedWidget = OpenHABWidget.updateFromEvent(widget, object);
+                        mLastWidgetList.set(i, updatedWidget);
+                        mCallback.onWidgetUpdated(updatedWidget);
+                    }
+                }
+            } catch (JSONException e) {
+                Log.w(TAG, "Could not parse SSE event ('" + payload + "')", e);
+            }
+        }
     }
 }

[maniac103]

Implemented this in my development branch in case anybody wants to try: https://github.com/maniac103/openhab.android/commit/a4d9cdaee02c2af23c79c77de8d249e0ac8c4a78 https://github.com/maniac103/openhab.android/tree/develop

Will submit a PR for it once the prerequisite patches are merged.