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

Allow profiles to access the entire context of the link they're applied to #3151

Merged
merged 1 commit into from
Dec 11, 2022

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Nov 7, 2022

So that they can vary their processing based on the item or channeluid itself, instead of (or in addition to) solely on the prior states passing through it.

This was inspired by/extracted from #3135, since that PR is still being discussed.

@ccutrer ccutrer requested a review from a team as a code owner November 7, 2022 21:50
…ed to

So that they can vary their processing based on the item or channeluid itself,
instead of (or in addition to) solely on the prior states passing through it.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@ccutrer ccutrer force-pushed the profile_context-link branch from e8f3d3b to 0e1de50 Compare November 8, 2022 00:13
@ccutrer ccutrer mentioned this pull request Nov 8, 2022
@ccutrer
Copy link
Contributor Author

ccutrer commented Nov 26, 2022

Ping?

@wborn wborn added the enhancement An enhancement or new feature of the Core label Nov 30, 2022
@kaikreuzer
Copy link
Member

Hm, it was by design that profile implementations do not have access to concrete items or channels as there is a high risk that this is misused and might lead to a tighter coupling (and therefore breaking changes if anything in the core framework is changed).
I understand that profiles might want to know the item type and channel type that they are linked to, if they are profiles that are applicable to multiple types. But imho this type information should suffice and I would hide the actual item/channel/link.
Wdyt?

@ccutrer
Copy link
Contributor Author

ccutrer commented Dec 11, 2022

Hmm... I see what you're saying. But no, type information alone would not be enough for my use case. I'm now using profiles to implement a "veto" feature my user "rules", as we've discussed before at #3101. My actual profile code:

profile "shades_vetoing" do |callback:, item:, command: nil|
  next true if command.nil?
  next true if command == UP
  next true if command == REFRESH
  next true if item.tags.include?("AlwaysClose")

  shade_base_name = item.name[0..-20].sub("Black", "").sub("Roman", "")
  contact_item = items["#{shade_base_name}Window_Contact"]
  contact_item = DeckDoor_Contact if item.eql?(GreatDrapes_Rollershutter)
  contact_item ||= items["#{shade_base_name}Door_Contact"]
  next true unless contact_item.open?

  if item.tags.include?("DoubleHung")
    target = (command == DOWN) ? PercentType::HUNDRED : command
    next true unless target > 45

    callback.handle_command(PercentType.new(45)) unless item.state == 45
    next false # veto
  end
  false # veto
end

and it's reference from a .items file:

Rollershutter BunkShade_Rollershutter "Bunk Room Shade [%d%%]" (lBunk, gMostBasementShades, gBunkShades, gInflux) { channel="mqtt:topic:072608:shade"[profile="ruby:shades_vetoing"], autoupdate="false", alexa="Shade" [ retrievable="true" ], homekit="WindowCovering" [instance=3] }
Rollershutter EntryShade_Rollershutter "Entryway Shade [%d%%]" (lEntry, gWestShades, gInflux) [ "DoubleHung" ] { channel="mqtt:topic:07278E:shade"[profile="ruby:shades_vetoing"], autoupdate="false", alexa="Shade" [ retrievable="true" ], homekit="WindowCovering" [instance=3] }
... <~30 shades in total>

As you can see, I use the Rollershutter item that the command is destined for to find a related Contact item, and veto (or simply tweak) the command if the associated window or door is open. I could possibly work around this by using something like Rollershutter EntryShade_Rollershutter ... { channel="mqtt:topic:07278E:shade"[profile="ruby:shades_vetoing", item="EntryShade_Rollershutter"] ... and then fetching the item name from the profile configuration at runtime, but that just seems to violate a few principles of clean coding.

This seems like a reasonable thing to do from a user-provided profile. But definitely not a profile that's in core, and probably not one provided by a binding or other addon. I'm also only accessing the Item, not the link itself, or the channel, but I could see someone writing a profile on a multiply-linked item that would want to behave differently depending on which channel is sending the update to the item. Perhaps only exposing the item and the channel, and not the link itself? I don't know that that helps the encapsulation much though.

Also, I was just checking things, and the ProfileContext sent to the ProfileFactory does already include the acceptedCommandTypes, acceptedDataTypes, and handlerAcceptedCommandTypes. Not quite complete type information, but close.

@kaikreuzer
Copy link
Member

Ok, I see from where you're coming.
Such custom/user profiles are more or less a way to create rules that intercept events from bindings and allows to tweak them individually. This is indeed nothing that system profiles are ever supposed to do, but I indeed see the power of such a feature for the users when using scripts. So let's merge and hope that we don't see too much misuse of it. 😬

@kaikreuzer kaikreuzer merged commit e2bcdcc into openhab:main Dec 11, 2022
@kaikreuzer kaikreuzer added this to the 3.4 milestone Dec 11, 2022
@florian-h05
Copy link
Contributor

@ccutrer Are there any docs which describe how to use JSR223 languages in profiles? I’d like to try this out in JavaScipt.

@J-N-K
Copy link
Member

J-N-K commented Dec 11, 2022

Using JSR-223 languages in profiles would have been possible by re-using the SCRIPT transformation. Unfortunately we couldn't agree on a solution in #2872.

This PR is more important if you want to programmatically create profiles from languages.

@florian-h05
Copy link
Contributor

Okay, thanks for the information.

I think I found the parts of the JRuby lib where the logic for creating profiles from languages is, maybe I‘ll also add that functionality to the JavaScript lib.

@J-N-K
Copy link
Member

J-N-K commented Dec 11, 2022

IMO this will result in a great mess. Instead of having one way to create scripting profiles we'll now have tons of different ways for each scripting language.

@florian-h05
Copy link
Contributor

That‘s true. Then maybe means it‘s unlikely to happen, I won‘t provide such a solution in the JavaScript library. And some point, we will maybe get a SCRIPT profile to solve that in a common way.

@kaikreuzer
Copy link
Member

IMO this will result in a great mess. Instead of having one way to create scripting profiles we'll now have tons of different ways for each scripting language.

Would you prefer to revert this change and rediscuss it, @J-N-K?

@J-N-K
Copy link
Member

J-N-K commented Dec 11, 2022

I'm fine with this change, I think there might be a use-case for programmatically adding profiles for very specific use-cases.

I just think that using scripting for a profile should be more easier than writing a script to add another script as profile. And this approach should not be promoted as the way to do it for general use.

My comment was more directed to @florian-h05 and adding this to JS Scripting. I would prefer to have standard way and this as "last resort" if the standard is not sufficient.

@ccutrer ccutrer deleted the profile_context-link branch December 11, 2022 15:10
@ccutrer
Copy link
Contributor Author

ccutrer commented Dec 11, 2022

Okay, thanks for the information.

I think I found the parts of the JRuby lib where the logic for creating profiles from languages is, maybe I‘ll also add that functionality to the JavaScript lib.

https://github.com/ccutrer/openhab-jrubyscripting/blob/main/lib/openhab/core/profile_factory.rb is the interface you have to implement and the factory you have to register for openHAB to even care to create profiles from your code (I nested the former inside the latter). The processEvent method combines all of "hook" methods, and calls back to the user-defined block, which is provided at https://github.com/ccutrer/openhab-jrubyscripting/blob/5c8ff249d979cd1ca9545db09927e246b962d006/lib/openhab/dsl.rb#L97. But keep in mind this currently just my own fork.

@ccutrer
Copy link
Contributor Author

ccutrer commented Dec 11, 2022

I'm fine with this change, I think there might be a use-case for programmatically adding profiles for very specific use-cases.

I just think that using scripting for a profile should be more easier than writing a script to add another script as profile. And this approach should not be promoted as the way to do it for general use.

I definitely agree here. Most users will just want to select a SCRIPT profile in the UI, and select which script to run (which is also created in the UI somewhere). There are a few competing PRs on the best way to do that, though, and they haven't made it into 3.4.0. I view using a construct from a JSR223 helper library to create profiles is akin to generating items from JSR223 scripts - only for the most power-user of users.

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
…ed to (openhab#3151)

So that they can vary their processing based on the item or channeluid itself,
instead of (or in addition to) solely on the prior states passing through it.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
GitOrigin-RevId: e2bcdcc
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.

5 participants