Fix to Fan TURN_ON and TURN_OFF features required for HA 2025.1 (and fix merge issues) #35
Conversation
fix: remove deprecated code
Fix supported Fan features as needed for HA 2025.1. Adds FanEntityFeatures.TURN_ON and FanEntityFeatures.TURN_OFF
# Conflicts: # custom_components/vesync/__init__.py
# Conflicts: # custom_components/vesync/__init__.py
|
Replaces #33 with all the conflicts resolved |
gdgib
changed the title
|
|
||
| await hass.config_entries.async_forward_entry_setups( | ||
| config_entry, list(PLATFORMS.keys()) |
There was a problem hiding this comment.
This PR is undoing the fixes from #32
There was a problem hiding this comment.
Not quite, and I'm definitely not sure I've got the logic right between all the merges, so bear with me for a moment.
If you look at #32, the platforms_list was never populated in the code you were fixing. This fork already had code to populate the platforms_list from PLATFORMS.keys() after filtering through the device_dict. Take a careful read through and let me know if you still think I've got it wrong. I'm going to run some quick tests, and then re-review this.
Oh, and I agree I really should have made a separate PR in a better world.
There was a problem hiding this comment.
The key is that my fork had platforms_list.append(p) (see https://github.com/haext/custom_vesync/blob/main/custom_components/vesync/__init__.py#L93) already, which solved SOME of the problem you were fixing. I had to deal with the mismatch while resolving merge conflicts, which is how I got a bit turned around and why this change ended up in this PR.
Anyway, I think the logic is correct now. The only difference between my code and what you wrote in that my code won't setup platforms which were excluded from the output of async_process_devices (see https://github.com/haext/custom_vesync/blob/main/custom_components/vesync/common.py#L34), which I think is actually slightly more correct.
As I mentioned I ABSOLUTELY could be wrong about this, so don't feel shy about pointing out anything I missed, or opening a PR that you think is necessary to correct things.
|
Seems I was too late, this PR just undid my work. Do you want me to submit a new PR or are you just undoing the last merge and resubmitting this PR? |
|
Check the comment I just added. I don't think this actually undoes your work, I think it does the same thing through a slightly different code path. Read carefully, and let me know if you disagree, because if I'm wrong it's the least I can do to fix it myself and not annoy you any more. |
|
You're right, it's not undoing my code. I was on my phone when I saw it, and didn't check it carefully, I just wanted to type as fast as possible before it got merged (I wasn't fast enough 😄 ) I'm surprised the linter didn't give you an error, because my PR erred at this exact line. Then again, you did update a few of the packages versions, and that might have solved it. |
|
I mean chances are we weren't BOTH going to be wrong, and I honestly wasn't sure either. I'm just relieved that I didn't completely screw things up with all the merges and linter updates at the same time. I can't tell you how much I appreciate your keeping an eye on that and warning me you thought something was wrong, on top of your original bug fix. No need to apologize for the haste, it was warranted and appreciated. Thank you so much for all your hard work, and care. |
No description provided.