Skip to content

Pass event source to profiles - #4990

Merged
holgerfriedrich merged 1 commit into
openhab:mainfrom
ccutrer:profile-source
Sep 14, 2025
Merged

Pass event source to profiles#4990
holgerfriedrich merged 1 commit into
openhab:mainfrom
ccutrer:profile-source

Conversation

@ccutrer

@ccutrer ccutrer commented Aug 27, 2025

Copy link
Copy Markdown
Member

So that a user can define a profile that takes different actions depending on what's generating the command.

@ccutrer
ccutrer requested a review from a team as a code owner August 27, 2025 15:47
@ccutrer
ccutrer marked this pull request as draft August 27, 2025 15:47
@ccutrer
ccutrer force-pushed the profile-source branch 3 times, most recently from 1ecdfe6 to b02811b Compare August 27, 2025 18:10
So that a user can define a profile that takes different
actions depending on what's generating the command.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@ccutrer
ccutrer marked this pull request as ready for review August 27, 2025 19:12
@wborn
wborn requested a review from Copilot August 31, 2025 10:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the StateProfile interface to pass the event source to profiles, enabling users to define profiles that take different actions based on what generates the command.

  • Adds a new overloaded onCommandFromItem method to StateProfile that accepts an optional source parameter
  • Updates the CommunicationManager to pass the source parameter through the profile application chain
  • Updates test cases to verify the new method signature and source parameter handling

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
StateProfile.java Adds new overloaded method with source parameter and default implementation
CommunicationManager.java Updates profile application methods to propagate source parameter
CommunicationManagerOSGiTest.java Updates test verifications to match new method signature

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +35 to +38
/**
* Will be called if a command should be forwarded to the binding.
*
* @param command

Copilot AI Aug 31, 2025

Copy link

Choose a reason for hiding this comment

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

The javadoc is missing parameter documentation for the command parameter. Both parameters should be properly documented with @param tags.

Suggested change
/**
* Will be called if a command should be forwarded to the binding.
*
* @param command
* @param command the command to forward

Copilot uses AI. Check for mistakes.

@holgerfriedrich holgerfriedrich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@holgerfriedrich holgerfriedrich added the enhancement An enhancement or new feature of the Core label Sep 14, 2025
@holgerfriedrich holgerfriedrich added this to the 5.1 milestone Sep 14, 2025
@holgerfriedrich
holgerfriedrich merged commit 5f72352 into openhab:main Sep 14, 2025
5 checks passed
@ccutrer
ccutrer deleted the profile-source branch September 14, 2025 14:39
@florian-h05

Copy link
Copy Markdown
Contributor

Please don’t forget updating the docs.

@holgerfriedrich

holgerfriedrich commented Sep 21, 2025

Copy link
Copy Markdown
Member

@florian-h05 Do you think we should extend this section of the docs and basically state that custom implementations of StateProfile can override onCommandFromItem to grab the event source?

ccutrer added a commit to openhab/openhab-jruby that referenced this pull request Sep 22, 2025
Only applicable to commands from items.

Dependent on openhab/openhab-core#4990

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@florian-h05

Copy link
Copy Markdown
Contributor

Yes, would be good I think.

@florian-h05

Copy link
Copy Markdown
Contributor

Ideally the ScriptProfile should support the source and inject it or pass it into the script.

@ccutrer

ccutrer commented Sep 23, 2025

Copy link
Copy Markdown
Member Author

Ideally the ScriptProfile should support the source and inject it or pass it into the script.

I'm not sure this is going to be possible. ScriptProfile just uses a TransformationService, and TransformationService can only accept one (unnamed) input parameter. Perhaps we could add a configuration option to the script profile that said "pass data as JSON", and then stuff both the command value and the source into a JSON structure before passing to the script? Seems kinda kludgey for some pretty advanced stuff. I'm able to utilize it easily because the JRuby helper library can directly create a profile, with optional named parameters.

@ccutrer

ccutrer commented Sep 23, 2025

Copy link
Copy Markdown
Member Author

@florian-h05 Do you think we should extend this section of the docs and basically state that custom implementations of StateProfile can override onCommandFromItem to grab the event source?

openhab/openhab-docs#2561

@florian-h05

Copy link
Copy Markdown
Contributor

TransformationService can only accept one (unnamed) input parameter

What if we add a new method to it so we can optionally pass the second source parameter, and then inject the source into the script like it’s done with the arguments?

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