Add Daikin Madoka thermostat integration #45314
Conversation
|
Black and isort were stuck in a loop, each of them modifying the other changes so I had to disable isort. Any solution for this? |
The pre-commit hook gets stuck in a file formatting loop where black and isort modify each other results. According to the isort webpage, the option --profile black has to be used to enhance compatibility. This has been run successfully in my local and has solved the loop. See: home-assistant#45318 home-assistant#45314
…ck (#45321) The pre-commit hook gets stuck in a file formatting loop where black and isort modify each other results. According to the isort webpage, the option --profile black has to be used to enhance compatibility. This has been run successfully in my local and has solved the loop. See: #45318 #45314
|
@frenck can you have a look at the isort error, please? |
There was a problem hiding this comment.
Some comments to get you started.
Additional:
- There is an image in this PR? Please remove it.
- The Configuration Flow requires tests, with a 100% coverage
I cannot reproduce the black/isort issues you've been pointing out. The changes isort wants to make (as shown in the CI), are correct and the same in my local development environment. black doesn't trigger on the changes isort created on my end either.
Please make sure you have the correct version of black & isort.
mduran80
force-pushed
the
daikin-madoka-integration
branch
4 times, most recently
from
eeaa069
to
a40e85c
Compare
|
Hey @frenck ! Could you have another look at this? I guess you must be busy with other stuff, but this is my first PR (noob) and I don't know if I have to ping you or.. just wait? Thank you! |
| def device_info(self): | ||
| """Return a device description for device registry.""" | ||
|
|
||
| return self.dev_info |
There was a problem hiding this comment.
I am not sure if this is allowed, since you don't specify all properties yourselves. Are you sure it only contains valid device properties? Best would be to do it as the example below, where you can always pull the info from self.dev_info.
return {
"identifiers": {
# Serial numbers are unique identifiers within a specific domain
(hue.DOMAIN, self.unique_id)
},
"name": self.name,
"manufacturer": self.light.manufacturername,
"model": self.light.productname,
"sw_version": self.light.swversion,
"via_device": (hue.DOMAIN, self.api.bridgeid),
}See https://developers.home-assistant.io/docs/device_registry_index/#device-properties
There was a problem hiding this comment.
No, the properties may no be fully aligned. When I reviewed what was being done with them I just thought it would do no harm to let HA list only those that match. I will change it to the supported properties as suggested.
Updated for lint tests Removed climate platform and unused variables Force CI workflow run Clean up Fixed discover devices wrong parameters Linter fixes Ensure unique id is set and early abort Added config flow tests Remove platform till next PR Updated requirements Bleak error workaround Bleak init error workaround
mduran80
force-pushed
the
daikin-madoka-integration
branch
from
9732cda
to
a9dd5cb
Compare
|
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. |
Breaking change
Proposed change
Add integration for Daikin BRC1H bluetooth thermostats (a.k.a madoka)
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.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: