Skip to content

feat(hardware): SIYI A8 gimbal and RTSP H.265 camera (PX4 stack 1/5) - #4289

Open
Ez4ezka wants to merge 1 commit into
dimensionalOS:mainfrom
Ez4ezka:ezen/feat/px4-1-payload
Open

Ez4ezka wants to merge 1 commit into
dimensionalOS:mainfrom
Ez4ezka:ezen/feat/px4-1-payload

Conversation

@Ez4ezka

@Ez4ezka Ez4ezka commented Sep 25, 2026 •

Copy link
Copy Markdown

What is this feature?

Camera and gimbal drivers for the PX4 stack. Nothing here depends on PX4.

  • RtspCamera (dimos/hardware/sensors/camera/rtsp): RTSP H.265 in; video (encoded, untouched), color_image (decoded, rate-capped), color_jpeg (small, for a slow link) out. PyAV only. url is an RTSP URL, a file, or synthetic. RPCs: set_video_enabled(enabled), set_jpeg_rate(hz).
  • SiyiA8Gimbal (dimos/hardware/gimbal/siyi): reads gimbal_attitude, publishes the gimbal tf chain and camera_info, turns target_los into gimbal_target aim requests. Opens no MAVLink socket. The SIYI SDK (zoom poll) is off unless ip is set.
  • px4 extra: av.

Why do we need this?

The A8 is the drone's camera. dimOS has no RTSP/H.265 source and no gimbal model.

How to Test

uv sync --extra px4
uv run pytest dimos/hardware/gimbal/siyi dimos/hardware/sensors/camera/rtsp
dimos run rtsp-camera-vis --rtspcamera.url=synthetic

The last command streams 25 fps video with no camera attached. An earlier revision decoded the real A8 stream on a Jetson Orin Nano.

Stack

Five PRs, all against main; each contains the ones above it. Merge in order. Review only this PR's own commit: 5bbf22c

  1. feat(hardware): SIYI A8 gimbal and RTSP H.265 camera (PX4 stack 1/5) #4289 SIYI A8 gimbal and RTSP camera (this PR)
  2. feat(msgs): NavSatFix, BatteryState and PX4 VehicleStatus (PX4 stack 2/5) #4290 NavSatFix, BatteryState, PX4 VehicleStatus
  3. feat(px4): Px4DroneConnection, flight supervisor and SITL gate (PX4 stack 3/5) #4291 PX4 connection, flight supervisor, SITL gate
  4. feat(px4): agent skills and agentic blueprints (PX4 stack 4/5) #4292 agent skills
  5. feat(px4): target following on dimos/perception detections (PX4 stack 5/5) #4293 target following on dimos/perception detections

Which issue(s) does this PR close?

None. New platform: PX4 multicopters with a SIYI A8 gimbal camera.

Checklist

  • I have read and approved the CLA.

🤖 Generated with Claude Code

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).
@greptile-apps

greptile-apps Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 0/5

[Medium risk] Adds gimbal and RTSP camera hardware modules.

Not safe to merge until the transform, zoom-calibration, replay-timing, and credential-logging failures are fixed. The viewer and test-coverage concerns do not independently block merging.

Findings

  1. P1 Gimbal pitch assumes level flight ▶
  2. P1 Failed polls retain calibration ▶
  3. P1 Replay changes capture timing ▶
  4. P1 Security RTSP credentials enter logs ▶
  5. P2 Displayed images lack calibration ▶
  6. P2 Independent camera tests are skipped ▶

Summary

The PR adds a SIYI A8 gimbal, an H.265 RTSP camera with file and synthetic replay, a viewer, and a PX4 dependency extra. Before merging, the gimbal transform must account for nonlevel flight, calibration publication must stop when zoom verification is lost, file replay must preserve variable frame timing, and RTSP credentials must be kept out of logs. The viewer calibration and local test-coverage concerns are non-blocking improvements.

Reviews (1) · Last reviewed commit: "feat(hardware): SIYI A8 gimbal and RTSP ..."

Comment on lines +255 to +259
gimbal_rot = Quaternion.from_euler(
Vector3(
math.radians(att.roll_deg), -math.radians(att.pitch_deg), -math.radians(att.yaw_deg)
)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Gimbal pitch assumes level flight

When the airframe pitches, this body-relative transform applies the A8’s earth-stabilized pitch without compensating for airframe attitude. The published optical frame is misoriented, giving camera rays and target locations incorrect directions. With 20° of airframe pitch, the optical orientation was 20° from the expected orientation.

Artifacts

Authored camera-orientation check and invocation

  • The executed Python source feeds a synthetic gimbal attitude through the real publisher and compares its optical TF orientation with an earth-stabilized model.

Level-airframe control output

  • The command, working directory, exit code, and output show that the published optical orientation matches the expected orientation when the airframe is level.

Nonlevel-airframe defect output

  • The command, working directory, exit code, and output show a 20° optical-orientation error after modeling 20° of airframe pitch, confirming the defect.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +239 to +242
zoom = self._sdk.query_zoom()
if zoom is not None: # a lost reply keeps the last known zoom
with self._lock:
self._zoom = zoom

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Failed polls retain calibration

After a successful 1× zoom reading, failed polls leave _zoom at 1×. publish_camera_info() keeps issuing newly timestamped 1× intrinsics although the current zoom can no longer be verified. If the camera changes zoom during that loss of communication, image consumers receive incorrect calibration.

Artifacts

Camera-info service before poll failures

  • Ran the service methods with unknown zoom and then a valid 1x reply; publication began only after the valid reply.

Camera-info service after poll failures

  • Ran the same service methods through three failed polls and a later 2.5x reply; fixed 1x intrinsics continued publishing through the failures.

Authored zoom-poll service repro source

  • This script supplies controlled SDK replies and asserts the observed service returns and publication counts.

Authored artifact upload script

  • This script PUT the two execution logs and both scripts to preallocated artifact slots.

Artifact upload command and responses

  • Captured the upload command, working directory, exit code, and four HTTP 200 OK responses.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +220 to +224
if self._is_file() and cfg.replay_realtime:
due = t_start + packets / fps
delay = due - time.monotonic()
if delay > 0:
time.sleep(delay)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Replay changes capture timing

For a file with variable frame durations, replay schedules packets by count and average frame rate rather than their timestamps. A captured 320 ms frame gap replayed as 87 ms, shifting frame timestamps relative to other streams and preventing faithful synchronized replay.

Artifacts

Variable-timestamp H.265 replay check source

  • The authored script generates a variable-PTS file and runs both the timestamp-paced reference and the PR relay, showing exactly how timing was measured.

Timestamp-paced reference output

  • The executed reference paced packets from file timestamps and observed the intended 320 ms gap, establishing the comparison.

PR camera replay output

  • The executed PR relay emitted all six frames but reduced the 320 ms file gap to 87 ms, confirming the timing defect.

View artifacts

T-Rex Ran code and verified through T-Rex

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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security RTSP credentials enter logs

If an RTSP URL contains credentials, a connection failure writes the unredacted URL to console output and the persistent JSON log. Anyone with access to those logs can recover the camera credentials. Log a credential-free endpoint instead; the stream-ended warning has the same exposure.

How this was verified: A failed connection with a dummy password placed that password in both the console warning and the JSON log.

Artifacts

Failed RTSP connection before mitigation

  • The executed PR code logged a dummy password to the console and persistent JSON record, confirming credential exposure.

Failed RTSP connection with in-memory mitigation

  • The same simulated failure ran with raw-URL logging removed in memory and left the dummy password out of both outputs.

Local RTSP failure reproduction script

  • The script invoked the changed relay path with a fake credential and inspected the real logger handlers' JSON output.

Artifact upload command script

  • The script uploaded the captured proof through allocated artifact slots without using camera credentials.

Artifact upload output

  • The captured upload command reported HTTP 200 OK for the after log and reproduction scripts; the before log was already uploaded.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +49 to +51
"max_hz": {"world/color_jpeg": 2.0},
# Raw decoded frames stay on this machine; the viewer decodes the H.265 itself.
"visual_override": {"world/color_image": None},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Displayed images lack calibration

This viewer suppresses color_image, the only image output to which the bridge attaches camera calibration. Its displayed video and JPEG outputs receive no pinhole calibration, limiting their usefulness for interpreting camera geometry. This is a non-blocking viewer limitation; JPEG resizing also requires appropriately scaled intrinsics.

Artifacts

Bridge output without RTSP image suppression

Bridge output with RTSP image suppression

Authored and executed bridge reproduction script

Executed artifact upload script

Successful artifact upload output

Final upload receipt

  • The final upload command recorded the prior uploads and an HTTP 200 for the upload-output image, confirming submission.

View artifacts

T-Rex Ran code and verified through T-Rex

from dimos.hardware.sensors.camera.rtsp.camera import RtspCamera
from dimos.hardware.sensors.camera.rtsp.synthetic import SQUARE, square_origin, write_synthetic_h265

pytestmark = pytest.mark.skipif_no_turbojpeg

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Independent camera tests are skipped

The module-wide turbojpeg marker skips all 15 camera tests locally when the native library is unavailable, including passthrough, decoded-image, lifecycle, and missing-library fallback tests. A successful local run can therefore exercise none of those paths. This loss of useful local coverage does not independently block merging.

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

Camera pytest execution commands

  • The uploaded script records and executes collection, local, and CI-mode camera pytest commands from the repository root, making the verification reproducible.

Camera pytest collection and run output without libturbojpeg

  • The captured execution shows 15 collected, 15 locally skipped, and 12 passing plus 3 failing in CI mode, confirming that the module-wide skip conceals runnable tests.

View artifacts

T-Rex Ran code and verified through T-Rex

@greptile-apps

greptile-apps Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These 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.

  • P1 Earth-stabilized pitch is published as a body-relative gimbal rotation ▶

    • Bug
      • A nonlevel airframe's pitch is added to the reported earth-stabilized gimbal pitch in the world-to-camera TF chain, misorienting the optical frame. The focused run measured a 20° error for a 20° airframe pitch.
    • Cause
      • In the new dimos/hardware/gimbal/siyi/gimbal.py, lines 255–259, gimbal_rot = Quaternion.from_euler(Vector3(math.radians(att.roll_deg), -math.radians(att.pitch_deg), -math.radians(att.yaw_deg))) places the reported pitch directly on the gimbal_base → gimbal_link body-relative edge without compensating for airframe attitude.
    • Fix
      • Supply synchronized airframe orientation and convert the earth-stabilized gimbal orientation into a body-relative rotation before publishing that TF edge; verify level and nonlevel attitudes.
  • P1 Failed zoom polls keep publishing 1x intrinsics without a current zoom reading ▶

    • Bug
      • Caching the last reading is useful for reporting state, but using a cached 1x reading to authorize new CameraInfo messages is unsafe when subsequent polls fail: the actual zoom can no longer be verified. The repro establishes continued publication during failures, not that the physical camera actually changed zoom.
    • Cause
    • Fix
      • Track whether the zoom reading is current, or its last-success time; withhold 1x CameraInfo after a failed poll or a defined freshness timeout until 1x is confirmed again. Last-known zoom may remain available separately in state().
  • P1 Variable-PTS file replay compresses and shifts capture timing ▶

    • Bug
      • A 320 ms interval in the captured file replayed as 87 ms, emitting the frame 139 ms ahead of its file timestamp. Because output stamps reflect replay time, this changes timing relative to other replayed streams.
    • Cause
      • In dimos/hardware/sensors/camera/rtsp/camera.py:220-224, file replay sets due = t_start + packets / fps using stream.average_rate rather than the packet’s timestamp and time base.
    • Fix
      • For realtime file replay, schedule packets from their timestamp offsets relative to the first valid packet timestamp; define a fallback for missing timestamps.
  • P1 RTSP credentials are written to console and persistent logs ▶

    • Bug
      • When a credential-bearing RTSP source fails, the warning exposes its username and password to anyone who can read console output or the JSON log. The stream-ended warning has the same URL-logging pattern.
    • Cause
      • New lines 177 and 185 of dimos/hardware/sensors/camera/rtsp/camera.py pass the unredacted self._source as a structured logging field. The configured console and rotating JSON file handlers retain that field.
    • Fix
      • Do not log the raw RTSP URL. Log a credential-redacted source identifier, and also sanitize exception text if it can contain a URL.
  • P2 RTSP viewer outputs lack camera calibration ▶

    • Bug
      • The configured world/video and world/color_jpeg views receive no Pinhole, so their displayed imagery is uncalibrated.
    • Cause
      • dimos/hardware/sensors/camera/rtsp/blueprints.py:49-51 caps JPEG output and suppresses world/color_image. The bridge attaches CameraInfo calibration only to tracked Image messages, not CompressedImage or CompressedVideo.
    • Fix
      • Attach appropriately scaled intrinsics to the displayed video and JPEG entities, accounting for JPEG resizing.
  • P2 Module-wide turbojpeg skip hides independent camera coverage ▶

    • Bug
      • Without native libturbojpeg, all 15 tests skip locally, including passthrough, decoded-image, lifecycle, and the missing-library fallback test at lines 126–141. This produces a successful run without exercising those paths.
    • Cause
      • pytestmark = pytest.mark.skipif_no_turbojpeg applies to the entire module at dimos/hardware/sensors/camera/rtsp/test_camera.py:36; dimos/conftest.py:301–304,315–321 converts it to a skip outside CI when the library is absent.
    • Fix
      • Restrict the skip to tests requiring successful JPEG encoding; allow independent and missing-library fallback tests to run.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-time-contributor PR opened by an author who had not previously committed to this repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant