Exclude querying certain modules for KL125(US) which cause crashes #451

brianthedavis · 2023-04-26T23:38:06Z

I've been running an old 0.4.0dev5 build for quite a while reliably and haven't had any python-kasa issues since #199 - but in downloading the latest version I had the exact same issues that @Nathanlclark mentioned in #345. I could run the command once, but subsequent commands would time out. This made me believe that this is a regression of the issue that #199 addressed - requesting modules that the bulb doesn't support causes it to crash and restart; thus being unresponsive for a few seconds.

I think I confirmed this theory -- if I modify the constructor of the smartbulb.py file to this:

def __init__(self, host: str) -> None:
        super().__init__(host=host)
        self._device_type = DeviceType.Bulb
        self.add_module("schedule", Schedule(self, "smartlife.iot.common.schedule"))
        self.add_module("usage", Usage(self, "smartlife.iot.common.schedule"))
        # self.add_module("antitheft", Antitheft(self, "smartlife.iot.common.anti_theft"))
        self.add_module("time", Time(self, "smartlife.iot.common.timesetting"))
        self.add_module("emeter", Emeter(self, self.emeter_type))
        # self.add_module("countdown", Countdown(self, "countdown"))
        # self.add_module("cloud", Cloud(self, "smartlife.iot.common.cloud"))

(removing the antitheft, countdown, and cloud modules), I can consistently run python-kasa over and over with no lag.

I've added in an exclusion filter into smartdevice.py to remove these modules on a model by model basis; I'm not sure if this is the "right" way to do this; I tried figuring out a more effective way to eliminate certain modules but only by model type, but we don't know enough information when the modules are added. I'm more than happy to refactor this based on any input.

I was able to successfully execute the tests against these code changes and they all passed.

Fixes #345

kasa/smartdevice.py

rytilahti · 2023-04-29T23:31:38Z

kasa/smartdevice.py


+# Certain module queries will crash devices; this list skips those queries
+MODULE_EXCLUSIONS = {
+    "KL125(US)": ["antitheft", "cloud", "countdown"]


Add a comment linking to the issue, just in case someone comes around wondering why this is done.

Btw, are you certain that cloud is also an issue? I'm seeing on my KL130 that it does not support countdown:

{'countdown': {'err_code': -2001, 'err_msg': 'Module not support'}

and one of the antitheft methods is also unsupported:

'smartlife.iot.common.anti_theft': {'get_next_action': {'err_code': -2000, 'err_msg': 'Method ' 'not ' 'support'}, 'get_rules': {'enable': 0, 'err_code': 0, 'rule_list': []}},

while the cloud query has worked on all on my test devices.

Could it just be, that the number of modules (and/or methods) is the thing that breaks it?

Add a comment linking to the issue, just in case someone comes around wondering why this is done.

Done!

Btw, are you certain that cloud is also an issue? I'm seeing on my KL130 that it does not support countdown:

I went back and did more testing and made a few additional interesting discoveries (that I'll document here in case someone else stumbles onto them).

I had a power outage last week and so all of my bulbs were powered off about 72 hours ago.

As a result, I discovered that querying the cloud, countdown, and antitheft modules did NOT cause most of my KL125 bulbs to consistently crash. This tells me that the "crashing" of the bulbs is loosely tied to how long they've been powered on.

I was able to get one of my bulbs to misbehave and confirmed your comment that the countdown and antitheft modules are indeed not supported, and the cloud module IS supported; however querying this module made the bulb crash with every other query.

So while the cloud module is_supported, it's interrogation seems to cause instability in the bulb.

I was able to reduce the exclude list down to just cloud and the bulb queries stabilized

I've reduced the exclusion list to cloud and that seems to be stable now for me.

kasa/smartdevice.py

kasa/tests/fixtures/KL125(US)_1.20_1.0.7.json

kasa/smartdevice.py

Co-authored-by: Teemu R. <tpr@iki.fi>

codecov-commenter · 2023-05-18T02:47:24Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.15 🎉

Comparison is base (39310f3) 79.17% compared to head (6acd809) 79.32%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #451      +/-   ##
==========================================
+ Coverage   79.17%   79.32%   +0.15%     
==========================================
  Files          25       25              
  Lines        1930     1935       +5     
  Branches      597      598       +1     
==========================================
+ Hits         1528     1535       +7     
+ Misses        361      359       -2     
  Partials       41       41

Impacted Files	Coverage Δ
kasa/smartdevice.py	`87.12% <100.00%> (+0.19%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

rytilahti

LGTM, thanks @brianthedavis! 👍

…ython-kasa#451) * commented out modules that break * added exclusion logic to smartdevice.py * cleaning up a name * removing test fixture that isn't related to this PR * incorporating PR feedback * fixed an if statement * reduced exclusion list to just 'cloud' * Tidy up the issue comment Co-authored-by: Teemu R. <tpr@iki.fi> * this seems to be what the linter whats --------- Co-authored-by: Teemu R. <tpr@iki.fi>

brianthedavis added 3 commits April 26, 2023 16:59

commented out modules that break

4ae9a32

added exclusion logic to smartdevice.py

a286b15

cleaning up a name

9a8cfcc

brianthedavis mentioned this pull request Apr 26, 2023

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

Closed

rytilahti reviewed Apr 29, 2023

View reviewed changes

rytilahti changed the title ~~[ISSUE 345] Exclude querying certain modules for KL125(US) which cause crashes~~ Exclude querying certain modules for KL125(US) which cause crashes Apr 29, 2023

brianthedavis added 4 commits May 1, 2023 20:20

removing test fixture that isn't related to this PR

9bee885

incorporating PR feedback

fe39462

fixed an if statement

f75468c

reduced exclusion list to just 'cloud'

36dd468

brianthedavis requested a review from rytilahti May 2, 2023 03:19

rytilahti reviewed May 17, 2023

View reviewed changes

kasa/smartdevice.py Outdated Show resolved Hide resolved

rytilahti added the bug Something isn't working label May 17, 2023

brianthedavis and others added 3 commits May 17, 2023 17:28

Tidy up the issue comment

c882768

Co-authored-by: Teemu R. <tpr@iki.fi>

Merge branch 'python-kasa:master' into issue_345

a017f35

this seems to be what the linter whats

6acd809

rytilahti approved these changes May 18, 2023

View reviewed changes

rytilahti merged commit 9550cbd into python-kasa:master May 18, 2023

rytilahti mentioned this pull request Aug 26, 2023

Split queries to avoid overflowing device buffers #502

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Exclude querying certain modules for KL125(US) which cause crashes #451

Exclude querying certain modules for KL125(US) which cause crashes #451

Uh oh!

brianthedavis commented Apr 26, 2023 •

edited by rytilahti

Loading

Uh oh!

Uh oh!

rytilahti Apr 29, 2023

Uh oh!

brianthedavis May 2, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented May 18, 2023 •

edited

Loading

Uh oh!

rytilahti left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Exclude querying certain modules for KL125(US) which cause crashes #451

Exclude querying certain modules for KL125(US) which cause crashes #451

Uh oh!

Conversation

brianthedavis commented Apr 26, 2023 • edited by rytilahti Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rytilahti Apr 29, 2023

Choose a reason for hiding this comment

Uh oh!

brianthedavis May 2, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rytilahti left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

brianthedavis commented Apr 26, 2023 •

edited by rytilahti

Loading

codecov-commenter commented May 18, 2023 •

edited

Loading