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.

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

[tapocontrol] Add support for P115 smart-socket #13225

Merged
merged 2 commits into from Aug 7, 2022
Merged

[tapocontrol] Add support for P115 smart-socket #13225

merged 2 commits into from Aug 7, 2022

Conversation

wildcs
Copy link
Contributor

@wildcs wildcs commented Aug 6, 2022

This PR now added the Tapo P115 smart-socket to tapocontrol.
Also fixed thing configuration warning occured since OH3.3

@wildcs
[tapocontrol] added P115 smart socket
0a377db 
Signed-off-by: Christian Wild <christian@wild-bw.de>
@lolodomo
Copy link
Contributor

lolodomo commented Aug 7, 2022 •
edited

Also fixed thing configuration warning occured since OH3.3

The configuration parameter is defined as an integer, there is no real sense to consider in the binding it could be a decimal.
Maybe the root cause leading to a decimal value in your setup should rather be determined and fixed.

If not, this would mean we have to fix all bindings to handle something that should not happen.

@J-N-K : WDYT ?

@lolodomo
Copy link
Contributor

lolodomo commented Aug 7, 2022 •
edited

Could you please exclude the new parameter handling and put it in a separate PR so that I can at least merge the PR which is about adding P115 smart socket ?

@lolodomo lolodomo added the enhancement An enhancement or new feature for an existing add-on label Aug 7, 2022
jlaur
jlaur requested changes Aug 7, 2022
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks! I have added a few minor comments.

...c/main/java/org/openhab/binding/tapocontrol/internal/structures/TapoBridgeConfiguration.java Outdated Show resolved Hide resolved
...c/main/java/org/openhab/binding/tapocontrol/internal/structures/TapoDeviceConfiguration.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tapocontrol/README.md Outdated Show resolved Hide resolved
@jlaur
Copy link
Contributor

jlaur commented Aug 7, 2022

@lolodomo - sorry, I missed your comments when checking the PR. At least our comments are consistent. :)

@wildcs
[tapocontrol] enabled energy-data for P115 socket
f8b764e 
Signed-off-by: Christian Wild <christian@wild-bw.de>
jlaur
jlaur approved these changes Aug 7, 2022
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

LGTM

@jlaur jlaur merged commit 7a31cb2 into openhab:main Aug 7, 2022
@jlaur jlaur added this to the 3.4 milestone Aug 7, 2022
@jlaur jlaur changed the title [tapocontrol] included P115 smart-socket [tapocontrol] Add support for P115 smart-socket Aug 7, 2022
@wildcs
Copy link
Contributor Author

wildcs commented Aug 7, 2022

removed the parameter handling workaround.
But without that i get still an error:
NumberFormatException@203 "java.lang.NumberFormatException: For input string: "20.0"
So the setting is useless?

@jlaur
Copy link
Contributor

jlaur commented Aug 7, 2022

@wildcs - I assume you are running the binding in openHAB 3.3? The mentioned bugfix is included in 3.4. You could try to run a snapshot version to verify this. Probably the first 3.4 milestone release is not far away.

@wildcs
Copy link
Contributor Author

wildcs commented Aug 7, 2022 •
edited

@jlaur - no i'm using OH3.4 Build 3034 on my coding pc and there i can reproduce and debug the error.

@jlaur
Copy link
Contributor

jlaur commented Aug 11, 2022

no i'm using OH3.4 Build 3034 on my coding pc and there i can reproduce and debug the error.

Sorry, I missed that openhab/openhab-core#3024 is not merged yet. So we expect to have a generic fix for 3.4 so that we don't have to fix all bindings, but the PR needs review with the possibility of more work to be done.

@jlaur
Copy link
Contributor

jlaur commented Sep 27, 2022 •
edited

no i'm using OH3.4 Build 3034 on my coding pc and there i can reproduce and debug the error.

Sorry, I missed that openhab/openhab-core#3024 is not merged yet. So we expect to have a generic fix for 3.4 so that we don't have to fix all bindings, but the PR needs review with the possibility of more work to be done.

@wildcs - openhab/openhab-core#3024 was recently merged, so this should be fixed now. Perhaps you could verify with latest snapshot - otherwise it will be included in next milestone.

@wildcs
Copy link
Contributor Author

wildcs commented Sep 27, 2022

Hi,
testet with Snapshot #3103. Issue is still there when reading "discoveryIntervalM" :
image
Object x is for debugging only

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/tapocontrol-control-tapo-smart-wifi-devices-with-openhab/118389/185

@jlaur
Copy link
Contributor

jlaur commented Sep 27, 2022

Hi,
testet with Snapshot #3103. Issue is still there when reading "discoveryIntervalM"

Did you test with latest snapshot of core, or only binding?

@wildcs
Copy link
Contributor Author

wildcs commented Sep 27, 2022

Downloaded latest snapshot from oh-homepage and put it into debugging enviroment
image

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/tapocontrol-control-tapo-smart-wifi-devices-with-openhab/118389/197

leifbladt pushed a commit to leifbladt/openhab-addons that referenced this pull request Oct 15, 2022
@wildcs @leifbladt
[tapocontrol] included P115 smart-socket (openhab#13225)
c501ef2 
* [tapocontrol] added P115 smart socket
* [tapocontrol] enabled energy-data for P115 socket

Signed-off-by: Christian Wild <christian@wild-bw.de>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@wildcs @andan67
[tapocontrol] included P115 smart-socket (openhab#13225)
947d0cd 
* [tapocontrol] added P115 smart socket
* [tapocontrol] enabled energy-data for P115 socket

Signed-off-by: Christian Wild <christian@wild-bw.de>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
@wildcs @andrasU
[tapocontrol] included P115 smart-socket (openhab#13225)
4c81d9b 
* [tapocontrol] added P115 smart socket
* [tapocontrol] enabled energy-data for P115 socket

Signed-off-by: Christian Wild <christian@wild-bw.de>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@wildcs @psmedley
[tapocontrol] included P115 smart-socket (openhab#13225)
056b1a0 
* [tapocontrol] added P115 smart socket
* [tapocontrol] enabled energy-data for P115 socket

Signed-off-by: Christian Wild <christian@wild-bw.de>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@wildcs @nemerdaud
[tapocontrol] included P115 smart-socket (openhab#13225)
077aed5 
* [tapocontrol] added P115 smart socket
* [tapocontrol] enabled energy-data for P115 socket

Signed-off-by: Christian Wild <christian@wild-bw.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wildcs @lolodomo @jlaur @openhab-bot