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

Feature/deattach-lib #13

Merged
merged 12 commits into from
May 28, 2024
Merged

Feature/deattach-lib #13

merged 12 commits into from
May 28, 2024

Conversation

rokam
Copy link
Collaborator

@rokam rokam commented May 22, 2024

Migrate midea folder to a external library midea-local

@rokam
Copy link
Collaborator Author

rokam commented May 22, 2024

@wuwentao this should work. I did a test with my AC unit. Due to this being a big change, it should be better tested before merging.

@wuwentao
Copy link
Owner

yes, it should be a big changes, we need more user and device to test and confirm before merge it.
I have mark it as draft to flag it, once we can confirmed it , we can merge it later.

@wuwentao wuwentao marked this pull request as draft May 22, 2024 02:27
@wuwentao wuwentao marked this pull request as ready for review May 22, 2024 02:27
@wuwentao wuwentao marked this pull request as draft May 22, 2024 02:27
@wuwentao
Copy link
Owner

@attilaersek @Necroneco @chemelli74 @Qianli-Ma

Hi my friends,

any suggestion or comments? rokam has finished this midea-local lib and try to merge it.

  1. can we help rokam to test with it? and confirm our test device and test result? I will try to do some test with my device later and share my result later. as my WAHIN air-conditioners has been power off more than 6 months now, and I need to clean all of them, then try to power on and test it later.

  2. once this PR merged, most of the core feature will be migrate to rokam's midea-local repo, so there will be new feature/issue exist in midea-local repo and current repo, this repo will focus on hacs integration and midea-local will be focus on the core device feature support.
    we can found issue/bug in any repo and try to fix it , hope we can continue improve it in future.

Thanks

@Necroneco
Copy link
Collaborator

I have tested on my Water Drinking Appliance yesterday, works as before.

@attilaersek
Copy link
Collaborator

looks good to me but give me a day or so to test it with my AC. I'll give a feedback quickly.

@attilaersek
Copy link
Collaborator

looks good to me but give me a day or so to test it with my AC. I'll give a feedback quickly.

tested locally and works good, no errors or warnings in logs.

@attilaersek
Copy link
Collaborator

@rokam : resolved the conflict to help others testing the PR. if you have local changes pull the branch with git pull --rebase first.

@rokam
Copy link
Collaborator Author

rokam commented May 23, 2024

Another strategy will be to create a beta release. Maybe with the actual branch.

@chemelli74
Copy link
Collaborator

Another strategy will be to create a beta release. Maybe with the actual branch.

I would go that way honestly.
We easily make the user testing base larger.

@chemelli74
Copy link
Collaborator

Added library_test.py to the library to make testing easier.

I have a small authentication issue with new library + test script:

Traceback (most recent call last):
  File "/tmp/test.py", line 120, in <module>
    asyncio.run(main())
  File "/usr/local/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/base_events.py", line 664, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/tmp/test.py", line 106, in main
    ac.authenticate()
  File "/usr/local/lib/python3.12/site-packages/midealocal/device.py", line 165, in authenticate
    raise AuthException()
midealocal.device.AuthException

@Qianli-Ma
Copy link
Collaborator

@attilaersek @Necroneco @chemelli74 @Qianli-Ma

Hi my friends,

any suggestion or comments? rokam has finished this midea-local lib and try to merge it.

  1. can we help rokam to test with it? and confirm our test device and test result? I will try to do some test with my device later and share my result later. as my WAHIN air-conditioners has been power off more than 6 months now, and I need to clean all of them, then try to power on and test it later.
  2. once this PR merged, most of the core feature will be migrate to rokam's midea-local repo, so there will be new feature/issue exist in midea-local repo and current repo, this repo will focus on hacs integration and midea-local will be focus on the core device feature support.
    we can found issue/bug in any repo and try to fix it , hope we can continue improve it in future.

Thanks

I'll test with my Colmo central AC this weekend. I'll have more free time by the end of May.

@wuwentao
Copy link
Owner

Another strategy will be to create a beta release. Maybe with the actual branch.

I would go that way honestly. We easily make the user testing base larger.

got it, I will test current changes with my devices this weekend and share the test result.
then we can merge current PR next week.

@rokam
Copy link
Collaborator Author

rokam commented May 24, 2024

Added library_test.py to the library to make testing easier.

I have a small authentication issue with new library + test script:

Traceback (most recent call last):
  File "/tmp/test.py", line 120, in <module>
    asyncio.run(main())
  File "/usr/local/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/base_events.py", line 664, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/tmp/test.py", line 106, in main
    ac.authenticate()
  File "/usr/local/lib/python3.12/site-packages/midealocal/device.py", line 165, in authenticate
    raise AuthException()
midealocal.device.AuthException

authenticate only works with v3 devices.

@chemelli74
Copy link
Collaborator

authenticate only works with v3 devices.

midea-lan/midea-local#15 (comment)

@wuwentao
Copy link
Owner

did a simple test, seems it works with My midea AC device,
there is some PR about update the core device lib, we can merge this PR and move the core device feature to midea-local repo.

@rokam rokam marked this pull request as ready for review May 27, 2024 13:42
@rokam rokam requested a review from wuwentao May 27, 2024 13:42
@wuwentao wuwentao merged commit 4c1b1d1 into master May 28, 2024
18 checks passed
@wuwentao wuwentao deleted the feature/deattach-lib branch May 28, 2024 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants