Skip to content

feat(msgs): NavSatFix, BatteryState and PX4 VehicleStatus (PX4 stack 2/5) - #4290

Open
Ez4ezka wants to merge 2 commits into
dimensionalOS:mainfrom
Ez4ezka:ezen/feat/px4-2-msgs
Open

Ez4ezka wants to merge 2 commits into
dimensionalOS:mainfrom
Ez4ezka:ezen/feat/px4-2-msgs

Conversation

@Ez4ezka

@Ez4ezka Ez4ezka commented Sep 25, 2026 •

Copy link
Copy Markdown

What is this feature?

Message types the PX4 connection publishes.

  • sensor_msgs/NavSatFix, sensor_msgs/BatteryState: wrappers over the dimos_lcm types.
  • px4_msgs/VehicleStatus: armed, PX4 mode, landed state, battery, GPS quality, supervisor state. Hand-written LCM type (its proper home is dimos-lcm). Wire helpers in dimos/msgs/lcm_wire.py.

Why do we need this?

Px4DroneConnection (#4291) publishes gps, battery and vehicle_status. Split out to keep that PR about flight code.

How to Test

uv run pytest dimos/msgs/px4_msgs dimos/msgs/sensor_msgs/test_NavSatFix.py dimos/msgs/sensor_msgs/test_BatteryState.py

Round trips compare whole messages; a foreign fingerprint is refused.

Stack

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

  1. feat(hardware): SIYI A8 gimbal and RTSP H.265 camera (PX4 stack 1/5) #4289 SIYI A8 gimbal and RTSP camera
  2. feat(msgs): NavSatFix, BatteryState and PX4 VehicleStatus (PX4 stack 2/5) #4290 NavSatFix, BatteryState, PX4 VehicleStatus (this PR)
  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).
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).
@greptile-apps

greptile-apps Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 2/5

[Medium risk] Adds gimbal control, camera streaming, and message types.

Do not merge until RTSP credentials are removed from warnings and the gimbal transform accounts for aircraft tilt. The publication-count concern is non-blocking.

Findings

  1. P1 Security RTSP credentials enter logs ▶
  2. P1 Aircraft tilt misorients camera ▶
  3. P2 Count published packets ▶

Summary

This PR adds RTSP H.265 streaming and replay, SIYI gimbal transforms and aim requests, PX4-related messages, and dependency and registry entries. Before merging, fix RTSP warnings that disclose URL passwords and camera transforms that become inaccurate when the aircraft tilts. File-replay publication counts have a separate, non-blocking diagnostic issue when a filter call emits a number of packets other than one.

Reviews (1) · Last reviewed commit: "feat(msgs): NavSatFix, BatteryState and ..."

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 a password, a connection failure logs the full URL to console output and the structured log; the stream-closure warning at line 185 does the same. Anyone with access to those logs can read the credential. Redact credentials in both warnings without changing the URL used to connect. This must be fixed before merging.

How this was verified: Both warning paths wrote a password-bearing URL to console output and structured logs.

Artifacts

Isolated RTSP credential logging reproduction script

  • The executed script drives both camera logging branches with an offline password-bearing URL and checks the resulting JSON events.

Camera logs with the unmodified logger

  • Running both branches with the unmodified logger produced console and JSON entries containing the test password.

Camera logs with process-local URL sanitization

  • Running the same branches with an isolated sanitizing logger produced entries without the password, showing a viable redaction boundary.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +254 to +259
# MAVLink gimbal angles: yaw clockwise positive, pitch up positive; FLU is the reverse.
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 Aircraft tilt misorients camera

When the aircraft pitches or rolls, the A8 reports earth-stabilized attitude, but this code publishes it as a rotation relative to the aircraft. The camera transform then points observations and targets in the wrong direction: with 20° aircraft pitch and zero reported earth pitch, the published camera orientation is 20° off. Convert the reported attitude into the aircraft frame before publishing it. This must be fixed before merging.

Artifacts

