search

dominikamann / oekofen-pellematic-compact

A Ökofen Pellematic Compact Integration based on JSON/TCP-Inteface for Home Assistant.
Apache License 2.0
32 stars 7 forks source link

Sensors are not prefixed with the entered prefix during setup #21

Closed cnico closed 1 year ago

cnico commented 1 year ago

Hi @dominikamann,

Thanks again for your work on this custom integration.

I have another point of improvement for the integration : during setup, the user can choose a prefix for the generated sensors, by default pellematic. It is a good thing to have such a prefix. But once the setup is finished, the effectively created sensors have no prefix in their key. Here are some key I have for example : sensor.outside_temperature, sensor.heating_circuit_2_name, etc It would be better to have : sensor.pellematic_outside_temperature, sensor.pellematic_heating_circuit_2_name where "pellematic" is the entered prefix during the configuration.

I tried to find how to code this but I am not really at ease with this so I prefer to send you the issue.

If you could correct this, it would be great.

Thanks,

dominikamann commented 1 year ago

Hi @cnico, I will see if we can add this as an additional option when adding a heater. Because if I implement this we will have a breaking change. All current users will have to update all their dashboards, automations, etc.

cnico commented 1 year ago

@hi @dominikamann, People not doing the reconfig from scratch should not be impacted by such a change. A solution is that they could simply empty the field proposing the prefix at setup in order to be in the same configuration as today. Another idea : if the default prefix is empty, it should have the same behaviour as now.

dominikamann commented 1 year ago

@cnico yes that's the idea 👍 I will let you know once it is implemented. Thank you!

MrRagga- commented 1 year ago

I am interested in the prefix, too. Have you already started? I am also able to contribute.

dominikamann commented 1 year ago

Hi @MrRagga- no unfortunately I had no time yet 😩. If you want to contribute feel free to do it👍💪 It should be configurable as a new option (default "false"). To prevent migration issues. Thank you.

cnico commented 1 year ago

Hi @dominikamann, I finally found time to implement a solution to this problem. Please have a look to the pull request in order to merge it. Thanks,

dominikamann commented 1 year ago

Thank you @cnico !