search

tagyoureit / nodejs-Pentair

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

Add Friendly Name Support #61

Closed arrmo closed 7 years ago

arrmo commented 7 years ago

I can take care of this, no issue - but if the Friendly Name does not exist ... fall back to the current approach?

Thanks!

tagyoureit commented 7 years ago

Friendly name should equal the friendlyname in config.json, if it exists. Or if it doesn't, it should be set to the name retrieved from the intellitouch controller.

arrmo commented 7 years ago

Hi,

I just went in to add this (sorry, been a bit sidetracked) - and it seems that now you always set friendlyName ... correct? So I can just move to that completely, replace name - right?

Thanks!

tagyoureit commented 7 years ago

Yes. That should be correct. Also, did you see my PM on the Gitter chat room?

arrmo commented 7 years ago

Yep, did now ... ;-). Replied!

OK, re-opening this one ... I modified my code to use friendlyName - seems to work, but if I set the friendly name (in config.json), then I'm not getting the field passed across (which breaks things).

Thoughts?

arrmo commented 7 years ago

And BTW, this is only for circuit messages, right?

tagyoureit commented 7 years ago

I just tested this on the 3.0.1 code from the new repo, and it is working there. Here is one on my circuits.

{"number":14,"numberStr":"circuit14","name":"WtrFall 3","circuitFunction":"Generic","status":0,"freeze":0,"friendlyName":"Waterfall High"},

Maybe your code is out of sync or the FriendlyNames section in config.json has an error?

And BTW, this is only for circuit messages, right?

I think that the only other place that (should) use this is the schedules. And yes, I do need to add it there. Anyplace else you know of?

tagyoureit commented 7 years ago

It's added now to the schedule object/socket. I'll push it with the next set of changes.

arrmo commented 7 years ago

OK, added support in poolController, PR#1 over there - can you pull it in, confirm it works on your side? Then I'll go in and add schedule support. To your question - also add for pumps, agreed?

And BTW, if you change the name for Pool or Spa it causes grief. Ignore this, or test as a special case?

arrmo commented 7 years ago

Schedule - will add it after you push your changes (and pull in my PR). For schedule, this replaces CIRCUIT, right? And what about pumps?

And to make sure it's not missed ... do I need to add code to avoid issues with Pool and Spa (as those names need to stay).

tagyoureit commented 7 years ago

And what about pumps?

Let's address that separately. Pumps don't really have a name, so it wouldn't be hard to add, but it will be a different part of the code.

do I need to add code to avoid issues with Pool and Spa (as those names need to stay). I can take care of this on the server side, but just curious... is this because we hard code the UI to pull out "pool" and "spa" circuits? Or was there another error.

arrmo commented 7 years ago

Nope, you have it right - it's because we pull out / expect POOL and SPA to exist (hard coded, not real other way to do that, at least that I can think of).

tagyoureit commented 7 years ago 
{"number":1,"numberStr":"circuit1","name":"SPA","circuitFunction":"Spa","status":0,"freeze":0,"friendlyName":"SPA"},
{"number":6,"numberStr":"circuit6","name":"POOL","circuitFunction":"Pool","status":0,"freeze":1,"friendlyName":"POOL"},

We might be able to use the circuitFunction. I just don't know if it holds true for all controllers. Definitely for my Intellitouch it could work.

arrmo commented 7 years ago

Agreed - and also not sure how consistent this is. Just let me know if I need to test for it, or if you'll take care of it on the server end (i.e. make sure friendlyName is POOL or SPA).

arrmo commented 7 years ago

OK, updated on the client side - checks for POOL and SPA, seems to work (added a friendlyName, ignored for these two circuits). Included in PR 7 over on the new repository ... please pull in, then close this ... agreed?

BTW, in the documentation, is it worth noting that changes to config.json (for items like friendlyNames) are not taken into use until nodejs-poolController is restarted?

tagyoureit commented 7 years ago

BTW, in the documentation, is it worth noting that changes to config.json (for items like friendlyNames) are not taken into use until nodejs-poolController is restarted?

At some point in the future it would make sense to be able to update these on the fly and have the app pick up the changes. That would probably be through a UI (ideally). Not sure it's worth the effort to try to re-read the file every time that it is changed.

A 3rd party option would be use nodemon. Awesome command line drop-in replacement that will restart the app upon any file changes.

Closing for now!

arrmo commented 7 years ago

FWIW, completely agree with you - not really a big concern, as pool configs don't change very often ... :)