tagyoureit / nodejs-Pentair

An application to read/write to Pentair pool controllers.
14 stars 6 forks source link

Flag / display errors on Web Page? #22

Closed arrmo closed 7 years ago

tagyoureit commented 7 years ago

What exactly do you want to display? Console errors? Packet errors? Application errors?

arrmo commented 7 years ago

Whatever makes sense ... ;). We can define as we go along - but as an example, when my RPi was down, no indication in the GUI at all -> may be good to show the error seen (no socket connect), agreed?

arrmo commented 7 years ago

For example, saw this today - cause by the idiot (me), trying to run nodejs on two machines accidentally. But good to show in the web interface? 09:07:56.100 VERBOSE Express Server listening at port 3000 09:07:56.103 ERROR Error opening port: connect ECONNREFUSED 192.168.2.78:9801

arrmo commented 7 years ago

On this one ... just add the logLevel for what to send across ... right?

tagyoureit commented 7 years ago

Specifically on this:

09:07:56.103 ERROR Error opening port: connect ECONNREFUSED 192.168.2.78:9801

I exit out of the program when we can't connect (with an error). So there wouldn't be any message to display. Would you rather we keep the program open and send this error across? Or handle it another way?

And any changes on the other logs that we are sending across? I think they are good now on the visual side. On the nice to have front: It might be useful to limit the size, or have a "clear" button, or automatically clear the box when we restart the app?

arrmo commented 7 years ago

Sorry, got sidetracked with other items. I'm thinking we pass the message across, so it's seen in the web GUI -> thoughts?

Logs are good, perhaps just send across at the same debug level as the server? Or have a separate setting?

On a clear button - yep, I can add that.

arrmo commented 7 years ago

Clear button - added ... :)

tagyoureit commented 7 years ago

Awesome.

arrmo commented 7 years ago

I think some of this is still open ... ;). Not exiting, and sending the info across? And configuring the log level to send across?

Thanks!

arrmo commented 7 years ago

Please do let me know your thoughts on this - I think we need to reopen, finish the "server" side of it?

arrmo commented 7 years ago

To the question / comment above - yep, if the connection burps, I see nodejs exiting ... don't do this, rather recover (retry, say every 30 sec or so)?

arrmo commented 7 years ago

And somewhat related (or a new item?) - perhaps send across a heartbeat signal, at defined intervals (config file to set the interval)? The client then knows that the server is alive ... and the heartbeat signal could send connection status (to the pool controller)?

tagyoureit commented 7 years ago

To the question / comment above - yep, if the connection burps, I see nodejs exiting ... don't do this, rather recover (retry, say every 30 sec or so)?

I'll include this with 2.0. Trying not to make any more changes to the 1.0.x code line moving forward.

perhaps send across a heartbeat signal, at defined intervals (config file to set the interval)?

Are you looking for something simple like "is the client connected"? (http://stackoverflow.com/a/16518983) Or a little more in depth, like "is it connected and able to talk to the pool controller?".

arrmo commented 7 years ago

Sounds good (v2.0) - agreed, let's not break v1.0 ... ;)

Thinking connected to the pool controller (serial directory, or over socat) - agreed? The client should be able to check the connection to the Node.js server ... right?

tagyoureit commented 7 years ago

Thinking connected to the pool controller (serial directory, or over socat) - agreed? The client should be able to check the connection to the Node.js server ... right?

I have this in 2.0 such that it will not exit the program on a serial port (or net) error, but send the messages to the web client. It doesn't catch the very first exception (not sure why), but it does log the errors every 10 seconds thereafter when it tries to connect again.

Once the app is connected, if there is a break in the communications with the pool controller, it has no knowledge of that and just sits there and waits. So if we wanted to take this further we would need a timer to throw an error if no packet was received in X number of seconds. But I think for now this should take care of the immediate problem. Check it out and let me know if you like it.

arrmo commented 7 years ago

Sorry, a bit confused - so does it flag on a lost connection to the serial port / net, or not? Just not sure I understand your point above - likely me though .. :(

tagyoureit commented 7 years ago

Ya, no worries... 2 different scenarios:

  1. Serialport (whether through net/socat or physical serial port) can't connect. Now it retries every 10 seconds and we get error messages. This we are handling.
  2. Sometime when the app is running, it is no longer able to communicate with the pool. This could be right away (port is successfully open, but no data) or anytime later. This I don't do anything with on the app side. But I see that you are taking care of it (with the timer) on the client side.
arrmo commented 7 years ago

Gotcha - thanks! On the scenarios, 1) This is perfect! Do you send a log message across? And if you do ... what is the format of the message? I can display a warning as a result (just need to know the message). 2) Yep, the timer is there to say if messages are not received, and you can set config values for color coding. FYI, I am finding in 2.0-Alpha this is triggering more than I saw before ... can't put my finger on it yet, but it's like messages are not being sent across correctly.

And a couple more thoughts to help out here, a) Would it make sense to allow the client to trigger an EMIT ALL, to refresh if desired? b) I am refreshing my web page, and that does trigger EMIT ALL ... but here is what I see,

Does this make sense? Any thoughts on what is happening, particular with the missing data?

Thanks!

arrmo commented 7 years ago

One more thought on this - perhaps add an option to config.json, to output the log to a file? I run nodejs index.js as a service, so no console output ... :(.

Thanks!

tagyoureit commented 7 years ago

Ya, we can do that. It's just another transport in Winston. I'd like to output to a DB for reporting as well for reporting.

I originally had an output to a file but it was HUGE. But no problem to add it back.

arrmo commented 7 years ago

Thanks! Yes, to a DB sounds good as well. Does Winston natively support that?

tagyoureit commented 7 years ago

From above...

This is perfect! Do you send a log message across? And if you do ... what is the format of the message? I can display a warning as a result (just need to know the message).

Right now it's just a log at the error level. You can put in the wrong address/port and see it retry.

Would it make sense to allow the client to trigger an EMIT ALL, to refresh if desired?

It does this automatically upon connection now. Easy enough to add an explicit call for it.

Does this make sense? Any thoughts on what is happening, particular with the missing data?

I have a lot of missing packets in my end again. This means the long 2 packets from the controller are coming through corrupt. Every once in a while it gets bad on my end for no apparent reason. But maybe you are seeing it, too? I see lots of 'warn: packet mismatch' errors in the logs. Do you see them?

arrmo commented 7 years ago

You bet, will change the address, to get the error message.

On the packets - nope, no errors noted here (though the logs are at INFO level, but assuming missing packets is higher than this, right?). I'm seeing an EMIT: all on page refresh, but the data sent across is all empty. Then after that - no more EMITS. For example, did a refresh - then no more EMIT messages, in ~ 10 minutes.

tagyoureit commented 7 years ago

though the logs are at INFO level, but assuming missing packets is higher than this, right?)

The mismatch errors are at the WARN level, but only if you have logMessageDecoding=1

Then after that - no more EMITS. For example, did a refresh - then no more EMIT messages, in ~ 10 minutes.

When the time is working right, it should emit the time once per minute. But if nothing else changes on your pool than this isn't unexpected. The empty data shouldn't be sent over. I think my problems are due to the Screenlogic Wireless link. Will try hardwired tonight and see if it's better.

arrmo commented 7 years ago

NP, thanks!

tagyoureit commented 7 years ago

a) Would it make sense to allow the client to trigger an EMIT ALL, to refresh if desired?

I now have an all event. Just call socket.emit('all') and it will resend all of the data. We shouldn't need it... but it's there now.

arrmo commented 7 years ago

Thanks!

tagyoureit commented 7 years ago

There's another way the client can determine if it is talking with the server and then there is a break in the communications: socket.on('disconnect', function(){}); Might be worth checking out in lieu of the timer (or in addition to it).

arrmo commented 7 years ago

Agreed! Let me check on this. It handles the (web) client to server connection, and debug messages for the connection to RS-485 ... right?

tagyoureit commented 7 years ago

Let me check on this. It handles the (web) client to server connection,

Yes. This is true.

and debug messages for the connection to RS-485 ... right?

Well, the debug messages are coming in THROUGH a socket. The 'disconnect' will fire when the socket layer can't reach the server anymore. Not sure the exact details how it knows it is disconnected, though. For example, will it only fire if the app actively disconnects the client? Or will it fire if it passively (like the Wifi disconnects) looses connection.

arrmo commented 7 years ago

OK, will try to add support for the connect / disconnect -> some sort of display item. But not sure about the RS-485 connection. Overall, 2 connections we want to monitor, agreed? 1) GUI / Web Client to server (Node.js) => use socket.io connect / disconnect 2) Server (Node.js) connection to serial port (local or remote)

How to track / monitor 2) in the web client?

Thanks!

tagyoureit commented 7 years ago

I think for 2 we would need to do something like you have done... put a timer in the main app. However, it would need to be dependent on the setup. EG The intellitouch still communicates even when it's off, but some other equipment setups (pumps only) literally have no power so no communications when they are off. Maybe we just back-burner this for now?

arrmo commented 7 years ago

Works for me (back-burner for now). The timer pretty much serves the short term need ... agreed?

tagyoureit commented 7 years ago

Yup! Closing...