Restructure and merge circuit into zone if possible

[ml1nk]

I reworked the inheritance structure and created separate files by device. The only real change are some lines which merge the circuit into the zone if possible which was eased by this transition, a migration with fixed an inconsistency in the unique_id creation and refactoring the deprecated units.

As our work is conflicting and i probably need to redo it on the current codebase (just faster this time), i would ask if you approve this restructure?

[signalkraft]

I like the change!

I'm not a fan of the I in IBaseSystem and the other base classes. It's not a convention I've seen in Home Assistant. IMO BaseEntity and BaseSystemCoordinatorEntity would be more consistent with the naming in HA and give a good understanding of what the base class does.

Things like name_prefix could also get a default implementation in BaseSystemCoordinatorEntity, instead of being left to the entities:

  @property
  def id_infix(self) -> str:
      return f"{self.system.id}_home"

  @property
  def name_prefix(self) -> str:
      return f"{self.system.home.home_name or self.system.home.nomenclature}"

...which you could overwrite, or call from other base classes like:

class VentilationClimate(IBaseSystem, ClimateEntity):
    @property
    def name_prefix(self) -> str:
        vname = [d for d in self.system.devices if d.type == "ventilation"][
            0
        ].name_display
        return f"{super().name_prefix} Ventilation {vname}"

But again, I like the change and that's a minor gripe. Thanks for the PR, I'll stop changing main in the meantime.

[ml1nk]

Thanks for your comments, please fix the naming if necessary.

The merge looks like this:

State (from Zone) vs Heating State (from Circuit) is a little bit confusing but otherwise it looks cleaner. As State is always Idle (at least i never saw it otherwise) it does not seem relevant in this kind of setup. What is your opinion?

Fixes: #72 #67

[ml1nk]

There are some more typing errors occurring (visible through #74) not related to this merge:

[signalkraft]

I've pushed a new commit with slightly changed names, fixed the typing issue with the holiday entities, renamed the DailyDataCoordinator to DeviceDataCoordinator, and tested with my setup. Only issue I'm seeing:

ERROR (MainThread) [homeassistant.components.sensor] Platform mypyllant does not generate unique IDs. ID mypyllant_<system_id>_device_<device_id>_heating_energy_efficiency already exists - ignoring sensor.home_device_0_arotherm_plus_heating_energy_efficiency

I don't think that was caused by my changes, do you have any idea why I'm seeing that?

[ml1nk]

Yeah, I failed separating the EfficiencySensor. Should be fixed with the last commit. Has the migration worked for you, so that no duplicate SystemEfficiencySensor was created? If the answer is yes and you didn't find more problems, it should be ready to be merged.

[signalkraft]

The unique ID issue is fixed, and I'm not seeing a duplicate SystemEfficiencySensor. I do however still have the old circuit device and one unavailable "cooling allowed" entity on it. Is there a way to delete those in the migration? I suppose something like https://github.com/LaggAt/home-assistant-core/blob/f7f07214d8e3da824da75d327cfb88093fb2fcfc/homeassistant/components/deconz/services.py#L152 could work?

[ml1nk]

Interesting, at least the cooling device shouldn't be there. I will look at this, thanks.

[ml1nk]

I'm sorry, but I couldn't get to it this week. I try to get it ready until the end of the next week.

[ml1nk]

• change unique id of cooling allowed in migration to match schema
• script to remove all devices without entries (orphaned)

Please use a new test instance if you want to test the migration or force the migration to rerun. The version of your entries is already 2 after testing last time. Hopefully it is now ready to merge. Thanks for waiting.

[ml1nk]

No idea why the entity registry isn't available when testing services, but ignoring the exception should be unproblematic.

[signalkraft]

I just got to testing, it looked like this when going from main branch to your PR:

I found the log message Migration to version 2 successful, so it must have run.
I'm guessing that the two entities with circuit_0_... should have been migrated?

[ml1nk]

I couldn't reproduce your problem, but found another one: Some devices are missing device as part of the unqiue_id between system_id and device_uuid. I added this in the migration and retested it and couldn't find any misbehavior anymore. In addition I merged the latest changes.

The cooling_allowed migration which replaces " " with "_" in the unique_id seems to work fine and flow_temperate never get changed by the migration nor changes somewhere else. Can you please test it again and look up if there are new entities created and provide the log?

[signalkraft]

Vaillant migrated a bunch of users with VRC700 controllers over, so in my free time I'm dealing with that for now. I'll check back on this soon!