Intermediate Core downstream / prepare tagging upstream #700
Conversation
CoMPaTech
added
enhancement
WalkthroughThe recent updates to the Plugwise integration encompass enhancements to the CHANGELOG, manifest, HACS configuration, and project settings. The CHANGELOG now includes an entry for version v0.52.1, detailing code improvements and documentation updates. The manifest and HACS files reflect a simplified name and the addition of automatic device discovery. Additionally, the project configuration description has been refined, indicating a potential rebranding. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Plugwise
participant HomeAssistant
User->>HomeAssistant: Configure Plugwise
HomeAssistant->>Plugwise: Initiate async_setup_entry
Plugwise->>Plugwise: Setup entities
Plugwise->>HomeAssistant: Register services
HomeAssistant->>User: Confirm setup
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
CHANGELOG.md (1)
8-8: Ensure clarity and completeness in changelog entries.The entry "Downstream Core changes" is vague. Consider providing more details about the specific changes or updates to help users understand the impact and scope of these modifications.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- CHANGELOG.md (1 hunks)
- custom_components/plugwise/init.py (5 hunks)
- custom_components/plugwise/binary_sensor.py (6 hunks)
- custom_components/plugwise/button.py (1 hunks)
- custom_components/plugwise/climate.py (2 hunks)
- custom_components/plugwise/config_flow.py (4 hunks)
- custom_components/plugwise/const.py (1 hunks)
- custom_components/plugwise/coordinator.py (5 hunks)
- custom_components/plugwise/diagnostics.py (1 hunks)
- custom_components/plugwise/entity.py (3 hunks)
- custom_components/plugwise/manifest.json (1 hunks)
- custom_components/plugwise/number.py (6 hunks)
- custom_components/plugwise/select.py (4 hunks)
- custom_components/plugwise/sensor.py (7 hunks)
- custom_components/plugwise/switch.py (7 hunks)
- custom_components/plugwise/util.py (1 hunks)
Files skipped from review due to trivial changes (6)
- custom_components/plugwise/config_flow.py
- custom_components/plugwise/const.py
- custom_components/plugwise/coordinator.py
- custom_components/plugwise/diagnostics.py
- custom_components/plugwise/entity.py
- custom_components/plugwise/switch.py
Additional comments not posted (19)
custom_components/plugwise/manifest.json (2)
3-3: Renaming looks good.The integration name change to "Plugwise Beta" simplifies the branding and aligns with a more streamlined identity.
12-12: Zeroconf addition is beneficial.Adding the zeroconf entry enhances the integration by allowing automatic discovery of Plugwise devices on the network.
custom_components/plugwise/util.py (1)
20-21: Type annotations update is clear and beneficial.The updated type annotations improve clarity and type safety by allowing more explicit type definitions in the
plugwise_commandfunction.custom_components/plugwise/button.py (2)
28-32: Refactoring improves readability.The refactoring of
async_setup_entryusing a generator expression simplifies the code and maintains the existing logic.
16-16: Removal of unused import is appropriate.The removal of the unused
ATTR_NAMEimport helps streamline the code.custom_components/plugwise/select.py (3)
89-94: Refactor improves readability and efficiency.The use of a generator expression directly in
async_add_entitiesenhances readability and avoids unnecessary list creation.
8-8: Removal of unused importATTR_NAME.The removal of
ATTR_NAMEcleans up the code by eliminating an unused import.
36-36: Verify the impact ofPARALLEL_UPDATESset to 0.Setting
PARALLEL_UPDATESto 0 means updates are not parallelized. Ensure this aligns with performance expectations and does not cause delays during multiple updates.custom_components/plugwise/number.py (3)
85-90: Refactor improves readability and efficiency.The use of a generator expression directly in
async_add_entitiesenhances readability and avoids unnecessary list creation.
109-118: Improved clarity with upstream constant comments.The comments about upstream constants enhance clarity, and the logic for
self._attr_native_stepensures a reasonable minimum step size for temperature adjustments.
34-34: Verify the impact ofPARALLEL_UPDATESset to 0.Setting
PARALLEL_UPDATESto 0 means updates are not parallelized. Ensure this aligns with performance expectations and does not cause delays during multiple updates.custom_components/plugwise/__init__.py (3)
32-32: Docstring update improves clarity.The updated docstring accurately reflects the broader scope of setting up "Plugwise components," enhancing clarity.
Line range hint
71-71:
Comments enhance clarity on service registration.The comments regarding the
ServiceCallassociation clarify the role ofdelete_notificationin the service registration process.
96-96: Docstring update improves clarity.The updated docstring accurately reflects the broader scope of unloading "Plugwise components," enhancing clarity.
custom_components/plugwise/binary_sensor.py (2)
99-118: Simplified entity addition logic looks good!The refactoring using a generator expression enhances readability and performance. Ensure that the logic correctly filters and adds entities as intended.
Line range hint
125-157:
Verify notification handling logic.Ensure that the logic for handling notifications and setting extra state attributes aligns with upstream requirements and functions correctly.
Verification successful
Notification Handling Logic Verified
The usage of
PLUGWISE_NOTIFICATIONandSEVERITIESin thePlugwiseBinarySensorEntityclass aligns with upstream requirements. The logic for handling notifications and setting extra state attributes is correctly implemented.
PLUGWISE_NOTIFICATIONis used as a key for notifications.SEVERITIESis used to filter message types.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify notification handling logic in PlugwiseBinarySensorEntity. # Test: Search for the usage of PLUGWISE_NOTIFICATION and SEVERITIES to confirm correct handling. rg --type python $'PLUGWISE_NOTIFICATION|SEVERITIES'Length of output: 84
Script:
#!/bin/bash # Description: Verify notification handling logic in PlugwiseBinarySensorEntity. # Test: Search for the usage of PLUGWISE_NOTIFICATION and SEVERITIES to confirm correct handling. rg --type py 'PLUGWISE_NOTIFICATION|SEVERITIES'Length of output: 779
custom_components/plugwise/climate.py (1)
62-63: Verify the impact ofPARALLEL_UPDATES.The addition of
PARALLEL_UPDATES = 0may affect concurrency behavior. Ensure that this change aligns with the intended design and does not introduce unintended side effects.Verification successful
Consistent use of
PARALLEL_UPDATES = 0across componentsThe setting of
PARALLEL_UPDATES = 0is consistently applied across multiple components in the Plugwise integration. This suggests a deliberate design choice to handle updates sequentially, likely to avoid concurrency issues. Ensure that this aligns with the overall design and performance expectations.
custom_components/plugwise/climate.pycustom_components/plugwise/sensor.pycustom_components/plugwise/binary_sensor.pycustom_components/plugwise/select.pycustom_components/plugwise/switch.pycustom_components/plugwise/number.pycustom_components/plugwise/button.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of `PARALLEL_UPDATES` in Plugwise Climate component. # Test: Search for the usage of `PARALLEL_UPDATES` to understand its impact. rg --type python $'PARALLEL_UPDATES'Length of output: 66
Script:
#!/bin/bash # Description: Verify the impact of `PARALLEL_UPDATES` in Plugwise Climate component. # Test: Search for the usage of `PARALLEL_UPDATES` to understand its impact. rg 'PARALLEL_UPDATES'Length of output: 507
custom_components/plugwise/sensor.py (2)
463-479: Refactored entity addition logic looks good!The refactoring using a list comprehension enhances readability and efficiency. Ensure that the logic correctly filters and adds entities as intended.
504-504: Verify native value logic.Ensure that the logic for returning native values aligns with upstream requirements and functions correctly.
Verification successful
Verified: Native value logic is consistent.
The use of the
SENSORSconstant insensor.pyaligns with its definition and intended usage. The logic for returning native values appears to function correctly as per upstream requirements.
custom_components/plugwise/sensor.py: The line correctly accesses sensor data using theSENSORSkey.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify native value logic in PlugwiseSensorEntity. # Test: Search for the usage of SENSORS to confirm correct native value logic. rg --type python $'SENSORS'Length of output: 57
Script:
#!/bin/bash # Description: Verify native value logic in PlugwiseSensorEntity. # Test: Search for the usage of SENSORS to confirm correct native value logic. rg --type py SENSORSLength of output: 1521
Reverted but dropped ATTR_NAME just for clarity, tried wrapping/overloading the Core functionality but that would take considerable more mypy time. If it is better for diagnostics we might challenge this to core as well by adding debug check to that helpers class? (Outside of scope for this PR though) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- custom_components/plugwise/init.py (5 hunks)
- custom_components/plugwise/binary_sensor.py (7 hunks)
- custom_components/plugwise/button.py (1 hunks)
- custom_components/plugwise/number.py (7 hunks)
- custom_components/plugwise/select.py (5 hunks)
- custom_components/plugwise/sensor.py (8 hunks)
- custom_components/plugwise/switch.py (9 hunks)
Files skipped from review as they are similar to previous changes (7)
- custom_components/plugwise/init.py
- custom_components/plugwise/binary_sensor.py
- custom_components/plugwise/button.py
- custom_components/plugwise/number.py
- custom_components/plugwise/select.py
- custom_components/plugwise/sensor.py
- custom_components/plugwise/switch.py
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- custom_components/plugwise/binary_sensor.py (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- custom_components/plugwise/binary_sensor.py
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- custom_components/plugwise/init.py (6 hunks)
- custom_components/plugwise/climate.py (3 hunks)
- custom_components/plugwise/const.py (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- custom_components/plugwise/init.py
- custom_components/plugwise/const.py
Additional comments not posted (2)
custom_components/plugwise/climate.py (2)
108-109: Reordering attributes for clarity.The reordering of
_previous_modeand_homekit_modeimproves the organizational clarity of the code.
62-62: Verify the intention behind settingPARALLEL_UPDATESto0.Setting
PARALLEL_UPDATESto0suggests that updates should be processed sequentially. Verify if this is the intended behavior to prevent potential conflicts or race conditions.Verification successful
Consistent usage of
PARALLEL_UPDATESset to0across components.The
PARALLEL_UPDATESconstant is set to0in multiple files within thecustom_components/plugwisedirectory. This suggests a deliberate choice to handle updates sequentially, likely to prevent concurrency issues or align with upstream requirements. The consistent pattern across different components supports this interpretation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and impact of `PARALLEL_UPDATES` in the codebase. # Test: Search for the usage of `PARALLEL_UPDATES`. Expect: Understand its role in the update mechanism. rg --type python -A 3 $'PARALLEL_UPDATES'Length of output: 71
Script:
#!/bin/bash # Description: Verify the usage and impact of `PARALLEL_UPDATES` in the codebase. # Test: Search for the usage of `PARALLEL_UPDATES` across all Python files. rg 'PARALLEL_UPDATES' --glob '*.py' -A 3Length of output: 1565
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- custom_components/plugwise/init.py (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- custom_components/plugwise/init.py
|
|
||
| async def async_setup_entry(hass: HomeAssistant, entry: PlugwiseConfigEntry) -> bool: | ||
| """Set up the Plugwise Device from a config entry.""" | ||
| """Set up Plugwise components from a config entry.""" |
There was a problem hiding this comment.
Should we write here: """Set up the Plugwise Integration from...?
There was a problem hiding this comment.
It's a complete mixed bag ... I've rewritten most of them to Set up Plugwise x from config entry now though with your next comment we might need more of those. Core is a mix of Plugwise and Smile and ConfigEntry vs config entry :)
Maybje just Setup Plugwise from a config Entry?
There was a problem hiding this comment.
Maybe Set up the Plugwise Integration from a config entry?
Good to add config entry 👍
|
|
||
| async def async_unload_entry(hass: HomeAssistant, entry: PlugwiseConfigEntry) -> bool: | ||
| """Unload a config entry.""" | ||
| """Unload the Plugwise components.""" |
There was a problem hiding this comment.
Also here: """Unload the Plugwise Integration.? Or """ Unload the Plugwise platforms.? Or """Unload the Plugwise config entry.? The last two seem to make the most sense, to me :)
There was a problem hiding this comment.
Upload plugwise (also followed Core on this one)
There was a problem hiding this comment.
Unload the Plugwise config entry? Fits nicely with the async_setup_entry() docstring.
| LOGGER.debug( | ||
| "Migrating entity %s from old unique ID '%s' to new unique ID '%s'", | ||
| entity_id, | ||
| old_unique_id, | ||
| new_unique_id, | ||
| ) |
There was a problem hiding this comment.
Let's get rid of this debug-message.
There was a problem hiding this comment.
It's still in core, adjusting and flagging accordingly
| if control_state == STATE_OFF: | ||
| if (control_state := self.device.get(CONTROL_STATE)) in (HVACAction.COOLING, HVACAction.HEATING, HVACAction.PREHEATING): | ||
| return control_state | ||
| if control_state == HVACMode.OFF: |
There was a problem hiding this comment.
The strange thing is some .get constructs do work in Core but not in beta ...
There was a problem hiding this comment.
I meant the control_state - str vs HVACMode.Off - StrEnum.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
custom_components/plugwise/sensor.py (1)
Line range hint
470-493:
Consider removing commented-out code.The commented-out code might be useful for debugging but should be removed or documented before finalizing the PR to maintain code clarity.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- custom_components/plugwise/init.py (6 hunks)
- custom_components/plugwise/binary_sensor.py (7 hunks)
- custom_components/plugwise/button.py (1 hunks)
- custom_components/plugwise/climate.py (4 hunks)
- custom_components/plugwise/number.py (7 hunks)
- custom_components/plugwise/select.py (5 hunks)
- custom_components/plugwise/sensor.py (8 hunks)
- custom_components/plugwise/switch.py (9 hunks)
Files skipped from review as they are similar to previous changes (7)
- custom_components/plugwise/init.py
- custom_components/plugwise/binary_sensor.py
- custom_components/plugwise/button.py
- custom_components/plugwise/climate.py
- custom_components/plugwise/number.py
- custom_components/plugwise/select.py
- custom_components/plugwise/switch.py
Additional comments not posted (2)
custom_components/plugwise/sensor.py (2)
Line range hint
16-62:
Verify the necessity of theLOGGERimport.The
LOGGERimport is commented as "pw-beta," suggesting it might be temporary or for debugging. Ensure it's necessary for the final implementation.Verification successful
LOGGER is actively used in the codebase.
The
LOGGERis utilized in multiple files, includingsensor.py, for debugging purposes. Its presence is justified given its usage across various components for logging debug information.
custom_components/plugwise/sensor.py:LOGGERis used for debugging.No changes are necessary regarding the
LOGGERimport.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `LOGGER` is used in the codebase. # Test: Search for the usage of `LOGGER`. Expect: Occurrences of `LOGGER` usage. rg --type python 'LOGGER'Length of output: 56
Script:
#!/bin/bash # Description: Verify if `LOGGER` is used in the codebase. # Test: Search for the usage of `LOGGER`. Expect: Occurrences of `LOGGER` usage. rg 'LOGGER'Length of output: 2182
520-520: LGTM! Verify the availability of upstream constants.The
native_valueproperty implementation is correct. Ensure that all upstream constants used are available and correctly imported.Verification successful
Verification Successful: Upstream Constant Available
The
SENSORSconstant is defined incustom_components/plugwise/const.pyand is correctly used insensor.py. The implementation of thenative_valueproperty inPlugwiseSensorEntityis valid. No issues found.
custom_components/plugwise/const.py:SENSORS: Final = "sensors"Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the availability of upstream constants used in `native_value`. # Test: Search for the definition of `SENSORS`. Expect: Definition of `SENSORS`. rg --type python 'SENSORS'Length of output: 57
Script:
#!/bin/bash # Description: Verify the availability of upstream constants used in `native_value`. # Test: Search for the definition of `SENSORS`. Expect: Definition of `SENSORS`. rg --type py 'SENSORS'Length of output: 1872
|
In 'our' HA core there is a |
bouwew
left a comment
bouwew
left a comment
There was a problem hiding this comment.
Ok, let's merge these updates!
|
I added the zeroconf-line in my system, all looks normal after a HA restart. |
|
Confirmed for merge? |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
CHANGELOG.md (1)
7-7: Add missing comma for clarity.Consider adding a comma after "updates" for improved readability.
Apply this diff to add the missing comma:
- Downstream the latest Core Plugwise updates. + Downstream the latest Core Plugwise updates,.Tools
LanguageTool
[uncategorized] ~7-~7: Possible missing comma found.
Context: ...ersions from 0.40 and up ## v0.52.1 - Downstream the latest Core Plugwise updates. - Doc...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- custom_components/plugwise/manifest.json (1 hunks)
Additional context used
LanguageTool
CHANGELOG.md
[uncategorized] ~7-~7: Possible missing comma found.
Context: ...ersions from 0.40 and up ## v0.52.1 - Downstream the latest Core Plugwise updates. - Doc...(AI_HYDRA_LEO_MISSING_COMMA)
Additional comments not posted (3)
custom_components/plugwise/manifest.json (3)
3-3: Name change approved.The name change from "Plugwise Smile/Stretch Beta" to "Plugwise Beta" simplifies the integration's branding.
11-11: Version update approved.The version update from "0.52.0" to "0.52.1" reflects the recent changes and improvements.
12-12: Zeroconf addition approved.The addition of the
zeroconfkey with["_plugwise._tcp.local."]enables automatic device discovery, improving user experience.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- hacs.json (1 hunks)
- pyproject.toml (1 hunks)
Files skipped from review due to trivial changes (1)
- pyproject.toml
Additional comments not posted (3)
hacs.json (3)
2-2: Verify consistency of the integration name change.The integration name has been simplified to "Plugwise beta." Ensure that this change is reflected consistently across all related documentation and code references to avoid confusion.
5-5: Verify implementation and testing for the new "button" domain.The "button" domain has been added to the supported domains. Ensure that the necessary code to support this domain is implemented and thoroughly tested.
12-12: Verify compatibility with the updated Home Assistant version.The required Home Assistant version has been updated to "2024.8.0." Ensure that this version aligns with the integration's dependencies and functionalities, and that any new features or fixes are utilized.
Now handled by Core itself.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- custom_components/plugwise/climate.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- custom_components/plugwise/climate.py
Points to notice:
climate.pyhas issues when rewriting as separate pw-beta and core functionality (rough progress in gist; https://gist.github.com/CoMPaTech/664d3d15a4572be5769fb510df154f2b)Set up [Plugwise/Smile] ...descriptors :)Summary by CodeRabbit
New Features
Improvements
Documentation