Add caching for all entity IDs and debounce updates in sensors #978
Conversation
📝 WalkthroughWalkthroughThe changes introduce a caching mechanism for retrieving all entity IDs, with event-driven invalidation to keep the cache fresh. Sensor entities now debounce state updates, reducing redundant updates from rapid events. Integration setup is updated to initialize and clean up the cache invalidation logic during its lifecycle. Changes
Sequence Diagram(s)sequenceDiagram
participant HomeAssistant
participant SpookIntegration
participant EntityRegistry
participant StateMachine
participant SpookSensor
HomeAssistant->>SpookIntegration: Setup Entry
SpookIntegration->>SpookIntegration: async_setup_all_entity_ids_cache_invalidation()
SpookIntegration->>EntityRegistry: Listen for updates
SpookIntegration->>StateMachine: Listen for events
Note over SpookIntegration: Cache invalidation listeners registered
SpookSensor->>SpookSensor: Event triggers update
SpookSensor->>SpookSensor: Cancel pending update if any
SpookSensor->>SpookSensor: Schedule debounced update (5s delay)
Note over SpookSensor: Only one update runs after debounce period
HomeAssistant->>SpookIntegration: Unload Entry
SpookIntegration->>SpookIntegration: Cleanup cache invalidation listeners
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
custom_components/spook/util.py (1)
409-409:⚠️ Potential issueFix incorrect function call.
This line calls
async_get_all_label_idsbut should callasync_get_all_floor_idsbased on the function name and context.Apply this fix:
def async_filter_known_floor_ids( hass: HomeAssistant, *, floor_ids: set[str], known_floor_ids: set[str] | None = None, ) -> set[str]: """Filter out known floor IDs.""" if known_floor_ids is None: - known_floor_ids = async_get_all_label_ids(hass) + known_floor_ids = async_get_all_floor_ids(hass) return { floor_id for floor_id in floor_ids - known_floor_ids if floor_id and isinstance(floor_id, str) }
🧹 Nitpick comments (2)
custom_components/spook/ectoplasms/homeassistant/sensor.py (1)
584-603: Well-implemented debouncing mechanism!The debouncing correctly prevents rapid updates by cancelling pending calls before scheduling new ones. This will effectively reduce the load from frequent entity registry or state change events.
Consider making the 5-second delay configurable to allow fine-tuning based on system requirements:
+# Debounce delay in seconds +DEBOUNCE_DELAY = 5 + class HomeAssistantSpookSensorEntity(HomeAssistantSpookEntity, SensorEntity): """Spook sensor providig Home Asistant information.""" entity_description: HomeAssistantSpookSensorEntityDescription _unsub_debouncer: Callable[[], None] | None = NoneThen use it in line 602:
- self._unsub_debouncer = async_call_later(self.hass, 5, _debounced_update) + self._unsub_debouncer = async_call_later(self.hass, DEBOUNCE_DELAY, _debounced_update)custom_components/spook/util.py (1)
128-188: Well-designed caching mechanism with proper invalidation.The implementation correctly:
- Prevents duplicate listener setup
- Clears cache on relevant events (entity registry updates, HA start, component loads)
- Provides proper cleanup functionality
- Includes helpful debug logging
While the global variable approach works, consider encapsulating the cache in a class for better maintainability:
class EntityIDCache: """Manages cached entity IDs with invalidation.""" def __init__(self, hass: HomeAssistant): self.hass = hass self._cache: set[str] | None = None self._unsub_listeners: list[Callable[[], None]] = [] def setup_invalidation(self) -> Callable[[], None]: """Set up cache invalidation listeners.""" # Implementation here def get_all_entity_ids(self, include_all_none: bool = False) -> set[str]: """Get cached entity IDs.""" # Implementation hereThis would make testing easier and reduce reliance on global state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
custom_components/spook/__init__.py(2 hunks)custom_components/spook/ectoplasms/homeassistant/sensor.py(3 hunks)custom_components/spook/util.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Verify
🔇 Additional comments (3)
custom_components/spook/__init__.py (1)
103-105: LGTM! Cache invalidation is properly integrated.The cache invalidation setup is correctly placed after platform setup and properly registered for cleanup on unload.
custom_components/spook/ectoplasms/homeassistant/sensor.py (1)
611-617: Proper cleanup implementation.The cleanup method correctly cancels any pending debounced updates and calls the parent's cleanup method.
custom_components/spook/util.py (1)
190-224: Efficient caching implementation for entity ID retrieval.The cache correctly:
- Combines entity IDs from both registry and current states
- Filters out ignored domains
- Returns copies to prevent cache corruption
- Includes optional ENTITY_MATCH_ALL/NONE constants
Description
Performance improvements, but introducing caching around getting all entity IDs; which is fetched by all repairs constantly.
Motivation and Context
fixes #637
closed #618
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist