colinl / node-red-contrib-pid

A node-red PID loop controller node intended for the control of real world processes
Apache License 2.0
26 stars 18 forks source link

Derivative time as property type issue #26

Closed kevintrewern closed 2 weeks ago

kevintrewern commented 9 months ago

Excuse my lack of node red experience.

I have used this PID controller within a subflow to add some reusability for my project (mainly coupling it with PWM). I was getting "Integral Locked" due to the process value being far from the setpoint - as expected - but this was dropping the output off so nothing ever happened (heating system). Eventually I realised that if I hard-coded the values into the PID node within my subflow, the output stays on as expected. After much experimentation, I found that the Derivative time was causing this if it was set to a number type. For now, setting it to a string type seems to fix this. Presumably these variables should all expect a number as standard?

Thanks, Kevin.

colinl commented 8 months ago

Also could you add a debug node to the Poll state node feeding the pid and check that there is nothing unnexpected coming from there. Check the topic as well as the pv. I thought you had to put the smooth node back in, but I see it is not in the pid feed in the flow you exported.

kevintrewern commented 8 months ago

Sadly I can't make that fail. Was it actually showing NaN when you exported it?

Yes it was.

kevintrewern commented 8 months ago

Also could you add a debug node to the Poll state node feeding the pid and check that there is nothing unnexpected coming from there. Check the topic as well as the pv. I thought you had to put the smooth node back in, but I see it is not in the pid feed in the flow you exported.

See below, I think this is what you wanted. All I have done is add the extra Debug and change D_SET to number type. I thought removing the smoothing was helping but I was just being impatient, forgetting it takes longer to get integral locked with the process value updating less frequently.

So basically, I still cannot put my finger on what is triggering the problem, it seems to move around as I change other things. At the moment it can be triggered by setting D_SET to a number and will work if that is changed back to a String, but this isn't always the case. The worst kind of issue!

image

image

colinl commented 8 months ago

Do you know how to open a command window into the node-red container. I think you would do sudo docker ps which would show you all the containers. The hex string is the id of the container, note the one for node red then run sudo docker exec -it hex_id_here bash If that complains the bash is not recognised then try sh instead. exit will close the shell. Let me know if that works.

kevintrewern commented 8 months ago

Yes, after a bit of messing around with HassOS security I am in, let me know what I need to do in there:

image

colinl commented 8 months ago

First we have to find the folder where you flows file is, I think that is /data. So if you run cd /data ls -l you should see a flows.json file dated the last time you updated the flows, and there should also be a package.json and a node_modules folder (those are different ones to those in the image you posted, which are for node-red itself rather than your flows). If you do see those then you run npm install colinl/node-red-contrib-pid#param_debug_branch If that complains that git is not installed then add it to system_packages as described here and restart the node-red container, or reboot, and try the npm install ... again. You will have to get the container id again as it will change each time the container is restarted. If it installs this time then restart node-red and hopefully it will now be using my debug version which has additional debug in the payload sent out. So if you can get it to the stage where something there is showing NaN then fully expand the debug output and show us what you get. If you don't see the flows file in /data then run find / --name flows.json and see if that finds the right folder.

kevintrewern commented 8 months ago

OK its in /config which makes sense and has 01/01/2024 which seems right. I will run a backup and then attempt installing your debug version!

image

kevintrewern commented 8 months ago

Ok Installed.

image

kevintrewern commented 8 months ago

Battling with this at the moment, don't know how to step the version back and don't really need to as the server is always on so going to wait for the next update but annoying as every time I restart node-red I have to reinstall the package!

https://community.home-assistant.io/t/warning-do-not-update-websocket-to-0-61-1-with-add-on-version-16-0-2/653887

I am not convinced that it is using the debug version unless you kept the version details the same?

image

kevintrewern commented 8 months ago

I am not running it with settings that Fault it at the moment but I am pretty sure this is not your debug version. back to work tomorrow so I will have less time, especially with catching up from the break.

image

colinl commented 8 months ago

Yes it is, I have added the params object which contains the config values.

kevintrewern commented 8 months ago

I didn't spot that! Here's showing params:

image

colinl commented 8 months ago

Excellent, now we are getting somewhere. As you can see the TD is NaN, the question is why is it not a number?

I have added some more debug, if you run the install again it will pick up the new version and the debug should also now include a property configParams which should shows the actual values being picked up from the subflow settings.

kevintrewern commented 8 months ago

Great OK I will try to do this tomorrow. What time zone are you in, I am UK not sure if there is a best time of day for me to look or if we are running at a similar time anyway?

Cheers.

colinl commented 8 months ago

I am in the UK too (mid Wales), but retired so don't have set times generally. Playing table tennis tomorrow morning, at home in the afternoon, but it should be dry tomorrow, at last, so lots of catching up to do outside.

kevintrewern commented 8 months ago

I know what you mean about catching up outside, it seemed like the rain would never end. We are in a wet part of Devon/Cornwall as it is so even the "dry days" have had showers but so much better than it has been today at lease!

image

colinl commented 8 months ago

Well, I wasn't expecting that. As you can see, some of the parameters are not being substituted with the actual values, so when the pid node picks them up they are most definately Not a Number. How are the subflow values configured now? In terms of strings or numbers I mean. Can you expand the params object too please?

Do you know how to get a node-red startup log? I don't know how to do that when running in HA, but it is possible. It would be good to see that as there is something very odd going on.

colinl commented 8 months ago

Also could you show what is coming out of the other subflow, I am interested in whether it is the same params that are not getting substituted.

colinl commented 8 months ago

Also please right click on the background in the editor, and a context menu should popup, select Show Action List and search for System and you should find Show System Info. Select that and the resulting window has a Copy To Clipboard at the top. Do that and paste it here please. I have started a thread on the node red forum hoping that someone can suggest what might be going on. https://discourse.nodered.org/t/subflow-parameters-not-being-substituted/84321

kevintrewern commented 8 months ago

Well, I wasn't expecting that. As you can see, some of the parameters are not being substituted with the actual values, so when the pid node picks them up they are most definately Not a Number. How are the subflow values configured now? In terms of strings or numbers I mean. Can you expand the params object too please?

Do you know how to get a node-red startup log? I don't know how to do that when running in HA, but it is possible. It would be good to see that as there is something very odd going on.

Totally agree that substitution is the issue here! Un fortunately I wouldn't have the first clue why.

Everything is sent to the subflows as a number, changing D_SET to string allows me to use the PID loop.

Here is what you asked for, first one with D_SET as number (bugged) and second with D_SET as string, so running.

I will have a dig around for Node Red startup log. Must be available somewhere.

image

image

kevintrewern commented 8 months ago

Also could you show what is coming out of the other subflow, I am interested in whether it is the same params that are not getting substituted.

Sure here is the POT:

image

kevintrewern commented 8 months ago

Also please right click on the background in the editor, and a context menu should popup, select Show Action List and search for System and you should find Show System Info. Select that and the resulting window has a Copy To Clipboard at the top. Do that and paste it here please. I have started a thread on the node red forum hoping that someone can suggest what might be going on. https://discourse.nodered.org/t/subflow-parameters-not-being-substituted/84321

Here you go. I will follow the other thread as well, thanks!

{
    "report": "diagnostics",
    "scope": "basic",
    "time": {
        "utc": "Fri, 05 Jan 2024 17:16:27 GMT",
        "local": "1/5/2024, 5:16:27 PM"
    },
    "intl": {
        "locale": "en-US",
        "timeZone": "Europe/London"
    },
    "nodejs": {
        "version": "v18.18.2",
        "arch": "x64",
        "platform": "linux",
        "memoryUsage": {
            "rss": 165093376,
            "heapTotal": 107151360,
            "heapUsed": 95435104,
            "external": 20398513,
            "arrayBuffers": 19092419
        }
    },
    "os": {
        "containerised": true,
        "wsl": false,
        "totalmem": 8321105920,
        "freemem": 6658850816,
        "arch": "x64",
        "loadavg": [
            0.01,
            0.05,
            0.06
        ],
        "platform": "linux",
        "release": "6.1.63-haos",
        "type": "Linux",
        "uptime": 173839.38,
        "version": "#1 SMP PREEMPT_DYNAMIC Mon Dec  4 15:31:27 UTC 2023"
    },
    "runtime": {
        "version": "3.1.0",
        "isStarted": true,
        "flows": {
            "state": "start",
            "started": true
        },
        "modules": {
            "node-red": "3.1.0",
            "node-red-contrib-actionflows": "2.1.2",
            "node-red-contrib-bigtimer": "2.8.5",
            "node-red-contrib-cast": "0.2.17",
            "node-red-contrib-counter": "0.1.6",
            "node-red-contrib-influxdb": "0.6.1",
            "node-red-contrib-interval-length": "0.0.6",
            "node-red-contrib-modbus": "5.27.2",
            "node-red-contrib-moment": "5.0.0",
            "node-red-contrib-sunevents": "3.1.1",
            "node-red-contrib-time-range-switch": "1.2.0",
            "node-red-dashboard": "3.6.2",
            "node-red-node-base64": "0.3.0",
            "node-red-node-email": "2.1.0",
            "node-red-node-feedparser": "0.3.0",
            "node-red-node-ping": "0.3.3",
            "node-red-node-random": "0.4.1",
            "node-red-node-serialport": "2.0.2",
            "node-red-node-smooth": "0.1.2",
            "node-red-node-suncalc": "1.1.0",
            "node-red-node-twitter": "1.2.0",
            "node-red-contrib-pid": "2.0.0",
            "node-red-contrib-timeprop": "1.0.1",
            "node-red-contrib-simple-gate": "0.5.2",
            "node-red-contrib-bool-gate": "1.0.2",
            "node-red-contrib-queue-gate": "1.5.5",
            "node-red-contrib-message-gate": "0.1.0",
            "node-red-contrib-google-action": "1.2.1",
            "node-red-contrib-google-home-notifier-offline": "1.1.1",
            "node-red-contrib-google-home-notify": "0.0.7",
            "node-red-contrib-google-translate": "0.0.5",
            "node-red-contrib-google-tts": "1.1.1",
            "node-red-contrib-castv2": "4.3.0",
            "node-red-contrib-simplecast": "1.0.8",
            "node-red-contrib-alexa-smart-home": "0.4.68",
            "node-red-contrib-alexa-remote2": "3.10.4",
            "node-red-contrib-alexa-local": "0.3.24",
            "node-red-contrib-persistent-fsm": "1.2.1",
            "node-red-contrib-amazon-echo-oztourer": "0.1.12",
            "node-red-contrib-home-assistant-websocket": "0.59.0"
        },
        "settings": {
            "available": true,
            "apiMaxLength": "UNSET",
            "disableEditor": false,
            "contextStorage": {
                "store": {
                    "module": "localfilesystem"
                },
                "default": {
                    "module": "memory"
                }
            },
            "debugMaxLength": 1000,
            "editorTheme": {
                "projects": {
                    "enabled": false
                }
            },
            "flowFile": "flows.json",
            "mqttReconnectTime": 15000,
            "serialReconnectTime": 15000,
            "socketReconnectTime": "UNSET",
            "socketTimeout": "UNSET",
            "tcpMsgQueueSize": "UNSET",
            "inboundWebSocketTimeout": "UNSET",
            "runtimeState": "UNSET",
            "adminAuth": "UNSET",
            "httpAdminRoot": "/",
            "httpAdminCors": "UNSET",
            "httpNodeAuth": "SET",
            "httpNodeRoot": "/endpoint/",
            "httpNodeCors": "UNSET",
            "httpStatic": "UNSET",
            "httpStaticRoot": "UNSET",
            "httpStaticCors": "UNSET",
            "uiHost": "SET",
            "uiPort": "SET",
            "userDir": "SET",
            "nodesDir": "SET"
        }
    }
}
colinl commented 8 months ago

"node-red": "3.1.0",

There have been a couple of releases since that version (now 3.1.3). None of the issues in the release notes specifically fit your problem, but there have been changes in that area of the code. Are you able to upgrade to the latest?

kevintrewern commented 8 months ago

I don't think that I can until they release a newer home assistant version. I have updated everything that I can on my HA but Node Red didn't have an update.

image

colinl commented 8 months ago

Do you have to use HA? Many node red users think it just complicates things and prefer to use node red to do everything.

colinl commented 8 months ago

I wonder whether one can upgrade node-red within the container. What does this command show, from the shell inside the container, in the folder where you ran the npm command to install the pid node? npm list node-red also, for comparison npm list node-red-contrib-pid

kevintrewern commented 8 months ago

I wonder whether one can upgrade node-red within the container. What does this command show, from the shell inside the container, in the folder where you ran the npm command to install the pid node? npm list node-red also, for comparison npm list node-red-contrib-pid

image

kevintrewern commented 8 months ago

Do you have to use HA? Many node red users think it just complicates things and prefer to use node red to do everything.

A good point. The short answer is yes, for now....

I had all sorts of ideas for automating things when I first installed Home Assistant - and I also dabbled with OpenHab. I discovered Node-red quite a bit later. The reality is that I don't get much time to play with it so it has pretty much left as is. Ultimately, it provides me with a simple solution to control the two rooms heating and do a couple of other odds and ends. I don't have the time to go further with it currently or in the foreseeable future, and in fact had I to remove the controllers from the living room lights as I was fed up with them coming on randomly and wasting electricity - and getting stuck on overriding the manual switches, and the dimmers where rubbish anyway as LED lights really don't dim well unless you spend a fortune on them. I spent hours researching why this was happening and tried various things and in the end just gave up (sure maybe I should have also dropped the faceplates off and soldered on some pullup/pulldown resistors and tried that but it had stopped being fun by then!).

The family isn't interested in using it so I just carry on using it to control the heating in those two rooms! Perhaps I should just get rid of it altogether but there is no point in spending money on new heating controllers when it is installed and working and the one thing that really is useful is being able to pre-heat the outbuilding which takes about an hour so it makes the room much more usable in winter.

Having said all that, I appreciate your work and effort to sort this and there obviously is an issue there, so I am happy to keep bashing away at it when I get chance to help get the problem sorted.

In reality I can just set that one variable to string and use it as it is, so let me know if this isn't important to you and I wont waste any more of your time on it.

Again, thanks for all the effort on this. Let me know what you want to do going forward!

colinl commented 8 months ago

I downgraded one of my systems and am now seeing exactly the same symptom as you are. I think probably the best thing is just for you to configure the inputs to strings and check in the debug output that there are no NaNs in configParams or params. Then it should all work as expected. The ones that seem to problematic are derivative time, output power when disabled and Enable. Make them all strings. For Enable set it to string 0 to disable or 1 to enable. You don't need to change the value you feed in to dynamically enable/disable as that is not affected by the bug. Once the next release of the node red addon is available then the problem should go away (but it will continue to work with strings anyway.

kevintrewern commented 8 months ago

OK so it turns out to be some issue with older versions of Node Red then? Thanks for all this, I really appreciate you spending the time to look at it with me!

colinl commented 8 months ago

Investigating the problem has pointed out that I should do better validation of some of the properties, so certainly not a waste of time on my part.