search

amlinger / homebridge-telldus-tdtool

A Homebridge plugin for Tellstick without Live, interfaced with the CLI tool tdtool.
21 stars 11 forks source link

Handle callbacks properly for setting/getting characteristics. #7

Closed amlinger closed 8 years ago

amlinger commented 8 years ago

This is how the callbacks are handled for setting/getting characteristics. Omitting the callback parameters here:

callback()

instead of

callback(null, out.endswith('Success'))

should make Siri not wait too long for a response from the device, as @zobi pointed out in #6.

kskaret commented 8 years ago

Please fix! I experience the same issue as pointed out in #6 , when using homekit to turn on a device, the device is turned on but no feedback seems to return to ios (app just hangs). Anyway thanks for the plugin!

amlinger commented 8 years ago

Hi @kskaret! I do not at the moment, and will not for some weeks, have access to my Telldus setup. If you are interested, please try the branch https://github.com/amlinger/homebridge-telldus-tdtool/tree/issue-7/fix-siri-latency as I cannot release it since I can't verify that it will not break anything.

To build it pull down the branch, install the dev-dependencies, and issue npm run build to build the dist. Please let me know how it worked out for you :)

pkempe commented 8 years ago

I'd like to test this branch, could you please provide more detailed instructions for installing it (specifically, how do I install the dev-dependencies)? I have currently installed via npm, following the instructions in the Readme.

amlinger commented 8 years ago

That should do fine, the development dependencies are installed by default. Should this not be the case, try running

npm install --only=dev

and see if that works better.

After this, you should be able to place this directory in your node_modules folder, and you should be running the new one.

I'll make sure to add this to the README.

Let me know how it worked out for you!

pkempe commented 8 years ago

After testing, I don't see any difference with regards to timeouts waiting for response. It's possible though, that I screwed up the installation of this branch. Here's what I did:

sudo npm install --only=dev
npm run build
cd ..
cp -R homebridge-telldus-tdtool /usr/local/lib/node_modules/
sudo systemctl restart homebridge

and then tried switching stuff on/off via Siri. The units switch on/off as expected, but I always get the 'no reply' feedback from Siri.

Maybe I'm missing something? The /usr/lib/nodes_modules/ did not contain the homebridge-telldus-td-tool module before my copying, but perhaps that's expected. Is there any way that I can verify that the 'new' module is actually used?

Update: I finally understood that the modules should be installed in /usr/lib/node_modules/, where indeed the previous version exists. Now I can verify that the new telldus-accessory.js is in fact that and is in fact being used. The 'no reply' feedback is still there, though, so it appears this fix does not fix the problem.

kskaret commented 8 years ago

Hi! I have also tested the branch, but I don't see any difference. (Same as @pkempe) I simply installed the branch globally with sudo npm install -g.

amlinger commented 8 years ago

@pkempe; @kskaret did find a solution, hiding in a typo which is now fixed in the branch above. What you did on the command line should be working, as far as I can tell. You should not be needing sudo to install dependencies, if you like me think that is a very scary thing to do.

I will not merge https://github.com/amlinger/homebridge-telldus-tdtool/tree/issue-7/fix-siri-latency however until I've verified that it is indeed working on my setup as well. Hopefully this branch will do it for you guys in the meantime.

pkempe commented 8 years ago

Yes, the latest update does indeed work for me as well!