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

Alpha0.3: GarageDoorOpener and/or timer or wait function? #49

Closed
misc2000 opened this issue Oct 23, 2016 · 42 comments
Closed

Alpha0.3: GarageDoorOpener and/or timer or wait function? #49

misc2000 opened this issue Oct 23, 2016 · 42 comments

Comments

@misc2000
Copy link
Contributor

misc2000 commented Oct 23, 2016

hi,
I am on the way to implement a own handler for my 2 garage door openers.
Unfortunately I did only install one "Reed" sensor on the doors that detect if the door is closed from the hardware side.I can use this to detect the start of the movement and set "Characteristic.CurrentDoorState.OPENING = 2"
But to implement it "the Apple way" you also need on signal when the door is complete open.
My door need 18 seconds to open. And I need to send a "Characteristic.CurrentDoorState.OPEN = 0"
after that 18 seconds.

My idea was to simulate this signal by software inside my own Homebridge-KNX garage door Handler.
Just start a 18 second timer when the door starts to open (value change on the existing HW sensor)
And if the timer is over, and no interrupt/stop command was detected during the timer run, send the "open" value to the HK device.
Because I am not a Java Script expert at all my question is how could I implement this?

I think a easy
yield WaitForSeconds (18);
would not work because it will block everything in the handler for 18s.

My idea was more like:
function function1() {
// stuff you want to happen right away
console.log('All stuff needed when door start to open');
}

function function2() {
// all the stuff you want to happen after that pause
console.log('All stuff needed when door is complete open');
}

// call the first chunk of code right away
function1();
// call the rest of the code and have it execute after 18 seconds
setTimeout(function2, 18000);

Do this approach work in java script and also in your handler framework?
And if yes can someone give me a hint how to implement it (where to declare the functions)?
And how to interrupt the time if needed?
Thanks

@snowdd1
Copy link
Owner

snowdd1 commented Oct 23, 2016

The latter should work. I will have to test it myself because my garage door works as yours.

@snowdd1
Copy link
Owner

snowdd1 commented Oct 23, 2016

In JavaScript you can define the functions directly where the callback function is passed:

setTimeout(time, function () { bla; bla;});

@misc2000
Copy link
Contributor Author

misc2000 commented Oct 23, 2016

OK I can upload my handler later on when I am back home...

@misc2000
Copy link
Contributor Author

I did try to upload my handler to the addins folder but I need some more rights from you to do it. :-(

@snowdd1
Copy link
Owner

snowdd1 commented Oct 23, 2016

You can create a pull request: Fork the repo, upload your changes to github (into your fork), and then create a pull request.

@misc2000
Copy link
Contributor Author

@misc2000
Copy link
Contributor Author

about the timer again.
I did try this:
setTimeout(5000, function () { // 5 Seconds Timer
// Timer abgelaufen
console.log('INFO: Timer abgelaufen');
this.myAPI.knxWrite("KNX_Buero_Licht", 1, "DPT1"); // Test
} // Ende der Funktion
); // Ende des Timers

and get this error:

timers.js:92
first._onTimeout();
^

TypeError: first._onTimeout is not a function
at Timer.listOnTimeout (timers.js:92:15)

@misc2000
Copy link
Contributor Author

misc2000 commented Oct 23, 2016

did found the above problem myself.
You did write the parameter of the setTimeout in the wrong order ;-)
Should be setTimeout(function () { bla; bla;}, time );

Now I use this:
setTimeout( function () { // 5 Seconds Timer
// Timer abgelaufen
console.log('INFO: Timer abgelaufen');
this.myAPI.knxWrite("KNX_Buero_Licht", 1, "DPT1"); // Test
} // Ende der Funktion
,5000); // Ende des Timers

And get this error:

/usr/local/lib/node_modules/homebridge-knx/lib/addins/Misc2000Test.js:38
this.myAPI.knxWrite("KNX_Buero_Licht", 1, "DPT1"); // Test
^

