Pull request proposal - temporary node status

[BartButenaers]

Hi folks,

Currently when you set the status for a node, that status text will be displayed in the flow editor until a new status is set afterwards. Which makes sense of course...

However in some situations it is convenient to have a temporary status which is only visible for a specified time. For example we want to show a warning, but we don't want it on the screen during several days...

Currently we workaround this by repeating the same boilerplate code over-and-over again in our nodes, to reset the status via a timer. To keep our code clean, it would be nice to have this functionality available in the Node-RED core ...

I made some small changes to the Node.js file, to support following 3 scenario's:

1. Set a status text on the node, which will be displayed until a next status is set (i.e. like we always have been using it):

   this.status({fill:"yellow",shape:"dot",text:"warning occurred"});
   

   

2. Set a status text on the node, which need to disappear after N seconds (i.e. replaced by an empty status object {}). Example for 2 seconds:

   this.status({fill:"yellow",shape:"dot",text:"warning occurred"}, 2);
   

   

3. Set a status text on the node, which need to be replaced by another status object after N seconds. Example for 2 seconds:

   this.status({fill:"yellow",shape:"dot",text:"error occurred"}, 2, {fill:"blue",shape:"dot",text:"connected"});
   

   

I used seconds as a unit, because it is useless to show status changes at high speed. Moreover I want to avoid that people (accidentally) update the status to often, thus overloading the Node-RED websocket channel (which would make their flow editor user interface unresponsive).

When a node specifies a timeout in an older Node-RED version (which doesn't contain this functionality), it will behave as it does already today. So it will behave like it has always behaved ...

If this pull request would get approved, I will also create a pull request to update the documentation.

Thanks for previewing my change, and hopefully you like it!

Bart

[ristomatti]

This sounds like a no brainer to accept, great concept! I would suggest having the time defined in milliseconds though, just for consistency with setTimeout.

[BartButenaers]

@ristomatti

I would suggest having the time defined in milliseconds though, just for consistency with setTimeout .

I have considered that, but don't think it makes sense to use milliseconds for something visual. And I truly believe it will introduce a lot of performance problems...

@ristomatti

This sounds like a no brainer to accept

I'm really not sure whether the brains behind Node-RED share your opinion

[dceejay]

Like the basic concept, but what is the use case for option 3 ? The status should indicate an actual status - ie be triggered by something (like the connected event in your example). I can understand wanting to time it out - but not why it would then trigger another.

In two minds about mSecs though.. I like that it should be integers of seconds - so can't be set too small like you suggest. (though even 1 sec is a bit small to read anything - and no we don't want blinking lights...

[knolleary]

@dceejay

Like the basic concept, but what is the use case for option 3 ?

As Dave says, status is meant to reflect something happening. I completely get the idea of clearing it after a set time - good suggestion, lets have that.

But its option 3 that's a problem for me. It feels the start of a slippery slope that doesn't end with a sequence of just two statuses and ends up with an arbitrary long list of statuses to be cycled through.

I would go with Seconds - 99% of use cases will be multiple seconds long.

The possibly harder part will be the scheduling of the clear event. setTimeout is the obvious solution, but how well will that scale when you have 1000 nodes all wanting to do it.

[Steve-Mcl]

Nice work Bart

Just floating an idea with out looking into the code - would something like the below be feasible & less of a headache?

What if instead of each node having its own timer...

when a node registers a status update with a timeout argument, a timestamp(+timeout) is logged against its node.id in a lookup object.

Then with a single global gatekeeper routine that runs server side every second takes care of clearing any expired statuses?

If a new status arrives before expiry, the lookup tables expiry timestamp is simply updated (or removed if there is no timeout argument)

Hope that makes sense. Any use?

[dceejay]

Or could it be a CSS transition style that let's the browser do the hard work ?

[knolleary]

@dceejay

Or could it be a CSS transition style

Not if you want the Status node to be triggered by the clear.

[cinhcet]

@knolleary

Not if you want the Status node to be triggered by the clear.

Should the status node being triggered by the auto clear?

I like the concept. About option 3, what if the the status is replaced by a permanent status that has been there before? Just an idea, I would advocate for it.

[jeancarl]

Maybe make it a property duration inside the object, rather than as second argument. This would permit more flexibility in further expansion?

[BartButenaers]

Thanks everybody for your time reviewing this!

@dceejay

The status should indicate an actual status - ie be triggered by something (like the connected event in your example)

I had this use case in mind:

1. I see that the device is connected, so set status to "connected".
2. Then a warning occurs, so show it temporarily.
3. Then I show again "connected". This status will not be set by my node since the connection status e.g. arrives via mqtt only when it changes.

I have been thinking to replacing the second status parameter by a boolean parameter 'toPreviousStatus', to define the behaviour after the timeout:

• When false I reset the status (i.e. status {} ).
• When true I go to the previous status.

However I think that this might not work always correctly: suppose I show a warning for 10 seconds, and meanwhile another warning occurs. Then the 'previous' status will be 'warning' instead of 'connected', which is not what I want ...

@dceejay

we don't want blinking lights

Indeed. Perhaps a minimum interval of e.g. 2 seconds, to avoid this kind of blinking situations?

@knolleary

It feels the start of a slippery slope that doesn't end with a sequence of just two statuses and ends up with an arbitrary long list of statuses to be cycled through.

We have a case of paranormal activity here I have been thinking: what if someone ever does a feature request to pass an array of status fields...
So I can really imagine why you guys are against option 3, but then I can never go back to my original "connected" status (which is really useful for nodes that represent my devices...).
I have also been thinking about the third parameter being a callback function (or a promise ), which needs to return a status. But then again every node needs to store its own previous status, which doesn't really solve the boilerplate code issue that I'm trying to solve here ...

@knolleary

how well will that scale when you have 1000 nodes all wanting to do it.

You are absolutely right. Performance is a major issue. Therefore I would definately not use milliseconds: like you say it is not only about traffic through the websocket, but also useless creating and clearing timers.

@Steve-Mcl

What if instead of each node having its own timer...

Sounds logical, but I think that my "quick-win" principle doesn't count then anymore for this pull request, especially the "quick" part of it " " I didn't have a look at the whole code, so cannot really judge about your proposal ...

@jeancarl

make it a property duration inside the object, rather than as second argument.

Not sure. Suppose you want to show an error during a longer time, compared to a warning. Then you have to change the property value over and over again. Do you have any ideas about which "future expansions" you are referring to?

[jeancarl]

Thinking animations, but this is definitely a rabbit hole that's more aesthetic than functional. Fade out, dissolve, etc.

Moving the duration into the object also aligns more to the options object pattern that is seen throughout JavaScript.

[dceejay]

@knolleary

Not if you want the Status node to be triggered by the clear.

If you want it triggered then set it clear explicitly like today.

[dceejay]

Re reverting state. Why shouldn't it always revert ? If the point of the timed status is that it is temporary then it should always revert. If it was already blank then it goes back to blank etc.

[ristomatti]

@jeancarl

Maybe make it a property duration inside the object, rather than as second argument. This would permit more flexibility in further expansion?

I like this duration idea also. It would allow cleanly storing the temporary status on a constant variable.

const connected = {fill: "green", shape: "dot", text: "connected"};
const warning = {fill: "yellow", shape: "dot", text: "warning", duration: 3};

node.status(warning);

[knolleary]

@dceejay

If you want it triggered then set it clear explicitly like today.

I'm not keen on have an inconsistent behaviour here. But it all depends on what the requirement is for the feature.

There have been multiple interpretations already in this thread - all quite different in terms of assumed behaviour.

Is the purpose of this:

1. set a temporary status before reverting to whatever was there before
2. set a status for a set time before clearing it

They are both valid scenarios, but they are also quite different in terms of behaviour.

I also believe that any change to the status should trigger the Status node. Otherwise you end up with inconsistency between what the UI shows and what the Status node 'sees'.

@jeancarl

Maybe make it a property duration inside the object

I would agree with this. It minimises changes to the underlying API - the duration is an inherent property of the status message, not a separate value.

@BartButenaers

Suppose you want to show an error during a longer time, compared to a warning. Then you have to change the property value over and over again.

Not convinced here. In that scenario you would presumably already have two different status objects for the different types of event - assuming you want to use a different colours for errors vs warning.

[BartButenaers]

@jeancarl

Moving the duration into the object

I had considered that, but then I ran into troubles when a second status object was passed (scenario 3). Because the first status object parameter would contain a duration, but the second status object parameter not. Which would be confusing. For example:

this.status({fill:"yellow",shape:"dot",text:"error occurred", duration: 2}, {fill:"blue",shape:"dot",text:"connected"});

But if we don't go for scenario 3, it might indeed be cleaner to use the duration object property.

@knolleary

Is the purpose of this:

1. set a temporary status before reverting to whatever was there before
2. set a status for a set time before clearing it

I personally was aiming at option 1, because otherwise I loose my original "connected" status. It seems to me that it makes more sense to revert to the original status (instead of clearing it), because that status was the last one explicit set by the node ...

@knolleary

In that scenario you would presumably already have two different status objects for the different types of event - assuming you want to use a different colours for errors vs warning.

Do I understand you correctly: Suppose I introduce a new this.originalStatus to store the previous status. Do you mean that I should update that variable only with non-temporary statusses?
Something like this:

1. Original status is "connected".
2. I set a temporary "warning 1" status for 10 seconds, which means this.originalStatus becomes "connected"
3. Within those 10 seconds a new temporary status "warning 2" is set, which is NOT stored in this.orginalStatus.
4. When "warning 2" disappears, the this.originalStatus is set again (which means I get again the last persistent node status "connected").

[dceejay]

@knolleary

any change to the status should trigger the Status node

Yes I agree - but the change of state is the fact that a "warning" has occurred - and that will cause the trigger. How that is then displayed - whether permanently or temporarily is a UI thing . The node itself is still in (say) connected state - but is just letting you know it saw something odd. (which means I vote for option 1).

But the Editor is still NOT a dashboard !

[knolleary]

@BartButenaers

Do I understand you correctly: Suppose I introduce a new this.originalStatus to store the previous status. Do you mean that I should update that variable only with non-temporary statusses?
Something like this:

You're getting way down into the weeds of detail before we've agreed what the functionality is. How easy it is to implement is irrelevant to getting the function right.

@dceejay

Yes I agree - but the change of state is the fact that a "warning" has occurred - and that will cause the trigger.

You are assuming that's the only reason this would be used. Its a reason for using it, but not the only reason.

The Status node reflects updates to the status of a node. That's it. That's what it does. I do not like introducing exceptions to rules. If the user updates the status of a node, then the Status node is triggered.

[ristomatti]

After following the discussion I would side with the single parameter option with the duration. Further I would like to think setting the duration would also implicate the status is temporary (like the thread title) and therefore returning the previous status whether set or not. Consecutive calls would all revert back to the original status.

To reword - a status set without a duration would be stored for the duration of the temporary status but status with a duration would not.