Skip to content
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

feat(AC): support current_humidity for AC device climate #411

Conversation

semyonchetvertnyh
Copy link
Contributor

PR Description

Support current_humidity for climate

Screenshot 2024-12-07 at 23 04 54

@semyonchetvertnyh
Copy link
Contributor Author

semyonchetvertnyh commented Dec 7, 2024

I'm not sure is it enough to work like in the screenshot, but I could make additional changes if needed.

@wuwentao
Copy link
Owner

wuwentao commented Dec 9, 2024

thank you very much for your PR.

this current_humidity add to base Climate class, so it will works for all the AC/CC/CF/C3/FB/etc Climate device.....
I'm not sure with most of these types devices should have or support a indoor_humidity sensor attribute, and it may can 't provide this attribute data for it, anyway, maybe default value None can be instead.

@semyonchetvertnyh
Copy link
Contributor Author

Sure! Is it ok now?

Copy link
Collaborator

@chemelli74 chemelli74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Owner

@wuwentao wuwentao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, you can't add this property to base class MideaClimate.
you can add it to class MideaACClimate(MideaClimate) as it will works for AC device.

the reason:
as base class will be call for all the climate device, but only AC device support indoor_humidity, for example, like FB/C3/CC/CF device, these device don't have attribute indoor_humidity, so we can't add it in base class.

just add it in the same file, but move it under class name class MideaACClimate(MideaClimate)

Thanks

@wuwentao
Copy link
Owner

Sure! Is it ok now?

it's not the expected result.

class MideaACClimate(MideaClimate) start from line 255, you should add your code under class MideaACClimate(MideaClimate)

current code still in line 160 in base class MideaClimate

@semyonchetvertnyh
Copy link
Contributor Author

No problem, I moved it from base class.

@wuwentao wuwentao changed the title Support current_humidity for climate Feat(AC): Support current_humidity for climate Dec 16, 2024
@wuwentao wuwentao changed the title Feat(AC): Support current_humidity for climate feat(AC): support current_humidity for AC device climate Dec 16, 2024
@wuwentao
Copy link
Owner

No problem, I moved it from base class.

thanks, approved and merged.

@wuwentao wuwentao merged commit b0be2cb into wuwentao:master Dec 16, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants