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

Consider splitting the library into a device specific and a HA specific one #7

Closed
attilaersek opened this issue May 15, 2024 · 10 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@attilaersek
Copy link
Collaborator

Device type and model (or SN)

n/a

The description of new feature

@rokam already started a repository that contains only the device communication specific code. What's the status of that?
I think it would be beneficial to use that as a package and remove the code from here and only maintain what is specific to HA. What do you think?

@attilaersek attilaersek added enhancement New feature or request question Further information is requested labels May 15, 2024
@rokam
Copy link
Collaborator

rokam commented May 15, 2024

I do think it is a best pratice and a step tp merge this code in ha core. But I don't have enought time to code it all by myself. Any help is welcome.

@attilaersek
Copy link
Collaborator Author

I do think it is a best pratice and a step tp merge this code in ha core. But I don't have enought time to code it all by myself. Any help is welcome.

What is missing from your repository? Ofc, the test coverage, but that will be a huge work.

@rokam
Copy link
Collaborator

rokam commented May 15, 2024

If we just set it up with initial coverage and automatic deploy to pipy. Also we should start typing the variables.

@wuwentao
Copy link
Owner

agreen with us, and I'm not familiar with midea_ac_lan detail code now, as I just use it, but I will try to read all the source code later, then I will try to help on test coverage and automation.
we may need more time to do it, I'm not sure.

current repo don't have any lint rule or test coverage for PR merge, I will add github linter action to improve it fo PR and future changes, test coverage and new feature may have some delay.
also will discuss with us later.

@chemelli74
Copy link
Collaborator

Hi all,

I would like to contribute moving this integration to core.
We can move by steps:

  • introduce ruff
  • introduce the remaining pre-commit hooks
  • separate low level calls to device in a dedicated library
  • publish the library to pypi
  • move the HA integration to HA core

What you think about the plan ?
I know that also @Necroneco would contribute as well.

As we are going to be a Team , the library could be placed on https://github.com/home-assistant-libs

@caibinqing
Copy link
Collaborator

I agree with moving to core, but it's a lot of work to do.

I'd love to help but don't have the time right now. Hopefully I can start in a few (one or two) weeks.

@rokam
Copy link
Collaborator

rokam commented May 21, 2024

Hi all,

I would like to contribute moving this integration to core.
We can move by steps:

  • introduce ruff
  • introduce the remaining pre-commit hooks
  • separate low level calls to device in a dedicated library
  • publish the library to pypi
  • move the HA integration to HA core

What you think about the plan ?
I know that also @Necroneco would contribute as well.

As we are going to be a Team , the library could be placed on https://github.com/home-assistant-libs

I've already started a lib http://github.com/rokam/midea-local

@rokam
Copy link
Collaborator

rokam commented May 22, 2024

#13

@wuwentao
Copy link
Owner

wuwentao commented May 22, 2024

Thanks all, we have more contributers to continue work with it now.
I have invite/add @rokam @attilaersek @Necroneco @chemelli74 @Qianli-Ma as collaborators to current repo

All of use can merge PR in future and we don't need to worry about all the permission controlled in one people.

@chemelli74
Copy link
Collaborator

Thanks all, we have more contributers to continue work with it now. I have invite/add @rokam @attilaersek @Necroneco @chemelli74 @Qianli-Ma as collaborators to current repo

All of use can merge PR in future and we don't need to worry about all the permission controlled in one people.

Thx for the invite!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants