Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mcu_code 1st #122

Closed
wants to merge 4 commits into from
Closed

mcu_code 1st #122

wants to merge 4 commits into from

Conversation

NW-Lab
Copy link
Contributor

@NW-Lab NW-Lab commented Feb 23, 2024

Hello

I wanted to run Moddable code on Node-Red.
Removed sandbox from Node-Red core's function and made it possible to write moddable_manifest.

As an example, the following Flows can be executed.(This is the same code as moddable/examples/drivers/aw3641.)

[
    {
        "id": "a9469753d31dfb25",
        "type": "tab",
        "label": "フロー 2",
        "disabled": false,
        "info": "",
        "env": [],
        "_mcu": {
            "mcu": true
        }
    },
    {
        "id": "4043457111bf2b23",
        "type": "inject",
        "z": "a9469753d31dfb25",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "_mcu": {
            "mcu": true
        },
        "x": 360,
        "y": 200,
        "wires": [
            [
                "38a937622a3b18a3"
            ]
        ]
    },
    {
        "id": "c7d233f7729a0722",
        "type": "debug",
        "z": "a9469753d31dfb25",
        "name": "debug 1",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "false",
        "statusVal": "",
        "statusType": "auto",
        "_mcu": {
            "mcu": true
        },
        "x": 900,
        "y": 220,
        "wires": []
    },
    {
        "id": "38a937622a3b18a3",
        "type": "mcu_code",
        "z": "a9469753d31dfb25",
        "name": "mcu_code 1",
        "func": "msg.payload=\"abc\";\nreturn msg;",
        "outputs": 1,
        "timeout": 0,
        "initialize": "//const val=AW3641.off;\r\n//const val=AW3641.Time220ms_Brightness100;\r\n//const val=AW3641.Time220ms_Brightness90;\r\n//const val=AW3641.Time220ms_Brightness80;\r\n//const val=AW3641.Time220ms_Brightness70;\r\n//const val=AW3641.Time220ms_Brightness60;\r\nconst val=AW3641.Time220ms_Brightness50;\r\n//const val=AW3641.Time220ms_Brightness40;\r\n//const val=AW3641.Time220ms_Brightness30;\r\n//const val=AW3641.Time1_3s_Brightness100;\r\n//const val=AW3641.Time1_3s_Brightness90;\r\n//const val=AW3641.Time1_3s_Brightness80;\r\n//const val=AW3641.Time1_3s_Brightness70;\r\n//const val=AW3641.Time1_3s_Brightness60;\r\n//const val=AW3641.Time1_3s_Brightness50;\r\n//const val=AW3641.Time1_3s_Brightness40;\r\n//const val=AW3641.Time1_3s_Brightness30;\t\r\n\r\nconst lamp = new AW3641({pin:26,});\r\n\r\nTimer.repeat(() => {\r\n\tlamp.flash(val);\r\n\ttrace(\"flash\\n\");\r\n}, 2000);\r\n\r\n\r\n",
        "finalize": "",
        "libs": [
            {
                "var": "Timer",
                "module": "timer"
            },
            {
                "var": "AW3641",
                "module": "aw3641"
            }
        ],
        "moddable_manifest": "{\"include\":[\"$(MODDABLE)/examples/manifest_base.json\",\"$(MODDABLE)/modules/drivers/aw3641/manifest.json\"],\"modules\":{\"*\":[\"./main\"]}}",
        "_mcu": {
            "mcu": true
        },
        "x": 650,
        "y": 220,
        "wires": [
            [
                "c7d233f7729a0722"
            ]
        ]
    }
]

Thank you.

Copy link
Owner

@phoddie phoddie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting idea. As I understand it, mcu_code is a version of the function node with two differences:

  1. The functions are only executed on the MCU. This is for functions with code that is "MCU-only" and would generate an exception running under Node-RED on computers.
  2. There is an optional Moddable SDK manifest

This all seems convenient for developers building flows.

It might be more clear if the name of the node type included the word "function" so that developers understand it is a kind of function node. Maybe "MCU Function" or "Function - MCU"?

FWIW – sometimes it is useful to have a Function node that runs on both MCU and computers but has different behaviors. For example, the computer implementation might be a simulator for testing / development. To do that, you can check RED.mcu:

if (RED.mcu) {
   // running on MCU
}
else {
   // running on computer
}

Using that approach, a separate function node is less necessary. Of course, that doesn't provide the manifest feature.

@ralphwetzel – I vaguely recall you've thought about this kind of situation before. Do you have any input?

nodered.js Outdated Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a copy of the Function node's HTML with some changes to add the manifest. It will be difficult to keep this in sync with future changes to the Function node. I don't know if there is a good way to achieve that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree too.
If you are running only on the MCU, you may not need to synchronize. There may also be possibilities to add other features.
If I could write a simple Moddable code (function), I would be fine with just that function, but it was difficult for me, so I created it by changing the Function node.

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 24, 2024

Thank you for your response.

I'm not good at English, so I use Google Translate. I'm sorry if I didn't convey the nuance.

The idea was to execute Moddable code using Node-RED.

  • The Node-RED core node Function sandbox does not recognize drivers defined in the Manifest. An error occurs. This error may disable the Node-RED Plugin's "BUILD" button.
  • There is no way to write a manifest when using Node-RED MCU Plugin.
  • The functions defined in Node-RED's device target (for example, M5Stack buttons) are not available.

For example, you can save the following code (which causes an interrupt on an M5Button) to a node using Node-RED import/export to take advantage of the Node-RED ecosystem.
In this example, it becomes the function of Button (Digital in).

[
    {
        "id": "1017cc0e2a06da55",
        "type": "mcu_code",
        "z": "abf55a50195b9e0d",
        "name": "M5StackButton",
        "func": "\nreturn msg;",
        "outputs": 1,
        "timeout": 0,
        "initialize": "buttonA.onChanged = function () {\r\n  const up = this.read()\r\n  if (up === 0) {\r\n    msg.send(\"BtnA\");\r\n  }\r\n}\r\nbuttonB.onChanged = function () {\r\n  const up = this.read()\r\n  if (up === 0) {\r\n    msg.send(\"BtnB\");\r\n  }\r\n}\r\nbuttonC.onChanged = function () {\r\n  const up = this.read()\r\n  if (up === 0) {\r\n    msg.send(\"BtnC\");\r\n  }\r\n}",
        "finalize": "",
        "libs": [],
        "moddable_manifest": "",
        "_mcu": {
            "mcu": false
        },
        "x": 460,
        "y": 360,
        "wires": [
            [
                "087a9ce6e7125150"
            ]
        ]
    }
]

I didn't know about the RED.mcu function.
It would be good if there was a way to turn off the sandbox of the Function node and a function to write manifest.json.

Thanks,

@phoddie
Copy link
Owner

phoddie commented Feb 24, 2024

I'm not good at English, so I use Google Translate. I'm sorry if I didn't convey the nuance.

No problem. JavaScript is a universal language.

The functions defined in Node-RED's device target (for example, M5Stack buttons) are not available

They should be. You can access the M5Stack buttons, for example. I modified your "mcu_code" node, fixing the start-up script and making it a "function" node. It seems to work with M5Stack buttons.

    {
        "id": "1017cc0e2a06da55",
        "type": "function",
        "z": "8c60c58949866055",
        "name": "M5StackButton",
        "func": "\nreturn msg;",
        "outputs": 1,
        "noerr": 0,
        "initialize": "button.a.onChanged = function () {\n  const up = this.read()\n  if (up === 0) {\n    node.send({payload: \"BtnA\"});\n  }\n}\nbutton.b.onChanged = function () {\n  const up = this.read()\n  if (up === 0) {\n    node.send({payload: \"BtnB\"});\n  }\n}\nbutton.c.onChanged = function () {\n  const up = this.read()\n  if (up === 0) {\n    node.send({payload: \"BtnC\"});\n  }\n}",
        "finalize": "",
        "libs": [],
        "_mcu": {
            "mcu": false
        },
        "x": 460,
        "y": 180,
        "wires": [
            [
                "c274399f02e409b4"
            ]
        ]
    },
  • The Node-RED core node Function sandbox does not recognize drivers defined in the Manifest. An error occurs. This error may disable the Node-RED Plugin's "BUILD" button.
  • There is no way to write a manifest when using Node-RED MCU Plugin.

