feat(hosted-teleop): auto-select operator cockpit from robot kind#3120
feat(hosted-teleop): auto-select operator cockpit from robot kind#3120ruthwikdasyam wants to merge 5 commits into
Conversation
Command modules declare ROBOT_TYPE and push it to the broker provider at init (set_robot_type); the session-create POST carries it so the operator dashboard opens the right cockpit. No core coordination changes.
5292eb7 to
032e679
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #3120 +/- ##
==========================================
+ Coverage 73.23% 73.32% +0.09%
==========================================
Files 1037 1037
Lines 93895 94281 +386
Branches 8528 8638 +110
==========================================
+ Hits 68761 69132 +371
- Misses 22792 22800 +8
- Partials 2342 2349 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR adds broker-configured cockpit selection for hosted teleoperation. The main changes are:
Confidence Score: 5/5No additional blocking issue was found in this review.
Important Files Changed
Reviews (3): Last reviewed commit: "hosted teleop: robot_type as a BrokerCon..." | Re-trigger Greptile |
| def test_init_declares_robot_type_to_broker(module: ArmCommandModule) -> None: | ||
| """__init__ pushes ROBOT_TYPE to the shared broker provider, which sends it | ||
| in the session-create POST so the operator dashboard opens the arm cockpit.""" | ||
| from dimos.protocol.pubsub.impl.webrtc.providers.broker import BrokerProvider |
| @classmethod | ||
| def set_robot_type(cls, robot_type: str) -> None: | ||
| """Declare the robot kind sent to the broker at session create, so the | ||
| operator UI auto-selects the cockpit. Called by the hosted command module |
There was a problem hiding this comment.
Wait, you're calling it the cockpit too? I picked that name we don't confuse the two systems 😅 (hence why I renamed the broker to "relay")
|
|
||
| def __init__(self, **kwargs: Any) -> None: | ||
| super().__init__(**kwargs) | ||
| BrokerProvider.set_robot_type(self.ROBOT_TYPE) |
There was a problem hiding this comment.
Hm, I don't like how initializing this class modifies a global in BrokerProvider.
| # Robot kind for the create-session POST, declared by the command module at | ||
| # init (one session + one kind per process). No default: _require_robot_type | ||
| # raises rather than guess. | ||
| _robot_type: ClassVar[str | None] = None |
There was a problem hiding this comment.
I don't like using class variables since they're essentially namespaced globals. They might be justifiable in normal Python, but with the way we use processes for Python modules it will probably lead to issues.
Contribution path
Problem
The hosted operator UI required a manual Go2/Arm toggle before connecting - nothing on the robot side told the broker which cockpit to open, so an operator could connect an arm and land on the Go2 view.
Solution
Each hosted command module declares its cockpit kind and pushes it to the broker at init
robot_type.py - a
StrEnumof cockpit kinds (go2,arm); the value is the wire string. Kept in the app layer, not the transport (BrokerConfigstays a plainstr), so the generic provider doesn't depend on this enum.CommandModule declare
ROBOT_TYPEand callBrokerProvider.set_robot_type()at init.the session POST sends the declared kind; no silent default (raises if undeclared), and re-declaring a conflicting kind raises (two command modules in one process would open the wrong cockpit).
How to Test
Connect from the operator dashboard — the arm cockpit opens without the manual toggle. (
teleop-hosted-xarm7opens the arm cockpit the same way.)AI assistance
Claude Code (Fable 5 / Opus 4.8) - used for research, refactoring, review
Checklist