Conversation
- Implemented per-light status tracking to provide detailed states like "active," "inactive," "blocked," and "manual override." - Added new `sensor` platform for enhanced diagnostics. - Updated README to include diagnostic status sensors. - Introduced various status-related attributes and methods for better monitoring and troubleshooting.
Added a new configuration option `enable_diagnostic_sensors` to control the creation of per-light status sensors in the Adaptive Lighting integration. Sensors are now disabled by default and can be enabled via the options flow. Updated related files, including initialization logic, constants, translations, and documentation.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
florianhorner
left a comment
There was a problem hiding this comment.
Review Summary
Useful diagnostic feature — knowing why a light isn't adapting is a common user question. The architecture (opt-in sensors per profile) is well-chosen. Some issues to address before merge.
Critical Issues
1. Duplicate _expand_light_groups in sensor.py
This function already exists in switch.py (line 588). Import it from there instead of reimplementing — duplicated logic will drift over time.
2. Redundant double set_light_status(STATUS_ACTIVE, "apply")
Called in both _adapt_light AND execute_cancellable_adaptation_calls. The first call is immediately overwritten by the second. Remove the first — it's dead code.
3. Test/framework deps in runtime dependencies
pytest-asyncio==1.3.0 and homeassistant>=2024.12.5 are in [project] runtime deps. Both belong in [dependency-groups]. HA custom components cannot declare HA itself as a pip dependency.
Quality Issues
4. ensure_status_sensors_enabled overrides user-disabled entities
If a user intentionally disables a diagnostic sensor via the UI, this re-enables it. Check disabled_by != EntityRegistryEntryDisabler.USER before re-enabling.
5. STATUS constants should be StrEnum
The codebase already uses StrEnum for TakeOverControlMode. Plain strings miss typo detection. Define LightStatus(StrEnum).
6. Unused return type change
execute_cancellable_adaptation_calls return type changed to bool but neither call site checks the return value.
7. _status_priority allocates a new dict on every call
This is constant data — make it a module-level STATUS_PRIORITY constant.
Efficiency Issues
8. get_combined_status called twice per sensor write
Both native_value and extra_state_attributes call it independently. Cache the result.
9. get_manual_control_attributes called twice
Same pattern in extra_state_attributes. Cache it.
10. Unbounded light_status dict
Never pruned when lights are removed. Add self.light_status.pop(light, None) in reset().
11. Bare async_dispatcher_send bypasses dedup
In state_changed_event_listener, the dispatcher fires even when no status changed.
Main blockers are #1 (duplicated code) and #3 (runtime deps). Looking forward to the next iteration! 👍
337bc93 to
4751047
Compare
4751047 to
cc64fbf
Compare
- Centralized light group expansion logic in `helpers.py` with `expand_light_groups`. - Simplified light status management using a new `LightStatus` enum with priority mapping. - Replaced scattered status strings with `LightStatus` constants for consistency. - Updated dev dependencies in `pyproject.toml` and reorganized requirements. - Enhanced `LightStatusInfo` for better status tracking and debugging.
for more information, see https://pre-commit.ci
|
Based on the feedback found on the pull request PR #1414, here are the 11 key points and their current status after the recent refactoring:
|
for more information, see https://pre-commit.ci
Summary
This PR adds optional diagnostic status sensors per light, giving visibility into whether Adaptive Lighting is actively controlling a light, blocked, manually overridden, or idle. It also aggregates status across multiple profiles controlling the same light.
Key Changes
sensor.py) with one diagnostic sensor per light.switch.pywith per-profile and combined state.const.py.enable_diagnostic_sensors(defaultfalse) to toggle sensors per profile.docs/status_sensors.mdand README note.Behavior
inactive,adopting,active,manual_override,blocked,error.error > manual_override > adopting > active > blocked > inactive.status_profilesattribute exposes per-profile detail.inactive.enable_diagnostic_sensorsis true.Testing
python3 -m py_compileon updated files.Notes