Comfort node locks up node red if given unusual values

[colinl]

If the Comfort node is given values that are completely ridiculous then it can hang forever, hogging the processor and locking up node-red. This example demonstrates the problem. Be prepared to kill node-red after hitting the inject button.

[{"id":"7b60eaff.1fc824","type":"comfort","z":"514a90a5.c7bae8","name":"","tempField":"payload.temperature","tempFieldType":"msg","mrtField":"payload.mrt","mrtFieldType":"msg","humidityField":"55","humidityFieldType":"num","airspeedField":"0","airspeedFieldType":"num","metabolicRateField":"1","metabolicRateFieldType":"num","clothingLevelField":"1.0","clothingLevelFieldType":"num","comfortField":"payload","comfortFieldType":"msg","sensationField":"","sensationFieldType":"msg","x":507.5,"y":683,"wires":[["99c51b2e.2f30d8"]]},{"id":"99c51b2e.2f30d8","type":"debug","z":"514a90a5.c7bae8","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","x":674,"y":683,"wires":[]},{"id":"462841dd.8019e8","type":"inject","z":"514a90a5.c7bae8","name":"This is ok","topic":"","payload":"","payloadType":"date","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":153.5,"y":653,"wires":[["eed2d31.c7aa0b"]]},{"id":"15a573f0.adae44","type":"inject","z":"514a90a5.c7bae8","name":"This hangs","topic":"","payload":"","payloadType":"date","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":158,"y":710,"wires":[["eac66d03.147a9"]]},{"id":"eed2d31.c7aa0b","type":"change","z":"514a90a5.c7bae8","name":"Sensible values","rules":[{"t":"set","p":"payload","pt":"msg","to":"{\"temperature\":20,\"mrt\":20}","tot":"json"}],"action":"","property":"","from":"","to":"","reg":false,"x":322.5,"y":652,"wires":[["7b60eaff.1fc824"]]},{"id":"eac66d03.147a9","type":"change","z":"514a90a5.c7bae8","name":"Silly values","rules":[{"t":"set","p":"payload","pt":"msg","to":"{\"temperature\":-2040,\"mrt\":-73}","tot":"json"}],"action":"","property":"","from":"","to":"","reg":false,"x":317,"y":711,"wires":[["7b60eaff.1fc824"]]}]

[DeanCording]

Thanks Colin,

I'll look into it.

Dean

On Wednesday, 20 February 2019 20:24:01 AEST Colin Law wrote:

If the Comfort node is given values that are completely ridiculous then it can hang forever, hogging the processor and locking up node-red.This example demonstrates the problem. Be prepared to kill node-red after hitting the inject button. [{"id":"7b60eaff. 1fc824","type":"comfort","z":"514a90a5.c7bae8","name":"","tempField":"payload.temperatu re","tempFieldType":"msg","mrtField":"payload.mrt","mrtFieldType":"msg","humidityField": "55","humidityFieldType":"num","airspeedField":"0","airspeedFieldType":"num","metabolicR ateField":"1","metabolicRateFieldType":"num","clothingLevelField":"1.0","clothingLevelField Type":"num","comfortField":"payload","comfortFieldType":"msg","sensationField":"","sensat ionFieldType":"msg","x":507.5,"y":683,"wires":[["99c51b2e.2f30d8"]]},{"id":"99c51b2e. 2f30d8","type":"debug","z":"514a90a5.c7bae8","name":"","active":true,"tosidebar":true,"co nsole":false,"tostatus":false,"complete":"false","x":674,"y":683,"wires":[]},{"id":"462841dd. 8019e8","type":"inject","z":"514a90a5.c7bae8","name":"This is ok","topic":"","payload":"","payloadType":"date","repeat":"","crontab":"","once":false,"onceD elay":0.1,"x":153.5,"y":653,"wires":[["eed2d31.c7aa0b"]]}, {"id":"15a573f0.adae44","type":"inject","z":"514a90a5.c7bae8","name":"This hangs","topic":"","payload":"","payloadType":"date","repeat":"","crontab":"","once":false,"on ceDelay":0.1,"x":158,"y":710,"wires":[["eac66d03.147a9"]]}, {"id":"eed2d31.c7aa0b","type":"change","z":"514a90a5.c7bae8","name":"Sensible values","rules":[{"t":"set","p":"payload","pt":"msg","to":"{\"temperature\":20,\"mrt\": 20}","tot":"json"}],"action":"","property":"","from":"","to":"","reg":false,"x":322.5,"y": 652,"wires":[["7b60eaff.1fc824"]]}, {"id":"eac66d03.147a9","type":"change","z":"514a90a5.c7bae8","name":"Silly values","rules":[{"t":"set","p":"payload","pt":"msg","to":"{\"temperature\":-2040, \"mrt\":-73}","tot":"json"}],"action":"","property":"","from":"","to":"","reg":false,"x":317,"y": 711,"wires":[["7b60eaff.1fc824"]]}]

