Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thermostat Schedule Retrieval Code #38

Merged
merged 26 commits into from
Jan 29, 2019
Merged

Thermostat Schedule Retrieval Code #38

merged 26 commits into from
Jan 29, 2019

Conversation

john-b-yang
Copy link
Member

Created a new file thermSchedule.go within the types directory of the pelican driver code.

The Pelican Thermostat API does not have a way to retrieve thermostat schedule data. ThermSchedule.go sends AJAX Requests to the climate control website to scrape the desired data from the website itself. The thermSchedule.go exposes one method, getSchedule, that performs this task. Within main.go and params.yml, polling code and an interval parameter has been added. This code is very similar to the existing status and demand response polling code.

@john-b-yang
Copy link
Member Author

@gtfierro I had a question with regards to representing time. I took your recommendation from above and used RRule to standardize the time format for the Pelican, but I feel like the way I did it wasn't particularly elegant.

From reading the RRule documentation, it seems like every New RRule entry requires a "DTStart" field (which is really just a time.time golang object). DTStart specifies the first occurrence of the scheduled event, kind of like the start date of recurring events in Google Calendar.

My approach was to use RRule to standardize the time, but it seems like RRule events require a start date. However, when it comes to the Pelican, I'm not sure what I should set the start date to. The Pelican schedule only gives me hour, minute, and AM/PM of each event block, and there's no history of when the schedule might've been last edited. As a result, as you can see in the convertTimeToRRule function, when creating the DTStart, I arbitrarily set the month, date, and year to 0, 0, 0.

I think the main issue is that when it comes to time, Pelican only provides something like "6:00:AM", but it seems like creating an RRule event requires a lot more parameters, most notably, a start date. Should we either 1. put in dummy values for those parameters 2. use a different time format or 3. something else?

Also for context, Pelican scheduling occurs, at most, on a weekly basis. There are 3 different settings

  1. Daily: There is only one schedule that occurs every day
  2. Weekly: There is a unique schedule for each day of the week (Mon, Tues, Wed, ... Sun)
  3. Weekday/Weekend: there are two different schedules, one for Mon - Fri. and the other for Sat., Sun.

driver/pelican/main.go Show resolved Hide resolved
driver/pelican/main.go Outdated Show resolved Hide resolved
driver/pelican/main.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
@gtfierro
Copy link
Member

@john-b-yang Its probably fine to start the recurrence rule at the "0" date for now, but it is important that we capture the weekly pattern of the schedule and not just the hour. The pelican schedule scraper i wrote was able to take care of this. but I remember having to get the javascript to render the schedule viewer in order to extract that information

…Added wkst field to RRule time to track day of week.
@john-b-yang
Copy link
Member Author

@gtfierro No problem, sounds good. The way I have the results set up is as follows

The ThermostatSchedule struct contains a map between a string and a ThermostatDaySchedule struct. In this case, the "string" key will be the day itself ("Sunday", "Monday"..."Saturday"). Then, each ThermostatDaySchedule itself is a series of "Blocks". I created a struct that would account how a Pelican's daily schedule consists of several different "blocks" of settings. Each Block consists of 4 fields: heatSetting, coolSetting, System (Setting aka auto, heat, cool, off), and Time. The Time RRule, after your suggestions above, now reflects 1. the time 2. timezone and 3. the day of week.

Hopefully this answers your question? I was able to get all of this data by inspecting the AJAX requests that were called when the schedule settings pages were being rendered.

@gtfierro
Copy link
Member

Have you tested this among multiple types of schedule definition? Pelican supports at least 3, I think. Not sure where those are documented, but if you look at the requests the frontend makes, then there is some variation in them.

The overview of the schedule struct sounds good; can you document what some of the fields mean?

@john-b-yang
Copy link
Member Author

Yup! The Pelican has three different schedule types,

  1. Daily: Same schedule for every day
  2. Weekly: Unique schedule for every day
  3. Weekday/Weekend: Two unique schedules for weekdays / weekends
    My code handles each of these cases and it works properly! I've been testing it by changing the schedule settings for the dummy Pelican I'm using. There's no schedule setting that extends beyond one week.

Thanks! I've added some comments above each struct within the thermSchedule.go file describing the utility of each of the structs. I'm also writing the schedule setting code now so I'll be using this same set of structs as setpoint message for altering the schedule.

@john-b-yang
Copy link
Member Author

@jhkolb @gtfierro I added the markdown file describing the expectations for the schedule structs interface mentioned in SoftwareDefinedBuildings/XBOS#80. The file is called "interface.md" and I discuss a bit about how the schedule is defined given the Pelican thermostat's settings.

@gtfierro
Copy link
Member

Thanks! I added a few comments to clarify the schedule interface; let me know when you get a chance to address those

@john-b-yang
Copy link
Member Author

@gtfierro Thanks for the feedback! Made some updates accordingly :)

@gtfierro
Copy link
Member

@jhkolb do you still have outstanding requests? Your previous review is outdated with the current commit so its hard to track things down.

I'm good to merge this when you are

Copy link
Contributor

@jhkolb jhkolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I have a few more suggestions for changes.

driver/pelican/interface.md Outdated Show resolved Hide resolved
driver/pelican/interface.md Outdated Show resolved Hide resolved
driver/pelican/interface.md Outdated Show resolved Hide resolved
driver/pelican/interface.md Outdated Show resolved Hide resolved
driver/pelican/interface.md Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
@jhkolb
Copy link
Contributor

jhkolb commented Jan 24, 2019

Okay, I had a couple of additional questions. Once those are answered and you've made the changes to the NewPelican function, we should be about ready to merge!

…figured cookie and id setting logic to be set/refreshed once every month. Modified multiple method headers, eliminating the number of parameters and deferring to the pelican struct fields instead.
Copy link
Contributor

@jhkolb jhkolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more changes to make.

driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
driver/pelican/types/thermSchedule.go Outdated Show resolved Hide resolved
if err != nil {
fmt.Printf("Failed to create Schedule msgpack PO: %v", err)
}
currentSchedIface.PublishSignal("info", po)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gtfierro In the Pelican's status retrieval loop, we write to the done channel if an error occurs, which prompts the driver to exit. We chose not to do this in the DR event retrieval loop because we figured a problem with the DR interface should be logged but not prompt an exit.

Do you have a preference on which approach we take here? As it stands, an error during schedule retrieval will be logged but the driver will keep running.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the "log but keep going" approach, like we did with DR event retrieval

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I think this is ready then!

@gtfierro gtfierro merged commit f619dc2 into SoftwareDefinedBuildings:master Jan 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants