Skip to content

[ITEP-81197] Publish to scene topics when there are no detections#591

Merged
dpitulax merged 10 commits intorelease-2025.2from
empty-detections
Nov 21, 2025
Merged

[ITEP-81197] Publish to scene topics when there are no detections#591
dpitulax merged 10 commits intorelease-2025.2from
empty-detections

Conversation

@jakubsikorski
Copy link
Copy Markdown
Contributor

@jakubsikorski jakubsikorski commented Nov 17, 2025

📝 Description

ITEP-81197

Detections are no longer hanging on the scene UI and disappear as no detections occur.

Recording 2025-11-18 150125

✨ Type of Change

Select the type of change your PR introduces:

  • 🐞 Bug fix – Non-breaking change which fixes an issue
  • 🚀 New feature – Non-breaking change which adds functionality
  • 🔨 Refactor – Non-breaking change which refactors the code base
  • 💥 Breaking change – Changes that break existing functionality
  • 📚 Documentation update
  • 🔒 Security update
  • 🧪 Tests
  • 🚂 CI

🧪 Testing Scenarios

Describe how the changes were tested and how reviewers can test them too:

  • ✅ Tested manually
  • 🤖 Ran automated end-to-end tests

✅ Checklist

Before submitting the PR, ensure the following:

  • 🔍 PR title is clear and descriptive
  • 📝 For internal contributors: If applicable, include the JIRA ticket number (e.g., ITEP-123456) in the PR title. Do not include full URLs
  • 💬 I have commented my code, especially in hard-to-understand areas
  • 📄 I have made corresponding changes to the documentation
  • ✅ I have added tests that prove my fix is effective or my feature works

@jakubsikorski jakubsikorski changed the base branch from main to release-2025.2 November 17, 2025 15:12
@jakubsikorski jakubsikorski changed the title Publish to scene topics when there are no detections [ITEP-81197] Publish to scene topics when there are no detections Nov 18, 2025
@jakubsikorski jakubsikorski marked this pull request as ready for review November 18, 2025 14:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where detection objects were persisting in the scene UI when no new detections occurred. The change ensures that when a message contains no detection types, empty arrays are explicitly added for all tracked types before processing, which allows the tracker to properly clear stale detections.

  • Adds logic to handle messages with no detections by populating empty arrays for all tracked detection types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +411 to +412
detection_types = list(scene.tracker.trackers.keys())
for dtype in detection_types:
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable detection_types is reassigned from an empty list to a populated list, but this reassignment only affects the local scope. If detection_types is used elsewhere in the function after this block, the reassignment could lead to unexpected behavior. Consider using a different variable name (e.g., tracked_types) for the list of tracker keys to avoid confusion and maintain clarity about which detection types are being referenced.

Suggested change
detection_types = list(scene.tracker.trackers.keys())
for dtype in detection_types:
tracked_types = list(scene.tracker.trackers.keys())
for dtype in tracked_types:

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing it this way would be confusing. detection_types can be (line 400) a reference to the dictionary keys, not a copy. Modifying jdata (referenced keys) will also modify detection_types because it's a dictionary view.

The explicit reassignment makes this side effect visible without requiring readers to understand dictionary view semantics.

Copy link
Copy Markdown
Contributor

@rawatts10 rawatts10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified working, so approved. I don't really like the idea of caching past tracked_types, but because we publish per-class topics it may be necessary. We should revisit this in the future, particularly since there are other edge cases that should be considered (e.g. cameras going down -- if no frames are coming from the camera then the last detection stays in the scene). This is something we can take up with TTL and persistence on the scene.

@dpitulax dpitulax enabled auto-merge (squash) November 21, 2025 03:27
@dpitulax dpitulax merged commit 183b691 into release-2025.2 Nov 21, 2025
28 of 29 checks passed
@dpitulax dpitulax deleted the empty-detections branch November 21, 2025 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants