Better way to declare the output of a function node

[Steve555]

Hi,

Not sure if this is the right spot for this, but, here goes anyway.

In order to switch a particular relay I am using I have to send a boolean true or false to, specifically, msg.payload.v

I have mucked around a fair bit within function nodes and the only way I could get the desired outcome is create a object within an object.

This kinda seems a bit clumsy and sort of a bit like double handling. The code works fine, just wondering if there is a better way to do this in one step.

[jbudd]

Welcome to the forum @Steve555

Because you only posted a very low resolution picture of your code I had to run it through OCR and then manually fix it up.
It would be very helpful if in any future forum post you use the export ability of the Node-red editor and paste the actual code here using the </> button.

I think that you have this:

var stat = msg.payload.status;
if (stat == "connect") {
    var r1 = global.get("pRellSta", "persist");
    if (r1 === true) {
        var msg1 = {};
        var msg2 = {};
        msg1 = {v:r1}
        msg2= {payload:msg1};
        node.send(msg2);
    }
}
return null;

Except that I would not use a global context variable, I would probably have coded this as

if (msg.payload.status === "connect") {
    if (global.get("pRellSta", "persist") === true) {
        msg.payload = {"v": true}
        return msg
    }
}

Which gets rid of several unnecessary declarations and statements.

[Colin]

