mikemucc / screenlogic-api

API to control a pentair Screenlogic. Built on top of parnic's screenlogic-node library.
Apache License 2.0
9 stars 2 forks source link

Schedule Support Request #2

Closed bshep closed 2 years ago

bshep commented 4 years ago

Hi, I helped add schedule support to parnic's library but I'm not skilled enough to do it for the UI, can I request the UI side be implemented?

If you need any help figuring out the new commands let me know, i did a fair bit of reverse engineering the protocol.

Thanks in advance!

https://github.com/parnic/node-screenlogic/pull/24 and ##https://github.com/parnic/node-screenlogic/pull/25 (waiting on merge of the second one)

mikemucc commented 4 years ago

The first thing we'll need to do is add that functionality to this API, i.e. add endpoints (probably /schedule) and the appropriate GET & PUT calls.

I envision these calls:

GET:
/schedule -> returns the whole schedule
PUT:
/schedule/<some ID> create or update a scheduled task

The PUTs may be somewhat more complicated than current endpoints, as we may need to accept a JSON data payload, something like:

{
"circuit":"pool",
"start": "12:00"
"stop": "13:00"
}

although we could URL encode it.

Once Parnic merges those PRs and issues a new build to npm we can get started.

Would you like to do the API integration as well, I'll gladly accept pull requests.

A second question for you, since you were able to some reverse engineering on the protocol, any chance you can figure out how to get pump data (speed/energy usage)? I'd love to have that info show up in the UI as well.

I'm also considering re-writing the UI using https://github.com/plotly/dash, i.e. in Python (I consider myself a much stronger python coder), although I'd keep the API in node/express. Any thoughts on that/are you a Python guy?

parnic commented 4 years ago

A second question for you, since you were able to some reverse engineering on the protocol, any chance you can figure out how to get pump data (speed/energy usage)? I'd love to have that info show up in the UI as well.

@bshep - commit https://github.com/parnic/node-screenlogic/commit/81b3a61c2881d62cc84220c3d4bb1c4ccbf89e3d was added to support this, but no one has had time yet to decode what the bytes in the arrays of that message mean.

bshep commented 4 years ago

A second question for you, since you were able to some reverse engineering on the protocol, any chance you can figure out how to get pump data (speed/energy usage)? I'd love to have that info show up in the UI as well.

If I can figure it out i will post and update in parnic's repo

mikemucc commented 4 years ago

So, I can now get the schedule info (/api/schedules). I'll work on the other endpoints as I have time.

Work is in branch scheduling-support.

mikemucc commented 4 years ago

@bshep Do we know what day aligns to what bit in the daymask field? I.e. does 1100000 = Sunday & Monday, or does it equal Monday & Tuesday?

bshep commented 4 years ago

@bshep Do we know what day aligns to what bit in the daymask field? I.e. does 1100000 = Sunday & Monday, or does it equal Monday & Tuesday?

Looks like bit7 is sunday, bit6 is sat so the order from MSB to LSB seems to be sun/sat/fri/thu/wed/tue/mon

So in your example thats Sun/Sat selected

mikemucc commented 4 years ago

So it's literally the inverse of a week: ['Su', 'Sa', 'Fr', 'Th', 'We', 'Tu', 'Mo'] I'm not sure how I feel about that... :P

I think I'd like the API to do the DayMask translation for the UI, as doing that in Angular is a pain (at least to me, because my Angular skills aren't anything special).

So I'll probably keep working on the GET route for a little while.

Still considering the structure for the PUT route.

POST route will be easy, /schedules/0 || 1, get back the ID, then PUT that ID with the data.

Delete will be easy, /schedules/<ID>

bshep commented 4 years ago

So it's literally the inverse of a week: ['Su', 'Sa', 'Fr', 'Th', 'We', 'Tu', 'Mo'] I'm not sure how I feel about that...

I guess you could look at it as week starts on monday and its in order from LSB to MSB ;-)

I think I'd like the API to do the DayMask translation for the UI, as doing that in Angular is a pain (at least to me, because my Angular skills aren't anything special).

One easy way is to make the days equal constants: monday is 1, tuesday is 2, wed is 4, etc then add the constants.

In what format you pass the data back and forth to the API?

So I'll probably keep working on the GET route for a little while.

Still considering the structure for the PUT route.

POST route will be easy, /schedules/0 || 1, get back the ID, then PUT that ID with the data.

Delete will be easy, /schedules/<ID>

mikemucc commented 4 years ago

My thought is to decode the daymask into an array of strings for the days, and return that and the daymask via the API, so that the UI doesn't have to decode the daymask.

That said, I'll probably only accept a daymask for the PUT route, for simplicity, but maybe I'll decode an array of strings. Probably not, as I don't want to take JSON payloads if I don't have to...

bshep commented 4 years ago

That said, I'll probably only accept a daymask for the PUT route, for simplicity, but maybe I'll decode an array of strings. Probably not, as I don't want to take JSON payloads if I don't have to...

I think the encoding/decoding of the daymask should only go in one place, i can add it to the parnic API ( if its ok w @parnic ) or it can be done in your API, but it should not be on both or be half on one and half on the other. In the end its all JS anyway since the API is written in node so the code will be almost identical on both implementation.

parnic commented 4 years ago

It makes sense for the library to make this as easy as possible. My approach has generally been to provide constants or helper methods when the under-the-hood representation is difficult to work with.

That said, @bshep 's idea that each day could just be assigned a power-of-two value and added/or-ed together makes a ton of sense to me.

bshep commented 4 years ago

It makes sense for the library to make this as easy as possible. My approach has generally been to provide constants or helper methods when the under-the-hood representation is difficult to work with.

That said, @bshep 's idea that each day could just be assigned a power-of-two value and added/or-ed together makes a ton of sense to me.

I'll whip it up in a couple of mins and post a gist ( or post a branch in my repo? ) for you guys to test, if it works well i'll send a PR

bshep commented 4 years ago

Please let me know any comments:

https://github.com/bshep/node-screenlogic/tree/day_mask

bshep commented 4 years ago

https://github.com/bshep/node-screenlogic/tree/day_mask

@mikemucc this has been merged into master if you want to take a look, you should be able to call encodeDayMask/decodeDayMask

mikemucc commented 4 years ago

Ok, I'm working on the create new schedule endpoint. When I call createNewSchedule() I'm expecting to get an addNewScheduleEvent back. Does that event have the new Schedule ID?

If so, how do I access it? If not, can I get that ID back as part of calling createNewSchedule()?

bshep commented 4 years ago

Ok, I'm working on the create new schedule endpoint. When I call createNewSchedule() I'm expecting to get an addNewScheduleEvent back. Does that event have the new Schedule ID?

If so, how do I access it? If not, can I get that ID back as part of calling createNewSchedule()?

I dont see the function createNewSchedule() the one that is defined is called addNewScheduleEvent(), it will emit a addNewScheduleEvent event, and this will contain an object with a property called scheduleId which is the new schedule id to be used with the function setScheduleEventById()

Hope that helps, if you need some sample code i can post a gist with some.

mikemucc commented 4 years ago

So I finally had a few minutes to rub together to work on this. Added POST endpoint for creating a schedule. Now I'm working on adding update schedule support (PUT /schedules/:id), which I want to take a body.

@bshep Did you add any documentation for the encodeDayMask/decodeDayMask functions?

bshep commented 4 years ago

So I finally had a few minutes to rub together to work on this. Added POST endpoint for creating a schedule. Now I'm working on adding update schedule support (PUT /schedules/:id), which I want to take a body.

@bshep Did you add any documentation for the encodeDayMask/decodeDayMask functions?

I cant remember if I wrote documentation but you pass the encode function an array of strings for the days you want the schedule to run: ['Mon','Wed','Fri'] ( the strings are defined here ) the decode function does the opposite, you pass it a day mask and it returns an array of strings with they days that the schedule is active for.

mikemucc commented 4 years ago

So I get how the function works, but how do I bubble it up to the express level so I can call the functions? It doesn't seem to be exported by ScreenLogic as any kind of helper.

Is there something I can do like const encodeDayMask = ScreenLogic.messages.encodeDayMask() so I can call it from within an API route?

parnic commented 4 years ago

So I get how the function works, but how do I bubble it up to the express level so I can call the functions? It doesn't seem to be exported by ScreenLogic as any kind of helper.

Is there something I can do like const encodeDayMask = ScreenLogic.messages.encodeDayMask() so I can call it from within an API route?

Currently these are instance methods on base SLMessage, so all messages can use them. They could probably be made static to avoid needing an instance of a message, but right now that's how you'd use them (by getting or creating an instance of a message).

mikemucc commented 4 years ago

So I'm seeing some very odd behavior with the events (not) being emitted by .addNewScheduleEvent(). I'm hoping one of you can look at this code and be like: "Oh yeah, you're not doing {Insert stupid thing I missed here}".

The code is here: https://github.com/mikemucc/screenlogic-api/blob/scheduling-support/server.js#L731-L768

Basically, for some reason I'm never getting the "addNewScheduleEvent" back from the library. I'm seeing similar behavior in the .deleteScheduleEventById as well, but one thing at a time. Delete route is commented out for now.

Also, I'm noticing that "egg timers" never show up in the scheduled events object (i.e. they don't get an event ID).

parnic commented 4 years ago

I don't immediately see anything wrong with that code. You could set the DEBUG env var to sl:* before running the server and see what events are being fired that way, as a gut check. If done correctly, you should see something like this:

❯ node example
  sl:remote connecting to dispatcher... +0ms
  sl:remote connected to dispatcher +54ms
  sl:remote received message of length 29 +14ms
  sl:remote   it's a gateway response +1ms
no unit found by that name
  sl:remote remote login server connection closed +11m
mikemucc commented 4 years ago

I added the debug flag and did some tests:

Here's a delete schedule call. The JSON is my code printing the schedule ID from express server.

{ id: '705' }
Schedule Exists
  sl:unit connecting... +4s
  sl:unit connected +7ms
  sl:unit sending init message... +0ms
  sl:unit sending challenge message... +0ms
  sl:unit received message of length 32 +35ms
  sl:unit   it's a challenge response +0ms
  sl:unit sending login message... +0ms
  sl:unit received message of length 24 +9ms
  sl:unit   it's a login response +1ms
Logged in...
  sl:unit sending delete schedule event command for scheduleId: 705... +0ms
  sl:unit connecting... +16s

16s later, no delete acknowledgement. Now my code is polling every 5 seconds (by default). I've never seen behavior like this before, but is it possible I'm stomping on this API call's connection?

parnic commented 4 years ago

I'm not sure what's up with that. What version of node-screenlogic are you using?

Did the debugging help answer anything about adding schedules?

mikemucc commented 4 years ago

"node-screenlogic": "^1.6.0-1" Should I instead install from "main"?

parnic commented 4 years ago

No, I would expect that to be fine

mikemucc commented 4 years ago

Ok, seeing some interesting things in the debug logs...

When calling the delete schedule code:

 ...
  sl:unit received message of length 12 +14ms
  sl:unit   it's an unknown type: 8300 +1ms
  sl:unit received message of length 8 +5ms
  sl:unit   it's a delete schedule event ack +0ms

I see the same message with a Create Schedule:

  sl:unit received message of length 12 +10ms
  sl:unit   it's an unknown type: 8300 +1ms
  sl:unit received message of length 12 +0ms
  sl:unit   it's a new schedule event ack +1ms

Any idea what a message type 8300 is? It's not in @ceisnach's document. Sometimes I see the type 8300 but it is not immediately followed by the add/delete schedule ACK. Or perhaps that len 12 8300 is the ACK but it's being miscalculated? I only see the type 8300 when calling the add/delete schedule functions.

Also, on this.getScheduleData();

sl:unit sending set schedule data query for scheduleType: NaN... +1ms

Don't know if I should be concerned about that one either, but like I said before, Egg Timers don't show up in the Events object.

parnic commented 4 years ago

See this issue for the 8300 message.

The NaN sounds like a bug in the library.

parnic commented 4 years ago

Oh, for getScheduleData() you're supposed to pass the type of schedule you want the data for. So the library should probably guard against that, but to fix it, just pass a valid schedule type.

mikemucc commented 4 years ago

I am seeing all the behavior described by @jpennell in #37.

@jpennell said that moving to "local" fixed the missing ACKs for him. I'm running completely local (my API does not currently support remote login) and getting the 8300 messages and NOT getting the ACKs.

mikemucc commented 4 years ago

Ok, so if I call getScheduleData(0) and getScheduleData(1) they both return the same event, so getting the Egg Timer events back is problematic. I'd prefer it if we had 2 separate calls, getScheduleData() for Schedules, and getEggTimers() for egg timers, or at least have a different callback event for a 0 vs a 1.

parnic commented 4 years ago

Feel free to open an issue in the library if you feel the library needs improvement or is deficient in any way. 🙂

mikemucc commented 2 years ago

Closed by #14. Thanks @parnic!