Repository navigation
Add support for RPI-CM4 using Libcam CSI/MIPI camera and CADDX gimbal - #45
Conversation
24150e2 to
226a868
Compare
Previous review (2026-10-05)Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks for the Raspberry Pi target. One issue needs fixing before merge. With the gimbal on UDP passthrough (CAMERA_APP_UART=udp://FC:PORT), any ICMP error on the connected socket makes poll() return POLLERR without POLLIN. The non-SITL main loop at camera_app/src/main.cpp:608-610 then exits with 'backend descriptor failed', taking RTSP and recording down with it. The ECONNREFUSED handling added at https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-e8471bd3511b217f44ff1281348bf2174f3320bbdb5de0d3bbdddf7e8216fb24R145 is never reached, because handle_fd only runs on POLLIN. I reproduced this by sending to a closed UDP port (revents=0x8). Could you clear SO_ERROR for datagram backends on this target, as the SITL block at main.cpp:590-605 already does, or handle it inside the CADDX backend? Smaller points: The RTSP address fallback (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-862008e059849dbb1a8cdab3af55eb729c57bbfe996836b8f379ad2c54d054bbR651) prefers 192.168.144.0/24 and then the lowest address, not the 'first LAN IPv4 address' described at https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-57a13d56b565ab46e64820b7f564b4fe3925995aedb46c54c76dd0c32be481ccR77 and in the service file (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-32f99232c5a933efd47fedcc465f97fac0f611e3879d001b136483137e295f14R12). It would help to document the actual rule and suggest CAMERA_APP_VIDEO_ADDRESS when the Pi has both FC Ethernet and WiFi. The target enables SOC temperature (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-2634cbadec094948fa809974e819182dbcda9e23c52fe471c9c7a0ae0005ffc2R35) and the web shows it, but cpu_temperature() in camera_app/src/recording/system_stats.cpp:88 only reads the sysfs path on Z1-Mini, so the SYS log cpu_temp is NaN on the Pi. Gating that branch on SOC_TEMPERATURE_PATH would fix it. CI never runs the new rpi_libcam_caddx-test (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-57ae37da2bcb8c69ff956255d520603f1980d4d634a1845c6dfdbdb94b202508R399) or compiles the libcamera backend. A Not verified here: real Pi, IMX477 or CADDX hardware, a full camera-app link, and the libcamera < 0.5 branch. |
Companion-computer targets speak only MAVLink and run on a host OS that owns the network interfaces. Add APCAM_PROTOCOL_NONE, which opens no SIYI or XFRobot service, and APCAM_HAVE_NETWORK_CONFIG, which skips the camera's address configuration when 0. On such targets MAVLink usually arrives through a local router, so a loopback route says nothing about which address the GCS can reach. Use the interface address selection serial routes already use (192.168.144.0/24 first, then the lowest address) for advertised RTSP URIs instead, or the address in CAMERA_APP_VIDEO_ADDRESS. APCAM_HAVE_SITL 0 keeps a hardware-only target out of SITL: the launcher, the Windows simulator build and its package test use the targets in SITL_TARGETS. Existing targets keep the defaults and are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks very much for the AI review! I think the issues have been resolved now |
Previous review (2026-10-06)Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks for the quick turnaround. I checked head 5fba26e and agree the earlier points are resolved. The CADDX UDP link is no longer polled by the main loop, and periodic() drains it, so an ICMP error from the flight controller can't stop the camera app any more. Your rpi_libcam_caddx-test, including caddx_udp_unreachable, passes here in a network namespace. The address docs, the Pi SOC temperature and the CI step for the Pi target all look right too. One small, non-blocking point. With CAMERA_APP_UART unset, as in the packaged service, the CADDX backend runs without a gimbal (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-e8471bd3511b217f44ff1281348bf2174f3320bbdb5de0d3bbdddf7e8216fb24R107). camera-app still sends an ACTIVE MAV_TYPE_GIMBAL heartbeat and GIMBAL_DEVICE_INFORMATION with pitch and yaw axes (camera_app/src/protocol/mavlink_server.cpp:533-539 and 1141-1176). That doesn't match the README's 'the gimbal is reported as unsupported' (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-57a13d56b565ab46e64820b7f564b4fe3925995aedb46c54c76dd0c32be481ccR107). Could you either skip those two messages when no gimbal link is configured, or reword that README line? Not verified here: real Pi, IMX477 or CADDX hardware, the libcamera >= 0.5 branch, and a full camera-app run against a real flight controller. |
|
Thanks again for the review. The only change is to address the AI review - if CAMERA_APP_UART is unset no gimbal component will appear. BTW, this has been tested on real hardware. |
Support for a Raspberry Pi CM4 companion computer with an IMX477 CSI camera and a CADDX gimbal, same as XFRobot C20-T, running alongside Rpanion-server. The target name combines its parts (board, camera driver, gimbal driver); the media backend in src/backends/rpi_libcam and the gimbal backend in src/backends/caddx can be reused by future targets such as rpi_libcam_simplebgc. libcamera fills YUV420 dmabufs which are queued, without copying, to the Pi 4 / CM4 V4L2 H.264 encoder. One 1080p30 stream serves both RTSP paths, the live view and MP4 recording. The IMG_* image settings map to libcamera controls and are attached to the next request, so brightness, exposure and the other settings change without restarting the stream. Both the libcamera 0.5+ exposure/gain modes and the earlier zero-means-automatic convention are supported. The gimbal backend ports the protocol of ArduPilot's AP_Mount_CADDX driver: a 10-byte angle command (A5 5A, lock mode, roll/pitch/yaw as 12-bit fractions of a turn, big-endian CRC16-CCITT) sent every 20 ms at 115200 8N1. Roll and pitch are earth-frame locked and yaw follows the vehicle; requested rates are integrated into angle targets within the gimbal limits. The gimbal sends no feedback, so the reported attitude is the commanded target, as in ArduPilot. Nothing is sent until the first gimbal command, so starting the camera app does not move the gimbal. CAMERA_APP_UART selects a serial device or udp://IPv4:PORT, the latter for ArduPilot's serial passthrough to a network port. Without it the camera advertises no gimbal component (no gimbal heartbeat or device information, gimbal device ID 0), so a gimbal on the flight controller is not duplicated, and camera functions remain available. A UDP link is read from the periodic update rather than polled, so ICMP errors while the flight controller reboots cannot stop the camera app. Packets are logged under a new CA_PACKET_CADDX protocol. The SYS log reads the SoC temperature from the kernel thermal zone on any target defining SOC_TEMPERATURE_PATH, as the Z1-Mini already did. Build natively on Raspberry Pi OS with "make rpi_libcam_caddx". Tested on a CM4 with an IMX477 and libcamera 0.7.2: RTSP 1080p30 streaming, IMG_* changes applied live from the GCS and MP4 recording; also compiled against libcamera 0.2. The gimbal was tested through passthrough from the flight controller's SERIAL3: pitch and yaw move in the commanded direction and neutral repeats. Host unit tests cover the image control mapping and the CADDX encoding against ArduPilot's formula. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read the SoC temperature from SOC_TEMPERATURE_PATH whenever a target defines one, instead of only on Z1-Mini. Any other target with SoC temperature enabled maps the SS928 sensor registers through /dev/mem, which is correct for MT11 but would read an arbitrary physical address on a new target with a different SoC. Existing targets are unchanged: MT11 keeps the SS928 sensor, Z1-Mini already used the thermal zone, and A8 and ZR10 have no SoC temperature. Refuse photo capture on targets without APCAM_HAVE_PHOTO rather than sending a SIYI shutter packet to a MAVLink port. Of the existing targets this changes only Z1-Mini, whose Photos page now reports the capture as failed without sending anything. Add optional switches for targets running on a general-purpose OS, all defaulting to the existing behaviour: - WEB_HAVE_FIRMWARE_UPLOAD 0 hides the package upload and rejects it - WEB_MEDIA_IS_MOUNT 0 reports media on the root filesystem as "Recordings" instead of an unmounted microSD card - APP_LOG_COMMAND shows a command's output (such as journalctl) on the Debug tab instead of rotated log files - REBOOT_COMMAND reboots through the init system so services stop and filesystems unmount cleanly Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Build the shared web service for the Raspberry Pi with "make rpi_libcam_caddx", served on port 8080 (for example over Rpanion-server's WiFi access point). Configuration and the web password live in /etc/ap_cameragimbal, recordings in /var/lib/ap_cameragimbal and the web assets in /usr/local/share/ap_cameragimbal/webroot. systemd supervises camera-app: the web page restarts it by stopping it and waiting for a fresh instance. The Debug tab shows the camera service's journal, Reboot uses systemctl and firmware upload is not offered since updates come from the source tree. The web page sends its camera controls over MAVLink TCP, so the default camera.ini enables tcp_port 14570. Tested on a CM4 with an IMX477: status, live video in the browser, parameter changes applied live, the Debug log and camera app restart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add systemd services for the camera app and the web interface and a README covering the native build, installation, the web interface, the mavlink-router endpoint for Rpanion-server, gimbal setup (a Pi UART or ArduPilot's serial passthrough to a network port, with the parameters) and the environment variables, and list the target in the top-level README. The camera service restarts always, since the web page restarts camera-app by stopping it. The web service seeds the admin password "ardupilot" on first start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Run the rpi_libcam_caddx host test (image control mapping and the CADDX gimbal, including an unreachable UDP passthrough) and build camera-app and the web interface natively against Ubuntu 24.04's libcamera 0.2, the oldest supported version, so the libcamera/V4L2 backend and its pre-0.5 exposure controls are compiled on every change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks, I checked head 27c84d1. The no-gimbal change works as you described. With CAMERA_APP_UART unset there is no gimbal heartbeat, no GIMBAL_DEVICE_INFORMATION and no gimbal command replies, and CAMERA_INFORMATION reports gimbal_device_id 0 (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-862008e059849dbb1a8cdab3af55eb729c57bbfe996836b8f379ad2c54d054bbR548). Other targets are unaffected. Your rpi_libcam_caddx-test, including caddx_udp_unreachable, passes here in a network namespace. One optional note, not a blocker. If the encoder poll() ever failed with something other than EINTR, the encoder thread would exit while ready() and recording stay true (https://github.com/ardupilot/ap_cameragimbal/pull/45/files#diff-abb55061855e87b1e8671dae1cec60161e4e0334a67b6dbae839fc8bf65beef9R199). That is the same as the existing A8 and ZR10 capture threads, and in practice it can only happen on ENOMEM, so it's just a possible future hardening across backends. Not verified here: real Pi, IMX477 or CADDX hardware, a full libcamera build including the >= 0.5 branch, and a real GCS or flight controller reacting to gimbal_device_id 0. |
3 participants
This adds support for the camera gimbal setup on my aerial photography copter which is:
This was almost entirely written by Claude but I've tested that it works and I will review the code more thoroughly before we merge.
Changes outside the new target that reviewers may want to look at:
/dev/memon all non-MT11 targets, photo capture refused on targets without photo support, and optional switches for targets running on a general-purpose OSWhat's been tested:
There are these known issues:
As a side note, on my custom carrier board the XFRobot C20-T camera gimbal was connected to the Autopilot's SERIAL3 port. To make this available to the RPI I used AP's serial passthrough feature to make it accessible to the RPI via ethernet. This required these settings:
CAMERA_APP_UART=udp://<autopilot IP>:15002, and Rpanion-server needs a UDP output to127.0.0.1:14570.