search

wijnandtop / home_assistant_gardena

Custom component to support Gardena smart devices
32 stars 7 forks source link

Next steps #18

Closed wijnandtop closed 4 years ago

wijnandtop commented 4 years ago

This plugin is a bit outdated, decent support for Gardena Smart within HASS would be great. This page is to discuss options.

Existing implementations:

Options:

  1. gradually upgrade Mine

    • [ ] Fix issues #17
    • [x] Publish using HACS #12
    • [ ] Verify implementation of irrigation controller
    • [ ] Use official API #9

  2. Pick up the work of @grm

Who has an opinion and want to help out.

wijnandtop commented 4 years ago

About me:

supernial commented 4 years ago

I believe there were comments that HASS implementation by GRM has a limitation of a token valid only for some time , and its not a long lived token. I didn't check this yet but if its a fact then you will bump into problem of refreshing it every call by HA. Your solution on the other hand I have done myself even before I found this thread. Just dumbly playing around with Postman. And I think I like it better . Also I have a Gardena Smart Tap irrigation so I could test the funcitonality, also I have a garden light/humidity sensor. I think the quick win would be to follow your path

Jpsy commented 4 years ago

Unfortunately the GRM integration has many open issues, pull requests and has not been updated for over 6 months. But in the community discussion of @wijnandtop's integration there is mention of a fork of GRM by @tobiaskuntzsch that seems to address these problems: https://gitlab.com/tobiaskuntzsch/home-assistant-gardena-integration I will test this alternative over the next week with my Gardena Sileno Smart mower and will report back. I am also very curious whether this integration is limited to Gardena gadgets or whether it will be compatible with Husqvarna mowers too. After all the new open API is Husqvarna's and they acquired Gardena in 2007. But unfortunately I can't test that on my end.

Jpsy commented 4 years ago

I have just installed the integration of @tobiaskuntzsch and the first impression is good. I could could start and stop the mower and could read the status. The details of the feedback from the new API differ somewhat from the reverse engineer API in the integration of @wijnandtop. Especially the states are different and what was state info in the old integration now seems to be fed into the "activity" attribute of the new integration. Here is a screenshot: image

I also witnessed a problem in the logs that is already described in this issue report of the new integration.

Unfortunately the old and new integration cannot be installed side-by-side as they collide in their namings ("gardena") and in the device they create (vacuum.name_of_your_sileno).

This also made it difficult to switch from one custom integration to the other. Simply replacing one by the other in the custom_components folder and the config did not work, as HA still evaluated the configuration options of the removed component and a restart is prevented because HA concludes a config error. So I had to remove both of them from custom_components and their configs from configuration.yaml, restart, move the new files and config in and restart a second time.

But all in all this seems to be a viable way. I currently cannot evaluate the new integration any further, as I would have to change all my mower automations (most of them in Node Red) and I need my mower to run every day. I will do further testing and adapting on one of the next weekends.

osks commented 4 years ago

I looked into the HASS integration that @grm did. One of the main issues is that it's pretending to be an async integration, but py-smart-gardena is not an async lib. It uses a thread for the websocket connectino. That is what's causing the "HA Detected I/O inside the event loop" problem. The fork by Tobias Kuntzsch has the same problem (https://gitlab.com/tobiaskuntzsch/home-assistant-gardena-integration/-/issues/1).

I've experimented a little with @grm's integration and modified it to not use the async methods in HASS and got it working.

I have Gardena's Water Control and Smart Sensor. Also have a Husqvarna Automower with Connect, but it's not represented in the GARDENA smart system API. I assume it's available in the Automower Connect API, but I haven't looked into it. (I got the mower listed in Gardena's smart app by connecting it to the Automower Connect API, which was nice.)

wijnandtop commented 4 years ago

If the sync implementation using the websockets works properly, this for sure would be the best implementation.

Do any of you experience the issue which @supernial mentions regarding the expiring tokens?

@osks where is your implementation located? I can give it a try, i can validate if it works with:

(only smart pressure pump is missing)

@osks , @Jpsy do you allready have a preference on which project we should focus?

stevennikolic commented 4 years ago

I've played around with this library too and it worked until i got the same problems with expiring tokens. The following small script works for the first time, as soon as it is restarted within 15 minutes it fails on update_locations with a status code of 429 not having the r['errors'] as expected in the library (gardena.smart_system.SmartSystem.__response_has_errors).

from gardena.smart_system import SmartSystem smart_system = SmartSystem(email=gardena_user, password=gardena_password, client_id=gardena_key) smart_system.authenticate() smart_system.update_locations() for location in smart_system.locations.values(): smart_system.update_devices(location) pprint.pprint({ 'id': location.id, 'name': location.name, })

Jpsy commented 4 years ago

Hi guys, to be honest, I would very much prefer the solution that uses the official API. No offense meant @wijnandtop - I am still very thankful for you solution! BTW: @tobiaskuntzsch claims that he has solved the token expiration problem in his GRM fork. I also hope that the official API will allow a websocket based push approach that will eliminate the 5 minute poll intervals of the current solution. I have developed some fancy automations that derive the gras growth rate and the wearing of the blades by analyzing the length of the cutting intervals. These automations would heavily benefit from exact mowing and loading times without any quantization effects through polling that happen every 5 minutes.

wijnandtop commented 4 years ago

@Jpsy non taken, my goal is to have one full implementation supported by multiple contributors. I do agree on having a solution based on the official API.

Will give the fork of @tobiaskuntzsch a try, if it works properly it is probably the best and most complete implementation.

osks commented 4 years ago

The code for my integration is here now: https://github.com/osks/hass-gardena-smart-system

Based on @grm's code, but I renamed the integration from "gardena" to "gardena_smart_system" to make it possible to compare with others.

I added config flow for configuration, so it is now setup under Integrations instead of configuration.yaml. It also exposes devices (in HASS terms) with battery level sensors. I've only done that for sensors and water control so far, since that is what I have, but it's a minor thing to add for the other Gardena devices as well.

I forked py-smart-gardena and renamed it to py-smart-gardena2 so I can publish relevant fixes on PyPI. So far I've only added "modelType", which all devices had in the API. https://github.com/osks/py-smart-gardena2

I will try to figure out what @tobiaskuntzsch did to fix the token refreshing and incorporate that as well.

osks commented 4 years ago

I looked into the HASS integration that @grm did. One of the main issues is that it's pretending to be an async integration, but py-smart-gardena is not an async lib.

I was kind of right, but also wrong. It's no problem to use Home Assistant's async setup functions, I just had to schedule the synchronous call on the event loop, which you do like this: await hass.async_add_executor_job(gardena_system.start)

Jpsy commented 4 years ago

The code for my integration is here now: https://github.com/osks/hass-gardena-smart-system

Hi @osks would love to test your integration but the repo is non-existant. Also nothing similar in the list of your Github repos. Did you forget to create it? ;D

osks commented 4 years ago

Sorry, I forgot to make it public! Should work now.

osks commented 4 years ago

I've updated it now. I don't think it was usable before because of an issue with the py-smart-gardena2 0.7.1 package on PyPI (see osks/py-smart-gardena#3). Hopefully it works now.

Jpsy commented 4 years ago

Great, thanks! Will test over the weekend and report back.

osks commented 4 years ago

BTW: @tobiaskuntzsch claims that he has solved the token expiration problem in his GRM fork.

Where have he claimed that? I couldn't find anything related to that in the code on https://gitlab.com/tobiaskuntzsch/home-assistant-gardena-integration

I have seen the problems once or twice, or what I think is that problem. But after fixing the error handling bug in py-smart-gardena and added more logging, I have never seen it again...

Jpsy commented 4 years ago

Where have he claimed that?

I referred to this posting but reading it again, I am not sure, I interpreted it correctly. But it still sounds as if his changes removed the problem at least as a side effect. Not sure though.

Today I tried to install your integration but unfortunately it is in fact not possible to run it side-by-side with the integration of @wijnandtop. Although they have different integration names, they both create an entity with id vacuum.[your mower name]. [your mower name] is obviously drawn from the data received from Gardena/Husqvarna.

So I will try to deinstall Wijnand's integration over the weekand and replace it with yours. But I have many dependencies in my automations. I hope that I do not destabilize my whole system.

Jpsy commented 4 years ago

I tried again, uninstalling Wijnand's integration, installing Oskars. Then I found that I could rename the device that Oskars integration created and with it all its entities. After that I could reinstall Wijnand's integration without collisions. So I can keep on experimenting without killing my existing automations.

First test result: For me Oskars integration creates only one entity, which is the battery entity.

Integrations: image Devices: image Device infos: image

This has nothing to do with the renaming of the entity and reactivating Wijnand's integration. It was also like that when I had uninstalled Wijnand's and just freshly added Oskar's. When I had Tobias' integration installed some days ago, I could see a mower entity which exposed the necessary mower data in its attributes. Oskar, any ideas, what is happening here?

Jpsy commented 4 years ago

I found the problem (DOMAIN missing in from .const import() of vacuum.py. But I also found that you updated your integration again today. I will retest tomorrow with the newest version.

osks commented 4 years ago

Thanks for testing! Yeah, I did some updates earlier today. Hopefully fixing that and other mistakes. Need to figure out how to test it better without buying another mower...

Jpsy commented 4 years ago

@osks looks promising! I have both integrations running side-by-side now. The mower state of Wijnand's and the activity attribute of yours are in sync. I am very curious whether yours will show changes of the activity earlier than the 5 minute poll cycle of the old integration. @wijnandtop this is becoming more like a testing session for Oskar's integration now. Not sure if we should switch to a new issue on Oskar's repo or a community thread for that stuff. Suggestions welcome.

osks commented 4 years ago

It was not my intention to hijack this thread either. I created an issue where we can continue with specifics about my integration: osks/hass-gardena-smart-system#1

Jpsy commented 4 years ago

Just a quick sum up before I continue discussion over at Oskar's repo:

northpower25 commented 4 years ago

I‘ve too switch over to -> https://github.com/py-smart-gardena/hass-gardena-smart-system

Thanks for your work @wijnandtop but i think the Integration of @osks is now closer to the target to become an official HA integration status.

grm commented 4 years ago

Hey everyone,

Sorry if I hijack your thread @wijnandtop

Indeed I had to stop working on HASS integration and the library a few month ago due to a new job :)

As the python-smart-gardena library is using the official API, py-smart-gardena seems a better solution in the long term :) Changes will be documented and announced and we will be able to have some time before deprecation. This won't be the case with an unofficial API.

@supernial : py-smart-gardena is using a refresh token, in order to renew the access token, and the hass component is using websockets to get updates. So py-smart-gardena is using the gardena API the way it is meant by the devs :)

@jpsy : py-smart-gardena is using websockets in the hass integration. Modifications are available as soon as gardena pushes us the information.

This week, we decided with @osks to join our effort on the implementation of both the library and the hass component and thus we created an organisation which can be found here : https://github.com/py-smart-gardena/

For now, we will have port the changes from @osks to the python library in the next few days and the hass integration, so you are really welcome to give your feedback there if you are using our libraries.

We still have to do some small evolutions to make the migration operational, but you can already post issues there, as it will be our main repositories.

All help of any kind will be really appreciated.

Grm,

grm commented 4 years ago

@wijnandtop : as you also have some knowledge about gardena devices and code, maybe do you want to join us in the organisation ?

wijnandtop commented 4 years ago

yes please :-)