luxtronik icon indicating copy to clipboard operation
luxtronik copied to clipboard

Merge project with BenPru/Luxtronik

Open AJediIAm opened this issue 2 years ago • 10 comments

https://github.com/BenPru/luxtronik is making a lot of good progress and seems to have surpassed this integration. Please consider merging the two projects so development effort be pulled together and users do not need to choose which one to use.

AJediIAm avatar Dec 08 '22 10:12 AJediIAm

@BenPru For me this could absolutely happen! the main issue is that you've for some reason not forked my project but uploaded it as a "new" one to GitHub. I've no idea how to get that to work out properly 🤷🏽

I lack the time to develop this further at the moment but your repo seems to have some traction.

Let me know what you think!

Bouni avatar Dec 08 '22 12:12 Bouni

Hi Bouni, I'm just a user who likes to make a small contribution once in a while. I was hoping that if @BenPru agrees, you could become a maintainer of the BenPru Luxtronik project, remove this one from HACS and update the community forum to refer to the BenPru project.

I hope that by joining forces and giving this a bit of a push from the community, your hard work can become part of HA.

AJediIAm avatar Dec 08 '22 13:12 AJediIAm

@AJediIAm Thanks for your ticket, great idea. I totally agree. @Bouni I can fork your project and transfer my code into this fork. But we have some things to plan:

  • [X] The yaml configuration is compatible.

How to handle breaking changes:

  • [x] Sensor names: Bounis project creates the sensor names in the legacy ha style luxtronik.%name%. (Correct me if I'm wrong) My project creates the names in the newer ha style sensor.luxtronik2_%name%. If we merge the projects we need to inform the Bouni/luxtronik user or implement a legacy mode. I've tried that so far without success.

Stability, codebase and feature completeness

  • [ ] My codebase is not so clean and stable like bounis. #35, #39, #48
  • [ ] The main feature climate.heating is not finalized. #3

How do you think about this?

BenPru avatar Dec 08 '22 15:12 BenPru

I have tested the current version from bouni and I have seen that the sensor names uses a new scheme. E.g. "sensor.temperature_forerun". I think we can add a "legacy sensor name mode" for sensors created by the yaml configuration.

BenPru avatar Dec 09 '22 23:12 BenPru

Just a personal thought: Given the extend of the changes, this might be the right time to make a breaking change and switch to the new sensor names. If users are happy with the current setup, they do not need to upgrade. If users want keep up to date, share code, etc, using the new naming convention will benefit them in the long run.

AJediIAm avatar Dec 10 '22 08:12 AJediIAm

Hey, sorry for the long silence 😅

I totally agree with @AJediIAm we should defenitely go the "new sensor names" path as this allows for using the sensors in the energy dashboard for example.

I also would not take the effort of a legacy mode. If someone whants to keep their sensor they still can stay on their version.

My questions:

Should we keep a yaml config mode? I think a good GUI configurable integration is, especially given the thousends of parameter/calculations/visibilities superior to the yaml mode. Is this even possible, I've not yet looked really into the config side of HA integrations ... Selecting the entities one wants to have with a long list of checkboxes would be awesome!

Bouni avatar Dec 14 '22 14:12 Bouni

At least I'm still in favor of YAML editing and kind of dis-like all of the GUI only integrations. So, if possible, I would definitely prefer to keep YAML editing around :-).

Also I don't see a reason why a dynamic Config Flow only mode should be established, all of the data points are well known and don't change. Also authentication is not an issue like with some other integrations, or am I missing something?

kbabioch avatar Dec 14 '22 14:12 kbabioch

a good GUI configurable integration Is this even possible

Yes, I think so. The Android TV Integration has something like this.

BenPru avatar Dec 14 '22 23:12 BenPru

There are many integrations which have the more complex sensors disabled by default. This is a very user friendly solution in my opinion. It shows you the most relevant stuff which is what 80% of the users need and it does not cluter your interface. More advanced users can still add the sensors by simply enabling them.

AJediIAm avatar Dec 16 '22 22:12 AJediIAm

@AJediIAm This is an approach you can only follow if you know what your sensors mean, in our case you have so man unknown sensors that might be important for some heatpump models that I don't think this is feasable for us (unfortunately).

Bouni avatar Dec 19 '22 06:12 Bouni