varamfer / openhab

Automatically exported from code.google.com/p/openhab
0 stars 0 forks source link

Support streaming mode in the REST API for item/widget status changes #62

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
To reproduce the bug, use the latest version of the Sencha Touch UI and go to 
Main Menu -> Group Demo. Click on "All off". The text label sometimes get 
updated correctly, sometimes not. Try several times until you reproduce it.

The returned JSON data from the suspended connection contains the old text 
value, but the icon value is updated correctly.

Original issue reported on code.google.com by mishoboss on 16 Jan 2012 at 8:18

GoogleCodeExporter commented 9 years ago
That is not really a bug in the REST API, but rather a conceptual problem that 
we had discussed a while ago already. When you click "All off", there are many 
change events sent on the server side - for each light that changes state and 
for the group, if at least one light was on before. There is no control of the 
order of these events.

The suspended request is now resumed upon the first change event and it will 
only see the change of this single event. While the client processes it, other 
events happen on the server (and there is no suspended connection anymore!). 
New change events are only available to the client again as soon as it does the 
next poll-request.

I do not really see a good solution to this problem. It probably can only be 
solved, if the connection is never closed, i.e. we use streaming mode. But as 
mentioned on Issue 46, I won't integrate that in 0.9.0, but keep it for the 
next release (possibly a 0.9.1 soon?).

Original comment by kai.openhab on 16 Jan 2012 at 9:40

GoogleCodeExporter commented 9 years ago
I have changed the title of this issue and converted it into a feature request. 
Let me know, if I am missing your point :-)

Original comment by kai.openhab on 16 Jan 2012 at 9:41

GoogleCodeExporter commented 9 years ago
Now I see what was your point when we discussed it. I'm not a Java developer 
and I don't know how this is realized, but is there a chance the group push to 
wait for all its group members to change their state and then make a HTTP push? 
Another solution might be to put a delay in the push event method of type 
"Group", so you ensure while this time passes, all group members are updated. I 
guess a very short delay of about 100-150ms is more than enough and it won't be 
noticed by users.

Original comment by mishoboss on 17 Jan 2012 at 8:56

GoogleCodeExporter commented 9 years ago
This delay can only be a workaround to reduce the problem impact, it is not a 
solution to the problem. Note that in real-life, the status update events can 
take quite some time, so they are probably not there after 100ms.

Nonetheless, I have now introduced a delay of 200ms, which seems to solve the 
problem at least for the demo setup. Let me know if you think that this delay 
is too big - for me it seemed acceptable so far.

Original comment by kai.openhab on 17 Jan 2012 at 8:00

GoogleCodeExporter commented 9 years ago
Yes, the delay is definitely not a bullet-proof solution.
I see you changed the method:
"public void stateChanged(Item item, State oldState, State newState)"

The item parameter contains the Item object, which contains the item type, 
right? You could make a check for the item type and do this only for group 
items. There is no sense all items are delayed, I think. 

A question here - do the Item object of type Group contains its group item 
objects or can they be retrieved somehow here? The idea is this:

1. Every item object to have a boolean variable "isDirty" for example with 
initial value FALSE.
2. When a change value is initiated isDirty becomes TRUE.
3. When the stateChanged() method of TransportListener class is called, it 
checks if the item type is Group and if it is, it continuously checks all its 
group members for the isDirty value. Once all of them are TRUE, it broadcasts 
its status and then sets them all to FALSE again.

What do you think?

Original comment by mishoboss on 18 Jan 2012 at 12:02

GoogleCodeExporter commented 9 years ago
That approach won't work. Imagine, you are on the page 
http://localhost:8080/rest/sitemaps/demo/Lights, i.e. the page that lists all 
lights (do not wonder, this is not directly accessible from the sitemap, but 
yet it exists :-))

If now sombody sends the OFF command to all Lights, you get a status update 
event for each single light plus one for the group. You have no idea in which 
order these events will arrive. So even if you receive an event for a single 
light first, you will have to delay the rest response as you will otherwise 
miss the other events.

Original comment by kai.openhab on 18 Jan 2012 at 8:49

GoogleCodeExporter commented 9 years ago
Yes, understand it theoretically. My idea is exactly to delay the group 
response, but not just at a given time, but as long as needed to obtain all its 
child statuses. To accomplish this, the group item must be aware of its child 
items and their isDirty state I mentioned in the last post. I made a simple 
draft graphic illustrating the idea with pseudo-code:
http://m-design.bg/openhab_ui/openhab_group_send.png

Original comment by mishoboss on 19 Jan 2012 at 6:25

GoogleCodeExporter commented 9 years ago
You see why I don't want to implement this? It is a very complex solution and 
quite hacky code for just a temporary (does this exist?) workaround. I prefer 
keeping it as imperfect as it is for now and then implement proper streaming to 
have a clean solution to the problem.

Original comment by kai.openhab on 20 Jan 2012 at 11:30

GoogleCodeExporter commented 9 years ago
What is this proper streaming? HTTP streaming or?

Original comment by mishoboss on 22 Jan 2012 at 9:13

GoogleCodeExporter commented 9 years ago
"X-Atmosphere-Transport: streaming"
This will leave the request open and stream data back whenever an item state 
has changed. By not closing the connection and forcing the client to 
re-connect, you can be sure not to lose any change events.

Original comment by kai.openhab on 22 Jan 2012 at 11:21

GoogleCodeExporter commented 9 years ago

Original comment by kai.openhab on 31 Jan 2012 at 11:49

GoogleCodeExporter commented 9 years ago
Ok, you mean somethink like a caching mechanism that prevents data loss during 
reconnections. Good that Atmosphere has a Broadcaster cache included, I will 
implement it soon.

Original comment by oliver.m...@gmail.com on 1 Feb 2012 at 8:42

GoogleCodeExporter commented 9 years ago
No, to my understanding, there is no caching necessary as with the header 
"X-Atmosphere-Transport: streaming", the connection will never be closed from 
the server side. So when ever events occur on the server, they can be directly 
broadcasted through the already open connection. The main thing to think about 
is the format of these updates. We will probably need a new WidgetUpdateBean or 
so...

Original comment by kai.openhab on 1 Feb 2012 at 8:47

GoogleCodeExporter commented 9 years ago
Hi Kai,
what do you think about the following idea.
We can use a HTTP Header or a QueryParameter to determine if a single item 
should be returned or the complete resource (e.g a page). We can use jersey 
sub-resources to achieve this. In case a single item should be returned openHab 
will check if the requested (subscribed) resource has any sub-resources and 
will register a change listener for each resource. Then each change event will 
be broadcasted to the clients.

To ensure that the client will receive all updates a websocket or 
HTTP-Streaming connection is necessary. If we won't drop long-polling support 
we can easily add a Broadcaster Cache. This is just a configuration entry. I 
saw in the touch-UI requests that they allready send a timestamp as a request 
parameter (?_dc=1313412421). The atmosphere HeaderBroadcasterCache requires 
this timestamp in the X-Cache-Date HTTP_header. 

Original comment by oliver.m...@gmail.com on 23 Feb 2012 at 9:01

GoogleCodeExporter commented 9 years ago
Hi Oliver,
I think we have two different discussions here:
1. Using a broadcaster cache for the existing long-polling requests. I didn't 
really know this is possible. This cache has hopefully a (short) timeout for 
the case that the client does not do any following polling request? If you can 
implement that, it would be nice as it solves the above discussed problem with 
missing updates between a response and the next request.

2. This is what I mainly had in mind in this issue - the streaming or websocket 
support, i.e. the server can send at any time updates to the client. For me it 
still makes sense to say that a client subscribes for updates of a page. If any 
item on this page then changes, the server pushes the change. So yes, I think 
we can use the same URI and decide on HTTP headers what kind of response we 
will provide - imho streaming and websocket should always do the "single-item" 
response type, while long-pull uses the current "full-page" response.
What we need to do is to define the format of the "single-item" response type; 
remember that the ItemBean is not enough here as the UI will need to update 
icon and label and the formatted state on a change. So what we actually need is 
a WidgetBean being streamed back to the client. The second problem is that the 
client needs to be able to identify this widget - which one of the page does it 
correspond to, i.e. which one needs to be updated? We currently have no id for 
widgets on a page. Maybe the simplest idea would be to number them from 1 to n 
and add an according id property to the WidgetBean. What do you think?

Original comment by kai.openhab on 24 Feb 2012 at 12:36

GoogleCodeExporter commented 9 years ago
Hi, I'm really happy to see this discussion.

Personally I imagine it something like what Kai said - the client subscribes to 
a page via HTTP streaming or websocket, then if an item changes its state and 
is a child of that page, openHAB pushes to the subscribed clients ONLY these 
WidgetBeans in which the changed item belongs to (there might be two or more 
widgets on the page representing the same item). We need some unique ID to 
identify the widget. My suggestion to this ID is to use combination of the page 
ID and incrementing number for the widget ID (for example "0003_2" represents 
widget 2 in page 0003).

Original comment by mishoboss on 24 Feb 2012 at 5:49

GoogleCodeExporter commented 9 years ago
sounds like a good solution ;)

Original comment by oliver.m...@gmail.com on 27 Feb 2012 at 8:43

GoogleCodeExporter commented 9 years ago
just i little question, how the update for the item displayed is done. the 
values are not updating at all....

Frontend: Google Chrome
Items: String

Original comment by openhab.lb on 1 Mar 2012 at 6:25

GoogleCodeExporter commented 9 years ago
Hi,
sorry for the delay, now I'm nearly finished and I will publish a new version 
in the next two days. 

@openhab.lb
are you referring to the touch web front? 

Original comment by oliver.m...@gmail.com on 13 Mar 2012 at 7:14

GoogleCodeExporter commented 9 years ago
you can find my latest changes in my cloned repo.
To enable the item "streaming" mode you have to add the HTTP Header 
"X-openHAB-SingleObject" to the request. If the header is present a WidgetBean 
will be returned.
I didn't add a new ID to that Bean yet, because I'm not really sure if this is 
neccessary. Is the item link not a suitable identifier?

One more think to note. I also added a SessionBroadcaster Cache to the 
atmosphere config. The cache feature can be used by adding the "X-Cache-Date" 
HTTP_header.
The cache should prevent the loss of messages when using long-polling.
Moreover the internal broadcaster handling has changed. In the previous version 
we had one broadcaster per request. Now we have one broadcaster per resource. 
This broadcaster handles all connection types  (long-polling, websocket) and 
responds the accurate response Object.

Original comment by oliver.m...@gmail.com on 16 Mar 2012 at 8:55

GoogleCodeExporter commented 9 years ago
Hi, Oliver. First, I want to thank you for your work! I haven't yet tested your 
code, because of lack of time. However I have some questions to be sure I get 
you right.

1. What is the full HTTP header that should be present for streaming to work? 
HTTP headers are presented as name:value pairs.

2. As far as I understand the streaming works only for Item resources, not 
Sitemap resources? Is that right? That's why you see no reason to add a unique 
ID. However for UI developers like me it is more useful to have this 
functionality on a Sitemap resource. This way we have only one request per 
page, not for example 20 requests per page, because every item needs one this 
way. One more thing to note - I believe there was a browser limitation for only 
two concurrent HTTP connections at the same time.

So, basically what I need is to subscribe for a sitemap page changes for a 
given page. Whenever an item of that page changes, the server sends back to the 
subscribed clients the WidgetBeans where this item belongs to (there might be 
more widgets on the same page representing the same item). And here we need a 
WidgetBean unique ID, so the UI will know which widgets to update with the new 
data.

I'm sorry if I didn't understood you right. As I said I didn't have time to 
test your code, but I will do it as soon as I can :) Thank you one more time!

Original comment by mishoboss on 18 Mar 2012 at 11:36

GoogleCodeExporter commented 9 years ago
Hi Oliver,
I have just briefly tested your code. Here's some initial feedback:

1.
> To enable the item "streaming" mode you have to add the 
> HTTP Header "X-openHAB-SingleObject" to the request.
I have now used:
X-Atmosphere-Transport: streaming
X-openHAB-SingleObject: true
which enables the new feature. Question: Why is the X-openHAB-SingleObject 
header necessary? Couldn't you simply do the check if the 
X-Atmosphere-Transport header is set to "streaming"?

2.
> Is the item link not a suitable identifier?
No, there might be multiple widgets on a page referring to the same item. So we 
need the id as discussed above.

3.
I noticed that the HTTP connection is closed after sending a widget bean in 
streaming mode - this should not be the case! The connection should be 
preserved and on every broadcast a new widget bean should be sent by the server.

