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

Merging with ha-smartthinq-sensor wideq implementation #99

Open
ollo69 opened this issue May 28, 2020 · 5 comments
Open

Merging with ha-smartthinq-sensor wideq implementation #99

ollo69 opened this issue May 28, 2020 · 5 comments

Comments

@ollo69
Copy link

ollo69 commented May 28, 2020

Hi,

think time is come to try to merge my project with your original work.
I start opening an issue and not a PR because I reorganize the project structure in many point and so create a PR is not so simple.
I start trying to recap here main changes of my implementation:

  • removed modue "client" because my implementation can use both old and new API and the client class is API related, so I moved definition inside the the core modules. I defined class "Client" inside core.py that implement APIv1 and class "ClientV2" inside core_v2.py that implement APIv2.
  • created a new module Device.py that containt the generic definition of Device and DeviceStatus and 2 class used to convert the model information for both old and new format. The Device class identify the correct Device type based on model_info initially load and perform the necessary conversion to properly load the device state.
  • for specific device type (at this moment Washer, Dryer, DishWasher and Refrigerator) I separate the states from the class itself also to make easier adding missing states.

Ready here for any suggestion on the best way to proceed or to provide additional detail of my changes.

@sampsyo
Copy link
Owner

sampsyo commented May 28, 2020

Sounds great; I would love to help with this!

Here are a few questions based on your summary of the reorganization:

  • Seems like the first target should be moving in the "new API" support. I still like having the core/client split—is there a specific reason to collapse those together? Can we start with just a v2 version of the core protocol, then we can build the higher-level client API on top of that?
  • A separate device.py file sounds fine at first blush, but maybe it will be helpful to see the code to understand why it should be separate from the client module(s).
  • Similarly, I'd be curious to see what you mean by "separate the states from the class itself."

Anyway, shall we start with the "v2" core and client stuff? IMO we can begin by merging the core stuff (i.e., the pure "functions" for communicating with the API). Then we can add the Client and Device classes on top of that. Does that seem feasible?

@ollo69
Copy link
Author

ollo69 commented May 28, 2020

Well,

I merged core and client because client class, as is implemented, cannot be independent by the version. Honestly I leaved both 2 module (core and core_v2) just because I want to preserve orginal API, but finally I discovered that the V1 version is not used anymore because core_v2 can manage both thinq1 and thinq2 devices.

I think that is complex to explain without viewing the code, maybe I can create a PR with all changes just with the objective to discuss directly on the code itself and let you take a look on that, than we can choose to discard PR and create a new one.

Let me know what do you think.

@sampsyo
Copy link
Owner

sampsyo commented May 28, 2020

OK cool, let’s abandon v1 then and just implement v2.

And sounds good; a PR might help me understand why the two (core and client) need to be coupled in v2.

@ollo69
Copy link
Author

ollo69 commented May 28, 2020

Ok, created PR #100, let's continue there....

@stefxx
Copy link

stefxx commented Jun 14, 2020

Great work in progress!

Do you already know how to control basic features of a v2 device? I guess it needs the thinq2Uri from the gateway-uri response... but do we have a json example of what the LG API expects?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants