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

ISY online/offline status should be maintained and reported #62

Open
mjwesterhof opened this issue Jun 4, 2016 · 13 comments
Open

ISY online/offline status should be maintained and reported #62

mjwesterhof opened this issue Jun 4, 2016 · 13 comments
Assignees

Comments

@mjwesterhof
Copy link
Collaborator

It would be useful if the lowest-level Polyglot-to-ISY layer would maintain a status flag indicating if the ISY is reachable and responsive, or not. Changes to this status should be logged.

This flag can be used to suppress excessive logging, at the very least. It may also be very useful to allow Polyglot to resync certain ISY information (such as the ISY version information (perhaps the ISY was just upgraded), or the node list (perhaps the ISY had a backup restored), as well as alert the message-handling mechanisms in Polyglot, or even alert node servers.

@mjwesterhof mjwesterhof self-assigned this Jun 4, 2016
@Einstein42
Copy link
Contributor

This is the same as #29, we will leave this and close that one.

@Einstein42
Copy link
Contributor

This is becoming a UDI driven issue to resolve. If the ISY is rebooted, Polyglot should recover gracefully. I will start looking into this today.

@Einstein42 Einstein42 self-assigned this Jul 2, 2016
@Einstein42
Copy link
Contributor

So currently the set_driver method in the nodeserver_api only reports to the ISY when a value is changed. Therefore when the ISY reboots, it loses all its values and shows blank fields. These fields remain blank, even on a query because the values are still the same from the Polyglot point of view therefore not reported to the ISY. Two options then, find a way to 'ping' or better yet, poll the ISY to determine if it is up every few seconds, or have the nodeservers automatically send ALL reports every 60 seconds or so.

Any opinions?

@mjwesterhof
Copy link
Collaborator Author

Glad you're asking about this - a discussion will certainly help. I've been messing about with this issue as well as a number of others that are closely related, and have a bunch of prototype code that isn't yet ready to commit. Rather like Thomas Edison, I don't yet know how to solve this, but I now know a whole LOT of ways to NOT solve this!

I do have code that "pings" the ISY. It's not quite correct (it pings unnecessarily, and thus adds to the problems when the ISY is under load). We can add a heuristic to determine the "up/down"-ness of the ISY; the details we can work out later (i.e. how many timeouts/errors in a row before we call it down, and how many sucesses in a row before we call it up again).

There is a latent issue with the set_driver method in that it assumes success, and thus if the status update to the ISY doesn't go through, then the set_driver method will never try again to update. This also can be fixed - and will ensure that status updates that fail while the ISY will be retried each time the Node's code does a status update. If a query doesn't resend the values, that's a bug -- if explicitly queried, the set_driver method should still send that along - does it not do that, then? (I'll have to look, I was hacking in that area as well, I wonder if I fixed it in my version here, or broke it earlier in something I committed?!)

Rebooting of the ISY is something distinct from a transient network issue. I've not found a good way via REST to get this information from the ISY. Perhaps there's an undocumented way to do this -- what I'd like would be an "uptime" counter that we can poll after every network outage -- if that counters goes backwards, then the network outage was caused by an ISY restart. Ideas on how to do this are welcome.

Regarding having node servers automatically send reports every 60 seconds or so, or even automatically send updates when the ISY has restarted, well, that's problematic. I think we need to do this on a nodeserver-by-nodeserver basis. I've been working on the Hue node server, and trying to model it's behavior after that of an insteon device. Insteon devices don't gratuitously send status updates, so neither should the Hue node server, IMO. If someone needs to know it's status after restart, then the query-on-restart will do that - or the user's program can query the Hue bulbs -- just like one would do with Insteon devices. Other node servers may need more sophistication, and should have the option to proactively re-establish their ISY state, I think -- so we need to add a new command to the polyglot/node-server communications to signal to the node server that the ISY has restarted. This command would also send along the ISY version information (that may have changed; the reason for the restart might be that the ISY just got upgraded!).

@Einstein42
Copy link
Contributor

A driver doesn't update even if you tell it to from the node server if the value hasn't changed. Check the set_driver method in the nodeserver_api. I think that inherent behavior should change and allow the node_server to control whether to report or not with the "set_driver(, report=False)" option. As for the rest, I agree. I found a way around this in my node servers and will implement it there using the aforementioned query from ISY on restart.

@Einstein42
Copy link
Contributor

I do still have a problem where the ISY gets the status update (after a reboot) but only 1 field out of like 20 shows blank. I requery and it populates. The HTTP request is successful to the ISY on the initial status set, so I don't know why it isn't showing. ISY 5.0.4

@mkohanim
Copy link
Contributor

mkohanim commented Jul 3, 2016

Hello all,

Upon reboot, ISY queries all nodes. I really don't think Node Server to return it's existing status for a node upon receiving a Query command. It really should do a query and post the status back to ISY.

With kind regards,


Michel Kohanim
CEO

(p) 818.631.0333tel:818.631.0333
(f) 818.436.0702tel:818.436.0702
http://www.universal-devices.comhttp://www.universal-devices.com/


On July 2, 2016 6:32:20 PM Mike Westerhof notifications@github.com wrote:

Glad you're asking about this - a discussion will certainly help. I've been messing about with this issue as well as a number of others that are closely related, and have a bunch of prototype code that isn't yet ready to commit. Rather like Thomas Edison, I don't yet know how to solve this, but I now know a whole LOT of ways to NOT solve this!

I do have code that "pings" the ISY. It's not quite correct (it pings unnecessarily, and thus adds to the problems when the ISY is under load). We can add a heuristic to determine the "up/down"-ness of the ISY; the details we can work out later (i.e. how many timeouts/errors in a row before we call it down, and how many sucesses in a row before we call it up again).

There is a latent issue with the set_driver method in that it assumes success, and thus if the status update to the ISY doesn't go through, then the set_driver method will never try again to update. This also can be fixed - and will ensure that status updates that fail while the ISY will be retried each time the Node's code does a status update. If a query doesn't resend the values, that's a bug -- if explicitly queried, the set_driver method should still send that along - does it not do that, then? (I'll have to look, I was hacking in that area as well, I wonder if I fixed it in my version here, or broke it earlier in something I committed?!)

Rebooting of the ISY is something distinct from a transient network issue. I've not found a good way via REST to get this information from the ISY. Perhaps there's an undocumented way to do this -- what I'd like would be an "uptime" counter that we can poll after every network outage -- if that counters goes backwards, then the network outage was caused by an ISY restart. Ideas on how to do this are welcome.

Regarding having node servers automatically send reports every 60 seconds or so, or even automatically send updates when the ISY has restarted, well, that's problematic. I think we need to do this on a nodeserver-by-nodeserver basis. I've been working on the Hue node server, and trying to model it's behavior after that of an insteon device. Insteon devices don't gratuitously send status updates, so neither should the Hue node server, IMO. If someone needs to know it's status after restart, then the query-on-restart will do that - or the user's program can query the Hue bulbs -- just like one would do with Insteon devices. Other node servers may need more sophistication, and should have the option to proactively re-establish their ISY state, I think -- so we need to add a new command to the polyglot/node-server communications to signal to the node server that the ISY has restarted. This command would also send along the ISY version information (that may have changed; the reason for the restart might be that the ISY just got upgraded!).

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/62#issuecomment-230130282, or mute the threadhttps://github.com/notifications/unsubscribe/ANlUvQNuncWnTHJ6uhZowSZG9GyG20fIks5qRxEfgaJpZM4IuLRk.

@Barrygordon
Copy link
Collaborator

Is there a reason polyglot can not subscribe to the ISY? An ISY subscriber is provided with heartbeats and a loss of heartbeats implies the ISY is no longer operating. I monitor the heartbeat in my iPad application that communicates with the ISY, and if there is a loss for 2 minutes I assume the ISY has gone offline and start probing it with a REST command to start a subscription. When the subscription request succeeds I know the ISY is back up.

Sent from my iPad

On Jul 2, 2016, at 10:28 PM, James Milne notifications@github.com wrote:

A driver doesn't update even if you tell it to from the node server if the value hasn't changed. Check the set_driver method in the nodeserver_api. I think that inherent behavior should change and allow the node_server to control whether to report or not with the "set_driver(, report=False)" option. As for the rest, I agree. I found a way around this in my node servers and will implement it there using the aforementioned query from ISY on restart.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@mjwesterhof
Copy link
Collaborator Author

@Einstein42 -- I checked the code to figure out how I'd addressed the status reporting issue; it turns out that I didn't mess with the Node object's code at all (that was unnecessary). Instead I fixed my node server. The code that responds to a query operation finishes up with a call to the report_driver() method, which is the magic way to cause Polyglot to iterate over all the driver fields and unconditionally send the current status to the ISY.

So the recommended approach is to use set_driver() when responding to device changes (from events or from inside the poll() methods) - this will send updates to the ISY only when something changes. In the query code, query the end device (per @mkohanim above) to get updated info, then use set_driver(..., report=False) to update the internal values without making any REST calls, and finally calling report_driver() to update the ISY with all of the values.

I note that I've not committed the fix to the Hue node server; I'll disentangle that minor fix from the other changes I've been hacking on, and get that done. I'll also review the Kodi node server to see if it needs fixing as well.

@Barrygordon -- the only reason I've avoided the subscription is that it vastly complicates the low-level Polyglot internals to add support for another API mechanism. Right now the low level stuff is asynchronous, with a single thread per node server that handles all I/O -- we'd need to add logic, and libraries, to handle the new API, as well as a thread that does nothing but maintain that connection to the ISY, along with a set of queues to send messages back and forth from said thread to the node servers themselves. We may end up needing such a housekeeping thread anyway, but since it wasn't coded into the original Polyglot framework, it's quite a job to do so now!

@mkohanim
Copy link
Contributor

mkohanim commented Jul 3, 2016

@mjwesterhof .. thank you. I think what you have outlined would work perfectly. At the same time, I think there should be an interface to the node server for ISY reboot. So, once ISY is rebooted, it notifies the node server of this event. I'll talk to @cjudi .

@Barrygordon I barry, I echo @mjwesterhof ... subscribing to ISY makes things much more complicated since we might end up with a loop (ISY sending node server events back to the node server from which the event originated).

With kind regards,
Michel

@Einstein42
Copy link
Contributor

@mjwesterhof this is exactly what I came up with last night to fix my node servers. Glad we are on the same page. Thanks.

@mjwesterhof
Copy link
Collaborator Author

I just pushed a change to the SimpleNodeServer Node object that will improve the reliability of status updates.

Basically, this change adds the notion of "in-sync" to each node's driver (ST, GV1, etc). This flag starts out in the "False" state, meaning that the value of the driver as known by the node server is not known to match the value in the ISY. After a successful status update to the ISY (as determined by the 200 status code returned from the REST call), the flag is set to "True". Whenever the set_driver method is called to set the internal status value, if the in-sync flag is False, then the new status will be sent to the ISY even if it happens to match the in-memory status value.

Note that this just makes things better in the face of errors -- it does not completely fix them. In particular, if a status update fails, but the node server does not call set_driver periodically, then even though the in-sync flag is False, there's nothing to trigger the status update. This would tend to indicate that most node servers should not try to do their own status-update-avoidance, but should regularly poll their end device and call set_driver with the results. This mechanism also does not detect and respond to the case where the ISY restarts -- in such a case, it is true that some of the status updates may encounter errors, and post corrected results to the newly-rebooted ISY, but many others may completely miss the ISY restart, and thus the values in the ISY would be out-of-sync unbeknownst to Polyglot. Further work is required to deal with that problem..

Finally, this addresses ONLY status updates. We need also to provide a means to handle commands sent by a node server. No support exists for commands in the current SimpleNodeServer objects -- that will have to added. So it would be ideal if we added it in a way that made it really easy for the existing node servers that send commands. Can anyone please post code fragments from their polyglot node servers that send commands, so we can open a discussion on how to handle that? Thanks.

@mkohanim
Copy link
Contributor

mkohanim commented Jul 6, 2016

@mjwesterhof thanks SO very much. This is an excellent remedy for now. I have already spoken to Chris and we are going to come up with a onStartUp service that ISY will call each node server every time ISY is rebooted.

With kind regards,
Michel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants