-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
Google calendar #2052
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
Google calendar #2052
Conversation
@@ -0,0 +1,389 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call this component google. Yes, for now it's only requesting calendar oauth scope but we could extend this to other scopes with support in other components in the future.
So this PR is quite blowing up… I would suggest to not include the automation trigger. Small steps at a time. Create a new PR once this once is merged. If you want to keep it in, you need to write tests for it to validate that it works. It also shouldn't just trigger every minute but instead listen to state changes of the entity id, that will be more efficient. Also new components need to be accompanied with a demo platform and test coverage for all offered functionality. |
Not sure how I'd create a demo/test because of the oauth that needs to happen. As far as the trigger happening every minute I was trying to avoid state changes every minute. Since someone could potentially change the event after it's registered here the offset could change causing the trigger to fire at the wrong time. Event: Test #Notify #-10, Start: In an hour Change event to Start in 2 hours If you can point me in the direction of how to implement a dynamic target for the trigger without checking every minute I'd appreciate it! I was doing a minute countdown in the state but that's noisy and logs a lot to the db as you pointed out. This is where I went next. The offset trigger can be changed at any time which makes this difficult for me to grasp. |
Just thought of how to do it. Watch for state changes to start_time and offset_time and set a specific time listener and hold onto the ref to that so I can remove/reset it if those change. I'll pull it for now. Have to spend the weekend learning tests. Still clueless on the demo part though. |
The tests would be for the calendar component. You can skip testing all of But since the calendar component needs a platform to provide get next event On Fri, May 13, 2016, 05:14 mnestor notifications@github.com wrote:
|
There's a bug I'm working on, can't keep the start/end as a datetime in the state. |
You can call |
Yeah, I've done that everywhere now and still getting the error. |
I was overthinking it. sigh Not sure why recorder can't do this itself. Seems like it'd be useful to have the datetime in the state for automation work comparing to now() |
@property | ||
def state_attributes(self): | ||
8000 """State Attributes for HA.""" | ||
start = self._cal_data.get('start', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not using these variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brain fart, supposed to return those, fixed. Had to copy from my prod system since I couldn't replicate in my dev.
Description:
Adds Google Calendar Event Devices and a generic Calendar Device for future calendar integration.
@Bart274 is working on iCloud calendars and will be utilizing the generic Calendar Device.
Fixes comments from previous PR #1982
Require other PR for Configurator changes #2035
Documentation: home-assistant/home-assistant.io#447
Apologies about the new PR, I completely messed up my fork and had to restart it.
Related issue (if applicable): #
Example entry for
configuration.yaml
(if applicable):Checklist:
If code communicates with devices:
tox
run successfully. Your PR cannot be merged unless tests passFails
REQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass