tbowmo / node-red-small-timer

timer node for node red
1 stars 0 forks source link

Error: Cannot find module 'node-red' - Breaks Node-Red #10

Closed Hisma closed 1 year ago

Hisma commented 1 year ago

You have a problem in the small-timer-runner.js file.
on start-up node-red console was giving me this error -

7 May 14:25:21 - [warn] ------------------------------------------------------
7 May 14:25:21 - [warn] [@tbowmo/node-red-small-timer/smalltimer] Error: Cannot find module 'node-red'

This was causing my node-red to halt execution and would not go away unless I uninstalled the node.

I went into the small-timer-runner.js and removed the require("node-red"); line (line 4 I believe), the error went away and I was able to launch and configure the node no problem. So node-red doesn't recognize this reference, at least on my installation. Worth noting I have about a dozen other external nodes installed & none of them have given this error, so I don't think it's a problem on my end.
Please look into this as others may encounter the same problem.

tbowmo commented 1 year ago

I need more info, so what is your environment?

Hisma commented 1 year ago

Sure. node-red version: 3.0.2 nodejs version: v18.16.0 not running in docker. Running ubuntu v22.04 I installed nodejs directly from source with command curl -sL https://deb.nodesource.com/setup_18.x | sudo -E bash -sudo apt-get install -y nodejs and then node-red directly from npm npm install -g --unsafe-perm node-red I'm running as root which I think may not be recommended, but I do as I run a closed system as a single & it's easier for me. So your node module is install in path - /root/.node-red/node_modules/@tbowmo/node-red-small-timer

Hisma commented 1 year ago

Oh, and may not be worth noting, but I launch node-red using pm2.
pm2 start node-red

tbowmo commented 1 year ago

That's weird, my dev system is Ubuntu, however nodered is installed as a global package, (npm - i - g nodered) and started with just nodered in cli.

My "production" server is running docker, without any issues.

Also came to think about it, the mentioned package is nodered itself, so obviously it's installed ;)

Btw, which version of small timer did you install?

Hisma commented 1 year ago

The latest, 0.5.0, but I tried several versions and all had the same issues.

Also, seems like I broke the code by removing that node-red reference.

image

Also, I am not quite understanding. I ran the code last night expecting it to trigger on/off commands at the times I specify.
I do see "ON/OFF" being shown below the node, with the correct time for when the timer should toggle the output. But when the time came, nothing happened. I don't even get any output on the debug window.

image

Do you think the error I'm getting is why this isn't working at all?

As you suggested, I'm not sure why you'd need to even check if node-red is installed. Anyone using this package will have it. I realize it's a good sanity check, but again, I have not encountered issues with any other of my added nodes, so I would not pass this off as "your system is the problem", just because it works in your particular dev & production environment. So perhaps look at how other popular nodes handle this check, or just remove it.

It's also not explained why there's an input on the node. All of your documentation explains how when you enter the time and location, it will generate an on or off command. In your example code, you have different payloads going into the input side of the node using the "inject" node, but those payloads would only be triggered when manually pressed. Please update the documentation to explain the purpose of the input side of your node and how it's used.

If I just want the timer to act on its own without any external input, shouldn't that work? I would expect the configuration of the timer to trigger the events to generate the on/off on the output. I love the concept and how you approached this node, but it needs some additional clarity in the docs.

tbowmo commented 1 year ago

The main issue with the node should be fixed now (release 0.5.1 is out) after trying to install it through node-red on my dev machine, I discovered the same issue. This was due to an incomplete dependency on a @node-red/util library. It's now added in the dependencies for the node.

And yes, it the error is causing the node to malfunction. the code you removed / is missing, is decoding the on / off message strings in the node, and since the code is missing, it can not decode the data internally, and hence not send any output.

It's still a work in progress.. However, the node documentation (inside node-red) does explain how to use the input / payload to instruct the node to do temporary overrides etc.

By itself, without any inputs, it just runs your scheduled on / off times, if that is all you want. The input can be used to temporarily send an on or off message, and then return to the default auto state after a set timeout.

This could perhaps be described better in the readme, but haven't been my focus atm. This as I wrote earlier, this is a work in progress :) (and PR's are welcomed, if there are any ideas / suggestions to enhancements, that be documentation, or code)

Hisma commented 1 year ago

Totally understandable it's a work in progress!

Glad I could at least help fix a bug and give some input as an early user :). My intent for now is to just use it without any inputs, so I'm happy it can do that.

I will update to the latest version and report if I have any other problems.

Thanks again for the quick action and good communication.

tbowmo commented 1 year ago

@Hisma Would you mind closing this issue, if the new version is working?

Created a separate issue for documentation

Hisma commented 1 year ago

@tbowmo just updated to 0.5.1 via npm and confirmed no errors and output is now working. image

Closing issue.