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

Script profile: Separate toHandlerScript for commands and states #4058

Merged
merged 4 commits into from
Feb 6, 2024
Merged

Script profile: Separate toHandlerScript for commands and states #4058

merged 4 commits into from
Feb 6, 2024

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Jan 22, 2024

This allows much more fine-grained control for the script profile.

E.g. it is now possible to mimic the behaviour of the system:follow profile, but apply a script transformation to the forwarded state. I discovered the need for this when trying to make a KNX channel follow an Item state, but at the same time needed to transform the Item state before sending it to KNX.

This allows much more fine-grained control for the script profile.
E.g. it is now possible to mimic the behaviour of the `system:follow` profile, but apply a script transformation to the forwarded state.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 requested a review from a team as a code owner January 22, 2024 11:26
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 requested a review from J-N-K January 27, 2024 14:06
// try to parse the value
Command newCommand = TypeParser.parseCommand(handlerAcceptedCommandTypes, returnValue);
if (newCommand != null) {
callback.handleCommand(newCommand);
Copy link
Member

Choose a reason for hiding this comment

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

If we try to keep it as "open as possible", do we need to apply the same workaround here that we have in openhab/openhab-addons#16263?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably yes, but I am not sure whether we want to "distribute" that work-around across the code or if it would be better to wait for a solution of the underlying issue/problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While having a look at ProfileCallbackImpl, the problem seems to be that CommunicationManager::toAcceptedCommand does not accept OpenClosedType, which IMO is correct as you normally do not command contact Items.

The question is, how to solve that.
I would think the cleanest solution is to add a method like CommunicationManager::toAcceptedState and to introduce a method called handleState or handleUpdate to the ProfileCallback.
Then all profiles could call handleUpdate instead of handleCommand for state updates from the Item to the Thing.

@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/channel-transformations-not-working-when-using-text-files-for-item-creation/153407/5

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 requested a review from J-N-K January 28, 2024 21:20
@florian-h05
Copy link
Contributor Author

@J-N-K I don't want to be pushy, but it would be great to get this PR merged soon.

discard the states and not pass them through.</description>
<limitToOptions>false</limitToOptions>
</parameter>
<parameter name="toHandlerScript" type="text">
Copy link
Member

Choose a reason for hiding this comment

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

Maybe show this one as "advanced"? That hides it for new users, while keeping it for this that already have it configured.

Side note: It would be great if Main UI would automatically show "advanced" parameters if something is configured there. This would also be helpful e.g. for HTTP binding things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Side note: It would be great if Main UI would automatically show "advanced" parameters if something is configured there. This would also be helpful e.g. for HTTP binding things.

There we go: openhab/openhab-webui#2313

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.

Otherwise LGTM

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
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!

@J-N-K J-N-K added the enhancement An enhancement or new feature of the Core label Feb 6, 2024
@J-N-K J-N-K added this to the 4.2 milestone Feb 6, 2024
@J-N-K J-N-K merged commit 6be00bd into openhab:main Feb 6, 2024
3 checks passed
@florian-h05 florian-h05 deleted the script-profile branch February 6, 2024 20:30
florian-h05 added a commit to florian-h05/openhab-docs that referenced this pull request Feb 16, 2024
Refs openhab/openhab-core#4058.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit to florian-h05/openhab-docs that referenced this pull request Feb 16, 2024
Refs openhab/openhab-core#4058.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit to florian-h05/openhab-docs that referenced this pull request Feb 16, 2024
Refs openhab/openhab-core#4058.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

@openhab/jruby-maintainers FYI in case you need to adjust documentation.

@jimtng
Copy link
Contributor

jimtng commented Feb 23, 2024

It would be great if an upgrade tool could automatically update managed links to rename toHandlerScript

@florian-h05
Copy link
Contributor Author

Yeah, good idea.

stefan-hoehn added a commit to openhab/openhab-docs that referenced this pull request Feb 26, 2024
…ns folder (#2235)

* Update transformation docs for script profile changes

Refs openhab/openhab-core#4058.

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

* Migrate actions docs from addons to configuration folder

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

* Clean-up addons docs

The removed files have not been picked up by the build process, i.e. they were not used by the website.
Looking at their content, they also seem outdated.

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

* Minor enhancements

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

* Address review

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

---------

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Co-authored-by: stefan-hoehn <mail@stefanhoehn.com>
J-N-K pushed a commit that referenced this pull request Mar 2, 2024
* Upgrade tool: Add upgrade task for script profile changes

Refs #4058.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
J-N-K pushed a commit that referenced this pull request Mar 2, 2024
Refs #4058.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
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 of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants