Proposal 1 - extend the trigger dropdown

[macinspak]

Originally from #24, submitted by @bartbutenaers

Currently the sensor node allows two trigger types ("all messages" and "msg.payload=true"). It would be nice if extra options could be added by using TypedInputs. For example:

Then it becomes possible to trigger an alarm when msg.payload=1 or msg.payload="OPEN" or ...

That way my flow would become much cleaner, since I could avoid a lot of Change-nodes (which I also see in the example flows on your readme page):

Of course the existing options should be migrated automatically, to make sure the existing flows are not impacted.

BTW It is not clear to me how the current filtering works. Because at first sight the triggerType seems only to be used in the sensor.html file, but I might have overlooked something. Would be nice if you could explain it!

[macinspak]

Great idea, I like this. It certainly would make things easier.

Can anyone else comment on common use cases? I would think maybe even a "msg.[enter path here]" = "string value also compared as numeric" or something would meet the majority of cases, but what about more complex multi-facet matching? Ie msg.payload.value==200 && msg.tag==3 ?

perhaps it's better to allow a small code snippet space like a basic function, then you could do any logic you like?

[bartbutenaers]

even a "msg.[enter path here]" = "string value also compared as numeric" or something would meet the majority of cases

Indeed that would solved most of the cases. Hadn't even thought about that...

Although that indeed might be sufficient for lots of use cases, I see some advantages in using the TypedInputs:

• The TypedInput fields is more Node-RED standard style. If I need to check for a boolean= true then every Node-RED user knows that he needs to select a boolean TypedInput from the dropdown. Although your expression syntax is VERY easy, you have to look it up in your readme page. While the TypedInputs are more self explaining. The expression seems (to me) a bit of cheating :-)

• The TypedInputs offer some extra types of triggers. You could introduce dynamic thresholds when using the "flow" or "global" types. An simple hypothetical example:

  • Normally you want to raise an alarm when there are more than 30 people in a bar, because that is required for fire insurance.
  • During those dark COVID days you want to raise an alarm when there are more than 10 people in the same bar, to be able to insure social distance.

  So you need some dynamic threshold, which should be easy to change live (without requiring a deploy)!

  You could again workaround this by adding some logic in your flow to determine whether this is a problem or not, and then pass that boolean to your Sensor node. But the whole idea of this feature request is to simplify our flows, by getting rid of lots of Change/Function/... nodes. By using a numeric TypedInput you can simply write your threshold in Context-memory via some user interface (e.g. the Node-RED dashboard) in flow memory flow.maximumVisistors=10 and read that threshold directly from your node ....

Note that processing the values from multiple TypedInputs can be done using evaluateNodeProperty. I can always create a proposal via a PR in a couple of weeks if you like!

[macinspak]

OK, had a think about this. Maybe a very easily understandable way to do it would be to replicate the switch node functionality? People are used to this node and it will make immediate sense.

Because technically we need to come up with a boolean value (To trigger or not to trigger? That is the question).

This switch node sets the type of expression at the top and then allows the user to set multiple and/or functions to trigger. It would just be for a single "output".

What do you think?

I think this would be really cool and we already have the design we can copy directly out of the switch node html and even look at the implementation of the js to tweak to our needs. Seems to me to be the most powerful and flexible option that is ultra simple to replicate.

If you feel like it, more than happy to have help doing it. It's the whole reason I released this set of nodes, to get people to help making it the best it can be.

[macinspak]

switch.html switch.js

Note there is only a delete button and no output selection.

And maybe we could change (checking all ..) to ( all must be true / at least one must be true) or (all / any) or (and/or)?

Oh, and the semantic for no rules is trigger on any message (so it's upgrade equivalent) we default to no rules and then people can migrate to the new node over time by adding rules and deleting any intermediary nodes as they go.

So if no rules are in the list, we might show a message in that rule box saying "will trigger on any message unless you add at least one rule" or something.

[bartbutenaers]

Maybe a very easily understandable way to do it would be to replicate the switch node functionality?

That would indeed be the most powerful option. Although it would make your code a LOT more complex to maintain. You also have to take into account that the Switch-node will become much more complex in the future: for example when the compound rules will be implemented (IF ... AND ... OR ...) in the Switch node, you would have to implement the same in your node. Unless you implement your own all / any rules (like you have stated above), but then your clone of the Switch node code will grow away from the Switch node in the future (which makes it very difficult to keep in sync with the new Switch node features in the future).

Personally I would keep it a bit simple. If we could only support (equality checking of) multiple trigger value types via TypedInputs, you can e.g. wire a GPIO input (from a read relay on a door) directly to your node: 0 = trigger alarm / 1 = don't trigger alarm. That way we can already simplify quite a large number of flows, by eliminating "basic" Switch/Change nodes. For less simple operations (i.e. not a simple equality check) I would suggest to keep using a Switch/Change node: each node should do only what it is good at ;-)

Of course it is your node: if you want to clone the whole Switch node functionality, then I rest my case. But I have not enough free time to implement such a big change... If you want to keep it simple by only typedinputs, then I can try to implement a first version if you want.

This switch node sets the type of expression at the top

That is indeed something very useful, again to avoid Switch nodes. If e.g. the trigger value arrives in the input message as msg.trigger_value, then you could simply specify that msg property name. I had that already in mind as 'proposal 4', but I didn't wanted to look greedy at the time being ;-).

[macinspak]

Thanks @bartbutenaers, you are probably correct it is overkill to go the complete duplication of the switch node functions AND try and maintain it to remain current.

I was actually not thinking to that degree. I was more thinking we use the same sort of approach, and just replicate one or 2 of the most useful ones that will fill our immediate need, and it provides a launchpad and precedent for anyone to chip in if they would like to add (or just request) a function they think should also be ported to meet a use case they have (with suitable justification of course).

I also would not care if it was not maintained in step with the switch node as I don't think that will really help us.

So we start with about the same (or less effort) to port a small portion of the switch code that is essentially already written for us. Then make a note that people can request or contribute more functions if they want.

What do you think? It provides a quick solution now and more flexibility if needed in future.

Oh, so if I understand about msg.trigger_value, then you could just put that in the field at the top and this mod idea would also cover it, right? Or did I misunderstand?

[bartbutenaers]

Oh, so if I understand about msg.trigger_value, then you could just put that in the field at the top and this mod idea would also cover it, right? Or did I misunderstand?

If you specify at the top msg.blabla then indeed your node will get the trigger value from that input message 'blabla' property. By adding such a typedInput field (type 'msg') you can avoid a lot of Change nodes in a flow. So I would definately add such a typedInput to your config screen.

It provides a quick solution now and more flexibility if needed in future.

To keep it simple and yet powerful, I'm not sure anymore if we should mimic partly the Switch node. It will be a lot of development, make your node much more difficult to maintain, and we will never be able to compete with the Switch node (especially if that is extended in the near future with AND/OR logic).

I am now wondering if your initial idea of using a simple expression would be a better solution perhaps? Suppose we would e.g. use this expression parser library. Don't think it is maintained anymore, but might be sufficient for our needs.

I haven't tested it yet, but I assume we could use it like this:

1. The user specifies an expression on your config screen, for example: "msg.payload == 1 and msg.topic == reed_relay".

2. We compile the expression and pass the input message to it:

   // Compile expression from the config screen to an executable function
   var myfilter = compileExpression(expression);
   
   // Execute function (with input message as parameter)
   myfilter({msg: msg});

Does this makes sense to you?

[macinspak]

I had another look at switch node and think you are right, on my second look at it, there is a lot of capability, but probably overkill. Something a simple expression can resolve.

So did you want to have a shot at doing the node the way you think with these considerations in mind? I think we are on the same page, we need to give a basic way to make these simple cases handled, and with an expression option it allows a lot of flexibility and it's still simpler to code.

As long as it covers those cases and works well I'd be happy to merge it in and publish an update.

Thanks again for helping work this out. We certainly get a far more powerful and better solution when more people collaborate on it.

[bartbutenaers]

Sure I can have a look at it. Only thing that bothers me, is that the library is probably not maintained anymore. On the other hand we don't need lots of features from it ...

If you now a more recent javascript logical expression parser library, please let me know!

And I haven't checked if the library is available on npm, otherwise we have to copy its js file to your repo...

My time is up for today...

[bartbutenaers]

Seems there is an NPM module available with the same name and functionality, but pointing to another Github repository.

As you can see it is a fork of the original repo I mentioned above:

This package is well maintained, which is much better in case we need assistance. So I am going to try that one as soon as I have time ...

[bartbutenaers]

Currently your node has a dropdown:

Am I correct that this dropdown simply need to be replaced by an input field, where the logical trigger expression can be entered?

If so, I need to write a short code snippet for existing nodes. Just to make sure we don't have impact on existing flows.

• Existing nodes with option msg.payload.open == true will get an identical expression in the new expression editor field.
• Existing nodes with option any message will get an empty expression editor field? And we consider an empty field as 'no condition' which means always true.

Is that ok for you?

[Allow2CEO]

Sure. Sounds great!

[bartbutenaers]

Hmm, found a partystopper... Would like to use nested input message properties:

However it looks like the Filtrex library doesn't support nested object properties (since it uses hasOwnProperty):

I could workaround this, but I think it is better if I create a pull-request for the Filtrex library to implement this feature ...

[macinspak]

Sounds like it will also make their library more useful/flexible.

Sent from my iPad

On 24 Aug 2021, at 8:25 am, bartbutenaers @.***> wrote:

 Hmm, found a partystopper... Would like to use nested input message properties:

However it looks like the Filtrex library doesn't support nested object properties (since it uses hasOwnProperty):

I could workaround this, but I think it is better if I create a pull-request for the Filtrex library to implement this feature ...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[bartbutenaers]

FYI: I have registered two issues for the filtrex library:

• One for accessing nested message properties (see here)
• One for using boolean values (see here)

Hopefully the author can help us soon with this, so I can continue with the implementation of expressions for alarms. He seems to be a very active contributor, so fingers crossed...

[macinspak]

Awesome, thanks for picking this up so quick and helping out for everyone! Will be a very useful addition I think.

On Tue, Aug 24, 2021 at 4:42 PM bartbutenaers @.***> wrote:

FYI: I have registered two issues for the filtrex library:

• One for accessing nested message properties (see here https://github.com/m93a/filtrex/issues/45)
• One for using boolean values (see here https://github.com/m93a/filtrex/issues/46)

Hopefully the author can help us soon with this, so I can continue with the implementation of expressions for alarms. He seems to be a very active contributor, so fingers crossed...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Anamico/node-red-contrib-alarm/issues/25#issuecomment-904366514, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGHK2LGMHB6GJQ7LHNWUWWDT6M5MLANCNFSM5BYCJDUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

[bartbutenaers]

I have implemented a small endpoint, that allows us to check the syntax of the expression (on the server side). That syntax check is runned for every character you type, to have a responsive config screen. The result of the last syntax check is also stored in the node, so the flow editor can draw a red triangle if necessary (based on the last syntax check):

BTW It is not clear to me how the current filtering works. Because at first sight the triggerType seems only to be used in the sensor.html file, but I might have overlooked something. Would be nice if you could explain it!

[macinspak]

Actually, I think it was a mistake and never implemented! So you have a blank slate. Not sure how that was missed! 🙂

Sent from my iPhone

On 25 Aug 2021, at 5:10 pm, bartbutenaers @.***> wrote:

 I have implemented a small endpoint, that allows us to check the syntax of the expression (on the server side). That syntax check is runned for every character you type, to have a responsive config screen. The result of the last syntax check is also stored in the node, so the flow editor can draw a red triangle if necessary (based on the last syntax check):

BTW It is not clear to me how the current filtering works. Because at first sight the triggerType seems only to be used in the sensor.html file, but I might have overlooked something. Would be nice if you could explain it!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[bartbutenaers]

Hi @macinspak, Sorry for the delay! But I don't think it is useful that I create a pull request, as long as the features haven't been added to the filtrex expression library... I see now that somebody else has posted the same feature request at the same day, but solving it in another way: see issue. Hopefully the author can find some time to help us, because I'm a bit stuck now with this...

[macinspak]

Hey, no problems, I think you are right. No need stressing, let’s just wait for the library fix.

Sent from my iPad

On 6 Sep 2021, at 7:40 am, bartbutenaers @.***> wrote:

 Hi @macinspak, Sorry for the delay! But I don't think it is useful that I create a pull request, as long as the features haven't been added to the filtrex expression library... I see now that somebody else has posted the same feature request at the same day, but solving it in another way: see issue. Hopefully the author can find some time to help us, because I'm a bit stuck now with this...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[bartbutenaers]

@macinspak, We got feedback from the Filtrex author. The poor devil needs to do his final examns in about two weeks... I will contact you again as soon as the requested Filtrex features are available. Bart

[bartbutenaers]

Hi @macinspak, I finally managed to create a pull request.

[macinspak]

Looks great, seems solid from a code review, but I am too busy to test heavily now, so will merge on your say-so. If you are confident I am good to merge it.

On Fri, Oct 1, 2021 at 8:58 AM bartbutenaers @.***> wrote:

Hi @macinspak https://github.com/macinspak, I finally managed to create a pull request https://github.com/Anamico/node-red-contrib-alarm/pull/32.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Anamico/node-red-contrib-alarm/issues/25#issuecomment-931759347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGHK2LEUGATO5JFYILXTKETUETTQBANCNFSM5BYCJDUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[macinspak]

Hi Bart,

Really keen to merge your pull request, just waiting to get your confirmation it is working well for you?

Thanks and also please check my other email about the other proposal you raised, I like it and I think we just need to work out the use cases to make sure it will work.

Thanks, Andrew

On Sat, Oct 2, 2021 at 9:15 PM Roo @.***> wrote:

Looks great, seems solid from a code review, but I am too busy to test heavily now, so will merge on your say-so. If you are confident I am good to merge it.

On Fri, Oct 1, 2021 at 8:58 AM bartbutenaers @.***> wrote:

Hi @macinspak https://github.com/macinspak, I finally managed to create a pull request https://github.com/Anamico/node-red-contrib-alarm/pull/32.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Anamico/node-red-contrib-alarm/issues/25#issuecomment-931759347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGHK2LEUGATO5JFYILXTKETUETTQBANCNFSM5BYCJDUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[bartbutenaers]

Hey Andrew, Sorry for the late reply. It is a busy week... Will get back to you this weekend! Bart

[macinspak]

No problems! Just happy to have some collaboration

Sent from my iPhone

On 15 Oct 2021, at 8:07 am, bartbutenaers @.***> wrote:

 Hey Andrew, Sorry for the late reply. It is a busy week... Will get back to you this weekend! Bart

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.