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

Idea: Parameterized thing label in thing builder. #323

Closed
clinique opened this issue Aug 13, 2024 · 12 comments · Fixed by #324
Closed

Idea: Parameterized thing label in thing builder. #323

clinique opened this issue Aug 13, 2024 · 12 comments · Fixed by #324

Comments

@clinique
Copy link

clinique commented Aug 13, 2024

I'm going my way with JRuby and I love it.
So much that I completely switched Thing creation to its ruby version taking the advantage of dynamically create things and using variables for this - far more flexible than traditional .things files.

I produced the following code that could maybe be interesting to consider for insertion in the thing builder:

# MyThing encapsulates thing class
class MyThing
  extend Forwardable

  def initialize(thing)
    @thing = thing
    @original_label = label
    @ids = uid.to_s.split(':')
  end

  attr_reader :thing

  def_delegators :@thing, :label, :location, :disable, :configuration, :uid, :status

  def bridge?
    thing.class.name.to_s.include?('BridgeImpl')
  end

  def reformat_label
    thing.configuration[:original_label] = @original_label
    thing.set_label parameterized_string(original_label)
  end

  def binding
    @ids[0]
  end

  def thing_type
    @ids[1]
  end

  def thing_id
    @ids[@ids.length - 1]
  end

  def parameter_to_string(key)
    case key
    when 'location' then location
    when 'type' then thing_type.capitalize
    when 'binding' then binding.capitalize
    when 'id' then thing_id.capitalize
    else
      element = thing.configuration[key.to_sym]
      element&.to_s
    end
  end

  def parameterized_string(label)
    new_label = label
    label.scan(/<([^>]+)>/).flatten.each do |key|
      new_label.gsub!("<#{key}>", parameter_to_string(key))
    end
    new_label
  end

  def parameterized?(chaine)
    chaine['<']
end

end

This enables me to make this type of thing creation:

things.build do
.
.
      bridge '1', '<type> <location>',
             binding: 'netatmo', type: 'welcome',
             location: 'Dining Room', config: {
               id: '70:ee:50:1d:16:73',
               icon: 'camera', tag: 'Camera'
             } 
.
end

This produces a thing labelled "Welcome Dining Room"

@clinique
Copy link
Author

Another usefull thing I found is that passing extra symbols to config hash works very well and I use it to store extra properties for each thing.

@jimtng
Copy link
Contributor

jimtng commented Aug 13, 2024

I don't fully understand the use case, but you could do something like this without the whole custom class:

things.build do
  type = "welcome"
  location = "Dining Room"
  bridge "1", "#{type} #{location}",
         binding: "netatmo", type:, location:, config: {
           id: "70:ee:50:1d:16:73",
           icon: "camera", tag: "Camera"
         }
end

@clinique
Copy link
Author

clinique commented Aug 13, 2024

Sure but this makes type and location global, not local to the thing.
My custom class only exists because I did not do it directly in the thing builder.
Currently I create things using the build and edit/save it after with this helper class.
My proposal would have been to push interesting parts (delimiter parser et label elements replacement) of it in the thing builder.
It also enables to use a config element as a parameter in the label :

    thing 'A1', 'Marmitek <subType> SS13 06B7 <deviceId>', type: 'lighting1',
                                         location: 'Garage',
                                         config: { deviceId: 'A.1', subType: 'X10',
                                                   icon: 'gate', ignoreMonitoring: true }

@clinique
Copy link
Author

Behind this, the thing name is quite important for me because I have a second logic that auto(or not) creates groups for corresponding things (reason why you see icon and tag entries in the thing configuration). Then the group label directly inherits the thing label. The same parameterized system is used to generate the group name.

This is quite practical and grandly reduces maintenance when renaming things or groups over large amount of the same things.

@clinique
Copy link
Author

Here's what I currently do after the thing builder if it's of any interest for you :

things.each do |thing|
  my_thing = MyThing.new(thing)

  my_thing.reformat_label

  if my_thing.location == PLACES.stock.display
    my_thing.disable
    BINDING.log_debug "'#{my_thing.label}' en #{PLACES.stock.display} il est donc désactivé."
  end

  # Mise en cache si nécessaire ________________________________________________
  if thing.configuration[:sharedCache] == true
    thing_name = my_thing.to_symbol
    shared_cache[thing_name] = thing
    BINDING.log_debug("Placé dans le cache:  :#{thing_name}")
  end

  # Gestion du groupe de l'item ________________________________________________
  create_group = !my_thing.skip_group_creation?
  thing.configuration[:create_group] = create_group

  if create_group
    thing.configuration[:linked_group] = my_thing.group_name
    thing.configuration[:icon] = 'none' if thing.configuration[:icon].nil?
    thing.configuration[:tag] = 'Equipment' if thing.configuration[:tag].nil?
    ALL_ITEMS << create_thing_group(my_thing)
  end

  next unless my_thing.configuration[:ignoreMonitoring] != true

  my_thing.configuration[:status_traker] = "#{my_thing.usable_name}#{Postfix::STATUS}"

  status_item = create_status_tracker(my_thing)
  ALL_ITEMS << status_item
end

@ccutrer
Copy link
Contributor

ccutrer commented Aug 13, 2024

Sure but this makes type and location global, not local to the thing.

I don't have a lot of time this week to look into this. I'm more than happy to extend things where they make sense. But I do want to point out that your example code does not create global (or even top-level local) variables. In Ruby, every block (i.e. the do/end in your example) creates a new scope, so those local variables stop existing outside of your thing builder.

@clinique
Copy link
Author

Sure but this makes type and location global, not local to the thing.

I don't have a lot of time this week to look into this. I'm more than happy to extend things where they make sense. But I do want to point out that your example code does not create global (or even top-level local) variables. In Ruby, every block (i.e. the do/end in your example) creates a new scope, so those local variables stop existing outside of your thing builder.

Sorry, should have said external to the thing instead of global. Happy to ear from you once you'll have time to take a look at it.

@ccutrer
Copy link
Contributor

ccutrer commented Aug 14, 2024

I've read through this more fully now. I think how you're doing it is exactly how you should do it - you're adding additional meaning to things like the label, and in order to facilitate that you're wrapping classes already in the library. Most of your additions are not generically applicable to openHAB users as a whole - only someone that specifically structures their things and labels like you do, and then wants to derive additional meaning from that structure would get use out of this. My own rules files have several similar things that I know only apply to how I do things, and wouldn't be useful for others.

To enumerate your additions to Thing (via your wrapper class):

  • bridge? - these is likely the most usable addition. I would say it's easily done with is_a?(Bridge), but we don't currently expose that interface easily in the library. Probably the most important addition we could make.
  • binding - this should probably be a method we add to ThingUID (and ThingTypeUID), and ideally in openhab-core, and then we just document in the library.
  • thing_type - Thing already has thing_type_uid (but to get the bare id, it would be thing_type_uid.id. Also, assuming the second segment of a ThingUID is the thing type is a false assumption (as I learned myself when I tried adding such a method on ThingTypeUID to openhab-core!) - a thing's type can change, without its UID changing. thing_type_uid continues to be correct in this case, but uid.all_segments[1] (or [[2] if the thing has a bridge) no longer has any connection to the thing's type.
  • thing_id - just use uid.id
  • parameter_to_string/parameterized_string - both of these are specific to your implementation. and as @jimtng originally pointed out - these are more commonly done in Ruby by simply using string interpolation directly, instead of scanning the string and doing replacements.

@jimtng
Copy link
Contributor

jimtng commented Aug 14, 2024

@jimtng
Copy link
Contributor

jimtng commented Aug 15, 2024

  element = thing.configuration[key.to_sym]  # there's no need to call to_sym here, because it gets converted back to string anyway inside the helper library

So

    thing.configuration[:original_label] = @original_label
    # stored as thing.configuration["original_label"] 

Some suggestions, mainly as general Ruby tips, and not specifically about what you're trying to achieve:

def parameterized_string(label)
    new_label = label
    label.scan(/<([^>]+)>/).flatten.each do |key|
      new_label.gsub!("<#{key}>", parameter_to_string(key))
    end
    new_label
  end

new_label = label just creates another reference to the same object as label, so when you changed new_label, it changed the original label data too.

Proof:

$ irb
irb(main):001> a = "test"
=> "test"
irb(main):002> b = a
=> "test"
irb(main):003> b.upcase!
=> "TEST"
irb(main):004> a
=> "TEST"

To avoid this, you could do label.dup

irb(main):005> a = "test"
=> "test"
irb(main):006> b = a.dup
=> "test"
irb(main):007> b.upcase!
=> "TEST"
irb(main):008> a
=> "test"

A better implementation of that method:

def parameterized_string(label)
  label.gsub(/<([^>]+)>/) { parameter_to_string($1) }
end

@jimtng
Copy link
Contributor

jimtng commented Aug 15, 2024

As for your main goal, you could probably do this:

  • After creating all your things with the templated labels, then run this code:
def format_thing_label(thing)
  return unless thing.label.include? "<"

  thing.label = thing.label.gsub(/<([^>]+)>/) do
    case $1
    when "location" then thing.location
    when "type" then thing.thing_type.uid.id.capitalize
    when "binding" then thing.uid.binding_id.capitalize
    when "id" then thing.uid.id.capitalize
    else thing.configuration[$1].to_s
    end
  end
end

things.each { |thing| format_thing_label(thing) }

@clinique
Copy link
Author

Thanks for your insights @jimtng - I'll integrate them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants