hultenvp / home_assistant_omnik_solar

Home Assistant Omnik Solar sensor component
MIT License
6 stars 3 forks source link

Migrate to config flow #2

Open hultenvp opened 2 years ago

hultenvp commented 2 years ago

Introduce config flow

MarijnS95 commented 2 years ago

Hi, thanks for picking up maintenance of this plugin!

I've also been hit by some of the issues you seem to have fixed here, thanks! But also noticed the deprecation by Hein, and already planned to port TCP support from this plugin to https://github.com/robbinjanssen/home-assistant-omnik-inverter - this HASS plugin from Robbin already has config flow support :partying_face:

The first step is already in progress, bringing it to the reusable python-omnikinverter https://github.com/klaasnicolaas/python-omnikinverter/pull/134 package.

When TCP support is done, perhaps you're interested in migrating to and aiding in maintenance of https://github.com/robbinjanssen/home-assistant-omnik-inverter and related Python packages? That way there won't be two more-or-less-identical plugins around, which has confused me as beginning HASS user.

hultenvp commented 2 years ago

Hi @MarijnS95

Thanks for the suggestion. Always happy to join forces and to learn from others. Relatively new to python myself, got a C/C++ background.

Let me know how you'd like to proceed and let's pick up from there.

Cheers

MarijnS95 commented 2 years ago

@hultenvp For now I'll keep working on the TCP backend and keep you / this project in the loop, you can also follow the above PR to track progress.

Then, as time goes on and TCP source support lands in python-omnikinverter and https://github.com/robbinjanssen/home-assistant-omnik-inverter it is probably up to everyone to test the new functionality and make the switch, after which it is - however weird it feels to say this - probably recommended to also archive this approach and recommend everyone to use the unified plugin. That implies the unified plugin works for everyone, which may take some time. You're of course always free to do however you wish with this plugin :smile:

MarijnS95 commented 2 years ago

Hi @hultenvp, apologies for not checking in yet. The TCP backend feature has finally landed in https://github.com/klaasnicolaas/python-omnikinverter/pull/134 and https://github.com/robbinjanssen/home-assistant-omnik-inverter/pull/110 so I can recommend you to switch to that integration instead of implementing/maintaining it yourself :tada:

Contributions are always welcome, I have some more improvements planned for it too!

MarijnS95 commented 2 years ago

@heinoldenhuis hijacking this thread to make you aware too; how do you feel about pointing to https://github.com/robbinjanssen/home-assistant-omnik-inverter as a viable alternative for your plugin?

rob-on-git commented 2 years ago

Would prefer if this extension still lives, because this version supports my Trannergy with TCP stream. The version of robbinjanssen not yet, however the implementation looks quit clean within the current releases of HA.

MarijnS95 commented 2 years ago

The version of robbinjanssen not yet

@rob-on-git What exactly do you need? I implemented the TCP stream in that plugin and it has has also been released.

If there's anything that plugin needs to do to be compatible with your Trannergy let me know! Even if it's just adding it to the list of supported models - because I can't test this for you :wink:

hultenvp commented 2 years ago

Hi both.

I agreed with Marijn to sunset this integration. Haven't done it yet, I'm a winter coder ;-). My proposal is that you check Marijn's integration. If it does not work then I'm fine keeping this one alive till support could be added. It will be here anyway till the weather turns sour again.

Cheers

rob-on-git commented 2 years ago

If there's anything that plugin needs to do to be compatible with your Trannergy let me know

I have to verify what might be the issue, but when configuring within the GUI, i cannot connect to the TCP stream (while i do see traffic)

It's a Trannergy model SGN4000TL

PS: I'll start in new issue in the other repo... because its a bit off-topic here :)