flz / iaqualink-py

Asynchronous library for Jandy iAqualink
BSD 3-Clause "New" or "Revised" License
30 stars 20 forks source link

Add support for eXO systems #21

Open flz opened 2 years ago

flz commented 2 years ago

Let's start with this, I need to look at feedback received recently.

codecov[bot] commented 2 years ago

Codecov Report

Merging #21 (533af72) into master (7a68192) will decrease coverage by 0.01%. The diff coverage is 97.07%.

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
- Coverage   97.10%   97.09%   -0.01%     
==========================================
  Files          19       24       +5     
  Lines        1518     1995     +477     
==========================================
+ Hits         1474     1937     +463     
- Misses         44       58      +14     
Flag Coverage Δ
full-suite 97.09% <97.07%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
tests/test_client.py 98.80% <ø> (ø)
src/iaqualink/systems/exo/system.py 88.75% <88.75%> (ø)
tests/systems/exo/test_device.py 98.36% <98.36%> (ø)
tests/systems/exo/test_system.py 98.75% <98.75%> (ø)
src/iaqualink/systems/exo/device.py 99.21% <99.21%> (ø)
src/iaqualink/client.py 100.00% <100.00%> (ø)
src/iaqualink/systems/exo/__init__.py 100.00% <100.00%> (ø)
TheKamakaZi commented 1 year ago

Hey @flz,

Been using the eXO branch of your integration for the past 2 months without issue. Would it be possible to merge this to master so that it can be included in HA's core?

Cheers, and thanks for all your hard work!

malatg commented 1 year ago

Is there any plans you incoporate this into the mail branch?

flz commented 1 year ago

Somehow I missed the comments here.

I'll rebase on top of the main branch. If things still work, I'll release a new version to make this available through HA.

malatg commented 1 year ago

Awesome! I hope it works 😊

flz commented 1 year ago

Awesome! I hope it works 😊

You let me know, I don't have exo systems :-)

malatg commented 1 year ago

Hi,

I am sorry if I have misunderstood something but when will this be included in the main branch?

/Marcus

flz commented 1 year ago

@malatg I meant I can't test the exo support since I don't have that hardware. If you can test the exo branch and let me know it works, I'll merge it to master.

malatg commented 1 year ago

I can definitely test it if you can point me in the direction on how I test the branch? I mean how do I switch the to this branch for that particular integration?

TheKamakaZi commented 1 year ago

Hey @flz,

Thanks for this! I'll deploy tonight, run some tests in the morning, and let you know.

@malatg, simply switch to the exo branch after cloning, and restart HA.

danricho commented 10 months ago

My only issue with this is that my pump data={'name': 'filter_pump', 'type': 1, 'state': 0}, will not change state by the .turn_on() method even if I force it to an ExoAuxSwitch or ExoAttributeSwitch.

As the current implementation stands, it is defaulting to an ExoAttributeSensor.

flz commented 7 months ago

@danricho Can you provide me with some more debug logging (the complete API output of systems/devices would be useful).

Veuchez commented 6 months ago

Excuse my ignorance, but where should these files be copied into home assistant?

mk-french commented 6 months ago

Hey @flz , is there a particular branch of your HA core fork to use with this branch?

I've been using my forks iaqualink-py/test-wo-shadow-client and core/iaqualink-eXO without any real issues for a while now but happy to help test this and provide some feedback to get it merged - then migrate over.

There's a few errors when using this branch and your core/iaqualink-http2 component. At initial glance they're related to type conversions etc. I haven't yet sifted through them in detail to debug but wanted to make sure I was starting in the right place first... :)

-Martin

flz commented 4 months ago

Hey @flz , is there a particular branch of your HA core fork to use with this branch?

I don't think there have been API changes in the iaqualink-py libraries for exo support but I'd need to double check. It's been a while since I looked at the code.

I've been using my forks iaqualink-py/test-wo-shadow-client and core/iaqualink-eXO without any real issues for a while now but happy to help test this and provide some feedback to get it merged - then migrate over.

There's a few errors when using this branch and your core/iaqualink-http2 component. At initial glance they're related to type conversions etc. I haven't yet sifted through them in detail to debug but wanted to make sure I was starting in the right place first... :)

I'll refresh the core/exo branch and make sure iaqua support works well. I'm at the point where I'm tempted to ship exo support as experimental in the library, just to make it easier for people to test.

flz commented 4 months ago

I've updated the core/exo branch.

The two ways to test I can think of:

Separate test home-assistant instance

Modify your existing HA installation