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

[basicprofiles] Add additional comparisons to State Filter profile #17323

Merged
merged 4 commits into from
Aug 28, 2024

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Aug 24, 2024

  • Add inequality comparisons to State Filter profile
  • Support symbolic operators ==, !=, etc.
  • Fix a bug where undefined mismatchState passed UNDEF instead of ignoring state updates
  • Support multiline conditions
  • Support comparing against the input state from handler to filter out
    unwanted data
  • Support comparing the states of item1 vs item2
  • Support comparing the input state against another item's state

https://community.openhab.org/t/state-filter-range-filter-profile/158025

@jimtng jimtng requested a review from a team as a code owner August 24, 2024 16:58
@jimtng jimtng added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Aug 24, 2024
@jimtng jimtng marked this pull request as draft August 24, 2024 17:00
@jimtng jimtng removed bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Aug 25, 2024
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/state-filter-profile/158025/1

@jimtng jimtng force-pushed the basicprofile-statefilter branch 2 times, most recently from be83319 to d68a005 Compare August 25, 2024 16:04
- Fix bug where undefined `mismatchState` passed `UNDEF` instead of ignoring state updates
- Support multiline conditions
- Support comparing against the input state from handler to filter out
unwanted data

Signed-off-by: Jimmy Tanagra <[email protected]>
@jimtng jimtng added the enhancement An enhancement or new feature for an existing add-on label Aug 25, 2024
@jimtng jimtng marked this pull request as ready for review August 26, 2024 02:05
@jlaur
Copy link
Contributor

jlaur commented Aug 26, 2024

@J-N-K - would you like to add yourself to CODEOWNERS for basicprofiles? I missed this in #16754.

@jlaur jlaur requested a review from J-N-K August 26, 2024 17:59
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/filter-transform-energy-meter-readings-to-delete-single-readings-for-state-machine/156276/7

@jimtng jimtng marked this pull request as draft August 27, 2024 08:19
Unquoted identifiers can refer to other items or have other meanings.

Signed-off-by: Jimmy Tanagra <[email protected]>
@jimtng jimtng marked this pull request as ready for review August 27, 2024 12:48
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

In general: nice addition.

@@ -27,13 +27,13 @@ Switch lightsStatus {

The Generic Toggle Switch Profile is a specialization of the Generic Command Profile and toggles the State of a Switch Item whenever one of the specified events is triggered.

### Configuration
### Generic Toggle Switch Profile Configuration
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change that? Isn't this already clear by the structure (## denotes the profile, ### the section within that profile)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I followed the markdown warning that the headings should be unique.
  2. This makes the anchor links better, but it's a very obscure benefit perhaps. Without this, the links would look like #configuration-1, #configuration-2, etc.

Should I revert this?


- The operator names must be surrounded by spaces, i.e.: `Item EQ 10`
- The operator symbols do not need to be surrounded by spaces, e.g.: `Item==10` and `Item == 10` are both fine.
- Only symbolic operators can be used when comparing against the incoming state, e.g. `> 10`. Using operator names isn't supported, i.e. this is not supported: ~~`GT 10`~~.
Copy link
Member

Choose a reason for hiding this comment

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

Why is that?

Copy link
Contributor Author

@jimtng jimtng Aug 28, 2024

Choose a reason for hiding this comment

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

The regex is more complicated to support it, but I've redone it now. Is it worth the extra complexity?

}
}

rhs = Objects.requireNonNull(parsedValue instanceof StringType ? parsedValue.toString() : parsedValue);
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need Objects.requireNonNull here? If parsedValue==null you exit in the if-clause above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The compiler complained about null mismatch further down in the calls to compareTo(rhs).

@jimtng jimtng marked this pull request as draft August 28, 2024 03:29
@jimtng jimtng force-pushed the basicprofile-statefilter branch 2 times, most recently from 16aa71f to 4d878eb Compare August 28, 2024 06:10
@jimtng jimtng marked this pull request as ready for review August 28, 2024 06:27
support named operator for input checking

Signed-off-by: Jimmy Tanagra <[email protected]>
@jimtng jimtng added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Aug 28, 2024
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks! Nice to see so many tests.

@J-N-K J-N-K added this to the 4.3 milestone Aug 28, 2024
@J-N-K J-N-K merged commit e4ce954 into openhab:main Aug 28, 2024
5 checks passed
@J-N-K J-N-K changed the title [basicprofiles] Add inequality comparisons to State Filter profile [basicprofiles] Add additional comparisons to State Filter profile Aug 28, 2024
@jimtng jimtng deleted the basicprofile-statefilter branch August 28, 2024 14:33
digitaldan pushed a commit to digitaldan/openhab-addons that referenced this pull request Aug 29, 2024
…penhab#17323)

* Add inequality comparisons to State Filter profile

- Fix bug where undefined `mismatchState` passed `UNDEF` instead of ignoring state updates
- Support multiline conditions
- Support comparing against the input state from handler to filter out
unwanted data

* Support comparing item to item or input to item

Signed-off-by: Jimmy Tanagra <[email protected]>
pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
…penhab#17323)

* Add inequality comparisons to State Filter profile

- Fix bug where undefined `mismatchState` passed `UNDEF` instead of ignoring state updates
- Support multiline conditions
- Support comparing against the input state from handler to filter out
unwanted data

* Support comparing item to item or input to item

Signed-off-by: Jimmy Tanagra <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants