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

Pyecobee - thermostat->reminders object definition #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fowlerk
Copy link

@fowlerk fowlerk commented Nov 20, 2020

Signed-off-by: fowlerk keith.fowler.kf@gmail.com

Added object definition and associated changes for the thermostat->reminder object

@sherif-fanous
Copy link
Owner

Hey @fowlerk

Thanks for the pull request. But I've got a few questions/requests

1-Why are there lots of changes to thermostat.py and utilities.py? It seems quite a few attributes have been deleted from thermostat.py (e.g. audio, energy, ...)
2-Can you please use "Black" to format the code so that it aligns with the rest of the codebase?
3-Do you have a response sample for a reminder?

Thanks

@fowlerk
Copy link
Author

fowlerk commented Nov 21, 2020

Hi @sfanous --

  1. Regarding the changes to thermostat.py and utilities.py, there should have been only minor changes to reference the new reminder definition. I obviously bodged something up there...I've been dealing with multiple library versions in attempting to troubleshoot this and may have somehow referenced a different version for these routines. Give me some time to take a look at it...sorry for the confusion.
  2. I'm not familiar with "Black" format, but I'll take a look and see what I can do.
  3. Here's a sample response snippet that includes a reminder, up to the "settings" attribute...I've obfuscated the ID and name just for privacy reasons:
    {\n "identifier": "999999999999",\n "name": "THERMOSTAT",\n "thermostatRev": "201110204737",\n "isRegistered": true,\n "modelNumber": "nikeSmart",\n "brand": "ecobee",\n "features": "Home,HomeKit",\n "lastModified": "2020-11-10 20:47:37",\n "thermostatTime": "2020-11-17 17:52:52",\n "utcTime": "2020-11-17 22:52:52",\n "alerts": [],\n "reminders": [\n {\n "type": "serviceReminder",\n "title": "Service Reminder",\n "description": "Service HVAC equipment notification.",\n "reminderDate": "Dec 25, 2020 12:00:00 AM",\n "remindMe": true\n }\n ],\n

@fowlerk
Copy link
Author

fowlerk commented Nov 21, 2020

@sfanous --
So, I did some digging on Black and installed it, then ran my changed modules through it prior to pushing the commit. I think the changes should be good now with your current version Pyecobee, though I did notice one somewhat strange anomaly. It looks like all instances of single-quotes (') were replaced with double-quotes (") throughout the changed modules. As I didn't do a replace on these explicitly, is this something that Black would have done on reformatting the code?

Let me know if the latest commit looks okay or if you have other issues. Again, sorry for the confusion earlier.

@sherif-fanous
Copy link
Owner

@fowlerk

You need to pass --skip-string-normalization when executing black to ignore the changing of the quotes.

@fowlerk
Copy link
Author

fowlerk commented Nov 22, 2020

@sfanous ... thanks, got it and have changed my default to add this switch. Let me know if you would like me to do another commit to change these quotes back to the "standard" formatting you have used for the other modules.

Signed-off-by: fowlerk <keith.fowler.kf@gmail.com>
@fowlerk
Copy link
Author

fowlerk commented Dec 5, 2020

Hey @sfanous...
I tried to clean up my prior PR and get the quotes back as they were before. Hopefully this helps; let me know if you have any questions. Thanks!

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.

None yet

2 participants