TypeError: Cannot read property 'knxWrite' of undefined
at null._onTimeout (/usr/local/lib/node_modules/homebridge-knx/lib/addins/Misc2000Test.js:38:17)
at Timer.listOnTimeout (timers.js:92:15)

Misc2000Test.js is my test handler

@snowdd1
Copy link
Owner

snowdd1 commented Oct 23, 2016

If the function is called from "outside" the object this is not the one inside the class. You can fix that by binding the function to "this" of the object:

function() { bla }.bind(this)

Inside the setTimeout()

Regards

@snowdd1
Copy link
Owner

snowdd1 commented Oct 23, 2016

my garage door opener is a little different after all.
However, I have made a little handler example (MonoStableMultivibrator.js) that shows how to include a timer. The sample itself doe not do anything on the bus, it just turns off a switch 1000ms after is was switched on.

https://github.com/snowdd1/homebridge-knx/blob/plugin-2.0/lib/addins/MonoStableMultivibrator.js

@misc2000
Copy link
Contributor Author

misc2000 commented Oct 23, 2016

OK timer is working.
Now I will test it with the garage door handler.

@misc2000
Copy link
Contributor Author

OK its working with the timer to set the open state now also in the garage door handler.
But I found a case that is not perfect when you open the door over HK with the iPhone and close it from the KNX side. Then the current status is not shown perfect at the phone (HK side).
I will debug this tomorrow evening and will upload the new version after i catches this issue...

@misc2000
Copy link
Contributor Author

misc2000 commented Oct 30, 2016

Updated Version 2.1 of Misc2000GarageDoorOpener.js with simulated DoorOpen contact
is uploaded.

@misc2000
Copy link
Contributor Author

misc2000 commented Nov 3, 2016

Version 2.2 of Misc2000GarageDoorOpener.js is now part of 0.3 Beta Version. If you want to use it please read the header section of the handler to understand the needed HW conditions.

@giase82
Copy link
Contributor

giase82 commented Nov 11, 2016

Cool stuff guys. I was searching for something like this. I currently use SmartHome.py logics to simulate the "Closing", "Opening", "Open". This thing will make my life easier 👍

@giase82
Copy link
Contributor

giase82 commented Nov 14, 2016

Hi @misc2000 ,

I just had a look into the example a bit. One comment:
I think you do currently not handle the case that on switches "TargetDoorState" in HK while the door is moving. So I think if somebody chooses to open or close while it is moving, it will just stop. Or did I miss that?

In addition, it would be great to have this stuff configurable in the knx_config, not in the addin itself:
' this.simulateDoorOpenContact= true;
this.GarageDoorRunTime= 20000; // in mil.seconds
'

Further, my door-contacts work exactly the other way around, meaning that a "1" means a contact is on. Can we make this configurable as well.

I could of course change this myself as well, but I think we should not create too many flavours of examples.

Makes sense?

Cheers,
//giase

@misc2000
Copy link
Contributor Author

misc2000 commented Nov 15, 2016

hi @giase82
regarding the change of "TargetDoorState" in HK I need to verify again but I thought I covered it.
I am right now on a bussines trip and first time able to veify it at the weekend.
Regarding configurable stuff I have the same idea to move it to knx_config in a updated version.
Btw. I was working on a small upadte to reduce some duplicate push messages but was not able to finish it before my trip.
Regarding the reversed contact value I (we) can try to make my handler flexible for both scenarios...
misc2000

@giase82
Copy link
Contributor

giase82 commented Nov 15, 2016

Hi,
No worries, I meanwhile started a parallel thing as I need this soon. I think we can merge that later, as it is based on yours. I currently have some trouble as for some other reasons I do not use the staircase-function. Therefore I need to reset the switch to create a pulse after 500 ms. The issue is basically that there is currently no good solution for something like "sleep()" in JS.

//giase

@snowdd1
Copy link
Owner

snowdd1 commented Nov 15, 2016

@giase82
Do NOT use sleep or something like that because that will cause homebridge to freeze (even shortly) and might get kicked from homekit!

use setTimeout() see here: https://github.com/snowdd1/homebridge-knx/blob/master/lib/addins/MonoStableMultivibrator.js#L45

