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

[Rules] [Feature Request] Adding support for substring and hex parsing #2828

Merged
merged 9 commits into from
Feb 24, 2020

Conversation

crathje
Copy link
Contributor

@crathje crathje commented Dec 20, 2019

Adding support for

  • substring
  • strtol (useful for converting hex or binary numbers)
  • 100ths division
    in eventvalue parsing.

Using this modification I am able to handle the messages sent from Opentherm Gateway ( http://otgw.tclcode.com/ ) and to set variables of a dummy device in rules.

Example:

  • Message coming from the serial interface: T101813C0
    • The B denotes that the message is from the
    • The next 4 bytes (actually 2bytes hex encoded) denote the status and type of the message.
    • the last 4 bytes (actually 2bytes hex encoded) denote the payload.
  • Message that ends up in rules when using ser2net (P020) and Generic handling: !Serial#BT101813C0

The room temperature in this sample is 19.75°C

To get this I need to access the last four bytes in packs of two bytes:
[substring:13:15:%eventvalue%]
and
[substring:15:17:%eventvalue%]

Parsing them to decimal representation each (using a base 16 call to strtol):
[strtol:16:[substring:13:15:%eventvalue%]]
and
[strtol:16:[substring:15:17:%eventvalue%]]

Last but not least the fraction is not correct, it needs to be divided by 256 (and multiplied by 100) with a call to div100ths:
[div100ths:[strtol:16:[substring:15:17:%eventvalue%]]:256]

And now a complete rule that I use to parse this and set a variable in a dummy device:

// Room temperature
on !Serial#T1018* do
TaskValueSet 2,1,[strtol:16:[substring:13:15:%eventvalue%]].[div100ths:[strtol:16:[substring:15:17:%eventvalue%]]:256]
endon

Now the variable can be used in the rules/controller/log/whatever.

* substring
* strtol (useful for converting hex or binary numbers)
* 100ths division
in eventvalue parsing.

Using this modification I am able to handle the messages sent from Opentherm Gateway ( http://otgw.tclcode.com/ ) and to set variables of a dummy device in rules.

Example:
* Message coming from the serial interface: T101813C0
** The B denotes that the message is from the boiler.
** The next 4 bytes (actually 2bytes hex encoded) denote the status and type of the message.
** the last 4 bytes (actually 2bytes hex encoded) denote the payload.
* Message that ends up in rules when using ser2net (P020) and Generic handling: !Serial#BT101813C0

The room temperature in this sample is 19.75°C

Rule used to parse this and set a variable in a dummy device:
// Room temperature
on !Serial#T1018* do
TaskValueSet 2,1,[strtol:16:[substring:13:15:%eventvalue%]].[div100ths:[strtol:16:[substring:15:17:%eventvalue%]]:256]
endon

Now the variable can be used in the rules/controller/log/whatever.
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
… the actual wildcard and not up to the wildcard.
Merged the functions as suggested by TD-er
Copy link
Contributor Author

@crathje crathje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case it gets dismissed: I fixed another issue regarding the wildcard matching in crathje@031888f

src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
src/ESPEasyRules.ino Outdated Show resolved Hide resolved
Checking for closing character ].
Switched to if else if.
Copied command string to get a lowercase representation.
TD-er added a commit to TD-er/ESPEasy that referenced this pull request Feb 24, 2020
@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

I fixed a merge conflict, created documentation and will very likely merge it tomorrow.

For any future pull request you may want to make.
Please create a branch on your own fork on which you will commit your changes and then create a pull request for that branch.
Now you were working on the main branch and if you would later had made any changes, those would also have been included in this pull request.

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

OK, I'm now testing it a bit more and these transforms do not work well on [taskname#varname], so I will try to fix those as well.

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

Hmm this looks like it is causing a major rewrite of the rules parsing engine.
The reason is we now parse a line from left to right and replace whatever we come across, while your new commands require a more depth-first approach.
The parameters you need are in the back of the command, or even nested.

So what should be done here is something like this: (just written here as memory dump, not tested)

String parseline(const String& input, int start, int& end, bool useURLencode) {
  int newStart = input.indexOf('[', start + 1);
  if (newStart != -1) {
    int newEnd = end;
    String subResult = parseline(input, newStart, newEnd, useURLencode);
    int sectionEnd = input.indexOf(']', newEnd);
    if (sectionEnd != -1) {
      // Found a closing bracket, so we have to interpret the content of this section.
      end = sectionEnd;
      String section = input.substring(start, newStart);
      section += subResult;
      section += input.substring(newEnd, sectionEnd);
      // interpret content
      return parseTemplate_padded(section, useURLencode);
    }
    addLog(LOG_LEVEL_ERROR, F("Error: bracket mismatch");
  }
  return input.substring(start, end);
}

This is just a dump of what I'm now thinking of and it does also need quite a bit of rework on the parseTemplate function as well, since we're essentially making the parser quite a bit more flexible.
But the problem is that recursive calls do need more stack and that's maybe already quite limited and may lead to crashes.
Also it does need quite a bit of String allocations.

N.B. this suggested parseline should also check if there is another opening bracket left, so it needs a while loop and also it does not take into account it can already be the inner part, so this will not work yet, but I think you will get the idea.

Edit:
Made an issue for it: #2899

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

Just couldn't let it go, so I just made it possible to use nested string commands.

So this is now also possible:

on DS-1#Temperature do
  logentry,[substring:0:2:[strtol:16:[substring:0:2:[DS-1#Temperature]][substring:3:5:[DS-1#Temperature]]]]
endon

And some debug log for illustrative purposes:

 221313 : Info  : EVENT: DS-1#Temperature=22.13
 221346 : Info  : parse_string_commands cmd: substring:0:2:22.13 -> 22
 221347 : Info  : parse_string_commands cmd: substring:3:5:22.13 -> 13
 221348 : Info  : parse_string_commands cmd: strtol:16:2213 -> 8723
 221349 : Info  : parse_string_commands cmd: substring:0:2:8723 -> 87
 221350 : Info  : ACT  : logentry,87
 221351 : Info  : Command: logentry
 221353 : Info  : 87

@TD-er TD-er merged commit 5d3813b into letscontrolit:mega Feb 24, 2020
@giig1967g
Copy link
Contributor

@TD-er
Are you sure this is the right approach?
We are adding another layer of syntax in the rules... a layer dedicated to a single device (will all respect to the OP, who will ever use the "div100ths" command?)
And maybe for just a change that helps a single device, we could have impact on memory, speed and adding potential bugs and complexity.
Also, if you go for this approach, you will need to potentially create commands inside the core for each new device on request.

Probably a more general solution should be to create a "Opentherm Gateway" plugin.
And maybe a REDEX format or command for manipulating strings.

These are my 2 cents, of course...

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

I was also a bit confused about this one: div100ths, and also thought about maybe making it one with 2 parameters (100 and 255 of this example).
But it is something that you may see in several sensors, but not always limited to a single byte.
Also the 2 decimals output is a bit strange here.

So in that respect I do share your concern about this one.

The pro for such a command is to make sure the result is a float in between steps, or else you will loose a lot of resolution.

For now, I will remove this specific one (comment out) as it may indeed lead to a lot of these.

The reason I put some work in this was also because I suddenly thought of a way to make parsing with nested brackets without using recursive calls and lots of memory allocation.
That may also be used to parse other variable notations (on which you also contributed in the past) and speed up rules where some expressions are used several times.

Apart from that there have been a number of requests to split longer numbers, like in the processing of RFID etc, so that was also very nice to see implemented by this PR.

But thanks for the critical look on the div100ths, as that's a bit too specific indeed.

@uzi18
Copy link
Contributor

uzi18 commented Feb 24, 2020

@TD-er maybe change [ ] to {} or <>

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

@TD-er maybe change [ ] to {} or <>

Just curious, why?

@uzi18
Copy link
Contributor

uzi18 commented Feb 24, 2020

because we use these chars already for something else and now your example line is completely unreadable

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

That's a valid point.
On the other hand we also have some formatting options on [task#var]
Using {} then? <> is already used in compares.

@uzi18
Copy link
Contributor

uzi18 commented Feb 24, 2020

{} left untouched
we should extend these formating options to support substring etc. to have everything consistent

maybe later we can add some user variables dedicated for strings manipulation?

@TD-er
Copy link
Member

TD-er commented Feb 24, 2020

Well the % notation I made for some standard conversions are also a bit unreadable, so maybe something like [c_to_f;<value>] may be more clear.
And in your suggestion, using {}

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 this pull request may close these issues.

4 participants