phoddie / node-red-mcu

Node-RED for microcontrollers
120 stars 18 forks source link

Function node: node.warn & node.error shall emit to debug tab #41

Closed ralphwetzel closed 1 year ago

ralphwetzel commented 1 year ago

Hello Peter! The node-red docu states that

The warn and error messages also get sent to the debug tab on the right side of the flow editor.

Currently, those just trace() to the log window of xsbug:

https://github.com/phoddie/node-red-mcu/blob/029cd1e6586435936b84ffe5c44e68ea9f4fc853/nodered.js#L336-L341

The plugin already understands a warn or error message in the following (standard) format (replace error vs warn twice for node.warn):

      trace.left(JSON.stringify({
        "error": {
          "error": <appropriate error message>
        }
      }), this.id);

I propose to add this functionality to class Node definition.

-- R.

phoddie commented 1 year ago

Thanks for the report. The documentation does seem to be ahead of the implementation. I've updated the implementation to relay the error and warning messages to the editor as you propose.

I also added a build configuration to control if messages should be sent to the editor. This avoids the overhead of constructing and sending messages that are unnecessary for the runtime itself. The MCU plug-in will probably always want to set that to true. Currently it is always true in manifest_runtime.json to avoid breaking the MCU plug-in. Please let me know when you update the MCU plug-in set this itself so I can change the default to false. (Thank you!)

ralphwetzel commented 1 year ago

I also added a build configuration to control if messages should be sent to the editor.

That's a good idea! Is my assumption correct, that the plugin is able to set this to true by defining

"config": {
    "noderedmcu": {
        "editor": true
    }
},

in a(ny) manifest.json?

ralphwetzel commented 1 year ago

That's a good idea!

There's one further aspect to consider: My understanding is that trace.left would be eliminated for release builds. https://github.com/phoddie/node-red-mcu/blob/4bbf3635970e0c2e9fab1303d33b79874b8295ec/nodered.js#L146-L147 Those guards yet (e.g. line 146) will stay & consume resources. Is there a way (e.g. like a #pragma) that nodered2mcu or another tool understands to remove them on demand?

phoddie commented 1 year ago

...that the plugin is able to set this to true by defining ... in a(ny) manifest.json?

Yes, you can define this in any manifest.

My understanding is that trace.left would be eliminated for release builds

The call to trace.left is skipped for any build that does not set config.noderedmcu.editor to a truthy value.

Those guards yet (e.g. line 146) will stay & consume resources. Is there a way (e.g. like a #pragma) that nodered2mcu or another tool understands to remove them on demand?

Whey will use a small amount of byte code in flash and the if will be evaluated at runtime. Both are fairly light, so that's a small concern. There are several ways that could be eliminated but none are trivial, so that feels like over-optimization at this stage.

ralphwetzel commented 1 year ago

Please let me know when you update the MCU plug-in set this itself so I can change the default to false. (Thank you!)

Done. You may switch the flag to default state false now.

phoddie commented 1 year ago

Perfect. Thank you! I've committed the change to make the default false.