—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub[1], or mute the thread[2].[3]

---

[1] https://github.com/DeanCording/node-red-contrib-comfort/issues/4 [2] https://github.com/notifications/unsubscribe-auth/AAPVItF41DB4x5j_XJP-W9_gHBryUOpeks5vPSJBgaJpZM4bE4kY [3] https://github.com/notifications/beacon/AAPVIsDgikgG3esXB5KvpvF-gfJXxsENks5vPSJBgaJpZM4bE4kY.gif

[dxdc]

@colinl As far as I can tell, this is an issue with https://github.com/CenterForTheBuiltEnvironment/comfort_tool/ (that code was used directly inside of this plugin).

There must be some infinite loop being triggered in the calculation.

I think sanitizing your inputs is probably a reasonable solution here, otherwise you'd have to figure out which aspects are causing a problem and/or submit this to the maintainers of the CBE.

[colinl]

Are you using the latest version of that library?

[dxdc]

There are no (significant) changes in the latest CBE library vs. this module, so, yes.

[colinl]

I understand what you are saying, and to stop the problem I have already done what you suggest, sanitising the inputs. However, as a matter of principle I think that a node should survive any data passed to it and not hang or crash. If that is correct then perhaps the node should sanitise the data, or just ignore obviously garbage inputs, with an appropriate status or error message.

[dxdc]

However, as a matter of principle I think that a node should survive any data passed to it and not hang or crash.

Sounds like you want a perfect world where computers don't crash even if their developers feed it garbage code :) We're not quite there yet...

There's no way for NR to know whether or not your process is supposed to take a long time. There may be some other kind of "node supervisor" processes that handle what you want.

I have 2 ideas that might benefit you:

1. You could try using an exec node to monitor NodeRED ram usage. If the RAM usage gets too high, just restart NR.

2. You could try to accomplish this more specifically to just the comfort node, IE:

• Pass input to both a trigger node and comfort node
• Set trigger to 3 seconds
• At the output of comfort node, send msg.reset to trigger node
• If msg.reset isn't set, trigger will restart NR

[colinl]

As I said, I have solved the problem for myself by sanitising the inputs. I am talking about matters of principle rather than a specific problem that I have. I think if you were to ask the node-red developers they would say that a well behaved contrib node should not hang or crash node-red under any circumstances.

[dxdc]

@colinl I suggest you bring up the issue with the developers of: https://github.com/CenterForTheBuiltEnvironment/comfort_tool/

The fact that those functions hang is an artifact of those libraries, and trying to figure out what specifically is causing the hang up is not trivial based on the cbe code.

[colinl]

I am not going to argue the point, it is of no relevance to me. If any of my contrib nodes hung or crashed then I would consider it my responsibility to work around the problem for my users. Don't get me wrong though, I am most grateful for the work you have done in making this available.

[dxdc]

Don't get me wrong though, I am most grateful for the work you have done in making this available.

This isn't my project, I'm trying to help you out.

[colinl]

I am confused, the npm page for node-red-contrib-comfort points to this repository.

[Ekristoffe]

what he want to say is, his project is a wrapper around another library not made for node-red. since he doesn't own the library he can't look in and solve the problem. so we have 3 solution possible: 1 - make sure your flow doesn't give strange value 2 - inside the node-red wrapper, make sure the data are viable before calling the library. 3 - goes to the original library, find a solution for the problem.

the third is the best but the owner of the library bust be contacted before. (there is no used to send a pull request if the project is dead ... The second is not bad either because other who use the node-red wrapper won't have the problem anymore. the first one is the fastest.

[dxdc]

@colinl @Ekristoffe just pushed a PR for this that validates the input values according to the allowable limits defined by their web tool.

[colinl]

That's great, thanks. Will give it a go.

[Ekristoffe]

Just updated node-red repo (need to use this page : https://flows.nodered.org/add/node ) And it seems to work fine. Thank you.

[alexciurea]

hi guys, after this update, i can see this error: "Invalid Air Speed value '12.51' (allowed: [0, 4]). " Air speed should be in meters per second, i guess it's a bit limiting if only this range is accepted...(e.g. 12m/s is about 45km/h)

[dxdc]

@alexciurea this is expected behavior. These calculations are not designed to work outside of specific ranges. This is also an unreasonable level for any HVAC system :)

Unexpected ranges not only produce wrong results, but they can also lead to infinite loops in the calculations.

This is just a node-red port of part of https://comfort.cbe.berkeley.edu/

[alexciurea]

thanks @dxdc maybe i am using wrongly the tools...? I agree that for HVAC/indoor this limitation makes sense, but I was using this node also for outside comfort calculations.

I am comparing the outside vs inside comfort/sensation and execute automation based on that... LE: i see this in the link you shared - thanks: Limits of Applicability: The elevated air speed comfort zone method is to all spaces where the occupants have activity levels between 1.0 and 2.0 met and clothing insulation between 0.0 and 1.5 clo. There is no upper limit to air speed if occupant's total clothing > 0.7 clo and metabolic rate > 1.3 met.

[dxdc]

Limits of Applicability: The elevated air speed comfort zone method is to all spaces where the occupants have activity levels between 1.0 and 2.0 met and clothing insulation between 0.0 and 1.5 clo.

I didn't notice this before. Perhaps something like this should be added @DeanCording, though not sure it's needed? It seems more of a warning than a requirement.

            var warnStr = '';
            warnStr += (metabolicRate < 1 || metabolicRate > 2) ? 'Metabolic rates below 1.0 or above 2.0 are not covered by ASHRAE Standard 55-2017. ' : '';
            warnStr += (clothingLevel > 1.5) ? 'Clothing levels above 1.5 are not covered by ASHRAE Standard 55-2017. ' : '';
            if (warnStr.length > 0) {
                node.warn(warnStr, msg);
            }

There is no upper limit to air speed if occupant's total clothing > 0.7 clo and metabolic rate > 1.3 met.

I didn't see this either. Still, when I try to manually type in a higher value for air speed it gets rejected as out of bounds. I'm not familiar enough with the ASHRAE standards to comment one way or the other. The PR I created just does the same thing that the web controls to do validate the input.

Are you getting drastically different results if you change the air speed to 4 m/s?

[alexciurea]

thanks @dxdc Indeed, the node implementation should follow the standard (despite the subjective inconvenience of its limitations). I played a bit with the inputs and despite that footnote where it explains no upper limit on airspeed under certain conditions, in no situation i can put airspeed > 4 (even if increasing metabolic or clothing). To solve the problem, in my flows I will use some range limiting for air speed input to the comfort node. Thanks for your inputs and investigation!! all the best, be safe -a