Cheers,
Kai 

Original comment by kai.openhab on 20 Mar 2012 at 6:32

GoogleCodeExporter commented 9 years ago
Hi Kai,

1.
I don't like the idea to use the X-Atmosphere-Transport header. This will 
needlessly restrict the clients. Maybe a client is intrested in the complete 
page feed and will use streaming or websockets or a client is intrested in the 
single items and will connect with long-polling. This is not possible when we 
couple the response entity with the transport. 

2.
I will add the ID to the widget

3. 
you are right, that's a bug.

@Mihail,
the "streaming" mode is for Sitemap resources.

Original comment by oliver.m...@gmail.com on 21 Mar 2012 at 12:42

GoogleCodeExporter commented 9 years ago
1. I do not agree with that. The content of what he is interested in should be 
defined by the resource (URI) he accesses, not by any HTTP headers. For the 
sitemap-page resource, streaming only makes sense for what we specified in this 
issue. So if the X-Atmosphere-Transport is set to streaming, the result should 
be a stream of widgets.
I hence do not see a need for an additional X-openHAB-SingleObject header, I 
rather find it confusing.

Original comment by kai.openhab on 21 Mar 2012 at 1:25

GoogleCodeExporter commented 9 years ago
Thanks, Oliver, I will test it in the next few days. I'm quite busy now.
One more question - what about websockets? Are they supported and if yes, how?

Original comment by mishoboss on 22 Mar 2012 at 7:23

GoogleCodeExporter commented 9 years ago
Hi Oliver, I managed to test your work. Congrats! However there is an issue - 
it always returns the first widget on the sitemap, no matter which item has 
changed. Also, as Kai said there need to be a unique ID. This ID must be 
presented in the result of non-suspended REST connections too, so the UI can be 
build giving a unique "name" of the widgets and then referencing them by this 
name. I would like to see the same functionality with websockets too. :)

Original comment by mishoboss on 22 Mar 2012 at 2:05

GoogleCodeExporter commented 9 years ago
Hi,
I try to fix the issues tomorrow.

@Kai
I would suggest to move our debat to mail.

Original comment by oliver.m...@gmail.com on 22 Mar 2012 at 8:15

GoogleCodeExporter commented 9 years ago
One more thing, Oliver. As Kai said, the streaming must not be terminated by 
the server. Now the connection is terminated at some random timeout.

Original comment by mishoboss on 23 Mar 2012 at 9:43

GoogleCodeExporter commented 9 years ago
Hi, Oliver. I saw you have worked on these issues and I tested your code. The 
"always return 1-st widget" issue is gone. Congrats! However the connection 
continues to terminate at some timeout. You wrote that automatic reconnection 
is implemented. Isn't that a client-side feature? Please, give me some more 
info on that topic. I'm waiting impatiently for the widget IDs, so I could 
start work on integrating this new communication interface to Sencha Touch UI.

What about websockets? I tried to test them, but without success? Are they 
implemented? Please, give some more info on that too.

Regards,
Mihail

Original comment by mishoboss on 29 Mar 2012 at 9:03

GoogleCodeExporter commented 9 years ago
Hi Mihail,

that true, I worked on that. I didn't posted it here because it's not completly 
finished and tested. I'll give you a more detailed update very soon.

Original comment by oliver.m...@gmail.com on 29 Mar 2012 at 2:49

GoogleCodeExporter commented 9 years ago
Hi Mihail,
I've added the widgetId. 
I also changed the response object of the single item. Now the response is a 
page object which contains only the updated items (I'm not sure if this is the 
final solution, becuase it's not commited from Kai yet). 
Moreover the current version doesn't check if a widget contains other widgets, 
I'm not sure if this is really necessarry. If we need this I can check-in a 
recursive version.

That the connection terminates after some time is intentional. The Jetty 
Webserver is not able to detect if a connection is closed. For this reason the 
server will close all connections after some time to prevent memory leaks. 
Please note that I've raised the timeout period from one minute to five minutes 
in the current version.
Websockets should work as long as you only receive updates. Posting updates 
over websockets won't work, this behavior depends on the underlaying Jersey 
framework. I hope that a REST over Websocket API is available soon. I think 
Jeanfrancois (the Atmosphere author) is working on this.
To test Websockets you can use the REST-API test bundle 
(http://localhost:8080/resttest/index.html). If you still have problems send me 
your code and I will check. 

Original comment by oliver.m...@gmail.com on 2 Apr 2012 at 9:01

GoogleCodeExporter commented 9 years ago
Hi, Oliver. I'm unable to get response body data with the new version. Nothing 
is returned to the client. 

What I do is to send GET requests with the following headers:

Request URL:http://192.168.90.176:8080/rest/sitemaps/demo/FF_Bath
Request Method:GET
Accept:application/json
X-Atmosphere-Transport:streaming

These same requests worked OK with the previous version. Do I have to do 
something more?

Original comment by mishoboss on 3 Apr 2012 at 10:16

GoogleCodeExporter commented 9 years ago
That's strange. With "Accept:application/json" it works as expected for me. 
With "Accept:application/xml" I have the same issue. I'm sure that both worked 
in the previous versions, but when I test the last two versions the 
responseBody contains nothing until the connection is resumed. 
Seems that I have to debug a little bit deeper.

Original comment by oliver.m...@gmail.com on 3 Apr 2012 at 5:28

GoogleCodeExporter commented 9 years ago
It seems that the response for xml will be buffered when streaming is used.
If you create many many update events the buffer will be written when it is 
full. Also the buffer will be written when the server closes the connection.

P.S. I updated the REST API and the test bundle to atmosphere 0.9.0.RC3. Now 
websockets should work with the test client and Chrome.

Original comment by oliver.m...@gmail.com on 4 Apr 2012 at 7:11

GoogleCodeExporter commented 9 years ago
This does not really sound like a useful behaviour... Is there possibly any way 
to configure it? Or even better, could the server do a flush to make sure that 
the content reaches the client immediately?

Original comment by kai.openhab on 4 Apr 2012 at 7:44

GoogleCodeExporter commented 9 years ago
How do you test the client-side HTTP streaming? From my experience it's a 
little bit tricky to test it with Chrome's Advanced REST Client or similar 
developer tools. It's even trickier to see what happens with Bugzilla or 
Chrome's Developer Tools. I had to write a little JavaScript client to handle 
it correctly and to see what happens. But I'm not sure if it works correct. 
Please, tell me how you test your code, so I could go the same way.

Original comment by mishoboss on 5 Apr 2012 at 9:20

GoogleCodeExporter commented 9 years ago
My favourite debugging tool is Fiddler (http://fiddler2.com). It's a HTTP 
debugging Proxy. For testing I use the atmosphere jquery plugin in combination 
with fiddler and bugzilla. The org.openhab.io.rest.test bundle is based on an 
atmosphere sample project and uses the atmosphere jQuery plugin. You can access 
it by pointing your browser to http://localhost:8080/resttest/index.html

Original comment by oliver.m...@gmail.com on 5 Apr 2012 at 7:48

GoogleCodeExporter commented 9 years ago
I was able to get the response body now, but just once. All my other tries were 
a fail, though I used the same HTTP request. When the response body was shown 
in Fiddler it was after the 5 minute timeout has passed, not immediately. But 
I'm not sure if this is due to how Fiddler works. It may expect the request to 
finish and then show it's body.

I can't find the /resttest/index.html. I get error 404.

Original comment by mishoboss on 6 Apr 2012 at 2:35

GoogleCodeExporter commented 9 years ago
Hi Oliver. I extracted from your source the jQuery files and index.html and 
tested your code with it. It works fine, so it seems the issues I experience 
are from my code and I'm going to investigate them.

One thing though. I see you use the page title as part of the widget id 
construction. Couldn't be better to use the page id for that? For example, 
"FF_Bath", not "Bathroom"?

Thank you for your work. I'm going to implement it in the Sencha Touch UI in 
the next few days.

Original comment by mishoboss on 8 Apr 2012 at 9:05

GoogleCodeExporter commented 9 years ago
I just made Sencha Touch UI to work with streaming and websockets. If 
websockets are not supported by the browser, then it falls back to HTTP 
streaming. It works like a charm! No delay, no missing status updates, no group 
items bugs. There are few more thing to do and I will release a new version for 
beta testing in the next new days. Congratulations, Oliver!

Original comment by mishoboss on 8 Apr 2012 at 2:14

GoogleCodeExporter commented 9 years ago
The only thing that has to be changed is the use of page id, not the page title 
in the widget id construction. The page title brings a lot of pain. For example 
the widget ID "No. of active heatings [(7)]_4" is no more valid when you change 
any of the heating switches... it will go "No. of active heatings [(6)]_4" for 
example.

Original comment by mishoboss on 8 Apr 2012 at 2:32

GoogleCodeExporter commented 9 years ago
Sorry for this comment flow, but an issue is found. For some strange reason no 
status updates are received for "Widget Overview" page in the demo sitemap. I 
checked with your "rest test" demo and the issue is present there too. I reused 
most of the code from the resttest page. Seems that the method 
request.onMessage is not called when on "Widget Overview" page. Any idea about 
this issue?

Original comment by mishoboss on 8 Apr 2012 at 3:30

GoogleCodeExporter commented 9 years ago
Hi,
I've changed the widgetId. Now the id contains the pageId.
I didn't checked the other issues yet, but I will try to have a look on them 
tomorrow.

P.S. if you right click a session in fiddler you can get the current received 
buffer by clicking on COMETPeek. 

Original comment by oliver.m...@gmail.com on 10 Apr 2012 at 8:26

GoogleCodeExporter commented 9 years ago
Hi, Oliver. Thanks for the IDs. But now the websockets don't work for some 
strange reason. HTTP streaming is working well. I get this in the openHAB 
console when using websockets and send some command via REST:

16:06:37.684 WARN  o.a.cpr.DefaultBroadcaster[:514] - This message 
Entry{message=Heating_FF_Bath (Type=SwitchItem, State=OFF), 
multipleAtmoResources=null, future=org.atmosphere.cpr.BroadcasterFuture@7096dc} 
will be lost

Original comment by mishoboss on 11 Apr 2012 at 1:13

GoogleCodeExporter commented 9 years ago
P.S. The same bug started to happen with streaming too.

Original comment by mishoboss on 11 Apr 2012 at 1:49

GoogleCodeExporter commented 9 years ago
Hi, I took short a time out, but I will continue on this now.

Original comment by oliver.m...@gmail.com on 16 Apr 2012 at 2:19

GoogleCodeExporter commented 9 years ago
The response object of the single item stream has been changed. Now it is a 
flat widget list. An incremental page object is too much overhead particularly 
with regard to nested sitemap structures like the WidgetOverview page. The 
WidgetOverview page should work now. 
The "message will be lost" error appers from time to time, don't know why until 
yet. 
I think it has somthing to do with the broadcaster lifecycle. If you can 
reproduce it in a test case please let me know.

P.S. Websocket support is still buggy. Jeanfrancois from the atmosphere project 
is working on a REST over websocket implementation called Swagger Socket. I 
hope that this will make things much easier.

Original comment by oliver.m...@gmail.com on 24 Apr 2012 at 9:07

GoogleCodeExporter commented 9 years ago
Hi Oliver. Where can I download the new version? The last change in your 
repository clone is from Apr 10, 2012.

Why have you changed the data schema? It was very easy to parse the data no 
matter which communication interface was used. For example in Sencha Touch UI I 
use all the 3 interfaces - HTTP long-polling, HTTP streaming and Websockets. I 
do a smart check for compatibility and use the most suitable interface. For 
example on Android devices prior to 4.0 ICS, the only available option is 
long-polling and Websockets are available only in Chrome for ICS. No matter 
which interface was used, the returned data has the same schema.

Original comment by mishoboss on 25 Apr 2012 at 7:59

GoogleCodeExporter commented 9 years ago
P.S. Unless the HTTP long-polling schema is changed too?

Original comment by mishoboss on 25 Apr 2012 at 8:02

GoogleCodeExporter commented 9 years ago
> Why have you changed the data schema?

Possibly because I suggested it. In streaming mode, you ask for single updates 
of widgets, not for updates of pages (including there structure etc.) - so the 
response should only be about widgets. And the response should imho look the 
same if two widgets change at the same time or slightly after one another. It 
should not return one page in the first case and two pages in the second...

Original comment by kai.openhab on 25 Apr 2012 at 10:32