boc-tothefuture / openhab-jruby

JRuby Libraries for Openhab
Eclipse Public License 2.0
6 stars 9 forks source link

execution blocks per trigger #602

Closed boc-tothefuture closed 1 year ago

boc-tothefuture commented 2 years ago

I have several methods where I have multiple triggers tied a single rule and the rule is the correct logical grouping for those triggers.

However, the execution logic varies for each trigger, I have been solving this indirectly with attachments.

rule 'Notify on HVAC not cooling' do
  changed Thermostats_Upstairs_Status, to: 2, attach: -> { after(2.hours, :id => :hvac) { notify "HVAC not cooling after 2 hours" }
  changed Thermostats_Upstairs_Status, from: 2, attach: -> { logger.info "Cancel HVAC Timer"; timers[:hvac]&.cancel }
  changed Thermostats_Upstairs_Temp, attach: -> { logger.info "Reschedule HVAC Timer";  timers[:hvac]&.reschedule }
  run { |event| event.attachment.call }
end

This works and could just be a pattern that is supported. I started to go down the path of an "implicit block run". In other words if there is no run block and every trigger is just a lambda/proc then automatically invoke event.attachment Procs are probably better here than lambda due to the argument matching requirements.

That would end up looking looking like this:

rule 'Notify on HVAC not cooling' do
  changed Thermostats_Upstairs_Status, to: 2, attach: -> { after(2.hours, :id => :hvac) { notify "HVAC not cooling after 2 hours" }
  changed Thermostats_Upstairs_Status, from: 2, attach: -> { logger.info "Cancel HVAC Timer"; timers[:hvac]&.cancel }
  changed Thermostats_Upstairs_Temp, attach: -> { logger.info "Reschedule HVAC Timer";  timers[:hvac]&.reschedule }
end

However, this relies on some magic and knowing that no run block and all proc attachments results in the proc being run. Therefore, I was considering adding a 'run' to triggers.

rule 'Notify on HVAC not cooling' do
  changed Thermostats_Upstairs_Status, to: 2, run: -> { after(2.hours, :id => :hvac) { notify "HVAC not cooling after 2 hours" }
  changed Thermostats_Upstairs_Status, from: 2, run: -> { logger.info "Cancel HVAC Timer"; timers[:hvac]&.cancel }
  changed Thermostats_Upstairs_Temp, run: -> { logger.info "Reschedule HVAC Timer";  timers[:hvac]&.reschedule }
end

@jimtng which do you like better? As it is now (and you just construct the run block to call the proc attachment), the automatically constructed run block when no run blocks exist and all attachments are procs, or the third option where you can specify a run block with each trigger.

On the third option, if a run block is specified that will be run in addition to any specified run block, so that would be used for to match all blocks.

jimtng commented 2 years ago

Have you considered simply using terse rules?

This looks very similar to

changed(Thermostats_Upstairs_Status, to: 2) { after(2.hours, :id => :hvac) { notify 'HVAC not cooling after 2 hours' } }
changed(Thermostats_Upstairs_Status, from: 2) { logger.info 'Cancel HVAC Timer'; timers[:hvac]&.cancel }
changed(Thermostats_Upstairs_Temp) { logger.info 'Reschedule HVAC Timer'; timers[:hvac]&.reschedule }
boc-tothefuture commented 2 years ago

I did think about it going with just the terse rules.

There was something that I liked about grouping like executions together in a single rule (for logging, etc). But it may not be worth it.