Skip to content

Controller breakdown#1271

Draft
saratpoluri wants to merge 29 commits into
mainfrom
controller-breakdown
Draft

Controller breakdown#1271
saratpoluri wants to merge 29 commits into
mainfrom
controller-breakdown

Conversation

@saratpoluri
Copy link
Copy Markdown
Contributor

📝 Description

Provide a clear summary of the changes and the context behind them. Describe what was changed, why it was needed, and how the changes address the issue or add value.

✨ 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

Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md Outdated

### MOT Tracking

- **Role**: perform 2D->3D projection and 3D Multi-Object-Tracking
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.

The projection should be its own service. It has complex logic about how we place the object on the surface, raycasting, heuristic based position adjustment, correcting depth inaccuracies in 3D Object Detections etc.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Currently this is implemented in the Tracker, but sure let's break it down as a separate entity here.

The main question is whether the complex logic itself justifies making it a service on its own (which requires maintaining its own API configuration, deployment, introduces latency etc.) and whether benefits (e.g. reuse) outbalance the costs.

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.

Same logic applies for the scene hierarchy service. In the case of hierarchy, latency may be less of a factor but rest is true.

Separate service for projection enables separation of concerns and each to evolve independently. Additionally, heuristics based on object library settings is a responsibility of projection logic. Not of tracker. Tracker input becomes well defined - objects positioned in the common coordinate system.

Downside is latency. We will need to see what the grpc inter service communication latency is. This would not be a phase 1 implementation but when considering the way forward with what is currently known, this is the right approach.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that same logic applies for separation of scene hierarchy and projection services. We need to make informed data-driven decisions for both of them, so it would be good to also gather enough experimental data if needed. I can recall that Jozef did JSON vs Protobuf serialization/deserialization benchmarking for Tracker Service PoC. It improved latency by 43% (below 1ms). We would need to consider MQTT throughput capacity and measure also gRPC vs MQTT.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Projection added as a separate entity in 0d2fc64

Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md
Comment thread docs/adr/controller-breakdown.mm.md
Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md
Comment thread docs/adr/controller-breakdown.mm.md Outdated
Comment thread docs/adr/controller-breakdown.mm.md
Comment thread docs/adr/controller-breakdown.mm.md
Comment thread docs/adr/controller-breakdown.mm.md
Comment thread docs/adr/controller-breakdown.mm.md
@tdorauintc
Copy link
Copy Markdown
Contributor

Do not merge. Used only for asynchronous discussion.

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.

3 participants