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

Some tweaks to have state behave better #5

Merged
merged 6 commits into from
Nov 20, 2018

Conversation

plainsane
Copy link

I did away with the fan device. I use the accessory to hold state that way the state survives a reboot.
I have also added a switch to reset the state Incase someone operates the fan with a remote.

Sadly stateless switches don’t work well from the client.

I did find via my rework that turning the fan off via tap and tapping back on seemed to provide a smoother experience.

Let me know what you think, this is my first crack at Homekit stuffs.

@iRayanKhan
Copy link

Does this plugin work for you? If so can you help me out with Issues #7

@plainsane
Copy link
Author

Yes it works. I commented on your thread

@iRayanKhan
Copy link

I comment on an issue and tagged you what happens when I try to use this pull:

[2018-11-6 23:46:54] [Bond] 1 cached accessories were loaded
[2018-11-6 23:46:54] Homebridge is running on port 51826.
[2018-11-6 23:46:54] [homebridge-tplink-smarthome.TplinkSmarthome] New Device Online: [My Smart Plug] plug [8006428AA29F7C0F1E774A04D4AE0C411A1C79F6] 10.0.0.5 9999
[2018-11-6 23:46:54] [homebridge-tplink-smarthome.TplinkSmarthome] Adding: [My Smart Plug] plug [8006428AA29F7C0F1E774A04D4AE0C411A1C79F6]
[2018-11-6 23:46:55] [Bond] Configure Accessory: null Fan
/usr/local/lib/node_modules/homebridge-bond/dist/index.js:168
            reset.getCharacteristic(Characteristic.On)
                 ^

TypeError: Cannot read property 'getCharacteristic' of undefined
    at BondPlatform.setupObservers (/usr/local/lib/node_modules/homebridge-bond/dist/index.js:168:18)
    at Timeout.setInterval [as _onTimeout] (/usr/local/lib/node_modules/homebridge-bond/dist/index.js:87:26)
    at ontimeout (timers.js:469:11)
    at tryOnTimeout (timers.js:304:5)
    at Timer.listOnTimeout (timers.js:264:5)

And any chance to make a reverse option and not be a switch?

@iRayanKhan
Copy link

Also when making fan speed 100, my fan only does the 2nd speed (3 speed fan) it makes it 3 for a second and makes it back to 2. The Bond App makes it 3 just fine

@iRayanKhan
Copy link

#8

@plainsane
Copy link
Author

plainsane commented Nov 9, 2018

You’re issue is caused by running the original version of the library then installing mine. There is not much in the way of an upgrade path right now...I’ll have to dig into it for a minute, but if you remove your accessories created by this module let it rediscover your devices from the api, it will work.

@plainsane
Copy link
Author

Ok, this new hotness should put the bond in James Bond. Should upgrade and add your switch. It should remove the reverse switch and it will add the direction characteristic for direction.

@plainsane
Copy link
Author

I’m hopeful that #8 will be mostly solved by this code once you install it, keep me posted

@methnen
Copy link
Contributor

methnen commented Nov 9, 2018

@plainsane Awesome to see this getting some more attention. I've been annoyed by the state issues myself but with no time to work on it. Have you pinged the original author so he knows you've got a pull request waiting? He was pretty responsive on Twitter when I bugged him that way for my last pull request.

@plainsane
Copy link
Author

@methnen no, I hate social media, I figured he got an email about this pr. Do this work for you using this pr?

@methnen
Copy link
Contributor

methnen commented Nov 12, 2018

@plainsane I hear you. I post to Twitter like once a year, if that, but I use the DM feature to get ahold of people when needed, basically the same reason I have a Facebook account these days though I used to use that for real quite a bit.

I'm not convinced he gets his GitHub alerts.

I know my pull request (the one that added light controls) never got any attention until I bugged him via a DM on Twitter. After which he was very responsive.

@plainsane
Copy link
Author

but have you installed this pr and given it a whirl? it should upgrade your existing devices...

@iRayanKhan
Copy link

Yes this worked for me after deleting the cache of the old bond, one issue though is that when I ask Siri to turn the rooms lights off it turns the fans lights on.

@methnen
Copy link
Contributor

methnen commented Nov 14, 2018

@plainsane No I haven't had a chance to test it yet. Meant to try your branch out this weekend but ended up being too busy. I'll try to spend some time with it today.

@plainsane
Copy link
Author

Yes this worked for me after deleting the cache of the old bond, one issue though is that when I ask Siri to turn the rooms lights off it turns the fans lights on.

I assume the remote you have setup in bond is a light toggle, not a discrete command for off and one for on. There really isn’t much that can be done with this respect except turn on the light state tracking in the bond app, but I found this as annoying.

@plainsane
Copy link
Author

Yes this worked for me after deleting the cache of the old bond, one issue though is that when I ask Siri to turn the rooms lights off it turns the fans lights on.

Sorry, I added an upgrade path for you

@plainsane
Copy link
Author

@plainsane No I haven't had a chance to test it yet. Meant to try your branch out this weekend but ended up being too busy. I'll try to spend some time with it today.

Back up your accessory cache, the code manipulates it for upgrade

@iRayanKhan
Copy link

Directional switch works :D Just the light thing is the only issue.

@evandcoleman evandcoleman merged commit 6289712 into aarons22:master Nov 20, 2018
@evandcoleman
Copy link
Contributor

I apologize for the delay on this. It's been merged and pushed to npm as 0.2.0. Thanks @plainsane for this!

@iRayanKhan
Copy link

Woah what your back!

@iRayanKhan
Copy link

Im sorry for being an ass over this but ayyy

@iRayanKhan iRayanKhan mentioned this pull request Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants