Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 Overkiz AtlanticPassAPCHeatingAndCoolingZone #78659
Add Overkiz AtlanticPassAPCHeatingAndCoolingZone #78659
Changes from all commits
daaea46
2b42f7b
4ea72b8
c483c44
5dc7948
3aea914
d51205f
b5c4f9f
2b6f4a4
6cfce13
dc20991
bd2548b
8a807ea
afb7367
bc6287e
4e3c9f8
bc1a51f
2e454b2
972a2df
533b1b8
82abacb
c817ba9
e36c56f
ed0c7eb
4c1f098
0d92ab0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What does this code do? Tbh, I don't find this code very clean / readable. And it seems that
self.index_device_url
is not typed / having None as default.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.
In other devices, like somfy_thermostat for instance, we use a constant:
core/homeassistant/components/overkiz/climate_entities/somfy_thermostat.py
Line 51 in 551fb44
core/homeassistant/components/overkiz/climate_entities/somfy_thermostat.py
Lines 67 to 74 in 551fb44
I think it might be clearer an less prone to error than doing
n+1
as you do hereThere 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.
This type of device don't work like that.
We have to use the climate device index and search for n+1 index, which will be the temperature sensor.
The n+1 is required
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.
Does it resolves the 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.
When this PR is merged and published, this code will be updated.
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.
OK, so, for now we are keeping it like 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.
It's working like this, it could be merged as it is.
Up to @iMicknl to decide I guess.