[issue/1002] Fix and enhance singleton sensor tagging with exposure analytics and cache persistence#1029
[issue/1002] Fix and enhance singleton sensor tagging with exposure analytics and cache persistence#1029
Conversation
- Environmental sensors: ambient readings with exposure tracking - Attribute sensors: persistent object-specific events - Fixed singleton_type database-to-runtime connection - Fixed scene-wide and circle region geometry - Fixed region cleanup outside debounce - Value-change optimization for memory efficiency
This commit addresses multiple issues discovered during testing of the singleton sensor implementation on Feb 17, 2026: Event Publishing Fixes: - Fix KeyError crash in _buildEnteredObjsList when sensor objects not in detections_dict (occurs during sensor MQTT value events) - Fix empty entered/exited arrays by clearing stale events after publishing while preserving sensor 'value' events - Remove redundant sensor "value" event publishing (sensor data already included in object entry/exit events via sensors field) Exposure Calculation Fixes: - Set scene.when timestamp in all message handlers (handleSensorMessage, handleMovingObjectMessage, handleSceneDataMessage) to enable exposure calculation in detections_builder - Use epoch time directly instead of string conversion when initializing sensor state to avoid precision loss in exposure calculations - Exposure now correctly calculates as: value × (exit_time - entry_time) Sensor Cache Resilience: - Preserve sensor cache (value, lastValue, lastWhen) during region geometry updates in scene._updateRegions() - Preserve sensor cache across scene invalidation/recreation in cache_manager by storing old cache during invalidate() and restoring sensor values when scene is deserialized - Preserve region state (entered, exited, objects, when) during updates Testing verified: - Objects entering/exiting sensor regions receive correct sensor data - Exposure accumulation is precise (matches manual calculation) - Sensor cache persists through all geometry changes (circle ↔ polygon) - No crashes during sensor MQTT message handling - Event data includes correct entered/exited object lists Files modified: - controller/src/controller/scene.py - controller/src/controller/scene_controller.py - controller/src/controller/cache_manager.py
There was a problem hiding this comment.
Pull request overview
This PR restores and enhances singleton sensor tagging for tracked objects in the SceneScape controller. It addresses issue #1002 where sensors were not properly tagging objects with data. The implementation adds support for two sensor types: environmental sensors (scene-wide ambient conditions like temperature, with cumulative exposure tracking via trapezoid integration) and attribute sensors (event-based data like card readers, with persistent event history).
Changes:
- Restored database-to-runtime linkage for singleton_type field enabling sensor type classification
- Implemented cross-detection-type tagging so sensors work with all object types (person, vehicle, etc.)
- Added sensor cache persistence across geometry changes and scene reloads
- Fixed circular region geometry handling and scene-wide sensor support
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| scene_common/src/scene_common/geometry.py | Fixes circle region handling by checking explicit area type before inferring from hasPointsArray |
| manager/src/django/models.py | Passes singleton_type from database to runtime Region objects |
| controller/src/controller/scene_controller.py | Adds include_sensors parameter to control sensor data publishing on different topics (full-rate vs regulated/external) |
| controller/src/controller/scene.py | Core sensor processing logic with exposure calculation, cross-detection-type support, and sensor state initialization |
| controller/src/controller/moving_object.py | Refactors ChainData to use separate dicts for environmental and attribute sensors instead of flat sensors dict |
| controller/src/controller/detections_builder.py | Builds structured sensor output with values and exposure for environmental sensors, values only for attribute sensors |
| controller/src/controller/cache_manager.py | Preserves sensor cache values (value, lastValue, lastWhen) across scene refreshes |
Convert cur_value to float once at the beginning to ensure consistent type comparison when checking if sensor values have changed. This prevents false positives/negatives in value comparison that could occur when mixing string and numeric types in the readings array.
Address code quality and robustness concerns identified in PR review: - Type consistency: Convert sensor values to appropriate types (float for environmental, string for attribute) once and use consistently throughout - Negative exposure prevention: Add dt > 0 checks in all exposure calculations to guard against out-of-order messages or clock skew - Analytics-only mode: Add tracker null check to prevent crashes when tracker is not initialized - Circle region validation: Improve error messages to distinguish missing vs invalid center values - Docstring accuracy: Update _clearSensorValuesOnExit to reflect actual behavior - Cache restoration: Use sentinel value to properly handle None as valid cached value vs attribute not existing - Performance optimization: Skip redundant isPointWithin checks for scene-wide sensors, remove unused objects_by_type tracking - Scene.when initialization: Add explicit None initialization in __init__ - Observability: Add debug logging for sensor cache restoration - TODO comments: Document unbounded cache growth and performance optimization opportunities for future work
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
a9e45bb to
d012fd1
Compare
- Remove all exposure tracking and calculation code - Environmental sensors now only store timestamped readings - Simplify sensor state structure (remove exposure fields) - Remove unused scene.when attribute - Update comments to reflect simplified sensor data model - Sensor output now contains only 'values' array without 'exposure' field
- Creates tc_singleton_sensor_tagging.py with 5 test scenarios: 1. Environmental sensor entry with cached reading 2. Environmental sensor exit with reading removal 3. Environmental sensor re-entry with cache 4. Attribute sensor entry/exit with persistence 5. Attribute sensor re-entry without cache (event-based only) - Adds make target 'singleton-sensor-tagging' to Makefile.functional - Test validates simplified sensor model (timestamped readings only) - Auto-cleanup of existing sensors before test and after completion
|
|
||
| # Initialize sensor state based on type | ||
| if region.singleton_type == "environmental": | ||
| # Use 'now' directly (already epoch time) to avoid precision loss from string conversion |
There was a problem hiding this comment.
Orphaned comment at line 589 states "Use 'now' directly (already epoch time) to avoid precision loss from string conversion" but there is no corresponding code that uses 'now'. The code at line 594 converts region.lastWhen to ISO string format using get_iso_time(region.lastWhen), which is the expected behavior for timestamp storage. This comment appears to be either misplaced or a leftover from a previous implementation. Consider removing this comment or clarifying its intent.
| # Use 'now' directly (already epoch time) to avoid precision loss from string conversion | |
| # Initialize environmental sensor state for this object, using cached region value if available |
| scene.last_published_detection[otype] = get_epoch_time() | ||
|
|
||
| # Rebuild detections list with sensor data included | ||
| jdata = jdata_base.copy() |
There was a problem hiding this comment.
Shallow copy of jdata_base could cause unintended mutation. At line 190, jdata_base.copy() creates a shallow copy, meaning nested mutable objects (lists, dicts) are still shared between the original and the copy.
When jdata['objects'] is reassigned at line 191, this is safe because it replaces the reference. However, if jdata_base contains other mutable nested structures that are modified later, both the original and copy would be affected.
While this may not cause issues in the current code flow (since the original jdata is not used after this point), it's safer to use jdata_base.copy() only if you're certain no nested structures will be mutated, or use copy.deepcopy(jdata_base) for complete isolation. Consider adding a comment explaining why shallow copy is sufficient here.
| if hasattr(region, 'exited'): | ||
| for detectionType in region.exited: | ||
| for exit_data in region.exited[detectionType]: | ||
| obj = exit_data[0] | ||
| if region.singleton_type == "environmental": | ||
| obj.chain_data.sensors.pop(region_name, None) | ||
| # Cleanup is now handled in _updateRegionEvents | ||
| # Environmental sensor state is already removed | ||
| # Attribute sensor events are intentionally preserved |
There was a problem hiding this comment.
Dead code: the nested loop at lines 377-382 iterates through region.exited but performs no operations - the loop body contains only comments. Since the comments indicate cleanup is now handled in _updateRegionEvents, this entire nested loop structure (lines 376-382) serves no purpose and should be removed for code clarity.
The method can be simplified to just clear the event arrays:
for event_type in scene.events:
for region_name, region in scene.events[event_type]:
region.exited = {}
region.entered = {}| event_data['objects'] = list(detections_dict.values()) | ||
| return detections_dict, num_objects |
There was a problem hiding this comment.
The method _buildAllRegionObjsList at line 331 (within the event_data assignment) calls buildDetectionsDict without the include_sensors parameter, which defaults to False. This means the detections_dict returned does not include sensor data.
However, in _buildEnteredObjsList at line 343, objects from this dict are added directly to the 'entered' array. This results in sensor data being missing from entered objects that are already in region.objects. Only objects that are NOT in detections_dict (handled as fallback in lines 348-350) will have sensor data included.
This creates an inconsistency where some objects in the 'entered' array have sensor data and others don't, depending on whether they were already tracked in the region. The fix is to pass include_sensors=True to buildDetectionsDict at line 331.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
This can't go in in RC1. |
|
@dmytroye : please test analytics only mode on this branch. |
📝 Description
This PR restores and enhances singleton sensor tagging for tracked objects, addressing multiple issues that broke the feature. Enables two types of sensors:
Environmental Sensors: Scene-wide sensors (temperature, noise, air quality) that track timestamped readings. Objects maintain a history of sensor values with timestamps while in the sensor region.
Attribute Sensors: Event-based sensors (card readers, barcode, weight, license plate) that tag objects with timestamped attribute values when MQTT messages arrive.
Key Features
[["2026-02-18T22:06:47.652Z", 20.5]])Major Fixes
Additional Bug Fixes
_buildEnteredObjsListwhen sensor objects not in detections_dictArchitecture Simplification
[(timestamp, value), ...]arraysscene.whenattribute and related tracking codevaluesarray without complexity of exposure trackingKnown Limitations
env_sensor_state[sensor_id]['readings']andattr_sensor_events[sensor_id]). For long-running objects that remain in sensor regions with frequent updates, these lists can grow indefinitely. For example, an object in a sensor for 24 hours with readings every second would accumulate 86,400 entries. The timestamp update optimization helps when values don't change, but frequent value changes can still cause memory exhaustion. Future work should implement bounded cache with FIFO eviction, time-based cleanup, or periodic consolidation. TODO comments added in code.Fixes #1002
✨ Type of Change
🧪 Testing Scenarios
✅ Checklist