Repository navigation
Conversation
rmackay9
added
Copter
Plane
Rover
AntennaTracker
Sub
AP_Periph
ReleaseAdmin
AIReview
labels
Previous review (2026-10-13)Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Reviewed at c6bae07, which is still the PR head; there are no comments on the thread yet. Nothing blocking was found. The backports match their master originals, SITL and the affected boards build, and the new vibration, camera and Aeron autotests pass; the vibration and gyro-log tests each fail on 4.7.2 as they should. Two optional follow-ups. The fixed-notch commits 3967818 and da6a914 say "Save the values instead", but they only remove the two default overrides (Copter, Plane). In SITL a converted pre-4.2 notch now reads MODE 1 / HMNCS 3 from the converting boot, the same values 4.7.2 reaches after its first reboot, so the code looks intended and only the commit messages (and PR 34251's results table) need correcting. Second, master's 337719d "AP_Periph: use stm32_toggle_line() for LED toggles" was not picked with the HAL pin-toggle fix; it applies cleanly here and AP_Periph builds with it for ZubaxGNSS and f103-GPS. For information: the new RateThreadPostFilterGyroLog test dropped log blocks and failed at the default speedup on the review machine (6 of 6 runs; the 4.7.2 binary drops blocks too) and passed at --speedup 5. CI was still queued when checked, so if the Copter job fails there with a DSF.Dp message, that is the cause. Separately from this PR, the Aeron driver's GPS publication uses the unlocked AP_GPS::handle_external() path that the other external-AHRS drivers already use; AP_GPS::update() can delete that backend after a 4 s timeout, which deserves a fix on master. Not covered: CI results, the full test suites, hardware, and the PRs listed as left out. |
andyp1per
left a comment
andyp1per
left a comment
There was a problem hiding this comment.
I am nervous about the blheli fix having not tested it myself as yet. It's probably fine, but we really don't want to make things worse for people in an official release.
(cherry picked from commit 902f7ff)
(cherry picked from commit 96507c5)
(cherry picked from commit cf40388)
(cherry picked from commit 5ea4727)
(cherry picked from commit d529404)
(cherry picked from commit 74420e8, manually adapted to the ArduPilot-4.7 autotest layout) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
(cherry picked from commit 663fc09)
(cherry picked from commit 9de61c2)
(cherry picked from commit 332de37)
(cherry picked from commit a1ecb8b)
The driver stamps last_sens_ms and friends at parse time, and only the reader thread ever parsed: update() was an empty stub. healthy() therefore measured how recently that thread had been scheduled rather than how recently the PLX had sent anything. In SITL a non-main thread's wall-clock scheduling delay becomes speedup-multiplied simulated time, so an ordinary 1-2 ms preemption at SIM_SPEEDUP=20 exceeds the 40 ms SENS_PARA deadline. The spurious unhealthy drops AP_AHRS to DCM, which fails the arming check and makes the AeronEAHRS autotest fail intermittently. update() now calls check_and_decode(), serialised against the reader thread by a new parse_sem, as other EAHRS backends already do. The lock is required for correctness, not hardening: the UART ring buffer is a single-consumer structure, so two unserialised readers would each copy and advance past the same bytes. A setup_complete guard is added to check_and_decode() so both callers share one readiness check. available() is called inside parse_sem rather than before it: on SITL it can reach a blocking accept() via _check_connection(), which two unserialised callers could race into. SENS_TIMEOUT_MS is unchanged. On HALs that serve UART reads only to the thread that claimed the port (ChibiOS, ESP32) the reader thread owns it and the main-thread call returns having read nothing. That is intentional: those targets schedule the reader thread in real time and do not exhibit this starvation, so on flight hardware this part of the change is a no-op. healthy() also sampled the clock before reading the packet timestamps. The reader thread runs at PRIORITY_SPI, above the main thread, and can stamp a newer millisecond between the two reads; (now - stamp) then underflows to ~2^32 and health reads false for one loop, bouncing AP_AHRS to DCM and back. Read the timestamps first: millis() is monotonic and the stamps are only ever assigned from it, so sampling the clock afterwards guarantees now >= stamp. This part applies on all targets. (cherry picked from commit 9771a00)
…SS disabled publish_sens_para() declared baro_ok and mag_ok unconditionally but only used them inside the AP_BARO_EXTERNALAHRS_ENABLED and AP_COMPASS_EXTERNALAHRS_ENABLED blocks. With either feature disabled the variable is unused and -Werror=unused-variable fails the build: ./waf configure --board sitl --disable-EXTERNALAHRS_BARO && ./waf copter AP_ExternalAHRS_Aeron_plx.cpp:432:16: error: unused variable 'baro_ok' Move each declaration inside the block that uses it. No behaviour change with the default feature set. (cherry picked from commit b9305b3)
HOLD uses rate-only heading control, but yaw_rad() previously falls through to the position controller's travel bearing. This leaves _yaw_angle_rad incorrect for later absolute CONDITION_YAW and mount yaw offsets. Keep HOLD aligned with the attitude controller's yaw target, like the other rate-only AutoYaw modes. (cherry picked from commit ad2d9cc)
The servo camera added the incoming rate value straight onto its scaled zoom and focus outputs each update, so continuous zoom and focus always took about 20 seconds to cover the full travel. CAM1_ZOM_RAT_MAX and CAM1_FOC_RAT_MAX now set that speed in percent of travel per second. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit e8ea9b0)
Assign CameraZoom and CameraFocus to spare servos and check SERVO_OUTPUT_RAW moves at CAM1_ZOM_RAT_MAX and CAM1_FOC_RAT_MAX. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit e92b993)
Clamp continuous zoom and focus rates to the MAVLink range of -1 to 1. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit f7a223e)
A Caddx GM2 booted with MOUNT_RP_LOCK HIGH does not hold roll and pitch in earth frame until the switch is toggled, although the lock bits are sent set from the first packet. The toggle only changes what the gimbal receives, so it appears to ignore a lock mode that has not changed since it powered up. Send body frame for the first 5s so the requested mode arrives as a change once the gimbal has booted. (cherry picked from commit 7b503ea)
(cherry picked from commit 4b83b49)
(cherry picked from commit 1ff6319)
…ottle The NEU to NED conversion dropped the division by GRAVITY_MSS when computing the throttle-per-acceleration gain used while vibration compensation is active. This made both the feed-forward and the integrator gains roughly 10x too large, driving the vertical controller into a limit cycle that pulses throttle between zero and full. (cherry picked from commit 9e9486a)
(cherry picked from commit a648631)
(cherry picked from commit b6e4af6)
The radians conversion in d56e91b changed the threshold from 2 * PHLD_BRK_RATE centidegrees to 2 * PHLD_BRK_RATE degrees (0.16 deg to 16 deg with the default rate of 8), so braking started while the lean angle was still fading out. Restore the original threshold. Fixes ArduPilot#34617 (cherry picked from commit f7bba98)
With the fast rate thread running, _gyro_filtered only takes the samples selected for the rate loop, but post-filter batch and raw gyro logging recorded it after every sample. With the rate loop at a quarter of the gyro rate each logged value was held for four samples. The hold folds the vehicle's own motion and the motor lines into images around the rate loop frequency, and smooths the motor lines themselves, so a post-filter spectrum misrepresents the filters in both directions. Log the sample just filtered instead; what the front end and the rate loop see is unchanged. With a 6.4 kHz gyro and a 1.6 kHz rate loop, the images at 1608 Hz fell from 38 dB to 52 dB below the 0.5-20 Hz band, and the runs of four identical samples are gone. (cherry picked from commit 9e98a8b)
(cherry picked from commit 30a9288) Adapted to ArduPilot-4.7: the test list and helper layout differ, and assert_log_has_no_dropped_blocks() is added from master as the test uses it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Claude and I have updated this PR to include PRs merged directly against the ArduPilot-4.7 branch. I'm not planning to include any additional PRs in 4.7.3. I've updated the PR description to move the reasoning why several other PRs were not included down to the bottom. I think this PR is ready to go now so all approvals are very welcome! |
|
Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head The head has moved from c6bae07 to d926e68 since the last review, and this review covers d926e68. The 25 code and test commits are unchanged. The SaamPixV1_1 and fixed-notch commits were dropped, and the BLHeli/pin-mode fix is now in the ArduPilot-4.7 base (patch-identical to before), so the earlier notch commit-message point no longer applies. SITL and several boards build, and the new vibration, camera, gyro-log and Aeron autotests pass at the new head. Nothing blocking was found. One optional item: master's two-line commit 337719d "AP_Periph: use stm32_toggle_line() for LED toggles" still applies cleanly. Tools/AP_Periph/AP_Periph.cpp:433 and Tools/AP_Periph/can.cpp:793 still use palToggleLine(), which rewrites the whole port output register. Adding the commit would complete the fix the AP_Periph release note lists. @andyp1per's BLHeli concern still stands: the fix builds for Pixhawk6C, but no bench test of passthrough has been reported. Separately, the Aeron GPS publication shares the unlocked AP_GPS::handle_external() path with seven existing external-AHRS drivers, so a fix on master is worth considering. Not covered: CI results (mostly queued when checked), full suites, hardware, and the PRs left out. |
Summary
This is the 4.7.3-beta1 release affecting Plane, Copter, Rover, Sub, Tracker and Periph
These PRs were included with little or no merge conflicts:
Classification & Testing (check all that apply and add your own)
These PRs have not been included due to merge conflicts but if the original dev (or anyone else) wants to create back-port PRs we can include them in the next point release (e.g. 4.7.4)
This backport PR has not been included because it includes changes which haven't been merged to master yet
These backport PRs have not been included because they need to be reviewed, approved and merged to the ArduPilot-4.7 branch