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.

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

[VeSync][WIP] Add outlets to the supported devices #15343

Closed
wants to merge 1,552 commits into from

Conversation

marcelGoerentz
Copy link
Contributor

@marcelGoerentz marcelGoerentz commented Jul 31, 2023

[VeSync][WIP] Add outlets to the supported devices

This enhancement will let you get information about your smart plug (outlet) and give the oppurtunity to control it.

Description

This is improvement will let you control your outlets that are uplinked to you VeSync account.
As I have smart plug which delivers information about the used energy and the current energy and is also contolable via the VeSync App, I wanted to observe and control it via openHAB. Therefore I decided to develop this improvement.
There are no changes done to the old functionality of this binding, there will only be features added.
To add a plug to your running system it will follow the same flow as it was for the humidifier and purifier.
So all you have to do is waiting for an automatic scan or trigger it by yourself.

Testing

Unfortunately I only have one smart plug (GreenSun Wifi Smart plug) that I could test with and this is working with firmware version 1.0.02.
So it would be great if someone else could test this binding with his/hers outlets to confirm its working properly.

You will find the jar file here: https://github.com/marcelGoerentz/openhab-addons/releases/tag/v0.0.2

Discussion

You can Discuss with me about this binding here: https://community.openhab.org/t/vesync-smart-plug/148039

@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/vesync-smart-plug/148039/3

@marcelGoerentz marcelGoerentz marked this pull request as draft July 31, 2023 11:19
@marcelGoerentz marcelGoerentz changed the title [VeSync] Add outlets to the supported devices [VeSync][WIP] Add outlets to the supported devices Jul 31, 2023
@dag81
Copy link
Contributor

dag81 commented Aug 1, 2023

Hi @marcelGoerentz, its great to see someone else extending it :)

I was just looking through your PR - need to continue later. Can you confirm if this is using any special endpoints. Only as I have a huge PR to support some of the bespoke endpoints for some of the other devices, and add 131 and other models here:

#15296

@marcelGoerentz
Copy link
Contributor Author

Hi @dag81 ,

as far as I know I've only adapted the already working DTOs to my needs. But I didn't add any endpoint.

@dag81
Copy link
Contributor

dag81 commented Aug 1, 2023

Hi @dag81 ,

as far as I know I've only adapted the already working DTOs to my needs. But I didn't add any endpoint.

Great news :) I was worried there would be overlap as a lot of the older outlets have bespoke endpoints, when I was looking through the PyVeSync codebase. Hence the changes in my branch were also the base so I could do some of the older devices with these bespoke paths.

@dag81
Copy link
Contributor

dag81 commented Aug 1, 2023

Sorry just had a quick look for the mistakes I've made in the past. I havn't looked through the code yet, but the new definitions for config items / channels - I think need adding to this file as well: bundles/org.openhab.binding.vesync/src/main/resources/OH-INF/i18n/vesync.properties

@marcelGoerentz
Copy link
Contributor Author

Hi @dag81 ,
as far as I know I've only adapted the already working DTOs to my needs. But I didn't add any endpoint.

Great news :) I was worried there would be overlap as a lot of the older outlets have bespoke endpoints, when I was looking through the PyVeSync codebase. Hence the changes in my branch were also the base so I could do some of the older devices with these bespoke paths.

Yes, I found those old outlets too, but since I could not test it, I didn't add them.

@marcelGoerentz
Copy link
Contributor Author

Sorry just had a quick look for the mistakes I've made in the past. I havn't looked through the code yet, but the new definitions for config items / channels - I think need adding to this file as well: bundles/org.openhab.binding.vesync/src/main/resources/OH-INF/i18n/vesync.properties

You are right, it will be necessary to generate a new version for this file.
So there will be a conflict when one of our PRs will be approved and merged.

@wborn wborn added enhancement An enhancement or new feature for an existing add-on work in progress A PR that is not yet ready to be merged labels Aug 1, 2023
@dag81
Copy link
Contributor

dag81 commented Aug 1, 2023

I had a read through it looks like a good clean job to me 👍 - just one key bit about removing the non WHO plug devices, because they won't work without some tuning using some of the new API's I've put in my latest PR. (Custom endpoints :( ). Also the other clash will be the readme file as well, which will need adding. I don't mind merging in your changes if it gets in first, I think the readme will be the fun one :( P.S Sorry I was going to download your jar just to check the new field in the base protocol layers, doesn't impact the purifiers API's. If it does it may need to be made a Integer instead of int I think it was so it can be set to null to exclude with the serialization update in my PR. Will download your jar now and test on a separate openhab install to hopefully confirm no changes needed here however 🤞

I just ran it against our Core400S and it was happy communicating with the changes, with several commands and polling. So no impact at all ! 👍

@marcelGoerentz
Copy link
Contributor Author

I just ran it against out Core400S and it was happy communicating with the changes, with several commands and polling. So no impact at all ! 👍

Great to hear that 👍

Thank you for your feedback 😊

@dag81
Copy link
Contributor

dag81 commented Aug 5, 2023

Hi @marcelGoerentz, Apologies for the delay I was trying to wrap up changes and testing for a different new binding, so I could open the PR. While doing it I was trying to think of a simple way to do this. I could make a new branch off the current one in my PR and pull in your changes. If you could provide the details of the API requests and responses, including postman curl output if pos (obviously remove token id and the other identifying bits :) ) details for the V1 commands I could update my simulator for that device, and cross check everything - prior to you checking on your end). I could then bundle your changes in with mine PR? The negative is you wouldn't get the credit with the PR, but I just through that may be the quickest way, just in case anything need's changes with the new API's in my PR (I think its possibly the cleanest way)? Or its wait until mine get's merged? What would be your pref, unless you have other ideas?

@marcelGoerentz
Copy link
Contributor Author

Hi @marcelGoerentz, Apologies for the delay I was trying to wrap up changes and testing for a different new binding, so I could open the PR. While doing it I was trying to think of a simple way to do this. I could make a new branch off the current one in my PR and pull in your changes. If you could provide the details of the API requests and responses, including postman curl output if pos (obviously remove token id and the other identifying bits :) ) details for the V1 commands I could update my simulator for that device, and cross check everything - prior to you checking on your end). I could then bundle your changes in with mine PR? The negative is you wouldn't get the credit with the PR, but I just through that may be the quickest way, just in case anything need's changes with the new API's in my PR (I think its possibly the cleanest way)? Or its wait until mine get's merged? What would be your pref, unless you have other ideas?

Hmmmm, I guess the cleanest way would be to wait until your PR got merged. Then I can add the logic and directly test it with my device. This will lead into a clean separation of your enhancement and mine. So as long as there is no reason to hurry, I can wait until your PR has been merged. 😄

@dag81
Copy link
Contributor

dag81 commented Aug 6, 2023

Makes sense @marcelGoerentz, 🤞 my merge doesn't too long to get looked at. Just eager to get your work added to it :) After its there I was going to setup my simulator for the other plugs covered by pyvesync to add the other models. As I know the first thing people will say what about XYZ model :)

@dag81
Copy link
Contributor

dag81 commented Aug 6, 2023

Hi @wborn could you label this up as depends on another PR please : #15296 (Sorry I saw that you labelled previously - hence the request :) )

ccutrer and others added 11 commits September 27, 2024 16:52
…ab#17400)

* [mqtt.homeassistant] Fix jinja usage in availability templates

Use the local Jinjava, instead of implicitly depending on the Jinja
transformation service.

Signed-off-by: Cody Cutrer <[email protected]>
* [emotiva] Fixes issue with missing data in source channels.

Signed-off-by: Espen Fossen <[email protected]>
* [pushbullet] Add link and file push type support

Signed-off-by: jsetton <[email protected]>
Fix spelling of occurred

Signed-off-by: Jacob Laursen <[email protected]>
* Make Markdown code block languages consistent

Signed-off-by: Jacob Laursen <[email protected]>

* Fix indentation

Signed-off-by: Jacob Laursen <[email protected]>

---------

Signed-off-by: Jacob Laursen <[email protected]>
* [linkTap] Initial Code Commit

[Signed-off-by: dag81 <[email protected]>
Regression of openhab/openhab-core#4389

Signed-off-by: Jacob Laursen <[email protected]>
holgerfriedrich and others added 8 commits December 2, 2024 21:38
Upgrade library which was not included in openhab#16588.

Signed-off-by: Holger Friedrich <[email protected]>
* [mqtt.homeassistant] implement Device Tracker

Signed-off-by: Cody Cutrer <[email protected]>
* [veSync] Device support enhancements

Device support enhancements
Signed-off-by: David Goodyear <[email protected]>
* myuplink skeleton

Signed-off-by: Alexander Friese <[email protected]>
* ☠️ Binding skeleton created for org.openhab.binding.huesync

Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
* Fix regionname
* Fix SAT
* Improve log line

Signed-off-by: Leo Siepel <[email protected]>
* fix step math so that the state description represents the step
   scaled to 0-100%

Signed-off-by: Cody Cutrer <[email protected]>
@lsiepel lsiepel removed the awaiting other PR Depends on another PR label Dec 3, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Dec 3, 2024

@marcelGoerentz the other PR is merged. Could you fix the conflicts and maybe @dag81 can review so we can get this in 4.3.0 expect feature freeze on sunday.

jimtng and others added 3 commits December 3, 2024 20:03
…, MIN, MAX) in State Filter (openhab#17362)

* [basicprofiles] Add support for functions (DELTA, MEDIAN, AVG, STDDEV, MIN, MAX) in State Filter

Support any type of operand on either side of the operator
e.g.: `ItemName > 10` and `10 < ItemName`

Signed-off-by: Jimmy Tanagra <[email protected]>
@dag81
Copy link
Contributor

dag81 commented Dec 3, 2024

@marcelGoerentz the other PR is merged. Could you fix the conflicts and maybe @dag81 can review so we can get this in 4.3.0 expect feature freeze on sunday.

@lsiepel sure - just setting up now to work through. I know I looked before age's ago - but a few bits changed since then such as unitHints. Will have a fresh look through shortly and put any suggestions in.

@dag81
Copy link
Contributor

dag81 commented Dec 4, 2024

Hi @marcelGoerentz

Just adding a few bits here as short on time, I'll try to add suggestions and more detailed feedback tonight. Adding this now in case your super quick :)

Overall really nice :) Just a few spots - mainly my fault from the last big update to the binding.

I quickly merged the changes into a clone from the main branch - yet to test to make sure the new subdevice field doesn't impact anything - I don't think it will.

The Handler will need the new i18n support added, so the constructor has two new parameters passed in and these are then passed in the super call. The errors can we add to the i18n please like now just added in the other two handlers for humidifiers and air filters, mainly just errors and user visible messages I did before.

Copyrights all need bumping 2023 -> 2024 (sorry).

Maybe good if the unitHiint's can be set, just to make sure all the units match how the app present's them, and don't default to potentially unexpected values such as %.

These two channels look like they are Kilowatt Hours (KwH - I think)? Should they possibly be a QuanityType with the units?

DEVICE_CHANNEL_ENERGY_WEEK
DEVICE_CHANNEL_ENERGY_MONTH

I couldn't quite figure out what the date calculation was doing. I suspect your getting milli's since the epoc at 0200, is this always in the local time zone / UTC do you know?

Other than the above nothing else jumped out yet. In case its of help - the merged version is here: https://github.com/dag81/openhab-addons/tree/reviews/vesync/outlets which I was going to build and spin up just to cross check no unexpected impact tomorrow, on at least the air filters I have here.

Hopefully the above makes sense, if you see this before I put the suggestions in. (Ironically I could never get them to commit without a DCO error, so always ended up manually adding them bit by bit). I give it another pass tomorrow on the big screen when everyone isn't asleep here :)

Adds the missing code owners and sorts the paths.

Signed-off-by: Wouter Born <[email protected]>
@marcelGoerentz
Copy link
Contributor Author

Hi @dag81,

thanks for your input I will check how fast I can fix this.
Someone was already asking for support of older devices. Unfortunately I don't have access to their payloads so I can add them by myself.

I can make this PR work in short term and would suggest to add the support for older outlets to then next SNAPSHOT build.

Signed-off-by: Marcel Goerentz <[email protected]>

# Conflicts:
#	bundles/org.openhab.binding.vesync/src/main/java/org/openhab/binding/vesync/internal/VeSyncHandlerFactory.java
#	bundles/org.openhab.binding.vesync/src/main/java/org/openhab/binding/vesync/internal/api/VeSyncV2ApiHelper.java
#	bundles/org.openhab.binding.vesync/src/main/java/org/openhab/binding/vesync/internal/discovery/VeSyncDiscoveryService.java
Signed-off-by: Marcel Goerentz <[email protected]>
…mart_plug

# Conflicts:
#	bundles/org.openhab.binding.vesync/src/main/java/org/openhab/binding/vesync/internal/VeSyncHandlerFactory.java
#	bundles/org.openhab.binding.vesync/src/main/java/org/openhab/binding/vesync/internal/api/VeSyncV2ApiHelper.java
#	bundles/org.openhab.binding.vesync/src/main/java/org/openhab/binding/vesync/internal/discovery/VeSyncDiscoveryService.java
@marcelGoerentz
Copy link
Contributor Author

Will recreate the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on work in progress A PR that is not yet ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.