Implemented a toggle method on Item #99

rkoshak · 2022-02-17T20:33:17Z

Signed-off-by: Richard Koshak rlkoshak@gmail.com

[Item] Added a Toggle Method to Item

Description

Added a toggle method to Item which will toggle the state of the Item based on its current state. Any Item that supports OnOffType command or update will receive the opposite of its current getStateAs(OnOffType) as an update or command. Any Item that supports OpenClosedType updates will receive the opposite of its current getStateOs(OpenClosedType). In all other cases an error is thrown (including attempting to toggle a Contact with a command). There is an argument update which, when provided with true will cause the new toggled state to be posted as an update. The default is false causing the new toggled state to be sent as a command.

There are many situations where one needs to toggle a binary type Item and this saves the user from needing to repeat that sort of logic over and over, similar to the already present sendCommandIfDifferent().

Testing

I tested most of the different permutations including:

toggle on an Item whose state is UnDefType: generates error
toggle on Switch without update argument: Item was commanded
toggle on Switch with update=false: Item was commanded
toggle on Switch with update=true: Item was updated
toggle on Contact with update=false: generates error
toggle on Contact with update=true: Item was updated

updateTimeout can reschedule a timer created by setTimeout. ZDTtoMillisFromNow parses the duration from now to the given ZDT in milliseconds. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

… about monkey patches Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

rkoshak · 2022-02-17T20:39:34Z

This is ready for review. I found myself using this over and over from my personal library and figured others would find it useful too. Since it's inline with sendCommandIfDifferent I figured it'd be appropriate to add.

rkoshak · 2022-02-17T21:14:46Z

Would it make sense to update/command the Item if its current state is NULL or UNDEF instead of throwing an error? It occurs to me that with the current error it might confuse users into thinking that the problem is the Item type and not that it's current state is the problem.

Time: Add updateTimeout & ZDTtoMillisFromNow

digitaldan

I made a couple comments, one thing that seems odd to me about this is that we have one method which handles both a command and an update, which is kinda awkward to explain given some types support update and commands and some don't. I get toggling switch like items and sending a command, rolling in updates and contacts seems to confuse this. Maybe we should stick to what we support in rules as well ?

openhab-js/rules/operation-builder.js

Line 405 in d1b3cd5

doToggle() {

we could probably move this onto an item and reuse/reduce the amount of code needed.

items/managed.js

rkoshak · 2022-02-18T21:05:06Z

we could probably move this onto an item and reuse/reduce the amount of code needed.

I did look for something like a toggle already in the library and never thought to look in the fluent rule builder stuff. Yes, I'd like to see that moved to Item so we don't duplicate code and everyone can benefit. I'll update that doToggle function as appropriate.

I do notice though that that existing implementation is incomplete. Dimmer too can be toggled. I like the approach of using getStateAs() to determine if it's toggle-able though because even if OH adds a new Item type that handles ON/OFF (unlikely I know) the function will still work.

I also don't mind splitting it into two separate functions but don't know what to call the one that does the updates. Maybe use toggleCommand and toggleUpdate?

The rest I'll change as suggested. I might experiment with Player Items too. It seems like Play/Pause is another type of toggle that should be supported.

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

rkoshak · 2022-03-01T18:51:08Z

I think I may have accidentally rebaselined this PR. I thought I was on main on my fork.

That wasn't my intention. I'm not sure how to proceed from here.

Clearly git and I do not get along. :-(

Anyway, I've addressed all the comments from above. I added a toggleCommand and toggleUpdate to Item and changed doToggle to call toggleCommand so it's all centralized.

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

digitaldan

Thanks Richard, it looks like there are a lot of changes related to your PR around time, I also left a very small comment. Can you fix this so it just includes toggle functionality? Might be easier to start fresh and cherry pick changes

digitaldan · 2022-03-05T20:34:12Z

rules/operation-builder.js

@@ -404,21 +404,7 @@ class ToggleOperation extends OperationConfig {
     */
    doToggle() {
        let item = items.getItem(this.itemName);


Since we only use the reference once, might be cleaner to do

Suggested change

let item = items.getItem(this.itemName);

items.getItem(this.itemName).toggleCommand();

digitaldan · 2022-03-05T20:35:36Z

time.js

+ * @returns {number} duration from now to the ZonedDateTime in milliseconds
+ */
+time.ZonedDateTime.prototype.millisFromNow = function () {
+  return time.Duration.between(time.ZonedDateTime.now(), this).toMillis();


Not sure if this should be in this PR?

Yes, I mucked things up and somehow merged upstream on this branch when I meant to on main instead. That's not actually my code. I'll try to get back to just my PR.

digitaldan · 2022-03-05T20:38:10Z

README.md

@@ -144,19 +143,23 @@ The string representations of each of these objects are appended together in the

 see https://developer.mozilla.org/en-US/docs/Web/API/console for more information about console logging.

-### SetTimeout
+### Timers


I think this is from a another PR, as well as all the other time changes?

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

rkoshak · 2022-03-15T18:17:33Z

Ready for review.

I've backed out the changes caused by my merging on the wrong branch and I've made the requested change in operation-builder.js.

digitaldan · 2022-03-19T15:44:43Z

items/managed.js

+     * command is sent).
+     * @throws error if the Item is uninitialized or a type that cannot be toggled or commanded
+     */
+    toggleCommand() {


Suggested change

toggleCommand() {

sendToggleCommand() {

wdyt? I'm thinking we should try and be consistant since we use sendCommand elsewhere

That works for me. I like consistency.

digitaldan · 2022-03-19T15:49:47Z

items/managed.js

+     * update is posted).
+     * @throws error if the Item is uninitialized or a type that cannot be toggled
+     */
+    toggleUpdate() {


Suggested change

toggleUpdate() {

postToggleUpdate() {

wdyt? I'm thinking we should try and be consistant since we use postUpdate elsewhere

Same as above. I'll get a PR submitted later today with these two changes.

digitaldan

Thanks @rkoshak ! Looks pretty good to me, i only had 2 small comments about naming

digitaldan · 2022-04-23T15:39:01Z

Hey @rkoshak any update on this?

rkoshak · 2022-04-23T16:04:44Z

I've been trying to figure out how to service the unit tests using Mocha but haven't had a lot of time these last few weeks to do much of anything. I've but forgotten and am still working it. I can commit the charges without the tests to get this moving.

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

rkoshak · 2022-04-27T19:05:19Z

I gave up on the uint testing. I might come back at a date with a new PR once I figure it out. Your requested changes have been made with this latest PR.

florian-h05

LGTM overall, but please have a look at my comment!

florian-h05 · 2022-04-30T07:22:15Z

rules/operation-builder.js

-            default:
-                throw Error(`Toggle not supported for items of type ${item.type}`);
-        }
+        items.getItem(this.itemName).toggleCommand();


Wouldn't this be sendToggleCommand instead of toggleCommand?

digitaldan · 2022-05-01T17:43:45Z

@rkoshak i'm ok without the testing , if you want to fix the one issue mentioned i'm happy to merge, thanks!

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

rkoshak · 2022-05-02T16:09:35Z

Should be good to go now.

florian-h05

LGTM, thank you @rkoshak.
I did no testing but it should work.

digitaldan · 2022-05-07T16:01:09Z

lgtm thanks!

This reverts the removal of `time.ZonedDateTime.millisFromNow()` and Timer docs from the README from PR openhab#99. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Emulating merge error with openhab#99 Signed-off-by: Sami Salonen <ssalonen@gmail.com>

Emulating merge error with openhab#99 Signed-off-by: Sami Salonen <ssalonen@gmail.com> Signed-off-by: Sami Salonen <ssalonen@gmail.com>

@runtime

* Docs: document UI rules event and @runtime Signed-off-by: Sami Salonen <ssalonen@gmail.com> * docs: additional disclaimer with String items Signed-off-by: Sami Salonen <ssalonen@gmail.com> * docs: TOC showing h2..h3 only Signed-off-by: Sami Salonen <ssalonen@gmail.com> * Docs: Capitalization of Java/JavaScript Signed-off-by: Sami Salonen <ssalonen@gmail.com> * docs: Wording correections. Mention "jsscripting" also Signed-off-by: Sami Salonen <ssalonen@gmail.com> * docs: cross-reference event object file/UI documentation sections Signed-off-by: Sami Salonen <ssalonen@gmail.com> * docs: Remove empty column Signed-off-by: Sami Salonen <ssalonen@gmail.com> * Docs: clarifying installation section Signed-off-by: Sami Salonen <ssalonen@gmail.com> Co-Authored-By: Dan Cunningham <dan@digitaldan.com> Co-authored-by: Dan Cunningham <dan@digitaldan.com> * docs: Advanced Scripting with @runtime Signed-off-by: Sami Salonen <ssalonen@gmail.com> * docs: Move scriptLoaded/scriptUnloaded hooks under File Based Rules Also: - title capitalization now reflecting code Signed-off-by: Sami Salonen <ssalonen@gmail.com> * Remove openhab timer Emulating merge error with #99 Signed-off-by: Sami Salonen <ssalonen@gmail.com> * Merge fixing Emulating merge error with #99 Signed-off-by: Sami Salonen <ssalonen@gmail.com> Signed-off-by: Sami Salonen <ssalonen@gmail.com> Co-authored-by: Dan Cunningham <dan@digitaldan.com>

* Revert unwanted removals from previous PR #99 This reverts the removal of `time.ZonedDateTime.millisFromNow()` and Timer docs from the README from PR #99. Signed-off-by: Florian Hotze <florianh_dev@icloud.com> * README.md: Style fixes Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 and others added 8 commits February 8, 2022 23:59

Time: Add updateTimeout & ZDTtoMillisFromNow

bb7d578

updateTimeout can reschedule a timer created by setTimeout. ZDTtoMillisFromNow parses the duration from now to the given ZDT in milliseconds. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Time: Remove unnecessary checks from updateTimeout

0d56361

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Time: Monkey-patch ZonedDateTime millisFromNow & Add README section…

4e0e2f0

… about monkey patches Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Time: Remove updateTimeout & Document the returned Timer

3a861df

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Time: millisFromNow improvements

d8cf009

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

README: Improvements for JS-Joda and Timers

840f290

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Implemented a toggle method on Item

28acaeb

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Fixed docstring error breaking the build

94e8b95

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Merge pull request openhab#93 from florian-h05/timer-improvements

d1b3cd5

Time: Add updateTimeout & ZDTtoMillisFromNow

digitaldan requested changes Feb 18, 2022

View reviewed changes

items/managed.js Outdated Show resolved Hide resolved

items/managed.js Outdated Show resolved Hide resolved

items/managed.js Outdated Show resolved Hide resolved

items/managed.js Outdated Show resolved Hide resolved

rkoshak added 4 commits March 1, 2022 11:42

Implemented a toggle method on Item

28b3a21

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Fixed docstring error breaking the build

2bc83d7

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Addressing comments

0bcb93f

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Merge branch 'toggle' of github.com:rkoshak/openhab-js into toggle

25ec0fb

Forgot to remove the old toggle method--

35f0108

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

digitaldan requested changes Mar 5, 2022

View reviewed changes

rkoshak added 5 commits March 15, 2022 12:08

README.md Undoing erroneous merge

1f8a800

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Addressing comment to make cleaner

a874def

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

time.js: removing changes from erroneous merger

94a2f92

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Removing remenants of bad merge

c743259

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

Removing remnants of bad merge

97c7297

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

digitaldan reviewed Mar 19, 2022

View reviewed changes

digitaldan requested changes Mar 19, 2022

View reviewed changes

changed names of toggle methods

e67d5a5

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

florian-h05 reviewed Apr 30, 2022

View reviewed changes

digitaldan mentioned this pull request May 1, 2022

Add standardx to enforce consistent codestyle #107

Merged

Addressing @florian-h05's comments.

5b925b8

Signed-off-by: Richard Koshak <rlkoshak@gmail.com>

florian-h05 approved these changes May 2, 2022

View reviewed changes

digitaldan merged commit ccea6bd into openhab:main May 7, 2022

florian-h05 mentioned this pull request May 7, 2022

Revert unwanted removals from previous PR #99 #113

Merged

ssalonen added a commit to ssalonen/openhab-js that referenced this pull request May 8, 2022

Remove openhab timer

95d08e3

Emulating merge error with openhab#99 Signed-off-by: Sami Salonen <ssalonen@gmail.com>

ssalonen added a commit to ssalonen/openhab-js that referenced this pull request May 8, 2022

Merge fixing

5f334f0

Emulating merge error with openhab#99 Signed-off-by: Sami Salonen <ssalonen@gmail.com>

ssalonen added a commit to ssalonen/openhab-js that referenced this pull request May 8, 2022

Merge fixing

1fa5bc5

Emulating merge error with openhab#99 Signed-off-by: Sami Salonen <ssalonen@gmail.com> Signed-off-by: Sami Salonen <ssalonen@gmail.com>

ssalonen mentioned this pull request May 8, 2022

Docs: document UI rules event and @runtime #110

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented a toggle method on Item #99

Implemented a toggle method on Item #99

rkoshak commented Feb 17, 2022

rkoshak commented Feb 17, 2022

rkoshak commented Feb 17, 2022

digitaldan left a comment

rkoshak commented Feb 18, 2022

rkoshak commented Mar 1, 2022

digitaldan left a comment

digitaldan Mar 5, 2022

rkoshak Mar 15, 2022

digitaldan Mar 5, 2022

rkoshak Mar 7, 2022

rkoshak Mar 15, 2022

digitaldan Mar 5, 2022

rkoshak Mar 7, 2022

rkoshak Mar 15, 2022

rkoshak commented Mar 15, 2022

digitaldan Mar 19, 2022 •

edited

Loading

rkoshak Mar 28, 2022

rkoshak Apr 27, 2022

digitaldan Mar 19, 2022

rkoshak Mar 28, 2022

rkoshak Apr 27, 2022

digitaldan left a comment

digitaldan commented Apr 23, 2022

rkoshak commented Apr 23, 2022

rkoshak commented Apr 27, 2022

florian-h05 left a comment

florian-h05 Apr 30, 2022

rkoshak May 2, 2022

digitaldan commented May 1, 2022

rkoshak commented May 2, 2022

florian-h05 left a comment

digitaldan commented May 7, 2022

	let item = items.getItem(this.itemName);
	items.getItem(this.itemName).toggleCommand();

Implemented a toggle method on Item #99

Implemented a toggle method on Item #99

Conversation

rkoshak commented Feb 17, 2022

[Item] Added a Toggle Method to Item

Description

Testing

rkoshak commented Feb 17, 2022

rkoshak commented Feb 17, 2022

digitaldan left a comment

Choose a reason for hiding this comment

rkoshak commented Feb 18, 2022

rkoshak commented Mar 1, 2022

digitaldan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkoshak commented Mar 15, 2022

digitaldan Mar 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitaldan left a comment

Choose a reason for hiding this comment

digitaldan commented Apr 23, 2022

rkoshak commented Apr 23, 2022

rkoshak commented Apr 27, 2022

florian-h05 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitaldan commented May 1, 2022

rkoshak commented May 2, 2022

florian-h05 left a comment

Choose a reason for hiding this comment

digitaldan commented May 7, 2022

digitaldan Mar 19, 2022 •

edited

Loading