-
Notifications
You must be signed in to change notification settings - Fork 8
Update JSON sorting (confirming to core) #998
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
Conversation
WalkthroughRestructures Plugwise Home Assistant integration's translation and configuration schemas by migrating from nested config structures to modular top-level sections (entity, exceptions, services, options). Adds new abort messages, expands error definitions, and introduces comprehensive entity naming for device types. Creates new translation files for language support. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @custom_components/plugwise/translations/nl.json:
- Around line 227-229: The translation value for the key "heating_setpoint"
contains an unintended leading space; open the translations object where
"heating_setpoint" is defined and remove the leading whitespace so the value is
"Instelling verwarmen" (ensure only the string value is changed, preserving the
key name and surrounding JSON formatting).
- Around line 60-62: Remove the stale translation key "low_battery" from the
Dutch translations file because it no longer exists in the canonical translation
sources (strings.json and en.json); locate the "low_battery" entry and delete
that object (the "name": "Batterij status" pair) so the NL file matches the
current keys, and run the i18n/translation validation to ensure no other orphan
keys remain.
In @custom_components/plugwise/translations/un:
- Around line 1-345: The translation file is using an invalid language code
("un") and lacks the required .json extension; rename the file to a valid locale
filename (for example en.json) so Home Assistant will load it, or remove it if
it duplicates the existing English translation; ensure the JSON content
(top-level keys like "config", "entity", "exceptions", "options", "services")
remains unchanged after renaming.
In @custom_components/plugwise/un:
- Around line 1-345: The file "custom_components/plugwise/un" is misplaced and
missing a .json extension; move/rename it into the translations directory (e.g.,
custom_components/plugwise/translations/un.json or translations/en.json) so Home
Assistant recognizes the JSON (contains keys like "options", "config", "entity",
"exceptions", "services"), and remove any duplicate translation file in
translations/ to avoid conflicts.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
custom_components/plugwise/strings.jsoncustom_components/plugwise/translations/en.jsoncustom_components/plugwise/translations/nl.jsoncustom_components/plugwise/translations/uncustom_components/plugwise/un
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (2)
custom_components/plugwise/translations/en.json (1)
1-345: LGTM!The English translation file structure is correct and consistent with
strings.json. Keys are properly organized across all sections.custom_components/plugwise/strings.json (1)
1-345: LGTM!The source strings file structure is well-organized with consistent formatting. All translation sections are properly defined.
| "low_battery": { | ||
| "name": "Batterij status" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale translation key low_battery not present in strings.json or en.json.
This low_battery key exists in the Dutch translations but is missing from both strings.json and en.json. This suggests it may be a removed or renamed entity that should be cleaned up from this file as well.
🔎 Suggested fix
"plugwise_notification": {
"name": "Plugwise melding"
- },
- "low_battery": {
- "name": "Batterij status"
},
"secondary_boiler_state": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "low_battery": { | |
| "name": "Batterij status" | |
| }, | |
| "plugwise_notification": { | |
| "name": "Plugwise melding" | |
| }, | |
| "secondary_boiler_state": { |
🤖 Prompt for AI Agents
In @custom_components/plugwise/translations/nl.json around lines 60 - 62, Remove
the stale translation key "low_battery" from the Dutch translations file because
it no longer exists in the canonical translation sources (strings.json and
en.json); locate the "low_battery" entry and delete that object (the "name":
"Batterij status" pair) so the NL file matches the current keys, and run the
i18n/translation validation to ensure no other orphan keys remain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an existing binary_sensor, please add this on to strings.json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if low_battery exists in strings.json
echo "=== Checking strings.json for low_battery ==="
cat custom_components/plugwise/strings.json | jq '.entity.binary_sensor' | grep -i "low_battery" || echo "Not found in strings.json"
echo -e "\n=== Checking en.json for low_battery ==="
cat custom_components/plugwise/translations/en.json | jq '.entity.binary_sensor' | grep -i "low_battery" || echo "Not found in en.json"
echo -e "\n=== Searching for low_battery binary_sensor in Python code ==="
rg -n "low_battery" --type=py -C2
Length of output: 926
@bouwew, you're absolutely right! I apologize for the incorrect assessment. The low_battery binary_sensor is indeed used in the codebase (defined in const.py as BATTERY_STATE) and should be present in strings.json.
Here's what needs to be added to strings.json in the entity.binary_sensor section:
"low_battery": {
"name": "Battery status"
}This should also be added to custom_components/plugwise/translations/en.json in the same location for consistency. Given that this PR is about JSON sorting, the key should be placed in alphabetical order within the binary_sensor section (between heating_state and plugwise_notification based on the current structure).
Would you like me to help identify the exact placement in strings.json to maintain proper alphabetical sorting?
✏️ Learnings added
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 998
File: custom_components/plugwise/translations/nl.json:60-62
Timestamp: 2026-01-07T19:13:26.648Z
Learning: The low_battery binary_sensor (defined as BATTERY_STATE in custom_components/plugwise/const.py) is a valid entity in the Plugwise integration and should be included in translation files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| "heating_setpoint": { | ||
| "name": " Instelling verwarmen" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leading space in translation string.
The value " Instelling verwarmen" has an unintended leading space before "Instelling".
🔎 Suggested fix
"heating_setpoint": {
- "name": " Instelling verwarmen"
+ "name": "Instelling verwarmen"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "heating_setpoint": { | |
| "name": " Instelling verwarmen" | |
| }, | |
| "heating_setpoint": { | |
| "name": "Instelling verwarmen" | |
| }, |
🤖 Prompt for AI Agents
In @custom_components/plugwise/translations/nl.json around lines 227 - 229, The
translation value for the key "heating_setpoint" contains an unintended leading
space; open the translations object where "heating_setpoint" is defined and
remove the leading whitespace so the value is "Instelling verwarmen" (ensure
only the string value is changed, preserving the key name and surrounding JSON
formatting).
|
|
Findings related to translation, besides the mismatches CRAI already suggests: the following should be either updated in beta OR in Core. Building a checklist starting with
This also induces changes to NL: full diff (as I didn't want an endless list) is at https://gist.github.com/CoMPaTech/07bd93f1a890c45a93df07ff430f396d
|
bouwew
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sorting looks fine 👍



No actual changes, just sorted JSON
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.