Skip to content

Added resetAutoReleaseOnDetection property to sensors#169

Open
Kellojo wants to merge 3 commits into
benzman81:masterfrom
Kellojo:master
Open

Added resetAutoReleaseOnDetection property to sensors#169
Kellojo wants to merge 3 commits into
benzman81:masterfrom
Kellojo:master

Conversation

@Kellojo

@Kellojo Kellojo commented Oct 2, 2022

Copy link
Copy Markdown

Solving #168

@benzman81 benzman81 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct the commented lines.

}

HttpWebHookSensorAccessory.prototype.setAutoReleaseTimeout = function () {
if (!this.autoRelease || !this.resetAutoReleaseOnDetection) return;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prevents autoreleases if not resetAutoReleaseOnDetection is set. The flag resetAutoReleaseOnDetection should be handled as optional.


HttpWebHookSensorAccessory.prototype.setAutoReleaseTimeout = function () {
if (!this.autoRelease || !this.resetAutoReleaseOnDetection) return;
clearTimeout(this[this.getTimeoutKey()]);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing and resetting timeout should only be done if resetAutoReleaseOnDetection is true. Otherwise it should always add the timeout as before, so nothing break.

HttpWebHookSensorAccessory.prototype.setAutoReleaseTimeout = function () {
if (!this.autoRelease || !this.resetAutoReleaseOnDetection) return;
clearTimeout(this[this.getTimeoutKey()]);
this[this.getTimeoutKey()] = setTimeout(this.resetToInitialState.bind(this), this.autoReleaseTime);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above.

@benzman81

Copy link
Copy Markdown
Owner

@Kellojo in case you did not see it, I requested some changes...

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.

2 participants