I understand these problems need to be solved in some way. Maybe your solution is good. I'm just not sure yet. I'd like to think about it. Also, I'm curious if @ralphwetzel has any ideas, since he knows the plug-in best.

@phoddie
Copy link
Owner

phoddie commented Feb 24, 2024

@NW-Lab – would you confirm that the modified node above allows you to access the M5 buttons? Thank you.

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 25, 2024

@phoddie
sorry. The original code was incorrect.
I confirmed that it works with the Function node.

It would be nice if there was a way to stop sandbox execution of a function and a node where you could freely write manifest.json. Since manifest.json is a driver specification, I think it accompanies the code. Therefore, it may be better to use it as a comment node in the flow rather than specifying it in the Plugin.

Thank you.

@phoddie
Copy link
Owner

phoddie commented Feb 25, 2024

sorry. The original code was incorrect.
I confirmed that it works with the Function node.

Excellent. Thank you.

It would be nice if there was a way to stop sandbox execution of a function

I want to confirm. You want to prevent the code of the Function node from running in Node-RED, but allow it to run on Node-RED MCU?

and a node where you could freely write manifest.json. Since manifest.json is a driver specification, I think it accompanies the code. Therefore, it may be better to use it as a comment node in the flow rather than specifying it in the Plugin.

A Comment node is a nice idea. It could eventually be a "manifest" node. But, your original idea to have the manifest with the Function/MCU-Code node is nice. As an experiment, I using the Description field of the Function node to hold the manifest. It is a small hack, but an easy one.

Here's a test flow that uses the qrCode module. The function scripts test RED.mcu so they only run on the MCU.

flows.json
[
    {
        "id": "8c60c58949866055",
        "type": "tab",
        "label": "Flow 7",
        "disabled": false,
        "info": "",
        "env": [],
        "_mcu": {
            "mcu": false
        }
    },
    {
        "id": "1017cc0e2a06da55",
        "type": "function",
        "z": "8c60c58949866055",
        "name": "M5StackButton",
        "func": "if (!RED.mcu) return;\n\nlet qr = qrCode({input: msg.payload});\nlet size = qr.size;\nqr = new Uint8Array(qr);\n\nmsg.payload = \"\";\nfor (let y = 0; y < size; y++) {\n\tfor (let x = 0; x < size; x++) {\n\t\tif (qr[(y * size) + x])\n\t\t\tmsg.payload += \"X\";\n\t\telse\n\t\t\tmsg.payload += \".\";\n\t}\n\tmsg.payload += \"\\n\";\n}\n\n\nreturn msg;\n",
        "outputs": 1,
        "noerr": 0,
        "initialize": "if (!RED.mcu) return;\n\nbutton.a.onChanged = function () {\n  const up = this.read()\n  if (up === 0) {\n    node.send({payload: \"BtnA\"});\n  }\n}\nbutton.b.onChanged = function () {\n  const up = this.read()\n  if (up === 0) {\n    node.send({payload: \"BtnB\"});\n  }\n}\nbutton.c.onChanged = function () {\n  const up = this.read()\n  if (up === 0) {\n    node.send({payload: \"BtnC\"});\n  }\n}",
        "finalize": "",
        "libs": [
            {
                "var": "qrCode",
                "module": "qrcode"
            }
        ],
        "_mcu": {
            "mcu": false
        },
        "x": 460,
        "y": 160,
        "wires": [
            [
                "c274399f02e409b4"
            ]
        ],
        "info": "{\n    \"include\": [\n        \"$(MODULES)/data/qrcode/manifest.json\"\n    ]\n}\n"
    },
    {
        "id": "c274399f02e409b4",
        "type": "debug",
        "z": "8c60c58949866055",
        "name": "debug 90",
        "active": true,
        "tosidebar": false,
        "console": true,
        "tostatus": false,
        "complete": "payload",
        "targetType": "msg",
        "statusVal": "",
        "statusType": "auto",
        "_mcu": {
            "mcu": false
        },
        "x": 690,
        "y": 160,
        "wires": []
    },
    {
        "id": "e68d8b42704484e3",
        "type": "inject",
        "z": "8c60c58949866055",
        "name": "",
        "props": [
            {
                "p": "payload"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": true,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "one two",
        "payloadType": "str",
        "_mcu": {
            "mcu": false
        },
        "x": 220,
        "y": 120,
        "wires": [
            [
                "1017cc0e2a06da55"
            ]
        ]
    },
    {
        "id": "1ee20679e607da19",
        "type": "inject",
        "z": "8c60c58949866055",
        "name": "",
        "props": [
            {
                "p": "payload"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": true,
        "onceDelay": "1",
        "topic": "",
        "payload": "three four",
        "payloadType": "str",
        "_mcu": {
            "mcu": false
        },
        "x": 220,
        "y": 180,
        "wires": [
            [
                "1017cc0e2a06da55"
            ]
        ]
    }
]

To use the manifest from the Description requires change mcmanifest.json. This is similar to the change in your PR.

				flows.forEach((node, i) => {
					let manifest;
					if (("function" === node.type) && node.info?.startsWith("{"))
						manifest = JSON.parse(node.info);
					else if (node.moddable_manifest)
					...

This approach works. It is a little strange. But... it requires no new nodes, so it is a convenient way to experiment further. What do you think?

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 25, 2024

I thought it was a good idea, but it seems like there might be an error in the sandbox.

The message "The flow has been stopped because an unknown module exists. aw3641-404" will be displayed and the Plugin BUILD button will be disabled.

image

image

image

flows.json

[
    {
        "id": "a9469753d31dfb25",
        "type": "tab",
        "label": "フロー 2",
        "disabled": false,
        "info": "",
        "env": [],
        "_mcu": {
            "mcu": true
        }
    },
    {
        "id": "4043457111bf2b23",
        "type": "inject",
        "z": "a9469753d31dfb25",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "_mcu": {
            "mcu": true
        },
        "x": 160,
        "y": 140,
        "wires": [
            [
                "73b0486d25bfe6dc"
            ]
        ]
    },
    {
        "id": "c7d233f7729a0722",
        "type": "debug",
        "z": "a9469753d31dfb25",
        "name": "debug 1",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "false",
        "statusVal": "",
        "statusType": "auto",
        "_mcu": {
            "mcu": true
        },
        "x": 520,
        "y": 140,
        "wires": []
    },
    {
        "id": "1298a2c40c7497d1",
        "type": "comment",
        "z": "a9469753d31dfb25",
        "name": "moddable_manifest",
        "info": "{\n    \"include\": [\n        \"$(MODDABLE)/examples/manifest_base.json\",\n        \"$(MODDABLE)/modules/drivers/aw3641/manifest.json\"\n    ],\n    \"modules\": {\n        \"*\": [\n            \"./main\"\n        ]\n    }\n}",
        "_mcu": {
            "mcu": true
        },
        "x": 150,
        "y": 60,
        "wires": []
    },
    {
        "id": "73b0486d25bfe6dc",
        "type": "function",
        "z": "a9469753d31dfb25",
        "name": "function 2",
        "func": "msg.payload = \"abc\";\nreturn msg;",
        "outputs": 1,
        "timeout": 0,
        "noerr": 1,
        "initialize": "if(RED.mcu){\n    //const val=AW3641.off;\n    //const val=AW3641.Time220ms_Brightness100;\n    //const val=AW3641.Time220ms_Brightness90;\n    //const val=AW3641.Time220ms_Brightness80;\n    //const val=AW3641.Time220ms_Brightness70;\n    //const val=AW3641.Time220ms_Brightness60;\n    const val = AW3641.Time220ms_Brightness50;\n    //const val=AW3641.Time220ms_Brightness40;\n    //const val=AW3641.Time220ms_Brightness30;\n    //const val=AW3641.Time1_3s_Brightness100;\n    //const val=AW3641.Time1_3s_Brightness90;\n    //const val=AW3641.Time1_3s_Brightness80;\n    //const val=AW3641.Time1_3s_Brightness70;\n    //const val=AW3641.Time1_3s_Brightness60;\n    //const val=AW3641.Time1_3s_Brightness50;\n    //const val=AW3641.Time1_3s_Brightness40;\n    //const val=AW3641.Time1_3s_Brightness30;\t\n\n    const lamp = new AW3641({ pin: 26, });\n\n    Timer.repeat(() => {\n        lamp.flash(val);\n        trace(\"flash\\n\");\n    }, 2000);\n}",
        "finalize": "",
        "libs": [
            {
                "var": "Timer",
                "module": "timer"
            },
            {
                "var": "AW3641",
                "module": "aw3641"
            }
        ],
        "_mcu": {
            "mcu": true
        },
        "x": 360,
        "y": 140,
        "wires": [
            [
                "c7d233f7729a0722"
            ]
        ]
    }
]

mcmanifest.js

if(("comment" === node.type)&&("moddable_manifest" === node.name)&&(node.info?.startsWith("{")))
	manifest = JSON.parse(node.info);
else if (node.moddable_manifest)
	manifest = {...node.moddable_manifest};
else if (nodeTypes[node.type])

It's not going well.

Thank you.

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 25, 2024

I confirmed that node-red-mcu works well.
An error occurs when "BUILD" is executed using Node-red-mcu Plugin (when "Deploy" of node-red is executed).

If possible, I think it would be better to take measures so that the Plugin's "BUILD" button does not become disabled. .

Thank you.

@ralphwetzel
Copy link
Contributor

Hi!
Some additional thoughts - most probably not exhaustive:

  • As you found out, you cannot use the "standard" feature of a function node to define references to MCU related libraries. If you dig into the Node-RED core, you'll discover that those modules are loaded not (on demand) just into the function node, but at first into the Node-RED environment. Thus they must exist - or you'll get the error you experienced.

  • To run portions of a flow only when this flow is executed on a MCU, I've once created node-red-mcu-gate. By concept it blocks incoming messages in standard Node-RED but forwards them on an MCU. When the flow is build via the plugin, it's completely removed to not generate any overhead. It's yet not a switch-like node that allows to execute either (MCU) / or (standard Node-RED) logic.

  • I've tried several ways to allow the definition of additional manifests - on plugin level, on flow level, on function node level.

    • Plugin level is easy to implement, but always needs plugin action to inject it into the build process. It's yet not part of the flow, thus will be lost when the flow is exported.
    • Flow level isn't too difficult to implement either. It can be stored in the flow's definition section in flows.json - and could be extracted from there by nodered2mcu (or by the plugin).
    • function node level is quite complex to implement (if the intension is to patch the standard node), with the additional downside to create an bad user experience: As explained above, users shall not use the standard features of the function node HMI. This could be overcome by a MCU dedicated, non-standard function node (with an adapted HMI), providing as well the option to define an additional manifest.json. The BUT here is - as @phoddie already mentioned - that it's to a certain extent a copy of the standard node, with the risc of additional effort to keep both in sync.

    For my personal use, I've created - similar to what @NW-Lab intended - that dedicated function node, even with a bit of extra functionality (error feedback). It's yet not released anywhere - but could be of course.

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 25, 2024

hello

@ralphwetzel, thank you for all the information.

For example, if the comment node is named "moddable_import", it seems like incorporating a mechanism to add imports would help organize the current request.
If you use "if(RED.mcu)", the syntax check of the Function node will not generate any errors.

Thank you

@phoddie
Copy link
Owner

phoddie commented Feb 27, 2024

@ralphwetzel – thank you for your extensive notes.

As you found out, you cannot use the "standard" feature of a function node to define references to MCU related libraries. If you dig into the Node-RED core, you'll discover that those modules are loaded not (on demand) just into the function node, but at first into the Node-RED environment. Thus they must exist - or you'll get the error you experienced.

My experience is that this is inconsistent. Some module specifiers do not generate an error, but others do. That makes it an impediment to using the standard Function module, as you note.

Your node-red-mcu-gate is an elegant solution to part of this challenge. We could achieve something similar with the switch node my defining an "MCU" environment variable in Node-RED MCU. The downside of both is that at least two nodes are now needed instead of one, which is a headache for developers.

The most developer-friendly approach is what both you and @NW-Lab have experimented with – a single node:

  • MCU only
  • Embeds manifest
  • Doesn't generate errors in Node-RED running under Node.js
  • Works with/without the plug-in

That's a maintenance headache, unfortunately.

(FWIW – based on what we've seen with @NW-Lab's work, this should only require minor changes to nodered2mcu and perhaps no changes to the Node-RED MCU runtime.)

For my personal use, I've created - similar to what @NW-Lab intended - that dedicated function node, even with a bit of extra functionality (error feedback). It's yet not released anywhere - but could be of course.

That would be interesting input into this discussion. I'm sure @NW-Lab would be interested to compare. Would you consider sharing a prerelease of that?

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 27, 2024

hello

I've published the revised version on Moddable.
Moddable-OpenSource/moddable#1317

This PR is no longer needed.
How about this?

Thank you.

@NW-Lab
Copy link
Contributor Author

NW-Lab commented Feb 28, 2024

Hello

I think there may be more than one way to create a manifest.

At the moment, I'm satisfied with the method of creating comments nodes, and it seems like it's not a good idea to increase the number of nodes as @phoddie wrote.
However, I feel that it is best for the Manifest required for code written in Node to be attached to Node. Let's leave this as a future topic.

For now, I will close this pull request.
For further discussion, please visit Moddable-OpenSource/moddable#1317

Thank you

@NW-Lab NW-Lab closed this Feb 28, 2024
@phoddie
Copy link
Owner

phoddie commented Mar 1, 2024

For developers, the best solution is to have a single Function Node. The alternative, creating an MCU Function node, is possible as @NW-Lab and @ralphwetzel have independently shown, but harder to maintain and more for developers to think about.

It seems like the Function Node could be a little more expressive in Node-RED. If it was, it would also be better for Node-RED MCU. I am sure proposing any changes to the Function Node would be tough, since it is so widely used. Still, let's discuss what's might be interesting.

  • If a Function node's imports are not found, the flow stops. Is that always the desired behavior? Imports could be marked as optional or required, and only stop the flow if they are required and not found. I suppose in Node-RED the alternative is not to use the import feature of the Function node and instead of require() or await import(). Similarly, in Node-RED MCU you could use Modules.importNow(). But, these are more work for developers and less readable.
  • Maybe a Function node should only be used in certain environments. A Function node could be set to only run if a certain Environment variable is set (or not set). Node-RED MCU Edition would always set an "MCU" environment variable. This is a more general version of @ralphwetzel 's clever MCU Gate. The Node-RED editor can show which Function nodes are disabled.

@NW-Lab NW-Lab deleted the mcu_code branch March 1, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants