I just ran into an issue where I had copied a file into another file, changed the logic of a rule but forgot to rename the rule. The end result was that my rule was sometime not firing. It was only ‘sometimes’ because when I would edit the rules file to debug, it would live reload and then this rule would be active. It made it very difficult to figure out why the rule was not firing.
I didn’t see an open ‘issue’ or ‘request for enhancement’ on this. I would think the preferred behavior would be to either have all rules with the same name be active all the time or to log an error if a rule with a duplicate name exists. Is this something that should have an issue opened? I know what to look for now if I have a rule that sometimes fires and sometime doesn’t, but for beginners they might be confused.
This is a known problem, but of course none of us take it on board until we run into it ourselves.
It might prove difficult to achieve where rules files can be reloaded in flight, after an edit. In that circumstance the normal process would be that a new rule appears with a duplicate name and replaces an existing live version.
That’s true. With no apparent effort going into Designer, nothing is likely to happen. Shame, as its probably the right place to warn of possible conflicts rather than trying to distinguish “error” from “edit” in runtime.
Just for the sake of letting of some steam. I just had the same issue and was looking for a solution since 4 weeks ago (shortly after blindly copying the rules but with enough time and other changes in between to not see the connection). Today i spent another 6 hours rewriting a big chunk of my rules file and trying all kinds of fixes before having that lucky desperate spark and disabling the copied rules alltogether. In case someone has the same issue as me and tries to search for it i will explain her in some short sentences:
My rule doesn’t trigger on the first time after restarting openhab. all consecutive triggers get detected just fine. so the first Item or group change which functions as trigger will be ignored and only after resetting the Items/group will the next trigger be detected.
hopefully i save someone the trouble of finding it by luck until a error will be reported in the logs or some other helping solution is implemented.
Every time you edit a rule file, you replace existing rules with ones with duplicate names.
In other words the warning would go off at every rule edit, as openHAB cannot determine if you meant to replace that rule or not.
Depending what you use to edit your rules, that is much better placed to detect inadvertent duplicates.
Yep! Been there just now for a couple of weeks!
I found this thread because suddenly I had a hunch that only the rules file last updated was being active.
Searching the documentation about Rules I see no warning that rule names and all variables have to be unique even though in hindsight this is obvious. It is not sufficient that the rules file name is unique. Just took me some time to realise that…
I use VS Code for editing but I agree it would be difficult to implement a warning. But maybe an update of the documentation is something to ask for?
Come on guys, this is common, very basic knowledge to all programming languages. You must not expect natural stuff to be documented, there’s just too much of that.
Anyone can edit the official docs. There’s a link at the bottom of each page, so take action yourself and just do.
Thanks Marcus for pointing out the obvious that eluded me!
I’m not a professional programmer and to me, it seemed obvious that the file name had to be unique but not that the rule name needed to be unique across all rule files.
Come on guys, this is common, very basic knowledge to all programming languages. You must not expect natural stuff to be documented, there’s just too much of that.
That isn’t true… There are all sorts of different scoping defaults in different programming languages. OpenHab is globally scoped for everything, but I don’t know why that should be assumed. Global scoping makes sense for things/files but there is not a reason (beyond ease of implementation and consistency with things/items) to naturally assume that rules are also globally scoped. It would, in my opinion, be optimal for Openhab rules to be file scoped. Highlighting that rules are globally scoped based on rule name in the documentation wouldn’t be a bad thing to do, so I don’t know why we are arguing against it.
Not addressing the technical complexities of implementing it, but a warning when you overwrite a rule with a rule of the same name would also be helpful. I can’t imagine a use case where a user wants to do that and it is fairly easy to make that mistake and fairly hard to track down why something isn’t working once you do.
Look, I haven’t volunteered to update the documentation or implement technical improvements for this and I get that. But there seems to be a theme in this thread that users are silly for assuming it works a certain way (and different users keep hitting this) which I think is unproductive and offputting.
I am not saying it is a trivial problem to solve within the context of OpenHab or rules engine. I haven’t developed inside the rules engine… But are you proposing it is intractable?
Compilers warn on duplicate assignment all the time… A naive solution to this problem would be a multi-pass system, where the first pass keeps a list of rules across all rules file and when a duplicate is found issues a warning. Is this possible in OpenHab, I don’t know as I haven’t looked at the rules engine. Would the additional burden of lexing all rules files before processing slow down systems that already have trouble keeping up with rules loading be an additional burden that isn’t worth it? Maybe.
I am not saying it is worth the development effort to resolve this. I am not saying it wouldn’t have drawbacks… But I don’t understand the pushback that the problem is unsolvable (I think it is solved in a lot of other domains/compilers) or would be an improvement.
The current answer in my mind is “Yeah, it’s a bummer. It would probably make Openhab better if it was implemented. It could be complex to implement based on the current design. Other items are higher on the priority list. If it is important to you, feel free to contribute code or create a bounty.”
Instead there is this weird, in my opinion, pushback at some users repeatedly running into the same problem and being told either they are the problem or its impossible to fix.
Well, never mind the implementation details.
Can a person solve the issue? The person in this case is being given a paper entitled “banana”. The person may or may not already have a paper “banana”. Is this a superceding version, or did the author of the paper mean to call it “apple”?
That’s the problem to solve before the nuts and bolts stuff.
There are many solutions - currently it is “assume the author knows what they are doing”.
You might prefer “let’s ask every time”.
Of course with computers, we can be more inventive. The equivalent of forcing the author to use particular typewriter, and have that call us up every time he types a title to see if we already have that, so we can be more selective about asking “are you sure”.
There’s no right way, not even much of a better way.
We aren’t. It doesn’t hurt to state this in the docs, correct, that’s why I told the OP to add it to the docs.
You seem to expect that to be done upfront though, for all sorts of potential questions and mistakes in understanding.
But there’s so much “natural” stuff - that is just too much to list - that devs cannot think of where a user might take a wrong turn like this one. Let alone they are busy on tasks only they can accomplish and improving docs is a task almost anybody else can take on …
It’s ok to highlight as was done here, but it’s not ok to expect “someone” to stand in and correct/improve it on your behalf. Frankly, we expect any user to contribute his share … and this thread shows that virtually anyone can, thanks @BGunnarB for demonstrating.
Well. First and foremost, noone’s calling anyone silly or stupid. I don’t and no other person did.
It’s not ok you impute that to any poster here. Be more cautious with statements like that.
Second - and yes it’s a thin line and unclear where exactly it is located -, everybody needs to apply a fair amount of common sense. Every user has to contribute his share, too. That’s even more so true for an Open Source product driven by volunteers that this is. Is it too much to expect anyone to know or at least assume that if there is no specific statement on rule names or variables that they must not be duplicated ? I am not the person to decide, but to me applying common sense would mean to be cautious here and avoid that in the first place.
You may feel “put off” when you’re hit by a response like this - and that’s an individual thing how quickly you feel hit - but on the other hand, any responder also has a point there in highlighting that you may NOT expect to be warned of anything that may potentially go wrong by neither docs nor forum members.
You still need to apply common sense yourself and may not waste other’s time in not doing so.
And unlike you do, I believe this is highly productive to remind people to apply common sense in situations like this one. Way more productive than an outright answer in fact. At least every now and then and of course only as long as posters don’t violate sense of decency, respect the posting rules and the community guidelines.
And you can trust me that as a moderator I’ve seen a LOT of stupid questions, responses and threads that are WAY more unproductive, offputting, arrogant and presumptuous [not sure that’s the proper term, I’m no native speaker] and waste of time.
If you are using the python rules engine in conjunction with standard rules, the following python rule will create a warning for duplicate rule names in the traditional rule engine whenever any rule file is changed.
"""
Watch for duplicate rule names
"""
from core.triggers import DirectoryEventTrigger, ENTRY_CREATE, ENTRY_MODIFY
from core.rules import rule
from core.log import logging, LOG_PREFIX
import glob
import os
import re
##When fixed in the library, can replace this with DirectoryWatcher from openhab-helper-libraries
from java.nio.file.StandardWatchEventKinds import ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY
try:
from org.openhab.core.service import AbstractWatchService
except:
from org.eclipse.smarthome.core.service import AbstractWatchService
class JythonDirectoryWatcher(AbstractWatchService):
def __init__(self, path, callback):
AbstractWatchService.__init__(self, path)
self.event_kinds = [ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY]
self.watch_subdirectories = False
self.callback = callback
def getWatchEventKinds(self, path):
return self.event_kinds
def watchSubDirectories(self):
return self.watch_subdirectories
def processWatchEvent(self, event, kind, path):
self.callback(os.path.dirname(str(path)))
#Remove comments from rule files to prevent triggering on commented rules with duplicate names
def comment_remover(text):
def replacer(match):
s = match.group(0)
if s.startswith('/'):
return " " # note: a space and not an empty string
else:
return s
pattern = re.compile( r'//.*?$|/\*.*?\*/|\'(?:\\.|[^\\\'])*\'|"(?:\\.|[^\\"])*"', re.DOTALL | re.MULTILINE)
return re.sub(pattern, replacer, text)
#Create a map of all rule names, mapped to files, warn on any duplicate rules names
def check_duplicates(path):
logging.debug("Checking path %s for duplicate rules" % path)
rules = {}
rule_pattern = 'rule\s+[\"\'](.*)[\"\']'
for rule_file in glob.glob(os.path.join(path,"*.rules")):
logging.debug("Checking file %s" % rule_file)
with open(rule_file,"r") as file:
rule_file_text = comment_remover(file.read())
for line in rule_file_text.splitlines():
result = re.search(rule_pattern, line)
if result:
rule_name = result.group(1)
files = rules.get(rule_name, [])
files.append(rule_file)
rules[rule_name] = files
logging.debug("Added Rule \"%s\" from %s" % (rule_name,rule_file))
for rule in rules:
files = rules.get(rule)
if len(files) > 1:
#Convert to a set to Only list a file once in case rule name is duplicated in same file.
files = set(files)
logging.warn("Rule \"%s\" is duplicated in file(s) %s" % (rule,', '.join(files)))
rule_dir = os.path.join(os.environ['OPENHAB_CONF'],'rules')
ruleWatcher = JythonDirectoryWatcher(rule_dir,check_duplicates)
def scriptLoaded(id):
ruleWatcher.activate()
def scriptUnloaded():
ruleWatcher.deactivate()
Below is an updated version that warns if rule name duplicates exist across both DSL style rules and python based rules:
"""
Watch for duplicate rule names
"""
from core.triggers import DirectoryEventTrigger, ENTRY_CREATE, ENTRY_MODIFY
from core.rules import rule
from core.log import logging, LOG_PREFIX
import glob
import os
import re
import ast
import time
##When fixed in the library, can replace this with DirectoryWatcher from openhab-helper-libraries
from java.nio.file.StandardWatchEventKinds import ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY
try:
from org.openhab.core.service import AbstractWatchService
except:
from org.eclipse.smarthome.core.service import AbstractWatchService
class JythonDirectoryWatcher(AbstractWatchService):
def __init__(self, path, callback):
AbstractWatchService.__init__(self, path)
self.event_kinds = [ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY]
self.watch_subdirectories = False
self.callback = callback
def getWatchEventKinds(self, path):
return self.event_kinds
def watchSubDirectories(self):
return self.watch_subdirectories
def processWatchEvent(self, event, kind, path):
self.callback(str(path))
rules_dir = os.path.join(os.environ['OPENHAB_CONF'], 'rules')
python_rules_dir = os.path.join(os.environ['OPENHAB_CONF'], 'automation/jsr223/python/personal')
lastCheck = time.time()
#Remove comments from rule files to prevent triggering on commented rules with duplicate names
def comment_remover(text):
def replacer(match):
s = match.group(0)
if s.startswith('/'):
return " " # note: a space and not an empty string
else:
return s
pattern = re.compile( r'//.*?$|/\*.*?\*/|\'(?:\\.|[^\\\'])*\'|"(?:\\.|[^\\"])*"', re.DOTALL | re.MULTILINE)
return re.sub(pattern, replacer, text)
#Create a map of all rule names, mapped to files, warn on any duplicate rules names
def dsl_rules():
logging.debug("Checking path %s for duplicate rules" % rules_dir)
rules = {}
rule_pattern = 'rule\s+[\"\'](.*)[\"\']'
for rule_file in glob.glob(os.path.join(rules_dir,"*.rules")):
logging.debug("Checking file %s" % rule_file)
with open(rule_file,"r") as file:
rule_file_text = comment_remover(file.read())
for line in rule_file_text.splitlines():
result = re.search(rule_pattern, line)
if result:
rule_name = result.group(1)
files = rules.get(rule_name, [])
files.append(rule_file)
rules[rule_name] = files
logging.debug("Added Rule \"%s\" from %s" % (rule_name,rule_file))
return rules
def python_rules():
rules = {}
for python_rule_file in glob.glob(os.path.join(python_rules_dir,"*.py")):
logging.debug("Checking file %s" % python_rule_file)
for stmt in ast.walk(ast.parse(open(python_rule_file).read())):
if isinstance(stmt, ast.FunctionDef):
for decorator in stmt.decorator_list:
if (isinstance(decorator, ast.Call) and decorator.func.id == 'rule'):
rule_name = decorator.args[0].s
files = rules.get(rule_name, [])
files.append(python_rule_file)
rules[rule_name] = files
logging.debug("Added Rule \"%s\" from %s" % (rule_name,python_rule_file))
return rules
def check_duplicates(path):
logging.debug("Check fired because of detected change to file: %s" % path)
filename, extension = os.path.splitext(path)
if extension in ['.py','.rules']:
global lastCheck
if time.time() - lastCheck >= 1:
rules = dsl_rules()
py_rules = python_rules()
merged_keys = set(rules).union(py_rules)
merged_rules = dict( (k, rules.get(k,[]) + py_rules.get(k,[]) ) for k in merged_keys)
for rule in merged_rules:
files = merged_rules.get(rule)
if len(files) > 1:
#Convert to a set to Only list a file once in case rule name is duplicated in same file.
files = set(files)
logging.warn("Rule \"%s\" is duplicated in file(s) %s" % (rule,', '.join(files)))
lastCheck = time.time()
else:
logging.debug("Skipping multiple checks within 1 second")
else:
logging.debug("Ignorning change to file: %s" % path)
ruleWatcher = JythonDirectoryWatcher(rules_dir, check_duplicates)
pyWatcher = JythonDirectoryWatcher(python_rules_dir, check_duplicates)
def scriptLoaded(id):
ruleWatcher.activate()
pyWatcher.activate()
lastCheck = time.time()
def scriptUnloaded():
ruleWatcher.deactivate()
logging.debug("Deactivated Rule Watcher")
pyWatcher.deactivate()
logging.debug("Deactivated Py Watcher")
