jvmahon / Homebridge-HomeSeer4

Homebridge Plugin for HomeSeer 3 and 4
28 stars 8 forks source link

When HomeSeer instance restarts, this plugin doesn’t reconnect #116

Closed abmagfab closed 3 years ago

abmagfab commented 3 years ago

HomeSeer on a separate box. Connect HomeBridge plugin to HomeSeer. Restart HomeSeer. This plug in fails and never reconnects.

I think that's the scenario. Both boxes reboot at roughly the same time. HomeBridge HomeSeer plugin repeatedly errors out and won't reconnect.

abmagfab commented 3 years ago

HomeSeer plugin 1.0.10 HomeBridge 1.2.3 Config UI X 4.28.0 Node 12.18.4

jvmahon commented 3 years ago

As a first thought, homebridge and the plugin use very little processor time - even on my system with hundreds of devices I never see more than a few percentage points of processor and that is only for a brief few seconds during a very busy activity (e.g., turning on/off dozens of devices). You will get much better performance if they are on the same box (the TCP / IP network processing between the boxes probably takes a good bit of processing compared to just leaving it all on one system).

As far as the connection problem, you should always start homebridge after HomeSeer. There's some code in the plugin to recover if it loses a connection to HomeSeer, but on startup it MUST always successfully connect or it won't recover. I run both HomeSeer and HomeBridge on the same laptop and I set homebridge to start up about a minute after HomeSeer.

I suppose the plugin could be adjusted to wait if HomeSeer isn't started, but I am not doing much development on this plugin anymore. I've begun to migrate to Hubitat so I plan to continue addressing bugs (if any -the code is pretty stable) rather than trying to do any significant code changes..

abmagfab commented 3 years ago

That's definitely the problem (startup). Is it a huge deal to just do something like sleep for 10 seconds on failure and try again? Are there other implications?

You're 100% right about low CPU and memory utilization (on RPi for me). I have them on separate boxes because it's preferential to have the HomeSeer home automation be separate from everything else to minimize maintenance and upgrades. For example, I just moved it from a Mac Mini to a RPi and it was painless because it's separate. I could decommission the RPi and use a pre-packaged HomeSeer box as well. The HomeBridge stuff is all on one box because it's all logically one solution.

Let me know if there's any chance of fixing the startup process?

Thanks! Mark

jvmahon commented 3 years ago

I tried getting this to work, but wasn't able to get it right. I'm sure this could eventually be done, but I just don't have the time. Again, the only suggestion I have at this time is to run both on the same box so you can better control the timing of when one or the other starts.

If you're familiar with Javascript programming and want to try and implement a fix, here's what I think needs to be done ...

The code needs to be altered such that there is a loop which keeps executing the fetch until it succeeds. Its also necessary that the code wait at this point until that success.

I tried converting this into "setInterval" loop that continually retries the fetch and returns a resolved Promise when its done, but the result seems to be wrong.

        var getStatusInfo;
        function resolveOnGetStatus() {
            return new Promise (resolve =>
            {
                var initialConnection = setInterval( function()
                    {
                    fetch(statusURL).then( response => 
                                {
                                    clearInterval (initialConnection);
                                    var getStatusInfo = response.Response.json();
                                    resolve ("Success")
                                } )
                                .catch( ()=>
                                {
                                    console.log(red(`Error - Failed to make initial connection to HomeSeer. HomeSeer may be offline. Retrying in 5 seconds.`))
                                });
                    }, 5000)
            })
        }

        await resolveOnGetStatus();

If anybody can figure this out, I welcome them posting a Pull request with the result.

sghoweri commented 3 years ago

I tried getting this to work, but wasn't able to get it right. I'm sure this could eventually be done, but I just don't have the time. Again, the only suggestion I have at this time is to run both on the same box so you can better control the timing of when one or the other starts.

If you're familiar with Javascript programming and want to try and implement a fix, here's what I think needs to be done ...

  • In the file "lib/HomeSeersystemObject" at line 472 is the function:
      var getStatusInfo =     await fetch(statusURL).then ( response => response.json() );

The code needs to be altered such that there is a loop which keeps executing the fetch until it succeeds. Its also necessary that the code wait at this point until that success.

I tried converting this into "setInterval" loop that continually retries the fetch and returns a resolved Promise when its done, but the result seems to be wrong.

      var getStatusInfo;
      function resolveOnGetStatus() {
          return new Promise (resolve =>
          {
              var initialConnection = setInterval( function()
                  {
                  fetch(statusURL).then( response => 
                              {
                                  clearInterval (initialConnection);
                                  var getStatusInfo = response.Response.json();
                                  resolve ("Success")
                              } )
                              .catch( ()=>
                              {
                                  console.log(red(`Error - Failed to make initial connection to HomeSeer. HomeSeer may be offline. Retrying in 5 seconds.`))
                              });
                  }, 5000)
          })
      }

      await resolveOnGetStatus();

If anybody can figure this out, I welcome them posting a Pull request with the result.

@jvmahon I think I fixed it! I'll try to toss in a quick PR but TLDR you were on the right track but had to wait for both fetch requests to finish before returning.

jvmahon commented 3 years ago

I don't think those changes fixed the original problem. The problem was that, if Homebridge starts first, the fetches will fail - so eve with the Promise.all structure you've put in place, the Promise will fail and you'll go to the .catch. Promise.all won't retry the fetch.

If you look at the code I was trying to get to work, I had a "setInterval" to retry the fetches every 5 seconds. Maybe your code gives me a cleaner way to do the fetches inside of an Interval timer.

Am I missing something?

jvmahon commented 3 years ago

Closed since there hasn't been a response.

marthoc commented 3 years ago

@jvmahon I don’t know JavaScript/nodejs but in python I would do this using a flag and a while loop, e.g. (pseudocode):

var flag = False
while !flag:
  try:
    get data
    flag = True (so that the while loop terminates)
  catch:
    log error
    wait 10s (flag is still false so the while loop will run again)

Is there a reason this wouldn’t work in nodejs?

jvmahon commented 3 years ago

Is there a reason this wouldn’t work in nodejs?

That section of startup code is pretty timing sensitive and there's a lot going on there - I'm hesitant to change anything at this point - I'm no longer using HomeSeer so I can't thoroughly test it. If anybody has a proposed change that they have tested, I'd be happy to integrate it, but I don't want to disturb the code without someone confirming that a proposed change works.

marthoc commented 3 years ago

Understood re your ability to test. Im getting my HomeSeer setup set back up after a move. Once I’ve got a few devices working I’ll make some changes and test. Since you’re not using HS anymore does it make sense for me to fork?

jvmahon commented 3 years ago

Since you’re not using HS anymore does it make sense for me to fork?

Up to you. I'm willing to integrated tested pull request into my codebase so that a single codebase is maintained. But for my own home system, I've switched to Hubitat so I'm not doing any significant work on this plugin anymore.

marthoc commented 3 years ago

@jvmahon I'm not familiar with how Homebridge works at starts up but I've tested this code yesterday, based on my earlier comment, and it achieves the stated goal; a connection to HomeSeer is retried until it is established:

        var getStatusInfo;
        var statusFlag = false;
        while (!statusFlag) {
            try {
                getStatusInfo = await fetch(statusURL).then(response => response.json());
                statusFlag = true;
            } catch (err) {
                console.log(red("Failed to fetch HomeSeer device data, retrying..."));
            }
        }

        var getControlInfo;
        var controlFlag = false;
        while (!controlFlag) {
            try {
                getControlInfo = await fetch(controlURL).then(response => response.json());
                controlFlag = true;
            } catch (err) {
                console.log(red("Failed to fetch HomeSeer control data, retrying..."));
            }
        }

The above code blocks the startup of Homebridge until a connection to HomeSeer is established, which may or may not be the desired behaviour - maybe if too many failures the code should exit without setting up the HomeSeer platform, to allow Homebridge to start successfully? HomeSeer is my only Homebridge platform so blocking until a connection is achieved is fine for me.

There's no wait or sleep in the catch as I found that the fetch takes several seconds to timeout anyway.