Remove warning about using incorrect schema for entity service registration #311
Conversation
WalkthroughThe pull request introduces updates to two files within the custom components of Node-RED. In Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
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 and nitpick comments (1)
custom_components/nodered/binary_sensor.py (1)
48-48: LGTM: Updated to use LockState.UNLOCKEDThe change from
STATE_UNLOCKEDtoLockState.UNLOCKEDis a good improvement. It uses a more type-safe enum value instead of a string constant, which aligns with best practices and addresses the warning about incorrect schemas.Consider updating other string constants in the
on_statestuple to use similar enum values if they exist in Home Assistant. For example,STATE_ON,STATE_OPEN, andSTATE_HOMEmight have corresponding enum values that could be used for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- custom_components/nodered/binary_sensor.py (2 hunks)
- custom_components/nodered/switch.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (4)
custom_components/nodered/binary_sensor.py (2)
6-6: LGTM: Import statement added correctlyThe new import statement for
LockStatefromhomeassistant.components.lockis correctly placed and necessary for the subsequent use ofLockState.UNLOCKED. This change aligns with the PR objective of updating the integration to use the correct schemas and constants.
Line range hint
1-80: Overall impact: Positive change with no negative side effectsThe changes made to this file are focused and beneficial:
- They address the warning about using incorrect schemas as mentioned in the PR objectives.
- They improve type safety by using an enum value (
LockState.UNLOCKED) instead of a string constant.- The changes maintain backwards compatibility and don't affect the existing logic of the binary sensor.
These updates enhance the integration's compliance with Home Assistant's expected schemas without introducing any apparent negative side effects or breaking changes.
custom_components/nodered/switch.py (2)
Line range hint
62-64: Verify compatibility with async_register_entity_serviceThe simplified
SERVICE_TRIGGER_SCHEMAis correctly used in theasync_register_entity_servicemethod. However, let's ensure that this method accepts a plain dictionary as the schema argument in the current Home Assistant version.To confirm this, please run the following command to check the
async_register_entity_servicemethod signature in the Home Assistant core:#!/bin/bash # Description: Check async_register_entity_service method signature # Test: Search for async_register_entity_service method definition rg --type python 'def async_register_entity_service' homeassistant/helpers/entity_platform.pyIf the method accepts a plain dictionary, no further changes are needed. If it requires a
vol.Schemaobject, we may need to update the registration code.
34-38: Approved: Schema simplification resolves the warning.The change from
vol.Schemato a plain dictionary forSERVICE_TRIGGER_SCHEMAis correct and addresses the warning about using an incorrect schema for entity service registration. This modification ensures compatibility with Home Assistant Core 2025.9 and beyond.The simplified schema maintains the same validation rules for the fields, which is good for maintaining consistency in the service's behavior.
Let's verify if there are any other occurrences of
vol.Schemain the file that might need similar updates:✅ Verification successful
Verified: No remaining
vol.Schemausages found.The change from
vol.Schemato a plain dictionary forSERVICE_TRIGGER_SCHEMAhas been successfully applied. No other instances ofvol.Schemawere detected incustom_components/nodered/switch.py, ensuring that the warning about incorrect schema usage is resolved.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other vol.Schema usages in the file # Test: Search for vol.Schema usages rg --type python 'vol\.Schema' custom_components/nodered/switch.pyLength of output: 126
Script:
#!/bin/bash # Description: Re-check for other vol.Schema usages in the file without specifying file type # Test: Search for vol.Schema usages rg 'vol\.Schema' custom_components/nodered/switch.pyLength of output: 54
|
Probably done this wrong somehow, since it seems to have included a minor change from my previous pull. I don't think it is an issue. |
2024-10-12 12:00:30.700 WARNING (MainThread) [homeassistant.helpers.frame] Detected that custom integration 'nodered' registers an entity service with a non entity service schema which will stop working in HA Core 2025.9 at custom_components/nodered/switch.py, line 62: platform.async_register_entity_service(, please create a bug report at https:/zachowj/hass-node-red/issuesSummary by CodeRabbit
New Features
Bug Fixes