search

metbosch / homebridge-http-temperature

HomeBridge HomeKit Plugin for HTTP temperature endpoints
https://www.npmjs.com/package/homebridge-http-temperature
Apache License 2.0
33 stars 18 forks source link

#6 Polling Status #8

Closed shidevil closed 6 years ago

shidevil commented 7 years ago

6 Fetch every x second/minute

metbosch commented 7 years ago

The changes look nice and the comments in issue #6 leave positive feedback. The only thing that I will add is dissable the fetch if the value in config.json is 0.

shidevil commented 7 years ago

Sure! No problem thank you!

metbosch commented 7 years ago

I just realized that this PR doesn't fix the issue #6 as we are not avoiding request when some ios device requests the state. In any case, the PR should be merged as it adds a needed feature 😀

shidevil commented 7 years ago

@metbosch it doesn't fix the issue of #6? it is polling the value right? hahah care to elaborate? thank you :) still learning as well.

shidevil commented 7 years ago

@metbosch it doesn't avoid request but fetch in x minutes. So it's still polling status back to home kit right?

shidevil commented 7 years ago

i have reverted the commit. As i notice i was editing the wrong index.js repo!

I have added disable fetch if refresh is 0

metbosch commented 7 years ago

@shidevil The issue #6 is about polling the status, you are right. But the user wants to avoid polling in the middle of the refresh period which is not implemented. And yes, after every fetch your code updates the homekit status.

shidevil commented 7 years ago

@metbosch the polling status will avoid poll in the middle of refresh period it will only poll every x/minus. Unless he opens the homekit app though. But that is HomeKit app function such that if you open the app it will refresh a new data. It's not a function that can be implemented if I'm not wrong.

metbosch commented 7 years ago

@shidevil I reviewed the code and I still see some problems. Mainly, the plugin will fail if the response time is larger than the refresh time. We developed a similar functionality in a motion-sensor plugin (https://github.com/lagunacomputer/homebridge-MotionSensor). Therefore, if it is okay for you, I will try to apply the changes here and we can test them together.

shidevil commented 7 years ago

@metbosch

Sure I will go and take a look at that.

metbosch commented 6 years ago

@shidevil Sorry for the late response :( Can you take a look at develop branch (https://github.com/metbosch/homebridge-http-temperature/tree/develop)? It's working on my board and I implemented your changes using promises to avoid duplicated requests on slow devices. Thanks!

shidevil commented 6 years ago

@metbosch will be taking a look at it and testing it.

shidevil commented 6 years ago

@metbosch i can confirm it is working. Good job. I have implemented your changes to my twine HTTP as well. It works good.

metbosch commented 6 years ago

Thanks @shidevil !!! I will merge the changes into the master branch, increase the version and publish it to npm. In addition, I'm closing this PR.