@snowdd1
Copy link
Owner

snowdd1 commented Nov 15, 2016

That creates a callback timer that will be called after given time, without interrupting the code execution much. The only thing to consider is that you can not be sure of order of execution - will the callback be called before or after a certain line in the rest of the code?
So make sure you do not depend on that, or you'll end up in http://callbackhell.com/

@giase82
Copy link
Contributor

giase82 commented Nov 15, 2016

That's exactly the issue, as I need to create up to three pulses (switch on/off) and therefore I need to be sure to only switch on again after it has been switched off!
I am going to try-out nested timeout functions tomorrow, unless you have another splendid idea? That's the only thing which comes to my mind at the moment...

@giase82
Copy link
Contributor

giase82 commented Nov 15, 2016

I started with setTimeout but this didn't work so far... if I don't get further by myself I'll post the code and you can check.

@snowdd1
Copy link
Owner

snowdd1 commented Nov 16, 2016

You need to keep the created timer object, so you can check if there is an active timer
https://github.com/snowdd1/homebridge-knx/blob/master/lib/addins/MonoStableMultivibrator.js#L40
There you can decide what to do in this case - in my example it just aborts the timer, so the callback function will never be called. You might need something else.

In the callback function you need to clear the timer variable
https://github.com/snowdd1/homebridge-knx/blob/master/lib/addins/MonoStableMultivibrator.js#L49
so the if from #L40 sees no timer any more.

@giase82
Copy link
Contributor

giase82 commented Nov 16, 2016

So what I have right now is this:

date = new Date(); console.log('INFO: KNXPulseMove = 1 - calling Up/Stop/Down, ' + date.getSeconds() +'s:'+ date.getMilliseconds() +'ms'); this.myAPI.knxWrite("KNXPulseMove", 1, "DPT1"); // Up/Stop/Down setTimeout(function(){ date = new Date(); console.log('INFO: KNXPulseMove = 0 - calling Up/Stop/Down, ' + date.getSeconds() +'s:'+ date.getMilliseconds() +'ms'); this.myAPI.knxWrite("KNXPulseMove", 0, "DPT1"); // Up/Stop/Down }, this.pulseLength);

(why doesn't the formatting work?)

But I get the following error currently:
'/usr/local/lib/node_modules/homebridge-knx/lib/addins/GiaseGarageDoorOpener.js:265
this.myAPI.knxWrite("KNXPulseMove", 0, "DPT1"); // Up/Stop/Down
^

TypeError: Cannot read property 'knxWrite' of undefined
at null._onTimeout (/usr/local/lib/node_modules/homebridge-knx/lib/addins/GiaseGarageDoorOpener.js:265:16)
at Timer.listOnTimeout (timers.js:92:15)'

My JS-skills are rather copy/paste-based currently, as I didn't do this for some years now. Can you guys point out what's wrong?

In addition, I got the issue that I see the needed address for the pulse switched on the bus, but for whatever reason, when I do this via Homebridge, it does not get switched off anymore... If I do it manually it works fine. I raised to timeout to 2 seconds already, but this doesn't change anything. But this is a different topic.

@giase82
Copy link
Contributor

giase82 commented Nov 16, 2016

I guess I need to use the 'bind'?

@snowdd1
Copy link
Owner

snowdd1 commented Nov 16, 2016

you can either use function () { ... }.bind(this) or do a that=this; before and use that inside the function instead of this - latter is the standard way, bind comes with a performance cost, I learned recently.

@snowdd1
Copy link
Owner

snowdd1 commented Nov 16, 2016

^ that=this: before the function definition

@giase82
Copy link
Contributor

giase82 commented Nov 16, 2016

Ok thanks. I did the .bind now and this works finally (also setting it correctly on the bus). I am not that much concerned about performance :)

When I've got time to test further, I'll send a pull request and we can probably merge it with the one from misc2000 later.

Am 16.11.2016 um 10:39 schrieb Raoul notifications@github.com:

^ that=this: before the function definition


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

