[Serial binding] OH2 Compatibility issue with RegexTransformation service #5184

marekhalmo · 2017-05-03T11:37:45Z

When Serial binding is used in OH2 there is no compatible interface to fill the regex transformation to and therefore the Regex functionality is absent if serial binding is used in OH2

Explanation
This is because OH2 RegexTransformationService implements interface org.eclipse.smarthome.core.transform.TransformationService but Serial binding requires implementation of interface org.openhab.core.transform.TransformationService available only in OH1

Because there is no way of including org.eclipse.smarthome.core.transform.TransformationService in the OH1 addons and OH2 will never have org.openhab.core.transform.TransformationService regex implementation the use of transformation service has to be removed and substituted with local code which is a slightly modified RegexTransformationService
This has also the benefit of REGEX functionality working even without the Regex transformation installed

Note
The OH2 RegexTransformationService can't fully cover the required cases for serial devices as serial device with fast communication can send multiple commands to buffer.

Example
Device sends temperature in format "temp:%C" (e.g. "temp:15C\n")
The corresponding regex would be: REGEX(temp:([0-9]{1,3}C)
If device sends temperature too fast, the Serial binding would receive "temp:15C\ntemp:16C\n"
in this case the original regex won't match the regex and therefore won't update the item at all.
This way both values are ignored!

Original issue and discussion can be found here

This patch fixes both issues

A jar for everybody to test can be found here

…H2 + Migrated functionality of OH2 RegexTransformationService to OH1 addon + Regex in serial binding can now handle fast rate

9037568

Thanks. Looks pretty good.

I think we need to get a few reviews from existing installations of serial binding users. You should ask for reviewers in the community forum.

9037568 · 2017-05-06T14:52:27Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

@@ -0,0 +1,130 @@
+/**
+ * Copyright (c) 2010-2016 by the respective copyright holders.


9037568 · 2017-05-06T14:53:12Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+ * Pattern matcher to match and cache patterns
+ *
+ * @author Marek Halmo
+ * @see RegexPatternMatcher#getMatches getMatches()


Add an "@SInCE 1.10.0" below "@author"

9037568 · 2017-05-06T14:55:27Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+
+    /**
+     * Returns true if given regExpression is a substitution pattern
+     * This fact is indicated by giving


"indicated by giving" ... what? Please complete this sentence.

9037568 · 2017-05-06T14:57:03Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+    }
+
+    /**
+     * Caches given regexpression, if the expression is substitution pattern it marks is as substitution


"Caches given regExpression. If the expression is a substitution pattern, it marks it as a substitution."

9037568 · 2017-05-06T14:58:29Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+     */
+    public static String[] getMatches(String regExpression, String source) throws TransformationException {
+        if (regExpression == null || source == null) {
+            throw new TransformationException("the given parameters 'regex' and 'source' must not be null");


"The given" ...

9037568 · 2017-05-06T15:02:24Z

...g.openhab.binding.serial/src/main/java/org/openhab/binding/serial/internal/SerialDevice.java

                                        }
+                                    } catch (TransformationException e) {
+                                        logger.error("Unable to transform!", e);


This should be a warn instead of an error.

Not shure, TransformationException is thrown only if you provide null pattern or result.
If the transformation fails due to some regex matching, it will be properly shown as a warning logger.warn("Unable to convert ....

See the logging guidelines.

OK, based on guidelines you are right.. changing to warn

marekhalmo · 2017-05-06T19:00:45Z

Thanx Chris.

I will check the forums when i have a bit of time.. i posted the jar here and on an issue report.. so a good feedback from users using regex would be great. I don't want to break compatibility..

marekhalmo · 2017-05-15T07:55:01Z

Hello,
the community test request is here

Did anyone try the provided JAR?

marekhalmo · 2017-05-16T14:05:24Z

Fixed, could you review again?

9037568 · 2017-05-16T23:22:52Z

....openhab.binding.serial/src/main/java/org/openhab/binding/serial/internal/SerialBinding.java

@@ -135,7 +140,7 @@ public String getBindingType() {
    @Override
    public void validateItemType(Item item, String bindingConfig) throws BindingConfigParseException {
        if (!(item instanceof SwitchItem || item instanceof StringItem || item instanceof NumberItem
-                || item instanceof RollershutterItem)) {
+                || item instanceof RollershutterItem || item instanceof ContactItem || item instanceof DimmerItem)) {
            throw new BindingConfigParseException("item '" + item.getName() + "' is of type '"


Because of the changes to supported item types, this throw message should probably be updated to something like "Items of type {} are not supported."

I will modify the message to include proper types, but i thing the user needs to be informed of the wrong item name so i will keep the old format.

9037568 · 2017-05-16T23:28:04Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+ */
+public class RegexPatternMatcher {
+    private static final Logger logger = LoggerFactory.getLogger(RegexPatternMatcher.class);
+    private static HashMap<String, Pattern> patternCache = new HashMap<String, Pattern>();


new HashMap<>();

9037568 · 2017-05-16T23:29:17Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+    private static final Pattern SUBST_PATTERN = Pattern.compile("^s/(.*?[^\\\\])/(.*?[^\\\\])/(.*)$");
+
+    /**
+     * Returns compiled pattern for specific regeExpression that is stored in patternCache


"regeExpression" should say "regExpression", or possibly "regular expression".

9037568 · 2017-05-16T23:32:50Z

...ab.binding.serial/src/main/java/org/openhab/binding/serial/internal/RegexPatternMatcher.java

+        }
+
+        Matcher matcher = getPattern(regExpression).matcher(source.trim());
+        List<String> results = new ArrayList<String>();


= new ArrayList<>();

9037568 · 2017-05-16T23:36:26Z

....openhab.binding.serial/src/main/java/org/openhab/binding/serial/internal/SerialBinding.java

+
+        if (parameterSplitterAt > 0) {
+            String[] split = bindingConfig.substring(parameterSplitterAt + 1, bindingConfig.length() - 1).split("\\),");
+            if (split.length > 0) {


Redundant. Should remove this if block.

marekhalmo · 2017-05-18T09:13:59Z

Ok.. fixes made. I added even more comments so the functionality is clear + found&fixed a small bug for the replacements.

Feel free to review again

marekhalmo · 2017-05-23T11:59:18Z

Hello, the reporter of the issue tested the jar, and says it's working. There is not much attention to the thread.. What is next?

Will this get merged?

9037568 · 2017-05-23T12:14:26Z

Yes, looks good to me. Thanks, @marekhalmo !

marekhalmo · 2017-05-23T12:15:48Z

Thank you!

marekhalmo · 2017-07-05T12:38:53Z

Hello @kaikreuzer , can you point me to the issue with this? Why did you label this with a bug?
Or did you mark this as a bug that is fixed in next release?

kaikreuzer · 2017-07-06T13:59:04Z

Or did you mark this as a bug that is fixed in next release?

Correct, I marked it as "bug" as it has been delivered with the 1.10.0 release and through this label it turned up in the release notes.

Marek Halmo added 3 commits May 3, 2017 13:08

OH2 Compatibility - Removed transformation service not available in O…

defa8ae

…H2 + Migrated functionality of OH2 RegexTransformationService to OH1 addon + Regex in serial binding can now handle fast rate

Fixed link in README.md

cb21796

Removed serial binding + RegexPatternMatcher comments update

30d13f1

marekhalmo mentioned this pull request May 3, 2017

No transformation service available openhab/openhab-core#128

Closed

9037568 suggested changes May 6, 2017

View reviewed changes

9037568 added the awaiting-feedback label May 6, 2017

Pull request review fixes, Added support for contact and dimmer

4c0e8cf

9037568 reviewed May 16, 2017

View reviewed changes

Comments and Pull request review modifications

f691479

9037568 removed the awaiting-feedback label May 23, 2017

9037568 added this to the 1.10.0 milestone May 23, 2017

9037568 merged commit 5c4c191 into openhab:master May 23, 2017

kaikreuzer added the bug label Jun 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serial binding] OH2 Compatibility issue with RegexTransformation service #5184

[Serial binding] OH2 Compatibility issue with RegexTransformation service #5184

marekhalmo commented May 3, 2017 •

edited

9037568 left a comment

9037568 May 6, 2017

9037568 May 6, 2017

9037568 May 6, 2017

9037568 May 6, 2017

9037568 May 6, 2017

9037568 May 6, 2017

marekhalmo May 15, 2017 •

edited

9037568 May 15, 2017

marekhalmo May 15, 2017

marekhalmo commented May 6, 2017

marekhalmo commented May 15, 2017 •

edited

marekhalmo commented May 16, 2017

9037568 May 16, 2017

marekhalmo May 17, 2017

9037568 May 16, 2017

marekhalmo May 17, 2017

9037568 May 16, 2017

9037568 May 16, 2017

9037568 May 16, 2017

marekhalmo commented May 18, 2017

marekhalmo commented May 23, 2017

9037568 commented May 23, 2017

marekhalmo commented May 23, 2017

marekhalmo commented Jul 5, 2017 •

edited

kaikreuzer commented Jul 6, 2017

		@@ -0,0 +1,130 @@
		/**
		* Copyright (c) 2010-2016 by the respective copyright holders.

[Serial binding] OH2 Compatibility issue with RegexTransformation service #5184

[Serial binding] OH2 Compatibility issue with RegexTransformation service #5184

Conversation

marekhalmo commented May 3, 2017 • edited

9037568 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marekhalmo May 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marekhalmo commented May 6, 2017

marekhalmo commented May 15, 2017 • edited

marekhalmo commented May 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marekhalmo commented May 18, 2017

marekhalmo commented May 23, 2017

9037568 commented May 23, 2017

marekhalmo commented May 23, 2017

marekhalmo commented Jul 5, 2017 • edited

kaikreuzer commented Jul 6, 2017

marekhalmo commented May 3, 2017 •

edited

marekhalmo May 15, 2017 •

edited

marekhalmo commented May 15, 2017 •

edited

marekhalmo commented Jul 5, 2017 •

edited