feat(nvr): prepare nvr for config reload#1250
Conversation
✅ Deploy Preview for viseron canceled.
|
There was a problem hiding this comment.
Pull request overview
This PR prepares the NVR (Network Video Recorder) component for configuration reload by implementing proper event listener tracking and cleanup mechanisms. The changes introduce an unload pattern that allows entities and NVR instances to properly clean up their resources (event listeners, signal handlers) when being removed or reloaded.
Changes:
- Added
__init__method to base Entity class to initialize event listener tracking list - Implemented
unloadmethod in Entity base class to unsubscribe from tracked event listeners - Modified NVR entities (sensor and toggle) to track their event listeners for proper cleanup
- Added
unloadmethods to FrameIntervalCalculator and NVR classes to clean up resources - Updated entity registration to include domain and identifier for proper entity tracking
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| viseron/helpers/entity/init.py | Added __init__ to initialize _event_listeners list and unload method to clean up tracked listeners |
| viseron/domains/camera/entity/init.py | Added super().__init__(vis) call to ensure event listener tracking is initialized |
| viseron/components/nvr/toggle.py | Modified to append event listeners to _event_listeners for tracking |
| viseron/components/nvr/sensor.py | Modified to append event listeners to _event_listeners for tracking |
| viseron/components/nvr/nvr.py | Added listener tracking, unload methods for FrameIntervalCalculator and NVR, updated entity registration with domain/identifier, removed unused DATA_STREAM_COMPONENT |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def unload(self) -> None: | ||
| """Unload nvr.""" | ||
| for unsubscribe in self._listeners: | ||
| unsubscribe() | ||
| for scanner in self._frame_scanners.values(): | ||
| scanner.unload() | ||
| self._nvr_thread.stop() # indirectly calls self.stop thru stop_target |
There was a problem hiding this comment.
The new unload() method lacks test coverage. Consider adding tests that verify:
- Event listeners are properly unsubscribed
- Frame scanners are unloaded correctly
- The NVR thread is stopped
This is important for ensuring the config reload functionality works correctly and doesn't leave dangling resources.
| def unload(self): | ||
| """Unload entity.""" | ||
| for unsubscribe in self._event_listeners: | ||
| unsubscribe() |
There was a problem hiding this comment.
The new unload() method in the Entity base class lacks test coverage. This is a critical piece of functionality for config reload. Consider adding tests to verify that entities properly clean up their event listeners when unloaded.
1 participant
No description provided.