(Brain failure) Need help understanding why it isn't working

Trying_to_learn · 21 July 2020 06:39

I am trying to optimise how a msg is constructed.

This is what I have:

    if (counter == 0)
    {
        msg = {
            payload: "OFF",
            icon: '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>',
            background: "black"
        };
    } else
    {
        msg = {
            payload: "ON",
            icon: '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>',
            background: "black"

        };
    }

My understanding is I can optimise it like this:

    msg = {
        icon: '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>',
        background: "black"
    };
    if (counter == 0)
    {
        msg = {
            payload: "OFF"
        };
    } else
    {
        msg = {
            payload: "ON"
        };
    }

Or you could have it like this:

    if (counter == 0)
    {
        msg = {
            payload: "OFF"
        };
    } else
    {
        msg = {
            payload: "ON"
        };
    }
    msg = {
        icon: '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>',
        background: "black"
    };

That way the icon and background are already set.
Just the payload changes.

It doesn't work.

Forgive my failure to remember why, but can you please explain why?

Steve-Mcl · 21 July 2020 07:02

Where does counter come from?

because you set payload then destroy the original msg by doing this...

msg = {
        icon: '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>',
        background: "black"
    };

Why not do this - much more clear and concise...

    if (counter == 0) {
        msg.payload = "OFF";
    } else {
        msg.payload = "ON";
    }
    msg.icon = '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>';
    msg.background = "black";

EDIT...
it is much more preferable to update the existing msg than create new ones. It is more efficient and you retain (sometimes necessary) properties in the original msg.

Trying_to_learn · 21 July 2020 07:04

Counter is set earlier on.

Isn't your second way the same as my second way?

I need a good slapping to get the brain working.

Trying_to_learn · 21 July 2020 07:06

Steve-Mcl:

because you set payload then destroy the original msg by doing this...
msg = {
        icon: '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>',
        background: "black"
    };

I don't see how that overwrites the original payload part of the message.

Sorry, I am trying to read your reply again.

Is it something to do with the { } I use?

Steve-Mcl · 21 July 2020 07:10

msg = { ... } // literally means "Create a NEW object"

Steve-Mcl · 21 July 2020 07:11

No

You destroy the original msg every time - I reuse it (as is the preferred way as far as I am concerned)

Trying_to_learn · 21 July 2020 07:12

Yeah, ok. So the { } were my undoing.

Thanks.

bakman2 · 21 July 2020 07:15

Or shorter version

payload = counter === 0 ? "OFF" : "ON"

msg.icon = '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>';
msg.background = "black";
msg.payload = payload

return msg

Steve-Mcl · 21 July 2020 07:16

Dont be throwing ternary confusion into the mix man

Trying_to_learn · 21 July 2020 07:16

You're just showing off now @bakman2

Steve-Mcl · 21 July 2020 07:17

msg.icon = '<font color = "orangered"><i class="fa fa-lightbulb-o fa-3x"></i></font>';
msg.background = "black";
msg.payload = counter === 0 ? "OFF" : "ON";
return msg

Shorter @bakman2

Steve-Mcl · 21 July 2020 07:18

PS, your ternary is wrong way around

bakman2 · 21 July 2020 07:18

Ah yeah you are correct ; but not anymore

Trying_to_learn · 21 July 2020 07:20

All this came about because I had buttons to control devices.

It was ok when there was only 1 button on 1 machine.

Now there are multiple machines with the button, when one changes the state, all machines have to update.

Of course that seemed easy at first, but then when the other machine's buttons were pressed the state was wrong and needed up dating.

Caused big problems and I had to rewrite a lot of stuff and conditional handling.

I was/am trying to cut down on redundant code. Though this is a bit petty, I think practice makes perfect. Or I can at least hope.

Colin · 21 July 2020 07:44

No, it was the msg = that was your undoing. By saying that you throw away any current contents of msg and set it the value you specified.
By using msg.payload = you are saying throw away any current data in msg.payload and replace it with a new value, leaving all other contents of msg, such as icon or background, as they were.

Trying_to_learn · 21 July 2020 07:45

But it was other parts of the message which I was changing, conditional to the payload contents.

And neither of the bits of code I posted specified a new payload. Just that it was said that the { } was saying build a new message from scratch.
Therefore the other parts are discarded.

Steve-Mcl · 21 July 2020 08:07

Technically, Colin is correct.

Anything left of the = is assignment.

Its as simple as that.

the object/variable on the left of = is overwritten in its entirety at the point of execution.

But your statement...

Yeah, ok. So the { } were my undoing "

... will suffice for your understanding of the situation.

krambriw · 21 July 2020 08:27

Would you not be interested in switching icons for on/off state as well???

Trying_to_learn · 21 July 2020 08:47

That is already in the system.

But the problem was because of multiple ~~devices~~ computers being able to control the one external device and all be in sync.

As said, the code sort of worked but then if another computer's button was pressed, it wasn't in sync.

This is an example:

Incoming messages of "ON" and "OFF" set the correct state of the switch.

Otherwise pressing the button (switch) an X message is local and toggles the button's condition.

[{"id":"9fb7e7ec.4e6b78","type":"switch","z":"26262ba1.62dcbc","name":"TOPICS *","property":"topic","propertyType":"msg","rules":[{"t":"eq","v":"BULB-1/cmnd/power1","vt":"str"},{"t":"eq","v":"BULB-1/cmnd/dimmer","vt":"str"},{"t":"else"}],"checkall":"true","repair":false,"outputs":3,"x":2660,"y":300,"wires":[["c0671188.49e67","b31baea6.87a2f"],["2eac36ef.582eca"],[]],"outputLabels":["State","","LWT"],"info":"This needs editing for different BULBS"},{"id":"c0671188.49e67","type":"function","z":"26262ba1.62dcbc","name":"Set state","func":"if (msg.payload == \"RESET\")\n{\n    let state = context.get(\"STATE\");\n    if (state === 0)\n    {\n        //\n        msg= {icon: '<font color = \"red\"><i class=\"fa fa-lightbulb-o fa-3x\"></i></font>'};\n        msg.background = \"black\";\n        return msg;\n    } else\n    {\n        //\n        msg= {icon: '<font color = \"lime\"><i class=\"fa fa-lightbulb-o fa-3x\"></i></font>'};\n        msg.background = \"black\";\n        return msg;\n    }\n}\nif (msg.payload == \"ON\")\n{\n        msg= {icon: '<font color = \"lime\"><i class=\"fa fa-lightbulb-o fa-3x\"></i></font>'};\n        msg.background = \"black\";\n        context.set(\"STATE\",1);\n} else\n{\n        msg= {icon: '<font color = \"red\"><i class=\"fa fa-lightbulb-o fa-3x\"></i></font>'};\n        msg.background = \"black\";\n        context.set(\"STATE\",0);\n}\nreturn msg;\n","outputs":1,"noerr":0,"x":2860,"y":340,"wires":[["22cee9b7.50c1ae","c8ea1741.fbff8"]]},{"id":"c8ea1741.fbff8","type":"ui_button","z":"26262ba1.62dcbc","name":"BULB#1","group":"7893c6c4.30ede","order":2,"width":"1","height":"3","passthru":false,"label":"{{msg.icon}}","tooltip":"","color":"","bgcolor":"{{msg.background}}","icon":"","payload":"X","payloadType":"str","topic":"","x":3070,"y":380,"wires":[["b31baea6.87a2f"]]},{"id":"b31baea6.87a2f","type":"function","z":"26262ba1.62dcbc","name":"Button control BULB1","func":"var msg1 = {};\nvar counter = context.get(\"counter\") || 0;\ncounter = (counter + 1) % 2;\ncontext.set(\"counter\",counter);\n\nvar x = msg.payload;\n\nif (x == \"ON\")\n{\n    counter = 1;\n    context.set(\"counter\",1);\n    return;\n} else\nif (x == \"OFF\")\n{\n    counter = 0;\n    context.set(\"counter\",0);\n    return;\n}\n\nif (x == \"X\")\n{\n    //\n    if (counter == 0)\n    {\n        msg = {\n            payload: \"OFF\",\n        };\n    } else\n    {\n        msg = {\n            payload: \"ON\",\n        };\n    }\n    msg.icon = '<font color = \"orangered\"><i class=\"fa fa-lightbulb-o fa-3x\"></i></font>';\n    msg.background = \"black\";\n}\nreturn msg;\n","outputs":1,"noerr":0,"x":3110,"y":420,"wires":[["d71b2e38.587d3","43c6ebaa.d58c44","c8ea1741.fbff8"]]},{"id":"7893c6c4.30ede","type":"ui_group","z":"","name":"BULB-1","tab":"aa487daa.33c1c","order":4,"disp":true,"width":"4","collapse":false},{"id":"aa487daa.33c1c","type":"ui_tab","z":"","name":"Real_World_Control","icon":"dashboard","order":3,"disabled":false,"hidden":false}]

If you want I could include more of the flow, but basically it is looking at a MQTT in node and it transmits to a MQTT OUT node.

krambriw · 21 July 2020 09:15

OK, then it's fine, I did not see this happening in the code you showed earlier, no cow on the ice as we say here up in north

Topic		Replies	Views
Why am I not getting this? msg.(stuff) General	4	507	1 August 2018
I'm not seeing the elephant - code help General	4	136	10 April 2023
Alarms according to events in msg.payload Dashboard	2	886	30 June 2019
Change node issue General	2	290	7 January 2019
Function node not working General	4	507	16 March 2021

(Brain failure) Need help understanding why it isn't working

Related topics