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

Merge project with BenPru/Luxtronik #51

Open
AJediIAm opened this issue Dec 8, 2022 · 10 comments
Open

Merge project with BenPru/Luxtronik #51

AJediIAm opened this issue Dec 8, 2022 · 10 comments

Comments

@AJediIAm
Copy link

AJediIAm commented Dec 8, 2022

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.

@Bouni
Copy link
Owner

Bouni commented Dec 8, 2022

@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!

@AJediIAm
Copy link
Author

AJediIAm commented Dec 8, 2022

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.

@BenPru
Copy link
Contributor

BenPru commented Dec 8, 2022

@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:

  • The yaml configuration is compatible.

How to handle breaking changes:

  • 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
Copy link
Contributor

BenPru commented Dec 9, 2022

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.

@AJediIAm
Copy link
Author

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.

@Bouni
Copy link
Owner

Bouni commented Dec 14, 2022

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!

@kbabioch
Copy link

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?

@BenPru
Copy link
Contributor

BenPru commented Dec 14, 2022

a good GUI configurable integration
Is this even possible

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

@AJediIAm
Copy link
Author

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.

@Bouni
Copy link
Owner

Bouni commented Dec 19, 2022

@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).

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

4 participants