Thermostat Occupancy Sensor Integration #37
Conversation
| @@ -14,6 +14,7 @@ import ( | |||
|
|
|||
| const TSTAT_PO_DF = "2.1.1.0" | |||
| const DR_PO_DF = "2.1.1.9" | |||
| const OCCUPANCY_PO_DF = "2.1.2.1" | |||
There was a problem hiding this comment.
I'm just using the existing i.xbos.occupancy interface. Let me know if I should create a different interface to use here.
| @@ -37,6 +37,8 @@ type Pelican struct { | |||
| target string | |||
| timezone *time.Location | |||
| req *gorequest.SuperAgent | |||
| drReq *gorequest.SuperAgent | |||
There was a problem hiding this comment.
SuperAgent instances aren't thread safe. I was routinely getting panics during my testing because we now have multiple goroutines for each Pelican thermostat simultaneously issuing requests.
Here, the Pelican structs now have one SuperAgent instance for each of their associated goroutines: The main status goroutine, the DR event goroutine, and the new occupancy data goroutine.
I'm open to suggestions if we want to take a different approach.
There was a problem hiding this comment.
The alternative I guess would be a whole locking wrapper around the SuperAgent, which doesn't strike me as worth it. This looks great!
|
Thanks for pulling this together @jhkolb Couple questions:
|
|
No, all of the Pelicans I saw had just one sensor associated with the each thermostat. I'm not sure how common these auxiliary sensors are. I haven't seen this anywhere besides the single deployment that motivated this PR in the first place. So while it's possible other kinds of sensors are out there, I haven't seen them. Also, it's worth noting that the sensors report both a |
driver/pelican/main.go
Outdated
| occupancy, err := currentPelican.GetOccupancy() | ||
| if err != nil { | ||
| fmt.Printf("Failed to read thermostat occupancy: %s\n", err) | ||
| done <- true |
There was a problem hiding this comment.
Can you change this so that the error is printed out, but the driver doesn't stop? Until we're more sure of the failure cases of the auxiliary sensors, I think it might make for a more robust deployment if the driver continues to run and try to access the sensors. What do you think?
There was a problem hiding this comment.
Sure, that makes sense. Just pushed an update to address this.
|
Thanks! |
These changes allow us to collect occupancy data from occupancy sensors that are associated with Pelican thermostats and report data through the Pelican API.
This has been tested on thermostats bound to Enlighted occupancy sensors.