-
Notifications
You must be signed in to change notification settings - Fork 8
Further code optimizations #838
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
2a7f0e6
Add THERMO_MATCHING constant
bouwew 762d16f
Rename to _het_appliances(), _get_locations(), optimize _get_applianc…
bouwew c9702bf
Use THERMO_MATCHING constant
bouwew 8ad9481
Rename _all_ to _get_
bouwew 6c71928
Rename to _get_p1_smartmeter_info()
bouwew 5311198
Rework _scan_thermostat() and related
bouwew 758b418
Type added constant with Final as suggested
bouwew 034d948
Update CHANGELOG
bouwew 33e1e4d
Break out extend_plug_device_class() function
bouwew 6a41486
Improve _match_and_rank_thermostats() as suggested
bouwew 3400aad
Optimize as suggested
bouwew 8a4202b
Improve detection and error-handling for home-location
bouwew 0c4271c
Adapt for legacy location too
bouwew 700073e
Clean up
bouwew 86a33b5
Use negative as suggested
bouwew e4efbbe
Revert back to short solution
bouwew File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| OFF, | ||
| P1_MEASUREMENTS, | ||
| TEMP_CELSIUS, | ||
| THERMO_MATCHING, | ||
| THERMOSTAT_CLASSES, | ||
| TOGGLES, | ||
| UOM, | ||
|
|
@@ -93,65 +94,64 @@ def item_count(self) -> int: | |
| """Return the item-count.""" | ||
| return self._count | ||
|
|
||
| def _all_appliances(self) -> None: | ||
| def _get_appliances(self) -> None: | ||
| """Collect all appliances with relevant info. | ||
|
|
||
| Also, collect the P1 smartmeter info from a location | ||
| as this one is not available as an appliance. | ||
| """ | ||
| self._count = 0 | ||
| self._all_locations() | ||
| self._get_locations() | ||
|
|
||
| for appliance in self._domain_objects.findall("./appliance"): | ||
| appl = Munch() | ||
| appl.available = None | ||
| appl.entity_id = appliance.attrib["id"] | ||
| appl.location = None | ||
| appl.name = appliance.find("name").text | ||
| appl.model = None | ||
| appl.model_id = None | ||
| appl.module_id = None | ||
| appl.firmware = None | ||
| appl.hardware = None | ||
| appl.mac = None | ||
| appl.pwclass = appliance.find("type").text | ||
| # Don't collect data for the OpenThermGateway appliance | ||
| if appl.pwclass == "open_therm_gateway": | ||
| continue | ||
|
|
||
| # Extend device_class name of Plugs (Plugwise and Aqara) - Pw-Beta Issue #739 | ||
| description = appliance.find("description").text | ||
| if description is not None and ( | ||
| "ZigBee protocol" in description or "smart plug" in description | ||
| ): | ||
| appl.pwclass = f"{appl.pwclass}_plug" | ||
| appl.zigbee_mac = None | ||
| appl.vendor_name = None | ||
|
|
||
| # Skip thermostats that have this key, should be an orphaned device (Core #81712) | ||
| if ( | ||
| # Don't collect data for the OpenThermGateway appliance, skip thermostat(s) | ||
| # without actuator_functionalities, should be an orphaned device(s) (Core #81712) | ||
| if appl.pwclass == "open_therm_gateway" or ( | ||
| appl.pwclass == "thermostat" | ||
| and appliance.find("actuator_functionalities/") is None | ||
| ): | ||
| continue | ||
|
|
||
| appl.location = None | ||
| if (appl_loc := appliance.find("location")) is not None: | ||
| appl.location = appl_loc.attrib["id"] | ||
| # Don't assign the _home_loc_id to thermostat-devices without a location, | ||
| # they are not active | ||
| # Set location to the _home_loc_id when the appliance-location is not found, | ||
| # except for thermostat-devices without a location, they are not active | ||
| elif appl.pwclass not in THERMOSTAT_CLASSES: | ||
| appl.location = self._home_loc_id | ||
|
|
||
| # Don't show orphaned thermostat-types | ||
| if appl.pwclass in THERMOSTAT_CLASSES and appl.location is None: | ||
| continue | ||
|
|
||
| appl.available = None | ||
| appl.entity_id = appliance.attrib["id"] | ||
| appl.name = appliance.find("name").text | ||
| appl.model = None | ||
| appl.model_id = None | ||
| appl.firmware = None | ||
| appl.hardware = None | ||
| appl.mac = None | ||
| appl.zigbee_mac = None | ||
| appl.vendor_name = None | ||
| # Extend device_class name of Plugs (Plugwise and Aqara) - Pw-Beta Issue #739 | ||
| description = appliance.find("description").text | ||
| if description is not None and ( | ||
| "ZigBee protocol" in description or "smart plug" in description | ||
| ): | ||
| appl.pwclass = f"{appl.pwclass}_plug" | ||
|
|
||
| # Collect appliance info, skip orphaned/removed devices | ||
| if not (appl := self._appliance_info_finder(appl, appliance)): | ||
| continue | ||
|
|
||
| self._create_gw_entities(appl) | ||
|
|
||
| # A smartmeter is not present as an appliance, add it specifically | ||
| if self.smile.type == "power" or self.smile.anna_p1: | ||
| self._get_p1_smartmeter_info() | ||
|
|
||
|
|
@@ -194,21 +194,24 @@ def _get_p1_smartmeter_info(self) -> None: | |
|
|
||
| self._create_gw_entities(appl) | ||
|
|
||
| def _all_locations(self) -> None: | ||
| def _get_locations(self) -> None: | ||
| """Collect all locations.""" | ||
| loc = Munch() | ||
| locations = self._domain_objects.findall("./location") | ||
| for location in locations: | ||
| loc.name = location.find("name").text | ||
| loc.loc_id = location.attrib["id"] | ||
| self._loc_data[loc.loc_id] = {"name": loc.name} | ||
| if loc.name != "Home": | ||
| continue | ||
|
|
||
| self._home_loc_id = loc.loc_id | ||
| self._home_location = self._domain_objects.find( | ||
| f"./location[@id='{loc.loc_id}']" | ||
| ) | ||
| loc.name = location.find("name").text | ||
| self._loc_data[loc.loc_id] = { | ||
| "name": loc.name, | ||
| "primary": [], | ||
| "primary_prio": 0, | ||
| "secondary": [], | ||
| } | ||
| if loc.name == "Home": | ||
| self._home_loc_id = loc.loc_id | ||
| self._home_location = self._domain_objects.find( | ||
| f"./location[@id='{loc.loc_id}']" | ||
| ) | ||
|
|
||
| def _appliance_info_finder(self, appl: Munch, appliance: etree.Element) -> Munch: | ||
| """Collect info for all appliances found.""" | ||
|
|
@@ -739,84 +742,72 @@ def _cleanup_data(self, data: GwEntityData) -> None: | |
| def _scan_thermostats(self) -> None: | ||
| """Helper-function for smile.py: get_all_entities(). | ||
|
|
||
| Update locations with thermostat ranking results and use | ||
| Adam only: update locations with thermostat ranking results and use | ||
| the result to update the device_class of secondary thermostats. | ||
| """ | ||
| self._thermo_locs = self._match_locations() | ||
|
|
||
| thermo_matching: dict[str, int] = { | ||
| "thermostat": 2, | ||
| "zone_thermometer": 2, | ||
| "zone_thermostat": 2, | ||
| "thermostatic_radiator_valve": 1, | ||
| } | ||
|
|
||
| for loc_id in self._thermo_locs: | ||
| for entity_id, entity in self.gw_entities.items(): | ||
| self._rank_thermostat(thermo_matching, loc_id, entity_id, entity) | ||
| if not self.check_name(ADAM): | ||
| return | ||
|
|
||
| for loc_id, loc_data in self._thermo_locs.items(): | ||
| if loc_data["primary_prio"] != 0: | ||
| self._zones[loc_id] = { | ||
| self._match_and_rank_thermostats() | ||
| for location_id, location in self._loc_data.items(): | ||
| if location["primary_prio"] != 0: | ||
| self._zones[location_id] = { | ||
| "dev_class": "climate", | ||
| "model": "ThermoZone", | ||
| "name": loc_data["name"], | ||
| "name": location["name"], | ||
| "thermostats": { | ||
| "primary": loc_data["primary"], | ||
| "secondary": loc_data["secondary"], | ||
| "primary": location["primary"], | ||
| "secondary": location["secondary"], | ||
| }, | ||
| "vendor": "Plugwise", | ||
| } | ||
| self._count += 5 | ||
|
|
||
| def _match_locations(self) -> dict[str, ThermoLoc]: | ||
| def _match_and_rank_thermostats(self) -> None: | ||
| """Helper-function for _scan_thermostats(). | ||
|
|
||
| Match appliances with locations. | ||
| Match thermostat-appliances with locations, rank them for locations with multiple thermostats. | ||
| """ | ||
| matched_locations: dict[str, ThermoLoc] = {} | ||
| for location_id, location_details in self._loc_data.items(): | ||
| for appliance_details in self.gw_entities.values(): | ||
| if appliance_details["location"] == location_id: | ||
| location_details.update( | ||
| {"primary": [], "primary_prio": 0, "secondary": []} | ||
| ) | ||
| matched_locations[location_id] = location_details | ||
|
|
||
| return matched_locations | ||
| for location_id, location in self._loc_data.items(): | ||
| for entity_id, entity in self.gw_entities.items(): | ||
| self._rank_thermostat( | ||
| entity_id, entity, location_id, location, THERMO_MATCHING | ||
| ) | ||
|
|
||
| def _rank_thermostat( | ||
| self, | ||
| entity_id: str, | ||
| entity: GwEntityData, | ||
| location_id: str, | ||
| location: ThermoLoc, | ||
| thermo_matching: dict[str, int], | ||
| loc_id: str, | ||
| appliance_id: str, | ||
| appliance_details: GwEntityData, | ||
| ) -> None: | ||
| """Helper-function for _scan_thermostats(). | ||
|
|
||
| Rank the thermostat based on appliance_details: primary or secondary. | ||
| Note: there can be several primary and secondary thermostats. | ||
| Rank the thermostat based on entity-thermostat-type: primary or secondary. | ||
| There can be several primary and secondary thermostats per location. | ||
| """ | ||
| appl_class = appliance_details["dev_class"] | ||
| appl_d_loc = appliance_details["location"] | ||
| thermo_loc = self._thermo_locs[loc_id] | ||
| if loc_id == appl_d_loc and appl_class in thermo_matching: | ||
| if thermo_matching[appl_class] == thermo_loc["primary_prio"]: | ||
| thermo_loc["primary"].append(appliance_id) | ||
| appl_class = entity["dev_class"] | ||
| if ( | ||
| "location" in entity | ||
| and location_id == entity["location"] | ||
| and appl_class in thermo_matching | ||
| ): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest turning this negative and return None
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line has been simplified, see the latest commits. |
||
| # Pre-elect new primary | ||
| elif (thermo_rank := thermo_matching[appl_class]) > thermo_loc[ | ||
| if thermo_matching[appl_class] == location["primary_prio"]: | ||
| location["primary"].append(entity_id) | ||
| elif (thermo_rank := thermo_matching[appl_class]) > location[ | ||
| "primary_prio" | ||
| ]: | ||
| thermo_loc["primary_prio"] = thermo_rank | ||
| location["primary_prio"] = thermo_rank | ||
| # Demote former primary | ||
| if tl_primary := thermo_loc["primary"]: | ||
| thermo_loc["secondary"] += tl_primary | ||
| thermo_loc["primary"] = [] | ||
|
|
||
| if tl_primary := location["primary"]: | ||
| location["secondary"] += tl_primary | ||
| location["primary"] = [] | ||
| # Crown primary | ||
| thermo_loc["primary"].append(appliance_id) | ||
| location["primary"].append(entity_id) | ||
| else: | ||
| thermo_loc["secondary"].append(appliance_id) | ||
| location["secondary"].append(entity_id) | ||
|
|
||
| def _control_state(self, data: GwEntityData) -> str | bool: | ||
| """Helper-function for _get_location_data(). | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.