-
-
Notifications
You must be signed in to change notification settings - Fork 37.4k
Fix WLED entity naming for multi-segment setups #169670
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,8 @@ def __init__(self, coordinator: WLEDDataUpdateCoordinator) -> None: | |
| """Initialize WLED main light.""" | ||
| super().__init__(coordinator=coordinator) | ||
| self._attr_unique_id = coordinator.data.info.mac_address | ||
| if coordinator.keep_main_light: | ||
| self._attr_name = None | ||
|
Comment on lines
+69
to
+70
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. It is expected. I covered this scenario in PR description. Migration would be too risky.
Comment on lines
+69
to
+70
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. For me, it's too risky to change an entity ID that's already assigned or to delete entities that have already been created. If the keep main light option is enabled, the entity will be marked as unavailable and the user can manually delete it from the UI.
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. Besides, I think we should discourage disabling the "Keep Main Light" option and instead describe how to achieve a similar effect using automation. I even have such automations for multi-segment installations, and now there will also be a recommendation to use it for single-segment installations too. One day, we might even retire the "Keep Main Light" option and leave it on by default but that's another thread. |
||
|
|
||
| @property | ||
| def brightness(self) -> int | None: | ||
|
|
@@ -122,9 +124,9 @@ def __init__( | |
| super().__init__(coordinator=coordinator) | ||
| self._segment = segment | ||
|
|
||
| # Segment 0 uses a simpler name, which is more natural for when using | ||
| # a single segment / using WLED with one big LED strip. | ||
| if segment == 0: | ||
| # With keep_main_light disabled, a single-segment setup uses segment 0 | ||
| # as the primary entity — it drops the "Segment N" qualifier. | ||
| if segment == 0 and not coordinator.keep_main_light: | ||
| self._attr_name = None | ||
| else: | ||
| self._attr_translation_placeholders = {"segment": str(segment)} | ||
|
|
||
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 intentional and consistent with how we handle options in other integration — storing no value distinguishes "user has never changed this setting" from "user explicitly set it to True/False". If we ever change
DEFAULT_KEEP_MAIN_LIGHTagain, we'll do it the same way we did here: write a migration that explicitly sets the option on all entries that should keep the old behavior, and let new entries inherit the new default. The 1.2 → 1.3 migration is exactly that pattern — it locks existing users toFalserather than leaving them at the mercy of the constant.