GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Convert the codebase to be more modular by rytilahti · Pull Request #299 · python-kasa/python-kasa

Conversation

rytilahti commented Jan 29, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

rytilahti force-pushed the modularize branch from 028f5b8 to fbecfd4 Compare

January 29, 2022 18:37

bdraco mentioned this pull request

Jan 30, 2022

Add effect support for light strips #293

Merged

bdraco mentioned this pull request

Mar 20, 2022

Add support for effects to tplink light strips home-assistant/core#65166

Merged

4 of 22 tasks

bdraco commented Mar 21, 2022

Copy link

Copy Markdown

Member

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'

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bdraco commented Mar 21, 2022

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rytilahti commented Mar 21, 2022

Copy link

Copy Markdown

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 reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rytilahti force-pushed the modularize branch from ce58a87 to ac01e4f Compare

March 24, 2022 14:11

bdraco commented Mar 24, 2022

Copy link

Copy Markdown

Member

Strips are fixed here #326

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bdraco commented Mar 25, 2022

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rytilahti force-pushed the modularize branch from 9af2b81 to 07d66b4 Compare

April 5, 2022 16:25

rytilahti marked this pull request as ready for review

April 5, 2022 16:29

codecov-commenter commented Apr 5, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rytilahti and others added 10 commits

April 5, 2022 19:12


          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


          Improve usage module, consolidate API with emeter (#249)

a76e520

* Consolidate API for both emeter&usage modules
* Add new cli command 'usage' to query usage


          Implement motion & ambient light sensor modules for dimmers (#278)

4dd3ca5


          Do not request unsupported modules after the initial update (#298)

2c1d7f5

* Do not request unsupported modules after the initial update

* debugify logging


          Raise an exception when trying to access data prior updating

6c30fcc


          Use device time for on_since for smartstripplugs

e85f720


          move get_time{zone} out from smartdevice + some minor cleanups

8ba68d0


          Fix modularize with strips (#326)

88e690a

* Fix test_deprecated_type stalling

* Fix strips with modularize

* Fix test_deprecated_type stalling (#325)


          Fix linting


          Bump version to 0.5.0.dev0

f48e75a

rytilahti force-pushed the modularize branch from e88ac97 to f48e75a Compare

April 5, 2022 17:13

rytilahti added the enhancement New feature or request label

Apr 5, 2022

rytilahti changed the title ~~Merge modularize back to master~~ Convert the codebase to be more modular

Apr 5, 2022

rytilahti merged commit 68038c9 into master

Apr 5, 2022

rytilahti deleted the modularize branch

April 5, 2022 17:27

rytilahti added the breaking change Breaking change label

Apr 24, 2022

rytilahti mentioned this pull request

Apr 24, 2022

Prepare 0.5.0 #342

Merged

rytilahti mentioned this pull request

May 29, 2022

Possible firmware issue with KL125 (1.0.7 Build 211009 Rel.172044) #345

Closed

sdb9696 mentioned this pull request

Apr 25, 2024

Do not automatically add emeter to iot strip #865

Merged

rytilahti pushed a commit that referenced this pull request

Apr 25, 2024


          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)

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

rytilahti commented Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdraco commented Mar 21, 2022

Uh oh!

bdraco commented Mar 21, 2022

Uh oh!

rytilahti commented Mar 21, 2022

Uh oh!

bdraco commented Mar 24, 2022

Uh oh!

bdraco commented Mar 25, 2022

Uh oh!

codecov-commenter commented Apr 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rytilahti commented Jan 29, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

codecov-commenter commented Apr 5, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.