Skip to content

Make device port configurable#471

Merged
rytilahti merged 1 commit intopython-kasa:masterfrom
karpach:master
Jul 9, 2023
Merged

Make device port configurable#471
rytilahti merged 1 commit intopython-kasa:masterfrom
karpach:master

Conversation

@karpach
Copy link
Copy Markdown
Contributor

@karpach karpach commented Jun 27, 2023

My use case is very similar to the discussion #410.

I have an internal network with IoT devices. The network is separated from my home network, so rouge IoT devices don't have the ability to hack my PCs (that happened once). I only expose ports that IoT devices need. Since I have multiple tp-link kasa devices, I need to do port forwarding to different ports.

@karpach karpach force-pushed the master branch 2 times, most recently from ae44a85 to aeef0ac Compare June 28, 2023 02:28
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 28, 2023 •
edited
Loading

Codecov Report

Patch coverage: 95.83% and project coverage change: +0.04 🎉

Comparison is base (6199521) 79.50% compared to head (a67e68e) 79.55%.

❗ 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     #471      +/-   ##
==========================================
+ Coverage   79.50%   79.55%   +0.04%     
==========================================
  Files          26       26              
  Lines        1952     1956       +4     
  Branches      598      599       +1     
==========================================
+ Hits         1552     1556       +4     
  Misses        359      359              
  Partials       41       41
Impacted Files Coverage Δ
kasa/cli.py 56.46% <75.00%> (+0.18%) ⬆️
kasa/discover.py 76.47% <100.00%> (ø)
kasa/protocol.py 86.36% <100.00%> (+0.10%) ⬆️
kasa/smartbulb.py 92.38% <100.00%> (ø)
kasa/smartdevice.py 87.16% <100.00%> (+0.03%) ⬆️
kasa/smartdimmer.py 84.26% <100.00%> (ø)
kasa/smartlightstrip.py 93.33% <100.00%> (ø)
kasa/smartplug.py 100.00% <100.00%> (ø)
kasa/smartstrip.py 88.34% <100.00%> (ø)

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

rytilahti
rytilahti reviewed Jun 30, 2023
View reviewed changes
Comment thread kasa/protocol.py Outdated
@karpach karpach force-pushed the master branch 3 times, most recently from e1eeb80 to ece85fb Compare July 6, 2023 20:38
rytilahti
rytilahti requested changes Jul 6, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, see my comments inline. Basically, I think we should avoid hardcoding the protocol-specific port constant on all the Device-inherited classes and leave it to the protocol class to handle. This not only makes it more extendable, if need to be (#471), but will also simplify the code a bit.

Comment thread kasa/cli.py
Comment thread kasa/discover.py Outdated
Comment thread kasa/protocol.py Outdated
Comment thread kasa/protocol.py Outdated
Comment thread kasa/smartbulb.py Outdated
Comment thread kasa/tests/test_discovery.py Outdated
Comment thread kasa/tests/test_protocol.py Outdated
Comment thread kasa/tests/test_protocol.py Outdated
@rytilahti rytilahti added the enhancement New feature or request label Jul 6, 2023
@karpach karpach force-pushed the master branch 6 times, most recently from 69d1bcd to b62f225 Compare July 7, 2023 19:37
@karpach karpach requested a review from rytilahti July 7, 2023 20:18
rytilahti
rytilahti reviewed Jul 9, 2023
View reviewed changes
Comment thread kasa/smartdimmer.py
@karpach karpach closed this Jul 9, 2023
@karpach karpach force-pushed the master branch 2 times, most recently from b62f225 to 6199521 Compare July 9, 2023 22:11
@karpach karpach reopened this Jul 9, 2023
rytilahti
rytilahti approved these changes Jul 9, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor issue with the exception messages, otherwise this is good to go. Thanks for the PR, @karpach!

Comment thread kasa/protocol.py Outdated
@rytilahti rytilahti changed the title Make 9999 port configurable Make device port configurable Jul 9, 2023
@rytilahti rytilahti merged commit 9b039d8 into python-kasa:master Jul 9, 2023
@sdb9696 sdb9696 mentioned this pull request Jul 14, 2023
Closed
@rytilahti rytilahti mentioned this pull request Dec 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kirichkov kirichkov kirichkov left review comments

@sdb9696 sdb9696 sdb9696 left review comments

@rytilahti rytilahti rytilahti approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@karpach @codecov-commenter @kirichkov @rytilahti @sdb9696