[basicprofiles] Fix StateFilterProfile to use linked Item system unit #18144
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
added
the
enhancement
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
added
bug
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrewfg
changed the title
andrewfg
changed the title
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
|
@jimtng good news that all your PRs have all now been merged; which means that this one is also now ready to go: => could you please have a look at it? |
jimtng
reviewed
Feb 9, 2025
.../src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java
Outdated
Show resolved
Hide resolved
| @@ -603,7 +635,7 @@ public int getWindowSize() { | |||
| // the previous state is kept in the acceptedState variable | |||
| return 0; | |||
| } | |||
| return windowSize.orElse(DEFAULT_WINDOW_SIZE); | |||
| return windowSize.isPresent() ? windowSize.get() : DEFAULT_WINDOW_SIZE; | |||
There was a problem hiding this comment.
It eliminates the yellow squiggly underline compiler warning in Eclipse IDE :)
.../src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java
Show resolved
Hide resolved
.../src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java
Show resolved
Hide resolved
Good question. If I recall correctly I think the code converts 'OtherItem' to the string representation of its State, and then the string is handled in the same way as one hard coded in the condition LHS / RHS. So if 'OtherItem' was a QuantityType we would attempt to convert it to the systemUnit. And in the case 'OtherItem1 < OtherItem2' .. if either or both would not convert to systemUnit we would have an error -- which I guess means that the input State is rejected?? PS in any case I will add some JUnit tests.. => EDIT: I see that there are actually already many such tests .. and they all still pass. |
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: Andrew Fiddian-Green <[email protected]>
jimtng
approved these changes
Feb 10, 2025
.../src/main/java/org/openhab/transform/basicprofiles/internal/profiles/StateFilterProfile.java
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #18122
There is a bug whereby the calculations over a StateFilterProfile set of time series state values are based on the Unit of the first member in the set of values. There are two problems: a) the order of the state values is not relevant, and b) the Unit of the incoming state values is not definitive to the Unit of the expected result. Therefore these calculations do produce variable and unexpected results.
This PR fixes the bug by normalising the calculations so that they are all based on the Unit of the target Item to which the StateFilterProfile is linked.
Furthermore this PR allows conversion between non- inverted and inverted Units so invertible conversions (e.g. Kelvin <=> Mirek) are supported.
And finally it also provides better support for cases where the binding may provide state updates in different mixed formats e.g. a mixture of QuantityType with different base units, UndefType, and various States that can yield a DecimalType value.
Signed-off-by: Andrew Fiddian-Green [email protected]