Executable SIYI frame comparison script

  • This authored script feeds synthetic attitudes through the real TF publisher and compares published and physically expected camera rotations.

Published TF with nonlevel aircraft attitude

  • Running the script in before mode shows 20° pitch error and 24.954° combined pitch-and-roll error in the published camera orientation.

Physical body-relative rotation with nonlevel aircraft attitude

  • Running the same inputs with the expected body-relative rotation substituted in the comparison reduces camera-orientation error to approximately zero.

Existing SIYI test results

  • Running the focused SIYI test file passes all 22 tests, showing that its existing coverage does not catch this nonlevel-aircraft error.

View artifacts

T-Rex Ran code and verified through T-Rex

self.video.publish(
CompressedVideo(bytes(unit), format="h265", frame_id=self.config.frame_id, ts=ts)
)
self._count("video", published=1)

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 Count published packets

If the file-replay filter emits zero or multiple packets from one call, this counter still increases by one. sensor_stats() and interval-rate logs can then disagree with the video messages sent, making diagnostics misleading. The tested native filter emitted one packet per call, so that clip was unaffected. Count successful publications inside the loop. This diagnostic concern is non-blocking.

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

Full authored H.265 replay and publish-count reproduction script

  • The complete executed Python source generates a clip, records filter output and publication counts, and applies an in-memory correction for comparison.

Replay output with the repository publish counter

  • The captured command, working directory, exit code, and output show ten per-call mismatches with buffered zero/two-packet output, while the native one-packet output matches.

Replay output with an in-memory per-packet counter

  • The captured command, working directory, exit code, and output show no per-call mismatches after moving the increment inside the publication loop in memory.

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 RTSP credentials are written to console and persistent logs ▶

    • Bug
      • A configured URL containing userinfo exposes its password when a stream fails or a live stream closes. Both events are warnings, so ordinary connection failures can repeatedly disclose the credential to anyone with access to console output or the JSON log.
    • Cause
      • dimos/hardware/sensors/camera/rtsp/camera.py:177 and :185 pass self._source directly as the logger's url field. The logging formatters retain that field without redaction.
    • Fix
      • Redact URL userinfo before either warning is logged, without changing the URL supplied to av.open. Also sanitize the exception text at line 177 if it can contain the URL.
  • P1 Earth-stabilized A8 attitude is published as a body-relative TF rotation ▶

    • Bug
      • In dimos/hardware/gimbal/siyi/gimbal.py, the relevant lines are 254–259, which build gimbal_rot directly from the reported angles, and 260–280, which publish it as gimbal_base -> gimbal_link. The alleged lines 223–226 are in the TF loop, not the rotation calculation. For an A8 reporting zero earth pitch and roll, the publisher emits an identity body-to-gimbal rotation regardless of aircraft attitude. At aircraft pitch 20°, the composed camera points 20° away from its physically expected orientation; at roll 15° plus pitch 20°, its orientation differs by 24.954°. The existing 22 tests pass but do not supply nonzero aircraft attitude. The full source of the executed, untracked test script is:
        """Exercise SIYI's real TF publisher against an independent world-frame expectation.
        
        Usage: PYTHONPATH=. .venv/bin/python trex-artifacts/siyi-earth-frame-repro.py before|after
        """
        
        import math
        import sys
        
        from dimos.hardware.gimbal.siyi.gimbal import SiyiA8Gimbal
        from dimos.hardware.gimbal.siyi.replay import AttitudeSample
        from dimos.msgs.geometry_msgs.Quaternion import Quaternion
        from dimos.msgs.geometry_msgs.Vector3 import Vector3
        
        def euler(roll_deg, pitch_deg, yaw_deg=0.0):
            return Quaternion.from_euler(
                Vector3(*(math.radians(a) for a in (roll_deg, pitch_deg, yaw_deg)))
            )
        
        def run(mode):
            assert mode in ("before", "after")
            gimbal = SiyiA8Gimbal()
            sent = []
            gimbal.tf.publish = sent.append
            cases = (("level", 0.0, 0.0, 0.0), ("pitch", 0.0, 20.0, 0.0),
                     ("roll", 15.0, 0.0, 0.0), ("pitch_and_roll", 15.0, 20.0, 0.0),
                     ("reported_pitch_with_aircraft_pitch_and_roll", 15.0, 20.0, -10.0))
            for name, aircraft_roll, aircraft_pitch, reported_pitch in cases:
                sent.clear()
                gimbal._on_attitude(AttitudeSample(123.0, 0.0, reported_pitch, 0.0).joint_state())
                assert gimbal.publish_transforms()
                edges = {(t.frame_id, t.child_frame_id): t for t in sent.pop().transforms}
                mount = edges[("base_link", "gimbal_base")].rotation
                reported_link = edges[("gimbal_base", "gimbal_link")].rotation
                optical = edges[("gimbal_link", "a8_optical")].rotation
                world_base = euler(aircraft_roll, aircraft_pitch)
                # A8 pitch-up positive maps to FLU pitch negative per gimbal.py:254-258.
                earth_gimbal = euler(0.0, -reported_pitch)
                expected_body_link = world_base.inverse() * earth_gimbal
                # Before: real published TF. After: corrected body-relative TF, without editing
                # the module; everything else (attitude input, frame chain, optical) is identical.
                link = reported_link if mode == "before" else expected_body_link
                actual_world_camera = world_base * mount * link * optical
                expected_world_camera = earth_gimbal * optical
                angle_error = math.degrees(actual_world_camera.angle_to(expected_world_camera))
                forward = actual_world_camera.rotate_vector(Vector3(0.0, 0.0, 1.0))
                camera_up = actual_world_camera.rotate_vector(Vector3(0.0, -1.0, 0.0))
                print(
                    f"{name}: aircraft FLU roll={aircraft_roll:.1f} pitch={aircraft_pitch:.1f}; "
                    f"A8 reported earth pitch={reported_pitch:.1f} roll=0 yaw=0; "
                    f"published body->link angle={math.degrees(reported_link.angle_to(Quaternion())):.6f} deg; "
                    f"physical body->link angle={math.degrees(expected_body_link.angle_to(Quaternion())):.6f} deg; "
                    f"camera world orientation error={angle_error:.6f} deg; "
                    f"camera forward world=({forward.x:.6f}, {forward.y:.6f}, {forward.z:.6f}); "
                    f"camera up world=({camera_up.x:.6f}, {camera_up.y:.6f}, {camera_up.z:.6f})"
                )
                if name == "level" or mode == "after":
                    assert angle_error < 1e-5
                else:
                    assert angle_error > 1.0
            gimbal.stop()
            print(f"PASS: {mode} checked real publisher on five attitudes and world-frame geometry")
        
        if __name__ == "__main__":
            run(sys.argv[1])
    • Cause
      • The module has no aircraft-attitude input. It places reported earth-stabilized pitch and roll directly on an edge whose parent is attached to base_link, so aircraft rotation is applied again when the TF chain is composed.
    • Fix
      • Supply the time-aligned aircraft orientation and convert the reported earth-frame gimbal orientation into the parent frame before publishing: R_base_gimbal = inverse(R_world_base) * R_world_gimbal, accounting for the mount and reported body-relative yaw. Add nonlevel-aircraft TF tests. This fix was demonstrated numerically, not applied to tracked source.
  • P2 Video published counter increments per filter call, not per published packet ▶

    • Bug
      • At camera.py:236–237,254–262, file replay passes all packets emitted by one filter call to _publish_video. When that iterable has zero or two packets, sensor_stats()['video']['published'] increments by one rather than by the number actually published. Interval rate logs at lines 314–329 use the same counter. The tested native filter emitted exactly one packet per call; the non-one-output case was exercised with a buffered wrapper, not observed from native PyAV.
    • Cause
      • self._count('video', published=1) at camera.py:262 is outside the loop at lines 258–261.
    • Fix
      • Increment published for each successful self.video.publish inside the loop, or count successful publications and increment by that total.

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