-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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 initial version of Vilfo Router integration #31177
Conversation
Sorry for the spam I made a mistake |
No worries! Were you trying to fix the |
Yeppa, I don't know why you have this now (also I was on a mobile app, and when I've add The recheck is not removing it. I'll let people who knows taking take of it. Or maybe "ManneW authored and Emanuel Winblad committed" on the first commit, try rebase and force push, so it will be you the author and committer. So, I'll review soon 😊 |
I actually did amend the "author" part to resolve the CLA issue from the beginning (before that it was only "Emanuel Winblad" and not "ManneW") 😄 But your suggestion was a good idea, so I did the rebase now to have things a bit cleaner 🙂Not sure if the |
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.
Why there is a src/vilfo-api-client
submodule here ?
It shouldn't be.
* Removed unit of measurement for boot time sensor, * Added support for a sensor not having a unit, * Updated the config_flow to handle when the integration is already configured, * Updated tests to avoid mocking the code under test and also to cover the aforementioned changes.
I've updated (but not pushed yet) this based on the last comments from @MartinHjelmare. Do you want me to squash all the work into a single commit? I'll push it as a single new commit in the meantime, but are ready to do some squashing to keep the repo as clean as possible 😄 |
Please don't squash commits after review has started to make it easier for readers to track changes. We'll squash when merging. 👍 |
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.
Very nice!
Can be merged when build passes. |
Thank you for great review feedback, @MartinHjelmare and @Quentame! 🙏 No worries, @Quentame! 😄 I guess it would be good if you also marked your original review as "approved" (or if one can remove the review or something?) so that the PR can be sorted accordingly 🙂 |
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.
Sorry, I think I've missed this.
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.
3 comments more before merging
…nt library doing I/O in async context.
This comment has been minimized.
This comment has been minimized.
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!
Very nice work @ManneW ! Thanks for contributing 👏 |
Proposed change
New integration added for the Vilfo Router (https://www.vilfo.com). The Vilfo Router is a VPN router with an HTTP API. In this initial version, the integration only provides a couple of sensors to monitor the status of the router. The idea is to add device tracking soon after this initial version (but as the guidelines states, this first version is limited to a single platform).
Type of change
Example entry for
configuration.yaml
:Integration only supports config_flow configuration.
Additional information
Checklist
black --fast homeassistant tests
)For the
config_flow
part of the integration: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
.