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.
github-actions
Bot
added
the
first-time-contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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 a password, stream failure logs the URL and potentially unredacted error text; a disconnect also logs the URL. Anyone with access to console or persistent logs can read the password. Redact credentials from both logged fields before merging.
How this was verified: A synthetic password appeared in console and JSON logs when the failure and disconnect paths ran.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
Synthetic RTSP logging check script
- This authored script drives the real relay loop through simulated failure and disconnect branches without real credentials or a camera connection.
- Running the unchanged logging path returned exit code 0 and exposed the synthetic password in both branches.
Stream logs with illustrative runtime redaction
- Running the same branches with a runtime-only redacting logger returned exit code 0 and kept the synthetic password out of both logs.
| # 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.
If congestion drops an H.265 access unit, later units can depend on the missing data. This stream uses droppable delivery, so the operator's video can freeze instead of merely skipping a frame. Preserve the access-unit sequence or provide tested loss recovery before merging.
Artifacts
Exact Python reproduction script
- The authored script generates H.265 camera output, reads the PX4 transport QoS, and decodes intact or loss-injected access units; it is the execution source for both captures.
Decoder output with every access unit delivered
- Running the reproduction script with `--condition intact` decoded all 50 frames without errors; the complete stream remains decodable.
Decoder output after one access unit was discarded
- Running the same script with `--condition loss` discarded access unit 10 and decoded only 10 frames; one loss can freeze the remaining clip.
| if now - self._last_aim_mono < 1.0 / self.config.aim_hz: | ||
| return False | ||
| self._last_aim_mono = now | ||
| self.gimbal_target.publish( |
There was a problem hiding this comment.
aim() and the line-of-sight path publish gimbal_target requests, but the PX4 connection has no matching input or MAVLink command path. An aim request can be reported as published while the physical gimbal does not move. Wire a consumer and command path before merging.
Artifacts
Synthetic gimbal service-contract probe
- The authored script runs the real aim and line-of-sight code against a fake consumer and the checked-out PX4 contract, without aircraft hardware.
Aim requests with a synthetic wired consumer
- The executed control run published two aim requests and delivered both to a fake consumer, showing what a wired path would receive.
Aim requests with the checked-out PX4 connection
- The executed checked-out-contract run published two aim requests but delivered zero commands because PX4 exposes no matching input.
| 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}") | ||
| 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.
Preflight accepts an unknown battery percentage, including when the battery measurement is unavailable, and does not check the age of GPS or battery data. It can proceed toward takeoff without usable current safety measurements. Require valid, fresh measurements before merging.
Artifacts
Synthetic preflight reproduction script
- The authored Python harness runs the real supervisor preflight path against synthetic telemetry and a fake actuator, without contacting hardware.
Current preflight behavior with missing and aged telemetry
- The captured command output shows all four defective telemetry cases entering STREAMING, confirming that current preflight accepts them.
Preflight behavior with an in-memory freshness guard
- The captured command output shows an uncommitted subclass guard retaining all four defective cases in PREFLIGHT while accepting healthy telemetry, demonstrating the comparison without changing production files.
| # HEARTBEAT is 1 Hz and px4_stale_s is 1.0 s, so heartbeat age alone sits on the | ||
| # threshold and a few ms of jitter would abort a flight (seen in SITL). Any message | ||
| # from 1/1 proves the link. | ||
| return min(st.heartbeat_age, st.px4_msg_age) |
There was a problem hiding this comment.
When heartbeats stop but other PX4 messages continue, taking the minimum message age treats the heartbeat as fresh. The supervisor can keep sending guidance setpoints using armed and mode information it can no longer verify instead of aborting. Check heartbeat freshness separately before merging.
Artifacts
Authored supervisor reproduction script
- This script calls the real supervisor methods with fake telemetry and compares repository behavior with a process-local age-policy alternative, without editing tracked files.
Repository behavior with a stale heartbeat
- The captured command exited 0 and shows GOTO sending a velocity setpoint with an eight-second-old heartbeat, confirming the defect.
Process-local alternative with a stale heartbeat
- The captured command exited 0 and shows the same telemetry causing ABORT and no setpoint after an in-memory age-policy change, demonstrating the expected safety distinction.
| status = self._poll(_settled, self.config.settle_timeout_s) | ||
| if status["state"] not in GUIDANCE_STATES: # pilot, abort, E-STOP: no setpoint left | ||
| return f"{said}, then interrupted: {status['reason']}. {_where(status)}" | ||
| return f"{said}. {_where(status)}" |
There was a problem hiding this comment.
When settling times out outside the 15 cm tolerance, _poll() returns its last status and _where_settled() still says “Arrived” or “Took off” if guidance remains active. The operator or agent is told the maneuver settled when it did not. Report the unsettled outcome before merging.
Artifacts
Synthetic skill service and timer harness
- The authored executable harness compiles the relevant source and calls both skills against deterministic statuses without editing tracked files.
Current skill responses after settling times out
- The captured before run shows both skills reporting success at 0.50 m error after the 2.00 s timeout.
Skill responses with an in-memory unsettled-status guard
- The captured after run uses the same statuses and timers with an in-memory guard and shows both skills reporting that they did not settle.
| cfg = self.config | ||
| self._acquire_writer_lock() | ||
| self._state = VehicleState(gimbal_mount=MOUNT_PRESETS[cfg.gimbal_mount_preset]) | ||
| self._core = SupervisorCore(cfg.limits, cfg.guidance, sitl=cfg.sitl) |
There was a problem hiding this comment.
If a physical-aircraft connection is configured with sitl=True, it accepts sitl_enable(True) independently of the MAVLink destination. That software flag replaces the physical RC enable switch, allowing preflight and continued guidance without fresh RC input. Prevent this override on physical-aircraft connections before merging.
Artifacts
Safe PX4 service-contract reproduction script
- The authored script invokes the connection and supervisor with fake vehicle state, I/O, and workers, allowing both constructor behaviors to be compared without aircraft contact.
Service responses with the RC safety gate retained
- The captured command output shows the in-memory `sitl=False` baseline rejecting the enable override and waiting for fresh RC input.
Service responses with configured SITL on a physical target
- The captured command output shows repository HEAD accepting the enable override, passing preflight without RC telemetry, and omitting the RC abort reason.
| 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.
If zoom replies stop after a 1× reading, this loop retains that reading indefinitely. Should the camera zoom change during the outage, publish_camera_info() continues sending 1× intrinsics, giving geometry consumers incorrect calibration. Treat a lost reply as unknown zoom before merging.
Artifacts
Fake SDK zoom-outage reproduction script
- The authored script polls a fake SDK and invokes the real CameraInfo publication path, with the proposed fix applied only in memory for comparison.
CameraInfo publication before the fix
- Running the repository code after a 1x reply and a lost reply shows CameraInfo still published after the camera changes to 2.5x.
CameraInfo withheld with the in-memory fix
- Running the same SDK sequence with only the zoom-loop replacement in memory shows unknown zoom and no new CameraInfo publication.
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?
Natural-language control of the drone.
Px4SkillContainer(skill_container.py):takeoff,go_to,land,set_guidance_mode,flight_status. Each skill calls the connection RPC and waits for the outcome, so the supervisor refuses a skill exactly as it refuses the RPC. A pilot takeover, abort or E-STOP is reported to the agent as such. FollowsUnitreeSkillContainer.connection_spec.py: the connection RPCs the skills use.blueprints_agentic.py:px4-agentic,px4-sitl-agentic. Separate file because it needs theagentsextra.Why do we need this?
Lets the dimOS agent fly the drone like the other robots.
How to Test
In SITL the agent flew "take off to 3 meters", "fly 3 meters north" (3.03 m), "what is your flight status?" and "land".
Stack
Five PRs, all against
main; each contains the ones above it. Merge in order. Review only this PR's own commit: 5ee3057Which issue(s) does this PR close?
None. New platform: PX4 multicopters with a SIYI A8 gimbal camera.
Checklist
🤖 Generated with Claude Code