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
Add foscam coordinator #92665
Add foscam coordinator #92665
Conversation
Hey there @skgsergio, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Split #81325 -> this includes the coordinator change only |
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.
Did a first pass.
There is quite some missing typing and also as you are bumping the library please include a release note or change-diff as mentioned in the PR description.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
d916d94
to
98167be
Compare
bef7150
to
933cbfc
Compare
933cbfc
to
c9d64bd
Compare
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.
Please make sure the CI is passing.
Also, please include the reason for creating a coordinator in the description.
The coordinator requests every 30 seconds two endpoints and the camera is not using this data et all. Therefore the coordinator is only being used for the availability property of the camera.
1b04387
to
2c0963d
Compare
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.
Add test coverage to the coordinator or exclude it from the tests (preferably you create tests 👍 )
f426ae3
to
c7ecca7
Compare
c7ecca7
to
09e7163
Compare
09e7163
to
7164983
Compare
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.
Thanks @krmarien 👍
Breaking change
Proposed change
Add foscam coordinator to support later on a switch for toggling the sleep mode
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: