Ivan/feat/markermerge#2114
Conversation
Skip the "Select device" prompt when discovery returns exactly one robot. Multi-device path still prompts.
Greptile SummaryThis PR adds ArUco marker detection with TF pose publication (
Confidence Score: 3/5Two bugs in memory2/module.py need fixing before merge: the logger warning silently drops every TF-miss event (wrong argument count), and all recorded messages now carry wall-clock time instead of the message's own timestamp, which corrupts replay. The memory2 recorder has two regressions introduced in this PR: the logger format-string mismatch makes TF-failure diagnostics invisible, and unconditionally using time.time() instead of the message timestamp causes every replayed stream to carry wrong absolute times. Both affect the recording path that the PR explicitly exercises. The rest of the changes (MarkerTfModule, cameracalibrate, lidar control, TF identity shortcut) look correct in isolation. dimos/memory2/module.py — both the logger format-string argument count and the ts=time.time() call need to be revisited. Important Files Changed
Sequence DiagramsequenceDiagram
participant Cam as Camera Stream
participant MTM as MarkerTfModule
participant TF as MultiTBuffer
participant TFPub as TF Publisher
Cam->>MTM: color_image (Image)
Cam->>MTM: camera_info (CameraInfo)
MTM->>TF: get(world, base_link, ts, tol)
TF-->>MTM: t_world_base (Transform)
MTM->>TF: get(base_link, optical, ts, tol)
TF-->>MTM: t_base_optical (Transform)
Note over MTM: ArucoDetector.detectMarkers(gray)
Note over MTM: solvePnP → rvec, tvec per marker
MTM->>MTM: "t_optical_marker = rvec_tvec_to_transform(...)"
MTM->>MTM: "t_base_marker = t_base_optical + t_optical_marker"
MTM->>MTM: "t_world_marker = t_world_base + t_base_marker"
MTM->>TFPub: "publish(world→markers, markers→marker_{id}, ...)"
Reviews (2): Last reviewed commit: "recording" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| getattr(msg, "ts", None), | ||
| ) | ||
| stream.append(msg, ts=ts, pose=pose) | ||
| stream.append(msg, ts=time.time(), pose=pose) |
There was a problem hiding this comment.
time.time() (wall-clock) instead of the message's own timestamp means that during replay, every stored message will carry the current real time rather than the replayed recording time. Downstream time-series queries against the stream will then see wrong absolute timestamps. The previous ts already fell back to time.time() when absent, so the old behaviour was safe for live use while preserving replay timestamps.
| stream.append(msg, ts=time.time(), pose=pose) | |
| stream.append(msg, ts=ts, pose=pose) |
| WavefrontFrontierExplorer.blueprint(), | ||
| PatrollingModule.blueprint(), | ||
| MovementManager.blueprint(), | ||
| MarkerTfModule.blueprint(marker_length_m=0.1), |
There was a problem hiding this comment.
desk_marker_tf or the 18 cm example in the module docstring), pose estimates will be off by the ratio of the expected vs. actual size. Consider surfacing this as a global-config knob or at least documenting the assumed physical size prominently.
| MarkerTfModule.blueprint(marker_length_m=0.1), | |
| MarkerTfModule.blueprint(marker_length_m=0.1), # physical marker edge length – update if markers differ |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
1 participant
recording of real go2 looking at markers
dimos --replay --replay-db markers_go2 run unitree-go2