8000 Convert the codebase to be more modular by rytilahti · Pull Request #299 · python-kasa/python-kasa · GitHub
[go: up one dir, main page]
Skip to content

Convert the codebase to be more modular #299

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Apr 5, 2022
Merged

Convert the codebase to be more modular #299

merged 10 commits into from
Apr 5, 2022

Conversation

rytilahti
Copy link
Member
@rytilahti rytilahti commented Jan 29, 2022 •
edited
Loading

This PR creates a modularized foundation to expose more features on the supported devices, making it more straightforward to implement new features in the future. This work forms the basis for 0.5 series.

At the moment, the most visible change is that each update cycle queries the information from all modules supported by the device type in a single query:

  • Basic system info
  • Cloud (new)
  • Countdown (new)
  • Antitheft (new)
  • Schedule (new)
  • Usage (new)
  • Time (existing, implements the time/timezone handling)
  • Emeter (existing, partially separated from smartdevice))
  • Ambientlight (new, for dimmers)
  • Motion (new, for dimmers)

Some of the new features are directly exposed to the cli while others are not, the follow-up PRs should add module-based tests and expose features where it makes sense.

Fixes #295 #268 #244

@bdraco bdraco mentioned this pull request Jan 30, 2022
Merged
@bdraco bdraco mentioned this pull request Mar 20, 2022
Merged
22 tasks
@bdraco
Copy link
Member
bdraco commented Mar 21, 2022

I think this is the outstanding issue

2022-01-30 14:53:03 ERROR (MainThread) [homeassistant.components.sensor] Error while setting up tplink platform for sensor
Traceback (most recent call last):
  File "/Users/bdraco/home-assistant/homeassistant/helpers/entity_platform.py", line 249, in _async_setup_platform
    await asyncio.shield(task)
  File "/Users/bdraco/home-assistant/homeassistant/components/tplink/sensor.py", line 133, in async_setup_entry
    entities.extend(_async_sensors_for_device(child))
  File "/Users/bdraco/home-assistant/homeassistant/components/tplink/sensor.py", line 124, in _async_sensors_for_device
    return [
  File "/Users/bdraco/home-assistant/homeassistant/components/tplink/sensor.py", line 127, in <listcomp>
    if async_emeter_from_device(device, description) is not None
  File "/Users/bdraco/home-assistant/homeassistant/components/tplink/sensor.py", line 99, in async_emeter_from_device
    if (val := getattr(device.emeter_realtime, attr)) is None:
  File "/Users/bdraco/home-assistant/venv/lib/python3.9/site-packages/kasa/smartdevice.py", line 88, in wrapped
    return f(*args, **kwargs)
  File "/Users/bdraco/home-assistant/venv/lib/python3.9/site-packages/kasa/smartdevice.py", line 462, in emeter_realtime
    return EmeterStatus(self.modules["emeter"].realtime)
KeyError: 'emeter'

@bdraco
Copy link
Member
bdraco commented Mar 21, 2022

I don't have an HS300 at my current location. I was hoping to get one locally but they aren't in stock. I have ordered one

@rytilahti
Copy link
Member Author

Okay, after the next release with the light effects, I'll simply merge this to master with all its potential flaws to avoid the need for keeping branches in sync. That'll also hopefully get some more eyeballs to spot if there are other issues that needs to be done prior shipping 0.5.

@bdraco
Copy link
Member
bdraco commented Mar 24, 2022

Strips are fixed here #326

@bdraco
Copy link
Member
bdraco commented Mar 25, 2022

I think all the tests will now pass as well with #326

@rytilahti rytilahti marked this pull request as ready for review April 5, 2022 16:29
@codecov-commenter
Copy link
codecov-commenter commented Apr 5, 2022 •
edited
Loading

Codecov Report

Merging #299 (e88ac97) into master (a744af4) will decrease coverage by 1.26%.
The diff coverage is 79.90%.

@@            Coverage Diff             @@
##           master     #299      +/-   ##
==========================================
- Coverage   82.34%   81.07%   -1.27%     
==========================================
  Files          13       25      +12     
  Lines        1376     1723     +347     
  Branches      202      240      +38     
==========================================
+ Hits         1133     1397     +264     
- Misses        201      285      +84     
+ Partials       42       41       -1
Impacted Files Coverage Δ
kasa/cli.py 57.79% <39.53%> (-2.60%) ⬇️
kasa/smartstrip.py 87.11% <59.09%> (-4.50%) ⬇️
kasa/modules/motion.py 66.66% <66.66%> (ø)
kasa/modules/ambientlight.py 68.75% <68.75%> (ø)
kasa/modules/usage.py 73.80% <73.80%> (ø)
kasa/modules/cloud.py 81.48% <81.48%> (ø)
kasa/modules/rulemodule.py 82.60% <82.60%> (ø)
kasa/protocol.py 90.35% <87.50%> (-0.09%) ⬇️
kasa/modules/module.py 88.57% <88.57%> (ø)
kasa/modules/time.py 91.66% <91.66%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a744af4...e88ac97. Read the comment docs.

rytilahti and others added 10 commits April 5, 2022 19:12
@rytilahti
Add module support & query their information during update cycle (#243)
a9c29d4 
* Add module support & modularize existing query

This creates a base to expose more features on the supported devices.
At the moment, the most visible change is that each update cycle gets information from all available modules:
* Basic system info
* Cloud (new)
* Countdown (new)
* Antitheft (new)
* Schedule (new)
* Time (existing, implements the time/timezone handling)
* Emeter (existing, partially separated from smartdevice)

* Fix imports

* Fix linting

* Use device host instead of alias in module repr

* Add property to list available modules, print them in cli state report

* usage: fix the get_realtime query

* separate usage from schedule to avoid multi-inheritance

* Fix module querying

* Add is_supported property to modules
@rytilahti
Improve usage module, consolidate API with emeter (#249)
a76e520 
* Consolidate API for both emeter&usage modules
* Add new cli command 'usage' to query usage
@rytilahti
Implement motion & ambient light sensor modules for dimmers (#278)
4dd3ca5
@rytilahti
Do not request unsupported modules after the initial update (#298)
2c1d7f5 
* Do not request unsupported modules after the initial update

* debugify logging
@rytilahti
Raise an exception when trying to access data prior updating
6c30fcc
@rytilahti
Use device time for on_since for smartstripplugs
e85f720
@rytilahti
move get_time{zone} out from smartdevice + some minor cleanups
8ba68d0
@bdraco @rytilahti
Fix modularize with strips (#326)
88e690a 
* Fix test_deprecated_type stalling

* Fix strips with modularize

* Fix test_deprecated_type stalling (#325)
@rytilahti
Fix linting
4889050
@rytilahti
Bump version to 0.5.0.dev0
f48e75a
@rytilahti rytilahti added the enhancement New feature or request label Apr 5, 2022
@rytilahti rytilahti changed the title Merge modularize back to master Convert the codebase to be more modular Apr 5, 2022
@rytilahti rytilahti merged commit 68038c9 into master Apr 5, 2022
@rytilahti rytilahti deleted the modularize branch April 5, 2022 17:27
@rytilahti rytilahti added the breaking change Breaking change label Apr 24, 2022
@rytilahti rytilahti mentioned this pull request Apr 24, 2022
Merged
@rytilahti rytilahti mentioned this pull request May 29, 2022
Closed
@sdb9696 sdb9696 mentioned this pull request Apr 25, 2024
Merged
rytilahti pushed a commit that referenced this pull request Apr 25, 2024
@sdb9696
Do not automatically add emeter to iot strip (#865)
8e0c093 
This fixes the current broken tests in
#844 caused by the
fixtures for `HS107(US)_1.0_1.0.8.json` and `KP400(US)_1.0_1.0.10.json`
having the `emeter` value in the fixture but no `get_realtime` value.
Other strips do not fail because they do not have the `emeter` value in
the fixture and the `FakeIotProtocol` creates one for them.

It seems that #299
introduced the adding of Emeter in the smart strip constructor (in
commit
c8ad99a)
and this was added after
#243 which adds the
Emeter in the IoDevice `update()` method so perhaps there is an
implication to this change that I'm missing, although it seems
`_create_emeter_request` is no longer called anywhere so maybe that was
a historical reason.

N.B. The tests all pass with this PR.

Link to [CI run on
branch](https://github.com/sdb9696/python-kasa/actions/runs/8831324308)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
breaking change Breaking change enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"on since" changes
3 participants
@rytilahti @bdraco @codecov-commenter
0