A further small refinement is that the first two lines can be combined to
if (msg.payload.status === "connect" && global.get("pRellSta", "persist") === true) {
Also, if the global context variable were only true or false then you could use
if (msg.payload.status === "connect" && global.get("pRellSta", "persist")) {
Though it seems a bit odd that you are defaulting the global variable to the string "persist" but also testing it for the boolean value true.

In the line msg.payload = {"v": true} the quotes round the v are optional, as the name does not include any special characters.

Personally, I don't like to see return statements in the middle of a function. It is too easy to add code to the function and not notice that you have a return in an inner block somewhere, so I would code this as

if (msg.payload.status === "connect"  &&  global.get("pRellSta", "persist") === true) {
  msg.payload = {v: true}
} else {
  msg = null
}
return msg

Though I agree with @jbudd's comment about global context, I would almost certainly not be using a global context variable, I would be passing the value around in messages.

[jbudd]

@Colin

Though it seems a bit odd that you are defaulting the global variable to the string "persist"

Isn't the OP specifying that this variable is in a store called "persist" ? There is no default value.

I considered the && approach, which would have removed a set of braces but at the expense of legibility

Yes, return in the middle of a function is potentially a problem but I sometimes do it (shrug)

[Steve-Mcl]

@jbudd

Isn't the OP specifying that this variable is in a store called "persist"

That is correct.

Ps, I see no need at all for a function node here. AFAICS, 2 switch nodes and a change node are all that is required.

[Colin]

@jbudd

Isn't the OP specifying that this variable is in a store called "persist" ? There is no default value.

Oops yes, brain fade setting in over here I think.

[Steve555]

Thanks for the welcome and sorry about the small images.

This I love:

@jbudd

    msg.payload = {"v": true}

I was always taught to declare and name everything at the top of my code, come rain hail or shine, then make them equal stuff later. Its kinda cool to see my 10 or so lines of code reduced to 6 and not really have to worry about names in the same way.

Bit more information about my system, the snippet of code I showed above is only representing the operation of one individual circuit. I have 9 relay modules and each module has 6 i/o, so in total I have approx. 50 circuits that I am controlling. The purpose of this section of code is to recover each individual circuit back to its previous state after a power blackout (unfortunately the relay modules I am using don't "persist" natively like the other Tasmota and Shellys I have). In Node Red I have the operation and control of 1 relay module per flow (so 6 individual circuits), this means the one "recovery" function node per flow has 6 outputs that operate individually. The code sequentially checks the last saved state of each of the 6 circuits and if the state is true, it just sends a true to that circuit again.

The globals are because I wanted all relay states available across all flows, the number of flows I am using and I wanted my own "database" of states, not have to rely on other things that "might" work.

I will modify my code and repost, should make all that a bit clearer

[Steve555]

I did make an interesting discovery when trying to implement this, msg.payload = {"v": true}, in my code. The message I receive from the relay modules upon re-powering up, the message that contains the word "connect", also contains a heap of other information I don't want or need. When I use msg.payload = {"v": true} to create my new output message, the output message does contain payload.v = true, but it also contains all the other junk information that I received from the original "connect" message. So I thought no worries I will just make it msg2.payload = {"v": true}, however, Node Red doesn't appear to like that as I get a ReferenceError: msg2 is not defined (line... col...). Anyone know why Node Red likes msg.payload = {"v": true} but does not like msg2.payload = {"v": true}? Its simple enough to put in a var msg2 = {}; but I didnt have to do that for the first case.

[jbudd]

That's because msg already exists (and is an object) so you can assign a value to msg.payload.

But msg2 does not exist so you have to declare it as an object before you can use it.

You could probably use

msg = {
"payload": { "v": true}
}

[E1cid]

@Steve555

I did make an interesting discovery when trying to implement this, msg.payload = {"v": true}, in my code. The message I receive from the relay modules upon re-powering up, the message

You could also rewrite the whole msg object
e.g

msg = {payload: {v: true}}

which would over write all msg properties.
(noting quotes are not necessary in Javascrpt property names, unless using square bracket notation.)
[edit] I see jbudd has already noted this info.

[Steve555]

@E1cid

msg = {payload: {v: true}}

which would over write all msg properties.

Works a treat, all the other junk properties gone. I can sort of see it now, the way you have done the { } within the { } is a much nicer version of my object in object.

[Steve-Mcl]

A word of caution. It is good practice to keep all properties of the message for downstream processing. One day you will need that extra data. Especially important for things like join node, http-response node and A few others.

Obviously it you are sending it to a single device it is not so much of an issue. also note, there is usually no harm in leaning all the properties in the original message and just ignore them.

[Steve555]

Ended up with this:

if (msg.payload.status == "connect") {
    node.send([null,null,null,null,null,null,{rcr:"recover1"}]);

    // curcuit 1
    if (global.get("pRel1Sta","persist") === true) {
        msg = {payload:{v:true}}
        node.send([msg,null,null,null,null,null,null]);
    }
    
    // curcuit 2
    if (global.get("pRel2Sta","persist") === true) {
        msg = {payload:{v:true}}
        node.send([null,msg,null,null,null,null,null]);
    }
    
    // curcuit 3
    if (global.get("pRel3Sta","persist") === true) {
        msg = {payload:{v:true}}
        node.send([null,null,msg,null,null,null,null]);
    }
    
    // curcuit 4
    if (global.get("pRel4Sta","persist") === true) {
        msg = {payload:{v:true}}
        node.send([null,null,null,msg,null,null,null]);
    }
    
    // curcuit 5
    if (global.get("pRel5Sta","persist") === true) {
        msg = {payload:{v:true}}
        node.send([null,null,null,null,msg,null,null]);
    }
    
    // curcuit 6
    if (global.get("pRel6Sta","persist") === true) {
        msg = {payload:{v:true}}
        node.send([null,null,null,null,null,msg,null]);
    }
}

return null;

Guess I could just put the {...} in the node.send line and not have the second line. Nuff for tonight , I am going to sleep

[jbudd]

Since you are sending the same message to all 6 circuits, you only need to send it once, but wire up the output to all 6 sets of downstream nodes:

[Steve555]

The test is per circuit (hence the 6 outputs from the function node) and their state is completely independent of each other. The states could be either true or false, dependent on if the circuit was in use prior to the power out. I am only wanting to send the true's, since after a power out, the relay modules default to false.

[Steve555]

Thanks all for the help! Small improvements to make things less clunky.

[bakman2]

As the elements are numbered, it could be done with a loop, something like;

const circuits = []

for(let x=1;x<7;x++){
    const circuit = `pRel${x}Sta`
    if(global.get(circuit,'persist')){
        circuits.push({payload:{v:true}})
    }else{
        circuits.push(null)
    }
}
if(msg.payload.status == 'connect'){
    node.send([null, null, null, null, null, null, { rcr: "recover1" }]);
    node.send(circuits)
}
return null

not tested.