Conversation
RtspCamera (dimos/hardware/sensors/camera/rtsp): RTSP URL, file or a generated clip in; video (encoded, untouched), color_image and color_jpeg out. PyAV only, url is required, set_video_enabled() and set_jpeg_rate() are RPCs. Blueprint rtsp-camera-vis. SiyiA8Gimbal (dimos/hardware/gimbal/siyi): gimbal tf chain, camera_info (a8_camera_info() is the one source of the A8 intrinsics), aim requests on gimbal_target. ip is a config field with no default; with it set the SIYI SDK client polls the zoom and camera_info is withheld off 1x. New px4 extra: av. Tests: pytest dimos/hardware/gimbal/siyi dimos/hardware/sensors/camera/rtsp -> 37 passed (gimbal 22, camera 15).
sensor_msgs.NavSatFix and sensor_msgs.BatteryState wrap the dimos_lcm types and take their enum values from them. px4_msgs.VehicleStatus is a hand-written LCM type with the wire layout of sensor_msgs/ImuInfo (fingerprint, Header, big-endian fields); its base hash is an arbitrary constant, there is no .lcm schema. The wire helpers are in dimos/msgs/lcm_wire.py. No registry, pyproject or lock change. The producer is Px4DroneConnection in the next commit: its gps, battery and vehicle_status outputs. Tests: pytest dimos/msgs/px4_msgs dimos/msgs/sensor_msgs/test_NavSatFix.py dimos/msgs/sensor_msgs/test_BatteryState.py -> 6 passed (43 with the payload tests below).
dimos/robot/px4: - connection.py: Px4DroneConnection, the one MAVLink link. The vehicle as streams (odometry, imu, gps, battery, gimbal_attitude, vehicle_status) and the operator RPCs takeoff, go_to, land, hold, set_guidance_mode, estop*. No arm, mode or raw-setpoint RPC. - supervisor_core.py: 20 Hz state machine, no I/O. Preflight, RC enable switch, fence and ceiling, pilot override, E-STOP latch, go-to, TELEOP. When the tick stops the setpoints stop and PX4's Offboard-loss failsafe takes over. - mavlink.py: pymavlink socket, vehicle state, PX4 modes, NED/FLU frames. - blueprints.py: px4-basic, px4-drone, px4-sitl, px4-teleop, px4-sitl-teleop. tool_sitl_gate.py runs px4-sitl against PX4 SITL. px4 extra += pymavlink; stubs/pymavlink/mavutil.pyi is extended for mypy. Tests: pytest dimos/robot/px4 -> 103 passed (connection 10, mavlink 21, supervisor_core 4, supervisor_operator 68); 146 with the payload and message tests below.
Px4SkillContainer: takeoff, go_to, land, set_guidance_mode, flight_status. Each skill is the connection RPC plus a wait for the outcome, so the supervisor refuses a skill exactly as it refuses the RPC; a takeover while the hover settles is reported, not read as an arrival. connection_spec.py holds the RPCs the skills call. px4-agentic and px4-sitl-agentic are in blueprints_agentic.py because they need the agents extra. supervisor_core.py gains TAKEOFF_STATES, which the takeoff skill waits on. Tests: pytest dimos/robot/px4/test_skill_container.py -> 13 passed, against a scripted connection plus one contract test against the real Px4DroneConnection and SupervisorCore.status(); pytest dimos/robot/px4 -> 116 passed; 159 with the payload and message tests below.
- dimos/perception/geolocation: pixel to line of sight to ground point, target estimators. - perception_bridge.py: PerceptionBridge turns the selected detection of the upstream Detection2DModule into target_state, target_valid and target_los. - follow.py: YAW_TRACK and FOLLOW guidance. The connection gains the target_* inputs, the gimbal aim path over MAVLink and global_pose. - timebase.py: vehicle clock; PX4's own streams are stamped at the vehicle instant (gimbal_attitude at receipt) and odometry carries the attitude interpolated to the position sample. - sitl.py: FakeA8 and BrightBlobDetector for the simulator twin. - blueprints_follow.py: px4-follow, px4-sitl-follow. tool_follow_gate.py, with an --offline mode that needs no simulator. Tests: pytest dimos/robot/px4 dimos/perception/geolocation -> 156 passed (new: follow 4, timebase 8, perception_bridge 8, geolocation 10, connection +8, mavlink +2); 199 with the payload and message tests below.
github-actions
Bot
added
the
first-time-contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Out of the connection: the supervisor. | ||
| ("supervisor_state", String): _zenoh_transport("supervisor_state", String), | ||
| # Camera and gimbal. | ||
| ("video", CompressedVideo): _zenoh_transport("video", CompressedVideo, latest_wins=True), |
There was a problem hiding this comment.
The H.265 stream sends interdependent units through a latest-wins transport. When the viewer falls behind, discarded units leave it unable to decode the complete stream until a usable keyframe arrives. A stalled-viewer check delivered only units 0 and 24 of 25 and decoded one frame. Video delivery must preserve a decodable sequence before merging.
Artifacts
Offline H.265 transport probe script
- The authored script generated a synthetic clip, published its units over local Zenoh, and decoded received units to compare unstalled and stalled viewer callbacks; it exercises the reported dependency loss.
Unstalled viewer: all video units decode
- The recorded command delivered all 25 H.265 units through local Zenoh and decoded 25 matching frames; the stream remained complete.
Stalled viewer: dependent video units missing
- The recorded command stalled the viewer callback while publishing the same clip and delivered only units 0 and 24, yielding one decoded frame; the stream became incomplete.
| batt = st.batt_pct | ||
| if batt >= 0 and batt < c.min_batt_pct: | ||
| f.append(f"battery {batt}%") |
There was a problem hiding this comment.
When battery status is missing or reports −1, this condition skips the 40% minimum. Both cases passed preflight and issued an arm request after a simulated Offboard acknowledgement, so the supervisor can attempt takeoff without establishing sufficient charge. PX4 arm acceptance was not established. Require a valid battery reading before merging.
Artifacts
Isolated battery preflight test script
- The executed script drives real supervisor methods with recorded actuator calls, separating preflight from later Offboard and arming acknowledgements.
Preflight output with a known 30% battery
- The control run held a 30% reading in PREFLIGHT until timeout and recorded no arm request.
Preflight output with missing and unknown battery readings
- The unknown-reading runs passed preflight and requested arming only after a simulated Offboard acknowledgement, confirming the gate bypass but not PX4 arm acceptance.
| if st.gps is None or st.gps.fix < c.min_fix_type: | ||
| f.append("GPS fix") | ||
| elif not math.isnan(st.gps.eph) and st.gps.eph > c.max_eph_m: | ||
| f.append(f"eph {st.gps.eph:.1f}") |
There was a problem hiding this comment.
A 3D fix with unknown horizontal accuracy (NaN EPH) passes this comparison, and the check does not consider when the fix arrived. A fix last received 120 seconds earlier passed preflight while other inputs remained fresh. Takeoff can therefore proceed without establishing the required current, accurate GPS fix. Enforce both requirements before merging.
Artifacts
Isolated GPS preflight reproduction script
- The authored script feeds a GPS message into VehicleState and runs supervisor preflight with fresh or stale GPS.
Preflight run with fresh GPS and unknown accuracy
- The captured command and output show a fresh 3D fix with NaN accuracy passing preflight and streaming a setpoint.
Preflight run with 120-second-old GPS and unknown accuracy
- The captured command and output show the same preflight and streaming result despite a GPS sample timestamp 120 seconds old.
| # PX4 refuses to arm for Offboard without an absolute position estimate; saying so | ||
| # here beats "arming refused" three seconds later. | ||
| if st.estimator is not None and not st.estimator.position_valid: | ||
| f.append("position estimate not valid") |
There was a problem hiding this comment.
This check rejects invalid flags only when estimator status exists; it neither requires a status nor checks its age. Missing status and an hour-old valid status both passed preflight and produced Offboard and arm requests. The supervisor thus cannot establish that absolute position is currently valid before attempting takeoff. PX4 arm acceptance was not established. Require current valid status before merging.
Artifacts
Offline estimator preflight check script
- The authored script exercises the snapshot and supervisor transitions with a recording actuator, without connecting to an aircraft.
Preflight output with a fresh invalid estimate
- Running the control case left the supervisor in PREFLIGHT with no mode or arm calls, showing that invalid flags are rejected.
Preflight output with missing estimator status
- Running without estimator status reached ARMING and recorded Offboard and arm calls, showing that absence passes the gate.
Preflight output with an hour-old estimator status
- Running with hour-old valid flags reached ARMING and recorded Offboard and arm calls, showing that age is not checked.
| def abort_reason(self, st: VehicleSnapshot) -> tuple[str, Rejection] | None: | ||
| c = self.cfg | ||
| if self._px4_age(st) > c.px4_stale_s: | ||
| return "PX4 heartbeat lost", Rejection.STALE_INPUT | ||
| if not self.enable_switch(st): | ||
| return "enable switch off / RC lost", Rejection.ENABLE_SWITCH_OFF | ||
| # The guidance laws and the fence below fly on this sample; a frozen one blinds both. | ||
| if st.local is None or st.local_age > c.px4_stale_s: | ||
| return "local position lost", Rejection.STALE_INPUT | ||
| batt = st.batt_pct | ||
| if 0 <= batt < c.min_batt_pct - 10: | ||
| return f"battery {batt}%", Rejection.BATTERY | ||
| if st.local and self.takeoff: | ||
| dist = math.hypot(st.local.n - self.takeoff.n, st.local.e - self.takeoff.e) | ||
| alt = self.takeoff.d0 - st.local.d | ||
| if dist > c.geofence_radius_m: | ||
| return f"geofence {dist:.0f} m", Rejection.FENCE | ||
| if alt > c.max_alt_m: | ||
| return f"altitude {alt:.1f} m", Rejection.CEILING | ||
| return None |
There was a problem hiding this comment.
If the estimator reports absolute position invalid while local-position messages remain fresh, this abort check does not react. FOLLOW and GOTO both continued issuing velocity setpoints, so guidance and fence decisions rely on a position estimate PX4 has declared invalid. Stop guidance on invalid estimator status before merging.
Artifacts
Offline supervisor reproduction script
- The authored script feeds estimator flags through the MAVLink state parser and exercises both guidance modes, providing a repeatable isolated check.
Unmodified supervisor with valid and invalid estimator flags
- The command ran against the unmodified product and shows FOLLOW and GOTO still sending velocity setpoints when absolute position is invalid.
Supervisor with a runtime-only estimator guard
- The command applied a guard only to the test instance and shows invalid-estimator cases aborting and requesting Hold while valid controls continue.
| heading = self._heading.at(capture_time) | ||
| pos = self._position | ||
| veh = VehicleGeo( | ||
| yaw_deg=heading["yaw"] if heading else None, | ||
| attitude_age_s=_gap_s(self._heading, capture_time) if heading else math.inf, | ||
| n=pos[0] if pos else None, | ||
| e=pos[1] if pos else None, | ||
| d=pos[2] if pos else None, |
There was a problem hiding this comment.
Heading is looked up at image capture time, but position comes from the latest odometry. When an image is processed after the aircraft moves, this shifts the projected ground target and changes FOLLOW's bearing. With 400 ms of latency and 4 m of movement, the calculated target shifted 4 m east. Use a capture-time position before merging.
Artifacts
Delayed-image moving-aircraft reproduction script
- The authored script sends timestamped odometry and a delayed wire-encoded detection through the bridge, then calculates FOLLOW commands; it also provides a script-only capture-position control.
Shipped behavior with delayed detection
- The executed shipped path used the latest 4 m-east position with capture-time heading and published an east target of 19.894 m.
Capture-position control with delayed detection
- The same executed path with only a script-side capture-position substitution published an east target of 15.894 m and a different FOLLOW bearing.
| def _on_target_valid(self, msg: Bool) -> None: | ||
| self._target_valid = bool(msg.data) |
There was a problem hiding this comment.
Receiving target_valid=False changes this input flag without forwarding the change to guidance. If no subsequent position or line-of-sight message arrives, FOLLOW continues treating the previous target as valid until its position times out. The next supervisor tick still sent a 2.0 m/s pursuit setpoint after invalidation. Forward validity changes immediately before merging.
Artifacts
Offline target-validity contract test
- The authored executable exercises the connection handler, supervisor tick, and recorded actuator setpoints without an aircraft.
FOLLOW setpoint before target invalidation
- The executed baseline records a fresh target and a 2.0 m/s north velocity setpoint.
FOLLOW setpoint after target invalidation
- The executed invalidation run shows velocity continuing despite a false input flag, then changing to position hold after a position update.
| self.relay_once() | ||
| except (av.FFmpegError, OSError, IndexError, ValueError) as exc: | ||
| self._count("video", errors=1) | ||
| logger.warning("camera stream unavailable", url=self._source, error=str(exc)) |
There was a problem hiding this comment.
If the configured RTSP URL contains user:password@host, stream errors and stream-end warnings write that userinfo into application logs; error text can expose it too. This non-blocking concern creates avoidable credential exposure wherever those logs are retained or read.
How this was verified: Both warning paths recorded dummy URL credentials in console and structured log fields.
Artifacts
Dummy-credential RTSP logging test script
- The authored script drives both warning paths and compares current logging with a test-only redaction wrapper, without contacting a camera.
Current logging exposes dummy RTSP credentials
- Running the unchanged warning paths captured the dummy username and password in console output and JSON records.
Test-only redaction removes dummy RTSP credentials
- Running the same paths with a test-only logger wrapper captured warnings without userinfo, demonstrating a comparison rather than a product change.
| zoom = self._sdk.query_zoom() | ||
| if zoom is not None: # a lost reply keeps the last known zoom | ||
| with self._lock: | ||
| self._zoom = zoom |
There was a problem hiding this comment.
Failed zoom polls retain the last successful reading indefinitely. If the camera zoom changes while replies are unavailable, CameraInfo continues publishing newly stamped 1× intrinsics. This non-blocking concern can cause consumers to geolocate detections with the wrong calibration.
Artifacts
Scripted zoom poll and CameraInfo publish probe
- The authored script runs the real poll and publish methods with scripted SDK replies and a simulated zoom change, alongside a fail-closed comparison.
Unmodified gimbal publishes after failed zoom polls
- The executed unmodified path retains zoom 1.0 and publishes on both failed polls despite the simulated physical zoom reaching 2.5×.
Fail-closed comparison withholds CameraInfo
- The executed comparison clears zoom on failed polls and withholds both subsequent CameraInfo messages.
Comments Outside DiffThese findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.
|
This branch has not been deployed
1 participant
What is this feature?
Follow a selected target, on the existing
dimos/perception/detectionstack.dimos/perception/geolocation/: pixel to line of sight to ground intersection, and the target filter. No PX4 or MAVLink in it.PerceptionBridge(dimos/robot/px4/perception_bridge.py): consumesDetection2DArrayfromDetection2DModule(track ids from the detector), publishestarget_state,target_valid,target_los. Selection by RPC (select_track,clear_selection).follow.py(YAW_TRACK,FOLLOW, target-loss ladder), the connection'starget_*inputs, the vehicle-clock timebase (frames and vehicle data on one clock), the gimbal aim path.px4-follow,px4-sitl-follow. SITL runs the realDetection2DModulewith a blob detector, so no GPU or model is needed. Gate:tool_follow_gate.py.Why do we need this?
Target tracking is the drone's main task. This reuses dimOS detection instead of a separate detector.
How to Test
SITL: line-of-sight error 0.00 deg;
YAW_TRACKturns toward the target at the law's rate;FOLLOWflies along the target bearing at the 2 m/s cap; target loss holds position and returns toHOVERafter 5 s. Geometry is ported from code flown on the aircraft; detection with dimOS tracker ids has not flown.Seen while wiring it:
Detection2DBBox.to_ros_detection2dnever setsresults_length(class id and score are lost on the wire).Stack
Five PRs, all against
main; each contains the ones above it. Merge in order. Review only this PR's own commit: 6f630cdWhich issue(s) does this PR close?
None. New platform: PX4 multicopters with a SIYI A8 gimbal camera.
Checklist
🤖 Generated with Claude Code