search

st-one-io / node-red-contrib-s7

A Node-RED node to interact with Siemens S7 PLCs
GNU General Public License v3.0
111 stars 58 forks source link

Selecting Debug for one Endpoint changes settings for other / all #17

Closed temmat closed 5 years ago

temmat commented 6 years ago

So...there is a "Debug" selection (Default/On/Off) that can be selected in the Endpoint config node. Ok, thing is that I have multiple config endpoints (multiple PLCs) and I just want to debug communication with one that is giving me problems. But....as soon as I select "On" for this config node, I get a lot of messages in the terminal...but not (or at least not only) for the selected config node, but for other ones too.

This doesn't seem to be the intended functionality, since each config node has its own setting.

Furthermore...is there any plan on supporting the node-red's "catch" node for issuing errors and messages ? At least for me it seems the errors are issued in the terminal only.

These two are maybe two separate issues but both deal with the same thing - debugging.

gfcittolin commented 6 years ago

Hi @temmat,

I'm aware of the issue. The problem is actually with the library we use, plcpeople/nodeS7. The log level there is configured globally - if you change in one instance, all of them will be affected. The fix for that shouldn't be that hard, I just haven't had the time for writing a PR for that :)

Regarding the catch node, I have to say I've never tested it with this node. We call node.error() - this is the call that is catched by the "catch" node - whenever the node is aware of a problem, but since the communication is handled by the config node (s7 endpoint), we call it from there. And I don't know whether Node-RED is capable of "catching" errors sent from config nodes. So maybe something to check out.

I'm currently working on a possible bug with this node, there might be a connection leak when a connection failure happens, so I'll publish a new version on the next days or week. I'll try to take a look on these 2 points and include them on the release :)

gfcittolin commented 6 years ago

I took a look on the catch node issue, and indeed there was an issue. We need to pass an argument to the node.error() function, so we kind of "promote" the event to be catched by the catch node. Just published 1.5.0 with this included.

Regarding the debug thing, it's a bit more complicated than I thought, because of the way the code was written in nodes7. I don't know if I'll find the time to implement it in the short term, but a PR there is for sure welcome ;)

temmat commented 6 years ago

I've finally been able to test this with the 1.5.1 version and...at least from what I can see, the Error catch node is now working with the S7 in node as expected.