Skip to content

Commit 2169fe0

Browse files
lolodomololodomo
authored
Fix syntax for time in trigger/condition in DSL rule (#5591)
* Fix syntax for time in trigger/condition in DSL rule Avoid conflict with switch-case statement Time as "7:0" is now also supported. Fix #5586 Signed-off-by: Laurent Garnier <lg.hc@free.fr> * Optimize method buildMsgWithLineNb Signed-off-by: Laurent Garnier <lg.hc@free.fr> * Consider Copilot review comments Signed-off-by: Laurent Garnier <lg.hc@free.fr> * Consider human comments Signed-off-by: Laurent Garnier <lg.hc@free.fr> --------- Signed-off-by: Laurent Garnier <lg.hc@free.fr>
1 parent 83881e6 commit 2169fe0

5 files changed

Lines changed: 106 additions & 19 deletions

File tree

‎bundles/org.openhab.core.model.core/src/main/java/org/openhab/core/model/core/internal/ModelRepositoryImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ private boolean validateModel(String name, InputStream inputStream, List<String>
353353
case "rules":
354354
if (d instanceof AbstractValidationDiagnostic vd
355355
&& d.getSeverity() == org.eclipse.emf.common.util.Diagnostic.ERROR
356-
&& "uid".equals(vd.getIssueCode())) {
356+
&& ("uid".equals(vd.getIssueCode()) || "time".equals(vd.getIssueCode()))) {
357357
errors.add(d.getMessage());
358358
} else {
359359
warnings.add(d.getMessage());

‎bundles/org.openhab.core.model.rule.runtime/src/org/openhab/core/model/rule/runtime/internal/DSLRuleProvider.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ private String removeIndentation(String script) {
457457
} else if ("midnight".equals(id)) {
458458
cfg.put(TimeOfDayTriggerHandler.CFG_TIME, "00:00");
459459
} else {
460-
cfg.put(TimeOfDayTriggerHandler.CFG_TIME, id);
460+
cfg.put(TimeOfDayTriggerHandler.CFG_TIME, formatTime(id));
461461
}
462462
yield TriggerBuilder.create().withId(Integer.toString(triggerId++))
463463
.withTypeUID(TimeOfDayTriggerHandler.MODULE_TYPE_ID).withConfiguration(cfg).build();
@@ -505,8 +505,8 @@ private String removeIndentation(String script) {
505505
Configuration cfg = new Configuration();
506506
return switch (condition) {
507507
case TimeOfDayCondition todCond -> {
508-
cfg.put(TimeOfDayConditionHandler.CFG_START_TIME, todCond.getStart());
509-
cfg.put(TimeOfDayConditionHandler.CFG_END_TIME, todCond.getEnd());
508+
cfg.put(TimeOfDayConditionHandler.CFG_START_TIME, formatTime(todCond.getStart()));
509+
cfg.put(TimeOfDayConditionHandler.CFG_END_TIME, formatTime(todCond.getEnd()));
510510
yield ConditionBuilder.create().withId(Integer.toString(triggerId++))
511511
.withTypeUID(TimeOfDayConditionHandler.MODULE_TYPE_ID).withConfiguration(cfg).build();
512512
}
@@ -588,6 +588,21 @@ private String removeIndentation(String script) {
588588
};
589589
}
590590

591+
/**
592+
* Format a time to have hour and minute on two characters with leading 0.
593+
* "8:5" => "08:05"
594+
* "10:25" => "10:25"
595+
*
596+
* @param time a valid time with hour and minute having the format "<int>:<int>"
597+
* @return the time "<hour>:<minute>" with <hour> and <minute> on two characters with leading 0
598+
*/
599+
private String formatTime(String time) {
600+
String[] splittedTime = time.split(":", 2);
601+
int hour = Integer.parseInt(splittedTime[0]);
602+
int minute = Integer.parseInt(splittedTime[1]);
603+
return "%02d:%02d".formatted(hour, minute);
604+
}
605+
591606
@Override
592607
public void onReadyMarkerAdded(ReadyMarker readyMarker) {
593608
for (String modelFileName : modelRepository.getAllModelNamesOfType("rules")) {

‎bundles/org.openhab.core.model.rule/src/org/openhab/core/model/rule/Rules.xtext‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ EventEmittedTrigger:
6464

6565
TimerTrigger:
6666
'Time' 'cron' cron=STRING |
67-
'Time' 'is' time=('midnight' | 'noon' | TIME)
67+
'Time' 'is' time=('midnight' | 'noon' | Time)
6868
;
6969

7070
DateTimeTrigger:
@@ -112,7 +112,7 @@ Condition:
112112
;
113113

114114
TimeOfDayCondition:
115-
'Time' 'is' 'between' start=TIME 'and' end=TIME
115+
'Time' 'is' 'between' start=Time 'and' end=Time
116116
;
117117

118118
DayOfWeekCondition:
@@ -207,8 +207,8 @@ Operator:
207207
'=' | '!=' | '>' | '>=' | '<' | '<' '='
208208
;
209209

210-
terminal TIME:
211-
(('0'..'1') ('0'..'9') ':' ('0'..'5') ('0'..'9')) | ('2' ('0'..'3') ':' ('0'..'5') ('0'..'9'))
210+
Time:
211+
INT ':' INT
212212
;
213213

214214
SIGNED_INT:

‎bundles/org.openhab.core.model.rule/src/org/openhab/core/model/rule/validation/RulesValidator.java‎

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,22 @@
1212
*/
1313
package org.openhab.core.model.rule.validation;
1414

15+
import java.util.List;
16+
import java.util.Set;
17+
18+
import org.eclipse.emf.ecore.EAttribute;
1519
import org.eclipse.emf.ecore.EObject;
1620
import org.eclipse.jdt.annotation.NonNullByDefault;
1721
import org.eclipse.jdt.annotation.Nullable;
1822
import org.eclipse.xtext.nodemodel.ICompositeNode;
23+
import org.eclipse.xtext.nodemodel.INode;
1924
import org.eclipse.xtext.nodemodel.util.NodeModelUtils;
2025
import org.eclipse.xtext.validation.Check;
2126
import org.openhab.core.automation.util.RuleUtil;
2227
import org.openhab.core.model.rule.rules.Rule;
2328
import org.openhab.core.model.rule.rules.RulesPackage;
29+
import org.openhab.core.model.rule.rules.TimeOfDayCondition;
30+
import org.openhab.core.model.rule.rules.TimerTrigger;
2431

2532
/**
2633
* This class contains custom validation rules.
@@ -32,24 +39,89 @@
3239
@NonNullByDefault
3340
public class RulesValidator extends AbstractRulesValidator {
3441

42+
private static final Set<String> ALLOWED_TIME_NAMES = Set.of("midnight", "noon");
43+
3544
@Check
3645
public void checkRuleUID(@Nullable Rule rule) {
3746
String uid;
3847
if (rule == null || (uid = rule.getUid()) == null) {
3948
return;
4049
}
4150
if (!RuleUtil.isValidRuleUID(uid)) {
42-
error(buildMsgWithLineNb(rule, "Rule UID '" + uid
43-
+ "' is invalid. A rule UID can't contain '/', '\\' or have leading or trailing whitespace."), rule,
44-
RulesPackage.Literals.RULE__UID, "uid");
51+
error(buildMsgWithLineNb("Rule UID '" + uid
52+
+ "' is invalid. A rule UID can't contain '/', '\\' or have leading or trailing whitespace.", rule,
53+
RulesPackage.Literals.RULE__UID),
54+
RulesPackage.Literals.RULE.getEStructuralFeature(RulesPackage.RULE__UID), "uid");
4555
}
4656
}
4757

48-
private static String buildMsgWithLineNb(EObject object, String msg) {
49-
ICompositeNode node = NodeModelUtils.getNode(object);
50-
if (node == null) {
51-
return msg;
58+
@Check
59+
public void checkTimerTrigger(@Nullable TimerTrigger timeTrigger) {
60+
String time;
61+
if (timeTrigger == null || (time = timeTrigger.getTime()) == null) {
62+
return;
63+
}
64+
if (ALLOWED_TIME_NAMES.contains(time)) {
65+
return;
66+
}
67+
if (!isValidTime(time)) {
68+
error(buildMsgWithLineNb("time '" + time
69+
+ "' in trigger is invalid. It must comply with (H)H:MM format with a range of 0 to 23 for the hour and a range of 0 to 59 for the minute.",
70+
timeTrigger, RulesPackage.Literals.TIMER_TRIGGER__TIME),
71+
RulesPackage.Literals.TIMER_TRIGGER.getEStructuralFeature(RulesPackage.TIMER_TRIGGER__TIME),
72+
"time");
73+
}
74+
}
75+
76+
@Check
77+
public void checkTimeOfDayCondition(@Nullable TimeOfDayCondition timeOfDayCondition) {
78+
String start, end;
79+
if (timeOfDayCondition == null || (start = timeOfDayCondition.getStart()) == null
80+
|| (end = timeOfDayCondition.getEnd()) == null) {
81+
return;
5282
}
83+
if (!isValidTime(start)) {
84+
error(buildMsgWithLineNb("start time '" + start
85+
+ "' in condition is invalid. It must comply with (H)H:MM format with a range of 0 to 23 for the hour and a range of 0 to 59 for the minute.",
86+
timeOfDayCondition, RulesPackage.Literals.TIME_OF_DAY_CONDITION__START),
87+
RulesPackage.Literals.TIME_OF_DAY_CONDITION
88+
.getEStructuralFeature(RulesPackage.TIME_OF_DAY_CONDITION__START),
89+
"time");
90+
}
91+
if (!isValidTime(end)) {
92+
error(buildMsgWithLineNb("end time '" + end
93+
+ "' in condition is invalid. It must comply with (H)H:MM format with a range of 0 to 23 for the hour and a range of 0 to 59 for the minute.",
94+
timeOfDayCondition, RulesPackage.Literals.TIME_OF_DAY_CONDITION__END),
95+
RulesPackage.Literals.TIME_OF_DAY_CONDITION
96+
.getEStructuralFeature(RulesPackage.TIME_OF_DAY_CONDITION__END),
97+
"time");
98+
}
99+
}
100+
101+
private static boolean isValidTime(String time) {
102+
String[] splittedTime = time.split(":", 3);
103+
if (splittedTime.length != 2) {
104+
return false;
105+
}
106+
try {
107+
int hour = Integer.parseInt(splittedTime[0]);
108+
int minute = Integer.parseInt(splittedTime[1]);
109+
return hour >= 0 && hour <= 23 && minute >= 0 && minute <= 59;
110+
} catch (NumberFormatException e) {
111+
return false;
112+
}
113+
}
114+
115+
private static String buildMsgWithLineNb(String msg, EObject object, EAttribute attribute) {
116+
List<INode> nodes = NodeModelUtils.findNodesForFeature(object, attribute);
117+
if (nodes != null && nodes.size() >= 1) {
118+
return buildMsgWithLineNb(msg, nodes.getFirst());
119+
}
120+
ICompositeNode node = NodeModelUtils.getNode(object);
121+
return node != null ? buildMsgWithLineNb(msg, node) : msg;
122+
}
123+
124+
private static String buildMsgWithLineNb(String msg, INode node) {
53125
int startLine = node.getStartLine();
54126
int endLine = node.getEndLine();
55127
return (startLine == endLine) ? "Line " + startLine + ": " + msg

‎itests/org.openhab.core.model.rule.tests/src/main/java/org/openhab/core/model/rule/runtime/DSLRuleProviderTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public void testAllTriggers() {
194194
" System started or\n" + //
195195
" Time is noon or\n" + //
196196
" Time is midnight or\n" + //
197-
" Time is 20:57 or\n" + //
197+
" Time is 8:57 or\n" + //
198198
" Time cron \"0 0/1 * * * ?\" or\n" + //
199199
" Item X received command ON or\n" + //
200200
" Item Y received update \"A\" or\n" + //
@@ -233,7 +233,7 @@ public void testAllTriggers() {
233233
assertThat(rule.getTriggers().get(2).getConfiguration().get(TimeOfDayTriggerHandler.CFG_TIME), is("00:00"));
234234

235235
assertThat(rule.getTriggers().get(3).getTypeUID(), is(TimeOfDayTriggerHandler.MODULE_TYPE_ID));
236-
assertThat(rule.getTriggers().get(3).getConfiguration().get(TimeOfDayTriggerHandler.CFG_TIME), is("20:57"));
236+
assertThat(rule.getTriggers().get(3).getConfiguration().get(TimeOfDayTriggerHandler.CFG_TIME), is("08:57"));
237237

238238
assertThat(rule.getTriggers().get(4).getTypeUID(), is(GenericCronTriggerHandler.MODULE_TYPE_ID));
239239
assertThat(rule.getTriggers().get(4).getConfiguration().get(GenericCronTriggerHandler.CFG_CRON_EXPRESSION),
@@ -298,7 +298,7 @@ public void testAllConditions() {
298298
"when\n" + //
299299
" Item X received command ON\n" + //
300300
"but only if\n" + //
301-
" Time is between 19:20 and 22:10 and\n" + //
301+
" Time is between 7:20 and 22:10 and\n" + //
302302
" Day is Thursday, Tuesday, Sunday and\n" + //
303303
" Day is weekday and\n" + //
304304
" Day is weekend and\n" + //
@@ -332,7 +332,7 @@ public void testAllConditions() {
332332

333333
assertThat(rule.getConditions().getFirst().getTypeUID(), is(TimeOfDayConditionHandler.MODULE_TYPE_ID));
334334
assertThat(rule.getConditions().getFirst().getConfiguration().get(TimeOfDayConditionHandler.CFG_START_TIME),
335-
is("19:20"));
335+
is("07:20"));
336336
assertThat(rule.getConditions().getFirst().getConfiguration().get(TimeOfDayConditionHandler.CFG_END_TIME),
337337
is("22:10"));
338338

0 commit comments

Comments
 (0)