-
-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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 config_flow to bluesound integration #115207
base: dev
Are you sure you want to change the base?
Conversation
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.
Hi @LouisChrist
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Hey there @thrawnarn, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
Before this integration can be migrated to a config flow, it should adhere to ADR-0010, which states that all code used to connect to a device or service should be encapsulated in a python library published on pypi.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
My Bad. My intent was to do it the other way around as this seemed to be the smaller task. I'm working on a library in parallel: pyblu. Should I do all that in one PR or should I split it into a separate PR? |
db6a4bc
to
19d87db
Compare
I created and integrated a separate library for interacting with the devices: pyblu. The parts regarding join/unjoin of devices are not tested with real devices as i have only one. They are tested to the degree that they do not return any 'Bad Request'. And there are unit tests in the library, which are based on the documentation. I tried to change as little as possible in the integration itself. |
Can we first do a prelimary PR to make the integration use the library? |
I created a separate PR: Integrate pypi library: pyblu |
19d87db
to
a64e8ee
Compare
Wow great work in getting Bluesound up to standards with a real config-flow. I didn't look before I created my own config-flow (with zeroconf discovery). Perhaps you find something useful from my code (or once we get this merged I can create a new PR with the zeroconf code). |
Thank you. I can integrate your zeroconf code into this PR, if you are ok with that. It seems to be a rather small addition. Or you can do it as a separate PR(doing zeroconf a separate PR was my initial plan). That is your decision, since it's your code i would be copying. |
Feel free to use "my" code in any way you want. I didn't think there were anyone actively developing for Bluesound and was quite annoyed that the integration didn't support config registry. |
Then i will integrate zeroconf into this PR, but it might take a while. I am currently working on Use pyblu library in bluesound, which has to be merged first. |
Proposed change
The documentation for using the
configuration.yaml
is removed and replaced by documentation for using the config_flow(see matching PR for docs)Type of change
Additional information
Checklist
ruff format 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: