Function node & var definition

I strongly suspect that the incoming message is not exactly what @pacmanmt thinks it is. The debug sidebar has its own formatting quirks, so node.warn messages and type checking are really good ideas.

First of all, thanks all for your availability and patience, as said I am a real dumb regarding coding.
This is the function where I hope having inserted all your suggestions:

const data = msg.payload.split("\t");
node.warn(`data[0]: ${data[0]}`)
let stateoc, voltag, curren;
switch (data[0]) {
    case 'SOC': {
        stateoc = parseFloat(data[1] / 10);
        break;
    }
    case 'V': {
        voltag = parseFloat(data[1] / 1000);
        break;
    }
    case 'I': {
        curren = parseFloat(data[1] / 1000);
        break;
    }    default: { 
        msg.payload = "Error";
    }
}
msg.payload = {
    SOC: stateoc,
    Voltage: voltag,
    Current: curren
}
return msg;

This is the debug result of part of the incoming stream from the serial port:
image

I have ran both my function and the modified one
This is the debug result of my first function (just 2 items of the continuos stream):
image

This is the debug result of the modified function with your suggestions (just 2 items of the continuos stream) :
image

The two examples you give where the input contains "I" and "SOC" contain spaces not tab characters, so data[0] is the whole string. Therefore none of the case statement lines match.
In this case the default: sets msg.payload to "Error" but you immediately overwrite it.

In short, you need better control of your input data format.

No the separator is tab, I just expanded the message payload to show it in a different way.
This is the actual output of I and SOC without expanding them:
image

As jbudd has suggested, no point in setting msg.payload in the default case as you overwrite it so you will never see the error.

But now we can see exactly what the real issue is.

Out of the 4 messages you've shown, only 2 of them have valid inputs. The other 2 will output nothing useful.

Which leads us to the bit we all missed - which was that your logic will never work. The reason is that you never get all of the data in a single message. So every time you process a message, you only ever get part of the output, never all of it.

To get round that, you will need to make use of context variables to temporarily hold the "missing" values.

The reason you are seeing the values when you don't specify let or const is a complete fluke which is why not using them is so dangerous. It appears that node.js is not acting as I'd expect and is somehow retaining the variables at a global level so that they are "still there" on the next invocation. This is absolutely not expected when working with a function node and might even be considered a bug.

Anyway, you NEED to use let or const:

// ALWAYS USE let OR const!!! No, really. ALWAYS!

let data
// This could fail so wrap in a try/catch
try {
    data = msg.payload.split("\t")
} catch (e) {
    node.warn(`Could not split the input. ${e.message}`)
    // give up if this is the case
    return
}

// Try to get the saved vars but return null if they don't yet exist
let stateoc = context.get('stateoc') || null
let voltag = context.get('voltag') || null
let curren = context.get('curren') || null

switch (data[0]) {
    case 'SOC': {
        stateoc = parseFloat(data[1] / 10)
        context.set('stateoc', stateoc) // save for future use
        break
    }
    case 'V': {
        voltag = parseFloat(data[1] / 1000)
        context.set('voltag', voltag)
        break
    }
    case 'I': {
        curren = parseFloat(data[1] / 1000)
        context.set('curren', curren)
        break
    } 
    default: {
        // Always handle the default
        node.warn(`Nope! didn't recognise input type ${data[0]}`)
        // Not interested in this input so give up
        return
    }
}

msg.payload = {
    SOC: stateoc,
    Voltage: voltag,
    Current: curren
}

return msg

// NB: semi-colons are optional and standard js now does not use 
// them unless necessary

Actually, I did point that out in post #4

What I don't understand is how the old function apparently populated all of them. Also the messages seem to be all out of sequence, and the node.warn is nowhere to be seen.

@pacmanmt I expected to see in the debug an input message, the node.warn and then an output message, then another input message and so on. Can you explain why that is not happening?

If you posted a simple flow including actual data instead of a photo of selected debug nodes after you "showed it in a different way" maybe there would be fewer wild goose chases and more actual diagnosis.

To soften my criticism a bit, yes I do see that the little triangle points downwards on the I and SOC lines. Still, real data is easier to experiment with.

I don't either. I thought that the point of the node.js vm is to isolate the code and variables completely. If that were true, there should be nothing left between incoming messages. But clearly that isn't quite true.

My understanding is that not using let/const/var means that the variable is hoisted to the global context.

It seems then that it does put something into a global location that is retained across executions.

That would be very strange.

If you can be so kind to suggest me how to capture actual data, I will post them. I used the picture because I don't know how to do what you ask.
I can also post the whole flow a (and I will), but without the device connected to a serial port I don't know how you can evaluate how is made the stream coming from the serial port.

You use a debug node to show the data. There is a button at the right of the debug pane "Copy value"
Untitled 4

In the example above, Copy value gives

{"SOC":100,"Voltage":13.136,"Current":0}

I used the </> button in the forum to paste the value in between two lines containing three back ticks.
Like this
Untitled 1

Now anyone can copy the value and insert it into an inject or function node to experiment with my data.

(There is a better pre-prepared example with a video but I don't have access to post it. Maybe someone else can do so?)

Thereā€™s a great page in the docs (Working with messages : Node-RED) that will explain how to use the debug panel to find the right path to any data item.

Pay particular attention to the part about the buttons that appear under your mouse pointer when you over hover a debug message property in the sidebar.

BX00Cy7yHi

This looks like a design glitch in Node-RED ... or perhaps a feature:
The function node uses a sandbox (node.js terminology: context) that is initialized once and used throughout the life of the node. This sandbox acts like globalThis, explicitly proposed to run several scripts within that (preserving) context. Changes to the global scope (like variables defined accidentally globally by omitting let & friends) thus are reflected in sandbox and available for the next run of the function node.

I would like to propose @knolleary to verify if this is the intended functionality...

2 Likes

Nice conversation here,
I think most people coming to nodered are not well trained coder.
They like nodered because it seems easy and graphic.
But soon they see the limit of basic library and begin to use function node.
What is tricky is that you can find a lot of example on the net and there are a lot of JS version and ways to code the same.
The syntax itself seems continuously changing ā€¦

What I donā€™t understand here with my small experience using nodered : each time I use a variable in a function node without declaring it with a var /let/const, the function node will indicate a syntax error and refuse to deploy the function node.
It seems here that the code can be validated by nodered syntax checking without the letā€¦

Pierre RUWET

Good point. As it looks like (on my system), there seems to be a difference between the two editor options:

Monaco:
image

Ace:
image

So we found another good reason to use Monaco - that already is the standard option since 3.0.

Yet... going on with the test, using Monaco:


image

Despite NR asks for confirmation of the deploy & shows the red triangle, the flow still runs & gives the expected result:
image

Thus: Monaco is supporting here - but it doesn't solve the issue.

This is a new feature in node-red that comes from having swapped the code editor from ACE to Monaco which has loads of goodies baked in, including "linting" of JavaScript code to point out obvious errors.

The editor isn't actually a syntax checker but a "linter". It cannot necessarily work out all errors. Normally, it would point out the first use of a variable without a let/const and would also warn about using var (which is valid but not often recommended). But, node-red's function node is not the same as raw JavaScript in a browser since it is a small part of a much larger application which may define things itself (such as the RED and node objects), so not everything can be checked.

Here you can see the linter telling me that it doesn't know about x or test. That is an indication (albeit somewhat roundabout) that you need let/const

Indeed. JavaScript is, in my view, going through the same mistakes that Python did in regard to syntax. It has a bunch of hard-core programmers on its committee making changes to it every year. In the process, it is getting ever more complex.

ACE is much older and has no linting built in.

That is correct, it isn't meant to. It is only linting. But that is massively helpful to people who don't do much coding because it points out many of the oddities of JavaScript that you should normally avoid. But there are always exceptions to the rules.

Thanks Jbud, below the stream coming from the serial port node, that repeats over time:

BMV	712 Smart
FW	0400
Checksum	6
H1	0
H2	0
H3	0
H4	0
H5	0
H6	0
H7	28
H8	13513
H9	0
H10	0
H11	0
H12	0
H15	23
H16	30
H17	0
H18	0
Checksum	
PID	0xA381
V	13298
VS	26
I	0
P	0
CE	0
SOC	1000
TTG	-1
Alarm	OFF
Relay	OFF
AR	0

I hope I did correctly to let others experiment with it.
Thank you so much for pointing me in the right direction.

I think one of the strengths of NodeRed is that allows "non programmers" like me to do things otherwise not possible if not after learning a completely new skill.
I am quite good at integrating systems, but a real dumb at programming because never had a coding training (at least not on the new programming languages).
I think the function node is used very often depending on the needs, or at least has been like that form me.
I am just transitioning from version 2.x to 3.x and I can testify the the ACE editor doesn't check variable declarations and Monaco does. This means that I will need to review all the other flows I developed to check the declarations and "some other bad habits" :wink:

1 Like

We all went through that when we moved to Monaco. It found a number of places where I had forgotten to declare variables, and it even found a bug that explained some odd behaviour very occasionally, that I had never managed to tie down.

Yup! Me too. :slight_smile:

OK, well done.

I have used the data you posted above and made a test flow with it.

[{"id":"f77cbf8b5446a20e","type":"tab","label":"Flow 1","disabled":false,"info":"","env":[]},{"id":"85c9b5d1e6d366ae","type":"function","z":"f77cbf8b5446a20e","name":"Setup and inject test data ","func":"const data = [\"BMV\\t712 Smart\", \"FW\\t0400\", \"Checksum\\t6\", \"H1\\t0\", \"H2\\t0\", \"H3\\t0\", \"H4\\t0\", \"H5\\t0\", \"H6\\t0\", \"H7\\t28\", \"H8\\t13513\", \"H9\\t0\", \"H10\\t0\", \"H11\\t0\", \"H12\\t0\", \"H15\\t23\", \"H16\\t30\", \"H17\\t0\", \"H18\\t0\", \"Checksum\\t\", \"PID\\t0xA381\", \"V\\t13298\", \"VS\\t26\", \"I\\t0\", \"P\\t0\", \"CE\\t0\", \"SOC\\t1000\", \"TTG\\t-1\", \"Alarm\\tOFF\", \"Relay\\tOFF\", \"AR\\t0\"];\nfor (let i = 0; i < data.length; i++) {\n    msg.payload = data[i]\n    node.send(msg)\n}","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"x":390,"y":160,"wires":[["900494fd1ae1094a"]]},{"id":"6d20b60fb97cb42d","type":"inject","z":"f77cbf8b5446a20e","name":"Go","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"","payloadType":"date","x":190,"y":160,"wires":[["85c9b5d1e6d366ae"]]},{"id":"26f180c7b205e559","type":"debug","z":"f77cbf8b5446a20e","name":"Errors come out here","active":false,"tosidebar":true,"console":false,"tostatus":false,"complete":"payload","targetType":"msg","statusVal":"","statusType":"auto","x":900,"y":120,"wires":[]},{"id":"900494fd1ae1094a","type":"function","z":"f77cbf8b5446a20e","name":"pacmant function ++","func":"const data = msg.payload.split(\"\\t\");\n//node.warn(`data[0]: ${data[0]}`)\nlet stateoc, voltag, curren;\nswitch (data[0]) {\n    case 'SOC': {\n        stateoc = parseFloat(data[1] / 10);\n        break;\n    }\n    case 'V': {\n        voltag = parseFloat(data[1] / 1000);\n        break;\n    }\n    case 'I': {\n        curren = parseFloat(data[1] / 1000);\n        break;\n    } default: {\n        msg.payload = \"Error\";\n    }\n}\nif (msg.payload == \"Error\") {\n    return [msg, null]\n}\nmsg.payload = {\n    SOC: stateoc,\n    Voltage: voltag,\n    Current: curren\n}\nreturn [null, msg];","outputs":2,"noerr":0,"initialize":"","finalize":"","libs":[],"x":640,"y":160,"wires":[["26f180c7b205e559"],["6fff34db69943fc3"]]},{"id":"6fff34db69943fc3","type":"debug","z":"f77cbf8b5446a20e","name":"Valid answers here","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"payload","targetType":"msg","statusVal":"","statusType":"auto","x":890,"y":180,"wires":[]}]

Since there are only three data lines which contain "SOC", "V" or "I", I tweeked your most recently posted function to divert these to a separate output so it's easy to include/exclude them in debug.
So ignoring the "Errors" I see three output messages, each with one valid value and two undefined.

I can't remember if this is what you are hoping to get, or if you want all three values combined in a single message?
Here is a flow which emits a single combined object.

[{"id":"f77cbf8b5446a20e","type":"tab","label":"Flow 1","disabled":false,"info":"","env":[]},{"id":"85c9b5d1e6d366ae","type":"function","z":"f77cbf8b5446a20e","name":"Setup and inject test data ","func":"const data = [\"BMV\\t712 Smart\", \"FW\\t0400\", \"Checksum\\t6\", \"H1\\t0\", \"H2\\t0\", \"H3\\t0\", \"H4\\t0\", \"H5\\t0\", \"H6\\t0\", \"H7\\t28\", \"H8\\t13513\", \"H9\\t0\", \"H10\\t0\", \"H11\\t0\", \"H12\\t0\", \"H15\\t23\", \"H16\\t30\", \"H17\\t0\", \"H18\\t0\", \"Checksum\\t\", \"PID\\t0xA381\", \"V\\t13298\", \"VS\\t26\", \"I\\t0\", \"P\\t0\", \"CE\\t0\", \"SOC\\t1000\", \"TTG\\t-1\", \"Alarm\\tOFF\", \"Relay\\tOFF\", \"AR\\t0\"];\nfor (let i = 0; i < data.length; i++) {\n    msg.payload = data[i]\n    node.send(msg)\n}","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"x":270,"y":160,"wires":[["9ff73188fa5ac0aa"]]},{"id":"6d20b60fb97cb42d","type":"inject","z":"f77cbf8b5446a20e","name":"Go","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"","payloadType":"date","x":90,"y":160,"wires":[["85c9b5d1e6d366ae"]]},{"id":"9ff73188fa5ac0aa","type":"function","z":"f77cbf8b5446a20e","name":"revised function","func":"const data = msg.payload.split(\"\\t\");\nvar newmsg = {};\nswitch (data[0]) {\n    case 'SOC': {\n        newmsg.topic = \"SOC\";\n        newmsg.payload = parseFloat(data[1] / 10);\n        break;\n    }\n    case 'V': {\n        newmsg.topic = \"Voltage\";\n        newmsg.payload = parseFloat(data[1] / 1000);\n        break;\n    }\n    case 'I': {\n        newmsg.topic = \"Current\";\n        newmsg.payload = parseFloat(data[1] / 1000);\n        break;\n    } \n    default: {\n        newmsg.payload = \"Error\";\n    }\n}\nif (newmsg.payload != \"Error\") {\n    return newmsg;\n}\n","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"x":480,"y":160,"wires":[["155719422ee31d55"]]},{"id":"155719422ee31d55","type":"join","z":"f77cbf8b5446a20e","name":"","mode":"custom","build":"object","property":"payload","propertyType":"msg","key":"topic","joiner":"\\n","joinerType":"str","accumulate":false,"timeout":"","count":"3","reduceRight":false,"reduceExp":"","reduceInit":"","reduceInitType":"","reduceFixup":"","x":650,"y":160,"wires":[["c5c49a8ab7950fa4"]]},{"id":"c5c49a8ab7950fa4","type":"debug","z":"f77cbf8b5446a20e","name":"Joined ","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"payload","targetType":"msg","statusVal":"","statusType":"auto","x":770,"y":160,"wires":[]}]

You can try this flow as I pasted it and then with your actual serial input. Does it give the result you need?