@giase82
Copy link
Contributor

giase82 commented Nov 18, 2016

Hi,

Hier ist der pull-request: #63

Ist jetzt doch n bisschen umfangreicher geworden...

@misc2000
Copy link
Contributor Author

hi Giase82,
I saw that you did a good progress this week. And you also did some of my todo list (translate all to english and working with constants) I will test your pull-request later today and will give feedback...
misc2000

@snowdd1
Copy link
Owner

snowdd1 commented Nov 19, 2016

@giase82
Chrsitof, could you have a look at it, because it shows merge conflicts.
You might need to merge the base branch first, and re-apply your changes.

@giase82
Copy link
Contributor

giase82 commented Nov 20, 2016

Hi Raoul, I saw this. How can I re-base in git or merge back the base? It just tells me:

This branch has conflicts that must be resolved
Only those with write access to this repository can merge pull requests.
Conflicting files
lib/addins/WindowCoveringTilt.js

@snowdd1
Copy link
Owner

snowdd1 commented Nov 20, 2016

I think it happens if you start modifying a file and haven't merged (pulled) the latest version. So you need to pull the latest version of the repository, do the changes again to WindowCoveringTilt.js and commit it, then I can click the merge pull request button.

@misc2000
Copy link
Contributor Author

hi Christof,
I did test the modified version today with this scenario:
"simulateDoorOpenContactMode": "knx",
"staircaseFunc": true,
"separatePulseUpDown": false,
"sensorOn": 0,
"pulseLength": 500,
"doorRunTime": 19000
It did work without any problems until now.
The only thing is that I once got a double/wrong push message (but I saw this also sometimes before you made your modifications).
The scenario was that I first open the door from knx side and then close it after some seconds again from knx side.
And I got the "wrong" message "door was opened" again direct after I press the knx button to close the door and the door start to move down.
Did you also see this double open messages in your setup sometimes because this behaviour is not reproducible in my case...

@giase82
Copy link
Contributor

giase82 commented Nov 21, 2016

Hi Raoul, Hi Michael,

Thanks for reviewing and testing!

I did indeed see the push-notification twice in this case as well. I only had a quick look, but from the implementation it seem to be fine, so I guess this is an Apple issue. I get a "door opened" once it starts showing "opening" in the App, and then again when the door is really opened.
If you spot anything in the code that proves me being wrong, just let me know. I am happy to change it then.

Regarding the merge: I am still trying to figure out how git works in its details. Just a matter of years .. ;-)

@snowdd1
Copy link
Owner

snowdd1 commented Nov 21, 2016

I see these two notifications as well, I think that it has to do with the TargetState and CurrentState - Apple has chosen to send a push upon receiving TargetState and CurrentState changes, might be a bug.
Also I am thinking about not sending an update if you get back the same value from the bus that you have just sent to it. Maybe I can investigate next weekend.

@misc2000
Copy link
Contributor Author

hi Christof/Raoul,
now I remember that i was able 1-2 week ago during some tests to reduce some double notifications by execute a ".setValue to the HK" only if the value changes.
We should try to place just a if =! "newValue" in front of every .setValue command....

@giase82
Copy link
Contributor

giase82 commented Nov 21, 2016

I agree with Raoul that sending a push on both TargetState and CurrentState is probably the issue. I think the if =! "newValue" wouldn't change that.
What I see from the logs there is no double setting of the same TargetState, at least not the scenarios I've tested. Nevertheless, it doesn't harm to have it...

@giase82
Copy link
Contributor

giase82 commented Nov 22, 2016

Hi guys,

I just did another update after running into a problem where there was only the "open-door" contact present. I am going to send another pull-request soon...

//Christof

@giase82
Copy link
Contributor

giase82 commented Nov 22, 2016

Here we go: #64

@snowdd1
Copy link
Owner

snowdd1 commented Nov 22, 2016

Having a bad night or are you in another time zone?

@giase82
Copy link
Contributor

giase82 commented Nov 23, 2016

I've got kids, so rather the former currently ;-)

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

3 participants