[BTB-455] feat. dimos to battlebang compatible#3
Conversation
📝 Walkthrough변경 사항 분석Walkthrough이 PR은 ROS 감지 메시지의 class_id 인코딩을 개선하고, 새로운 Battlebang WebRTC 브리지를 추가하며, ROS 타입 강제 변환 로직을 강화하고, 웹소켓 종료를 안전하게 처리합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Detection2D 앱
participant Det as Detection2DBBox
participant ROS as ROS ObjectHypothesis
participant Conv as 변환 로직
App->>Det: Detection2DBBox 생성<br/>(class_id=5, name="person")
Det->>Det: _encode_ros_class_id(5, "person")<br/>→ "5:person"
Det->>ROS: to_ros_detection2d() 호출
ROS->>ROS: ObjectHypothesis.class_id = "5:person"
ROS-->>App: ROS Detection2D 반환
App->>Conv: from_ros_detection2d() 호출
Conv->>Det: _decode_ros_class_id("5:person")
Det->>Det: 파싱: class_id=5, name="person"
Conv->>Det: Detection2DBBox 재구성
Det-->>App: 원본과 동일한<br/>Detection2DBBox 반환
sequenceDiagram
participant ROS as ROS 토픽
participant Bridge as BattlebangWebrtcBridge
participant RPC as GO2Connection RPC
participant GO2 as GO2 유닛
ROS->>Bridge: webrtc_req 메시지 도착
Bridge->>Bridge: _parse_parameter()로<br/>페이로드 파싱
Bridge->>Bridge: topic/api_id 검증
Bridge->>RPC: publish_request(payload)
RPC->>GO2: WebRTC 요청 전송
GO2-->>RPC: 응답
RPC-->>Bridge: 완료
ROS->>Bridge: cmd_vel Twist 도착
Bridge->>Bridge: ROS Twist→<br/>DimOS 표현 변환
Bridge->>RPC: move(velocity)
RPC->>GO2: 이동 명령 실행
GO2-->>RPC: 확인
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 Dimos 시스템과 Battlebang 간의 시뮬레이션 통합을 목표로 합니다. 주요 변경 사항은 Battlebang이 사람을 감지하고 행동을 실행할 수 있도록 ROS2 토픽을 통해 센서 데이터를 발행하는 기능, Dimos의 이동 명령을 Battlebang의 twist mux를 통해 라우팅하는 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR integrates the dimos and battlebang systems within a simulation environment, adapting dimos's object detection, TF, and movement commands for battlebang compatibility using ROS2 topics, and controlling movement via battlebang's twist_mux. It also bridges the webrtc_req topic to transmit API commands. A critical security concern has been identified: the newly introduced bridge between ROS topics and the robot's WebRTC API lacks access control, potentially allowing arbitrary API calls to the robot from the ROS network. Additionally, improvements are suggested for exception handling in the BattlebangWebrtcBridge module and the validation logic of Detection2DPerson.
| logger.warning("[BattlebangWebrtcBridge] dropping webrtc request: empty topic") | ||
| return | ||
|
|
||
| api_id = int(getattr(msg, "api_id", -1)) |
There was a problem hiding this comment.
getattr(msg, "api_id", -1)의 결과를 int()로 변환할 때 ValueError가 발생할 수 있습니다. 예를 들어, api_id 필드가 숫자로 변환할 수 없는 문자열(예: "abc")일 경우 프로그램이 비정상적으로 종료될 수 있습니다. try-except 블록을 사용하여 ValueError를 처리하여 안정성을 높이는 것이 좋습니다.
| api_id = int(getattr(msg, "api_id", -1)) | |
| api_id_raw = getattr(msg, "api_id", -1) | |
| try: | |
| api_id = int(api_id_raw) | |
| except (ValueError, TypeError): | |
| logger.warning( | |
| f"[BattlebangWebrtcBridge] dropping webrtc request: invalid api_id format '{api_id_raw}'" | |
| ) | |
| return |
| payload["parameter"] = parameter | ||
|
|
||
| try: | ||
| self._publish_request_rpc(topic, payload) |
There was a problem hiding this comment.
The _on_webrtc_req_raw method forwards ROS messages from the /webrtc_req topic to the robot's WebRTC API without any validation or authorization. This allows any user or process on the ROS network to call arbitrary API methods on the robot, potentially leading to unauthorized control or configuration changes. It is recommended to implement a whitelist of allowed topics or add an authorization check before forwarding the request.
| valid_keypoints = sum(1 for score in self.keypoint_scores if score > 0.5) | ||
| return valid_keypoints >= 1 |
There was a problem hiding this comment.
is_valid 메서드의 유효성 검사 조건이 크게 완화되었습니다. 키포인트 신뢰도 임계값이 0.8에서 0.5로, 필요한 키포인트 수가 5개에서 1개로 줄었습니다. 이로 인해 단 하나의 키포인트(예: 코)만 50% 신뢰도로 탐지되어도 유효한 사람으로 간주되어, 오탐(false positive)의 가능성이 크게 높아집니다. 시뮬레이션 환경을 위한 의도적인 조정일 수 있으나, 시스템의 안정성을 위해 조건을 조금 더 강화하는 것을 고려해 보시는 것이 좋겠습니다.
| valid_keypoints = sum(1 for score in self.keypoint_scores if score > 0.5) | |
| return valid_keypoints >= 1 | |
| valid_keypoints = sum(1 for score in self.keypoint_scores if score > 0.6) | |
| return valid_keypoints >= 3 |
There was a problem hiding this comment.
시뮬레이션 상에서 사람을 detection하기 위해 검사조건을 느슨하게 변경하엿습니다. 실제 go2에서 사람을 detection할때에 다시 조정할 필요가 있습니다.
| from go2_interfaces.msg import WebRtcReq # type: ignore[import-not-found] | ||
|
|
||
| return WebRtcReq | ||
| except Exception: |
There was a problem hiding this comment.
except Exception:은 너무 광범위한 예외 처리입니다. from go2_interfaces.msg import WebRtcReq 구문에서 발생할 수 있는 주된 예외는 ImportError이므로, 더 구체적인 예외를 명시하여 코드의 의도를 명확히 하고 예상치 못한 다른 예외가 숨겨지는 것을 방지하는 것이 좋습니다.
| except Exception: | |
| except ImportError: |
|
|
||
| try: | ||
| return json.loads(parameter) | ||
| except Exception: |
There was a problem hiding this comment.
except Exception:은 너무 광범위한 예외 처리입니다. json.loads()에서 발생할 수 있는 주된 예외는 json.JSONDecodeError이므로, 더 구체적인 예외를 명시하여 코드의 의도를 명확히 하고 예상치 못한 다른 예외가 숨겨지는 것을 방지하는 것이 좋습니다.
| except Exception: | |
| except json.JSONDecodeError: |
| if isinstance(class_id_raw, str): | ||
| class_id = int(class_id_raw) if class_id_raw.isdigit() else 0 | ||
| else: | ||
| class_id = int(class_id_raw) |
There was a problem hiding this comment.
if-else 조건문을 사용하지 않도록 하시죠. ros 에서 vision msg 에서 class_id 는 string 으로 확장성있게 변경이 되었다고 합니다. (uuid 나 namespace 적용을 위해)
따라서 string 으로 바꾸시는건 맞지만 단순히 int index 번호를 가지로 string 으로 변환하지는 않고 {class_id}:{name} 로 id 를 만들어서 주입해주시길 바랍니다
There was a problem hiding this comment.
해당형식으로 id를 변환하였습니다!
There was a problem hiding this comment.
해당 파일은 battlebang 레포로 이관 예정
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
dimos/protocol/pubsub/impl/rospubsub_conversion.py (1)
63-72: 현재는string타입만 강제 변환하지만, 향후 다른 타입 불일치가 발생할 수 있습니다.현재 구현은
class_id와 같은 특정 케이스를 해결하지만, ROS의 다른 타입 불일치(예:float32vsfloat64,int32vsint64)가 발생할 경우 확장이 필요할 수 있습니다. 디버깅 편의를 위해 강제 변환이 발생했을 때 디버그 로그를 추가하는 것을 고려해 보세요.🔧 선택적 개선안: 강제 변환 시 디버그 로깅 추가
+import logging + +_logger = logging.getLogger(__name__) + def _coerce_lcm_scalar_to_ros_type(value: Any, ros_type_hint: str) -> Any: """Coerce scalar values to match ROS runtime field assertions. ROS Python message setters perform strict runtime type checks. Some LCM-backed messages can carry numerics in fields that ROS expects as strings (e.g. vision_msgs/ObjectHypothesis.class_id). Coerce those primitive mismatches here. """ if ros_type_hint == "string" and not isinstance(value, str): + _logger.debug("Coercing %r to string for ROS field (hint=%s)", value, ros_type_hint) return str(value) return value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dimos/protocol/pubsub/impl/rospubsub_conversion.py` around lines 63 - 72, The helper _coerce_lcm_scalar_to_ros_type currently only coerces non-str to string; add a debug log when any coercion occurs (e.g., include the function name, original value, original type, and target ros_type_hint) to aid debugging, and prepare the function to easily extend coercion rules for numeric mismatches later (keep branching centralized in _coerce_lcm_scalar_to_ros_type so adding float/int widening like float32->float64 or int32->int64 is straightforward). Use the module logger (or pass a logger) to emit the debug message right before returning the coerced value so callers like rospubsub conversion paths can see what was changed.dimos/perception/detection/type/detection2d/person.py (1)
223-224: 유효성 임계값은 상수/Config로 분리하는 편이 좋습니다.Line 223-224는 튜닝 포인트(점수 기준, 최소 키포인트 수)가 하드코딩되어 있어 모델/환경 변경 시 코드 수정이 필요합니다. 클래스 상수나
Config로 올리면 실험/운영 조정이 쉬워집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dimos/perception/detection/type/detection2d/person.py` around lines 223 - 224, The hardcoded keypoint validity check in the method using self.keypoint_scores (currently "score > 0.5" and "valid_keypoints >= 1") should be replaced with configurable thresholds: add class-level constants (e.g., KEYPOINT_SCORE_THRESHOLD, MIN_VALID_KEYPOINTS) or read values from a Config object and use those symbols in the validity logic (replace the literal 0.5 and 1 with the new constants or config fields) so you can tune score and minimum-keypoint criteria without changing the code.dimos/robot/unitree/go2/blueprints/smart/unitree_go2_battlebang_bridge.py (1)
40-42: 네임스페이스robot0하드코딩은 재사용성을 제한합니다.실기/멀티로봇 환경에서 동일 블루프린트를 재사용하려면 네임스페이스를 외부 설정으로 받는 편이 안전합니다.
♻️ 제안 변경
+import os + -_GO2_ROS_KWARGS = {"ros_robot_namespace": "robot0"} -_ROS_ROBOT_NAMESPACE = _GO2_ROS_KWARGS["ros_robot_namespace"] +_ROS_ROBOT_NAMESPACE = os.getenv("ROS_ROBOT_NAMESPACE", "robot0").strip("/") or "robot0" +_GO2_ROS_KWARGS = {"ros_robot_namespace": _ROS_ROBOT_NAMESPACE}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dimos/robot/unitree/go2/blueprints/smart/unitree_go2_battlebang_bridge.py` around lines 40 - 42, Replace the hardcoded namespace by making _GO2_ROS_KWARGS["ros_robot_namespace"] configurable: read the namespace from an external parameter (environment variable or a function/class init arg) with a default of "robot0", and set _ROS_ROBOT_NAMESPACE from that value instead of the literal; update any references to _GO2_ROS_KWARGS and _ROS_ROBOT_NAMESPACE so callers can pass a different namespace (e.g., expose a parameter in the blueprint constructor or a factory function and fall back to os.environ.get("ROS_ROBOT_NAMESPACE", "robot0")).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dimos/perception/detection/type/detection2d/point.py`:
- Line 168: Detection2DPoint currently sends class_id as str(self.class_id)
which doesn't follow the decoder's expected "id:name" format, causing
from_ros_detection2d in dimos/perception/detection/type/detection2d/bbox.py to
lose the original class name; change the serialization in Detection2DPoint (the
place constructing class_id) to emit the combined "id:name" string (e.g.,
f"{self.class_id}:{self.class_name}" or equivalent using the object's class_name
field) so the decoder can restore name and id correctly.
In `@dimos/robot/unitree/go2/battlebang_webrtc_bridge.py`:
- Around line 84-106: Wrap the post-start RPC binding and subscription logic
(the block using get_rpc_calls, self._unsubs.append(...), and logger.info calls)
in a try/except/finally (or try/except that calls self._ros.stop() before
re-raising) so that if any exception occurs after RawROS.start() the ROS
resource is stopped; specifically, after calling RawROS.start() invoke the
subscription/RPC setup inside a try: and in except Exception as e: call
self._ros.stop() (and optionally clear/unsubscribe any partial entries in
self._unsubs) and re-raise the exception to preserve failure behavior—this
ensures RawROS.start() is always paired with RawROS.stop() on initialization
failure when using methods like get_rpc_calls, _on_webrtc_req_raw,
_on_cmd_vel_raw, and subscribing to RawROSTopic.
- Around line 110-114: The cleanup loop that calls each function in self._unsubs
currently swallows exceptions with a bare except/pass; update the except block
in the for unsub in getattr(self, "_unsubs", []) loop to log the failure
including exception details (e.g., using logging.exception or
self.logger.exception) and context about which unsub failed (include repr(unsub)
or its name) so cleanup errors are recorded for postmortem.
---
Nitpick comments:
In `@dimos/perception/detection/type/detection2d/person.py`:
- Around line 223-224: The hardcoded keypoint validity check in the method using
self.keypoint_scores (currently "score > 0.5" and "valid_keypoints >= 1") should
be replaced with configurable thresholds: add class-level constants (e.g.,
KEYPOINT_SCORE_THRESHOLD, MIN_VALID_KEYPOINTS) or read values from a Config
object and use those symbols in the validity logic (replace the literal 0.5 and
1 with the new constants or config fields) so you can tune score and
minimum-keypoint criteria without changing the code.
In `@dimos/protocol/pubsub/impl/rospubsub_conversion.py`:
- Around line 63-72: The helper _coerce_lcm_scalar_to_ros_type currently only
coerces non-str to string; add a debug log when any coercion occurs (e.g.,
include the function name, original value, original type, and target
ros_type_hint) to aid debugging, and prepare the function to easily extend
coercion rules for numeric mismatches later (keep branching centralized in
_coerce_lcm_scalar_to_ros_type so adding float/int widening like
float32->float64 or int32->int64 is straightforward). Use the module logger (or
pass a logger) to emit the debug message right before returning the coerced
value so callers like rospubsub conversion paths can see what was changed.
In `@dimos/robot/unitree/go2/blueprints/smart/unitree_go2_battlebang_bridge.py`:
- Around line 40-42: Replace the hardcoded namespace by making
_GO2_ROS_KWARGS["ros_robot_namespace"] configurable: read the namespace from an
external parameter (environment variable or a function/class init arg) with a
default of "robot0", and set _ROS_ROBOT_NAMESPACE from that value instead of the
literal; update any references to _GO2_ROS_KWARGS and _ROS_ROBOT_NAMESPACE so
callers can pass a different namespace (e.g., expose a parameter in the
blueprint constructor or a factory function and fall back to
os.environ.get("ROS_ROBOT_NAMESPACE", "robot0")).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
Run ID: c0b8fd0d-e45d-41ea-b7be-e4e79071be89
📒 Files selected for processing (14)
dimos/perception/detection/detectors/test_bbox_detectors.pydimos/perception/detection/module2D.pydimos/perception/detection/type/detection2d/bbox.pydimos/perception/detection/type/detection2d/person.pydimos/perception/detection/type/detection2d/point.pydimos/perception/detection/type/detection2d/test_imageDetections2D.pydimos/protocol/pubsub/impl/rospubsub_conversion.pydimos/robot/all_blueprints.pydimos/robot/unitree/go2/battlebang_webrtc_bridge.pydimos/robot/unitree/go2/blueprints/__init__.pydimos/robot/unitree/go2/blueprints/smart/unitree_go2_battlebang_bridge.pydimos/robot/unitree/go2/connection.pydimos/robot/unitree/go2/ros_sim_connection.pydimos/web/websocket_vis/websocket_vis_module.py
| ObjectHypothesisWithPose( | ||
| ObjectHypothesis( | ||
| class_id=self.class_id, | ||
| class_id=str(self.class_id), |
There was a problem hiding this comment.
Detection2DPoint의 ROS class_id 인코딩 포맷이 현재 디코더 규약과 불일치합니다.
Line 168에서 class_id=str(self.class_id)만 보내면, dimos/perception/detection/type/detection2d/bbox.py의 from_ros_detection2d 경로(디코더가 id:name 기대)에서 이름이 복원되지 않고 class_{id}로 대체됩니다. 이로 인해 ROS round-trip 시 클래스명이 손실됩니다.
🔧 제안 수정
- class_id=str(self.class_id),
+ class_id=f"{self.class_id}:{self.name}",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class_id=str(self.class_id), | |
| class_id=f"{self.class_id}:{self.name}", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dimos/perception/detection/type/detection2d/point.py` at line 168,
Detection2DPoint currently sends class_id as str(self.class_id) which doesn't
follow the decoder's expected "id:name" format, causing from_ros_detection2d in
dimos/perception/detection/type/detection2d/bbox.py to lose the original class
name; change the serialization in Detection2DPoint (the place constructing
class_id) to emit the combined "id:name" string (e.g.,
f"{self.class_id}:{self.class_name}" or equivalent using the object's class_name
field) so the decoder can restore name and id correctly.
| self._ros = RawROS(node_name=self.config.ros_node_name) | ||
| self._ros.start() | ||
|
|
||
| if subscribe_webrtc and webrtc_req_type is not None: | ||
| self._publish_request_rpc = self.get_rpc_calls("GO2Connection.publish_request") | ||
| self._unsubs.append( | ||
| self._ros.subscribe( | ||
| RawROSTopic(self.config.webrtc_req_topic, webrtc_req_type), | ||
| self._on_webrtc_req_raw, | ||
| ) | ||
| ) | ||
| logger.info(f"[BattlebangWebrtcBridge] subscribed to {self.config.webrtc_req_topic}") | ||
|
|
||
| if subscribe_cmd_vel: | ||
| self._move_rpc = self.get_rpc_calls("GO2Connection.move") | ||
| ros_twist_type = derive_ros_type(Twist) | ||
| self._unsubs.append( | ||
| self._ros.subscribe( | ||
| RawROSTopic(self.config.cmd_vel_topic, ros_twist_type), | ||
| self._on_cmd_vel_raw, | ||
| ) | ||
| ) | ||
| logger.info(f"[BattlebangWebrtcBridge] subscribed to {self.config.cmd_vel_topic}") |
There was a problem hiding this comment.
초기화 중간 실패 시 ROS 리소스 누수가 발생할 수 있습니다.
RawROS.start() 이후 RPC 바인딩/구독 단계에서 예외가 나면 정리 없이 start()가 종료될 수 있습니다. 실패 경로에서 self._ros.stop() 보장이 필요합니다.
🐛 제안 변경
- self._ros = RawROS(node_name=self.config.ros_node_name)
- self._ros.start()
-
if subscribe_webrtc and webrtc_req_type is not None:
self._publish_request_rpc = self.get_rpc_calls("GO2Connection.publish_request")
- self._unsubs.append(
- self._ros.subscribe(
- RawROSTopic(self.config.webrtc_req_topic, webrtc_req_type),
- self._on_webrtc_req_raw,
- )
- )
- logger.info(f"[BattlebangWebrtcBridge] subscribed to {self.config.webrtc_req_topic}")
if subscribe_cmd_vel:
self._move_rpc = self.get_rpc_calls("GO2Connection.move")
- ros_twist_type = derive_ros_type(Twist)
- self._unsubs.append(
- self._ros.subscribe(
- RawROSTopic(self.config.cmd_vel_topic, ros_twist_type),
- self._on_cmd_vel_raw,
- )
- )
- logger.info(f"[BattlebangWebrtcBridge] subscribed to {self.config.cmd_vel_topic}")
+ self._ros = RawROS(node_name=self.config.ros_node_name)
+ try:
+ self._ros.start()
+
+ if subscribe_webrtc and webrtc_req_type is not None:
+ self._unsubs.append(
+ self._ros.subscribe(
+ RawROSTopic(self.config.webrtc_req_topic, webrtc_req_type),
+ self._on_webrtc_req_raw,
+ )
+ )
+ logger.info(f"[BattlebangWebrtcBridge] subscribed to {self.config.webrtc_req_topic}")
+
+ if subscribe_cmd_vel:
+ ros_twist_type = derive_ros_type(Twist)
+ self._unsubs.append(
+ self._ros.subscribe(
+ RawROSTopic(self.config.cmd_vel_topic, ros_twist_type),
+ self._on_cmd_vel_raw,
+ )
+ )
+ logger.info(f"[BattlebangWebrtcBridge] subscribed to {self.config.cmd_vel_topic}")
+ except Exception:
+ if self._ros is not None:
+ self._ros.stop()
+ self._ros = None
+ raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dimos/robot/unitree/go2/battlebang_webrtc_bridge.py` around lines 84 - 106,
Wrap the post-start RPC binding and subscription logic (the block using
get_rpc_calls, self._unsubs.append(...), and logger.info calls) in a
try/except/finally (or try/except that calls self._ros.stop() before re-raising)
so that if any exception occurs after RawROS.start() the ROS resource is
stopped; specifically, after calling RawROS.start() invoke the subscription/RPC
setup inside a try: and in except Exception as e: call self._ros.stop() (and
optionally clear/unsubscribe any partial entries in self._unsubs) and re-raise
the exception to preserve failure behavior—this ensures RawROS.start() is always
paired with RawROS.stop() on initialization failure when using methods like
get_rpc_calls, _on_webrtc_req_raw, _on_cmd_vel_raw, and subscribing to
RawROSTopic.
| for unsub in getattr(self, "_unsubs", []): | ||
| try: | ||
| unsub() | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
구독 해제 예외를 무시하지 말고 로깅해 주세요.
정리 실패를 pass로 삼키면 운영 중 장애 원인 추적이 어려워집니다.
🛠️ 제안 변경
for unsub in getattr(self, "_unsubs", []):
try:
unsub()
- except Exception:
- pass
+ except Exception as e:
+ logger.exception(f"[BattlebangWebrtcBridge] unsubscribe failed: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for unsub in getattr(self, "_unsubs", []): | |
| try: | |
| unsub() | |
| except Exception: | |
| pass | |
| for unsub in getattr(self, "_unsubs", []): | |
| try: | |
| unsub() | |
| except Exception as e: | |
| logger.exception(f"[BattlebangWebrtcBridge] unsubscribe failed: {e}") |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 113-114: try-except-pass detected, consider logging the exception
(S110)
[warning] 113-113: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dimos/robot/unitree/go2/battlebang_webrtc_bridge.py` around lines 110 - 114,
The cleanup loop that calls each function in self._unsubs currently swallows
exceptions with a bare except/pass; update the except block in the for unsub in
getattr(self, "_unsubs", []) loop to log the failure including exception details
(e.g., using logging.exception or self.logger.exception) and context about which
unsub failed (include repr(unsub) or its name) so cleanup errors are recorded
for postmortem.
| track_id = int(str(ros_det.id)) | ||
| except (TypeError, ValueError): | ||
| track_id = 0 | ||
|
|
There was a problem hiding this comment.
제가 기억하기로는 track_id 는 dimos 에 자체 tracking 을 하기 위한 id 로 string 으로 되어있던데 괜찮은 건가요??
There was a problem hiding this comment.
이 파일 및 battlebang 과 관련된 것들은 더 이상 수정하지 말고 일단 simulation 에서 돌아가게끔 하고 battlebang 이 refactor 완료되면 그 때 하는걸로 하시죠~
2 participants
simulation상에서 dimos와 battlebang을 연결하였습니다.
Uploading 스크린캐스트 03-03-2026 06:10:46 PM.webm…
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
테스트
기타