Commit 14bffe5
committed
MZTC: address the review findings and correct the driver against the camera manual
Answers all eight Qodo findings and all eight questions from the review comment
on iNavFlight#11837, then corrects the driver against the BJ core serial protocol manual
that ships with the camera.
Serial protocol
The old sender wrote a variable-length prefix of a fixed-layout struct. The
checksum and the 0xFF terminator sat past that prefix. Neither was ever
transmitted and uninitialized bytes went out in their place. The size field was
payload+8 where the protocol says payload+4. For a full payload the computed
length ran past the end of the struct. The packet is now built
contiguously and matches the manual's own worked example byte for byte:
brightness 100 is F0 05 36 78 02 00 64 14 FF.
The receive parser is length driven. A 0xF0 or 0xFF byte inside a payload can
no longer split or truncate a packet. Responses are decoded by class and
subclass with per-payload length checks.
Corrections from the camera manual
Auto shutter values were off by one. The camera takes 0x01 temperature only,
0x02 time only and 0x03 time and temperature. It answers 0x00 with a threshold
error. The driver sent the zero-based setting straight through. The default
sent an out of range value.
The shutter interval belongs to the camera. 0x7C/0x05 takes two bytes of
minutes and the camera runs the schedule from it. The driver never sent that
command and ran a competing host-side timer instead. The interval is pushed on
connect now. mztc_ffc_interval accepts 1 to 60. TEMP_ONLY is how time-driven
correction is turned off.
The initialization status reply arrives on class 0x7D subclass 0x06. The host
asks on 0x7C/0x14. The decoder matched the request address and never fired.
Connection state
Opening the UART no longer counts as a connected camera. The driver probes for
the device model and reports connected once the camera answers. An established
link that stops answering for three seconds is closed and retried.
connection_quality is the share of recent probes answered. It was hardcoded to
100. The configuration burst moved out of the serial receive
interrupt.
Removed surfaces
The camera exposes a UART for control and a composite video output for the
picture. It has no digital data interface. Its manual defines 26 class and
subclass pairs. None of them read a frame or a temperature. 0x78/0x01 appears
only in the manual's invalid-subclass error example. 0x74/0x0C reads the ISP
parameter version number.
Frame data is removed. mztcFrameData_t, mztcGetFrameData(),
MSP2_MZTC_FRAME_DATA, the undocumented 0x78/0x30 request and the rand() based
simulator are all gone, along with frame_count and last_frame_time.
Temperature is removed. camera_temperature, ambient_temperature,
mztc_temperature_unit, mztc_temperature_alerts, mztc_alert_high_temp,
mztc_alert_low_temp and MSP2_SET_MZTC_ALERTS are gone.
Three settings were stored and validated but never transmitted. They did
nothing. mztc_bad_pixel_removal drove an interactive on-screen cursor that a
flight controller cannot walk. mztc_vignetting_correction is a one-shot action
at 0x7C/0x0C that needs the lens on a uniform surface first. It becomes the
mztc_vignetting command and MSP2_SET_MZTC_VIGNETTING. mztc_crosshair_enabled
had no camera command at all.
MSP
The commands move out of 0x3000, where MSP2_BETAFLIGHT_BIND and MSP2_RX_BIND
already live, into a contiguous block from 0x2240 to 0x2249. The identifiers
nothing handled are dropped along with the duplicate aliases.
Every field is serialized with sbufRead and sbufWrite. No struct is cast over
the stream buffer. Padding and alignment stay off the wire. The config payload
is 15 bytes and the status payload 7.
SET_MZTC_CONFIG validates the whole request before applying any of it. An
update_rate of zero was a reachable divide-by-zero in the camera task and an
unbounded baudrate indexed past baudRates[].
OSD
The module formatted strings and drew nothing. OSD_MZTC_STATUS replaces it as a
real element driven by INAV's own layout and render pipeline. That grows
OSD_ITEM_COUNT. Adding items moves the per-layout offsets in osdLayoutsConfig.
PG_OSD_LAYOUTS_CONFIG therefore goes to version 4.
Dead code
mztc_camera_cli.c was not in CMakeLists, was never initialized and called APIs
that do not exist. msp_mztc.c had a dispatch that was never invoked. Both are
removed. msp_mztc.h stays for the command IDs and the payload layouts.
mztc_shutter was identical to mztc_calibrate and mztc_simulate faked link
liveness. mztc_save and mztc_defaults are added so the camera flash commands
are reachable.
Configuration
The reset template is driven from the SETTING_*_DEFAULT macros. The
fresh-EEPROM defaults and the CLI defaults cannot diverge. That settles the
baudrate at 115200 and the mode at STANDBY. The limits live in the
settings.yaml constants block. Compile-time assertions check that the C limits
still match. A value the CLI rejects cannot be accepted over MSP.
last_calibration widens to uint16 after wrapping at about 4.25 hours. The PG
version goes to 1 because the PR test builds are already out.
Tests
The unit test properties were applied after the loop that creates every target.
USE_MZTC never reached the compiler. The whole test body was preprocessed away.
They move ahead of the loop.
The tautological assertions are replaced with 40 tests across three suites.
They cover the wire format against the hardware capture, the configuration
validator, and the receive path driven through the serial callback. The receive
tests exercise framing resynchronisation, payloads containing framing markers,
checksum rejection, response dispatch, and the exact bytes the configuration
burst puts on the wire. Five mutations confirmed the tests fail when each fixed
bug is reintroduced.
Docs
The feature doc is rewritten around commands that exist. Its preset, camera
management, video input and OSD settings were never implemented.
docs/Settings.md is regenerated. The MSP messages are added by hand to
msp_messages.json per docs/development/msp/README.md, with the MSP docs
regenerated.
Validated on three builds. SPEEDYBEEF405AIO with MZTC on, MATEKF722SE with MZTC
off, and SITL. All three compile with no warnings in any MZTC file. 556 of 556
unit tests pass.1 parent 1faf409 commit 14bffe5
25 files changed
Lines changed: 2721 additions & 3530 deletions
File tree
- docs
- development/msp
- src
- main
- config
- fc
- io
- osd
- msp
- test/unit
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3519 | 3519 | | |
3520 | 3520 | | |
3521 | 3521 | | |
3522 | | - | |
3523 | | - | |
3524 | | - | |
3525 | | - | |
3526 | | - | |
3527 | | - | |
3528 | | - | |
3529 | | - | |
3530 | | - | |
3531 | | - | |
3532 | | - | |
3533 | | - | |
3534 | | - | |
3535 | | - | |
3536 | | - | |
3537 | | - | |
3538 | | - | |
3539 | | - | |
3540 | | - | |
3541 | | - | |
3542 | 3522 | | |
3543 | 3523 | | |
3544 | 3524 | | |
| |||
3551 | 3531 | | |
3552 | 3532 | | |
3553 | 3533 | | |
3554 | | - | |
3555 | | - | |
3556 | | - | |
3557 | | - | |
3558 | | - | |
3559 | | - | |
3560 | | - | |
3561 | | - | |
3562 | | - | |
3563 | | - | |
3564 | 3534 | | |
3565 | 3535 | | |
3566 | | - | |
| 3536 | + | |
3567 | 3537 | | |
3568 | 3538 | | |
3569 | 3539 | | |
3570 | | - | |
| 3540 | + | |
3571 | 3541 | | |
3572 | 3542 | | |
3573 | 3543 | | |
| |||
3591 | 3561 | | |
3592 | 3562 | | |
3593 | 3563 | | |
3594 | | - | |
3595 | | - | |
3596 | | - | |
3597 | | - | |
3598 | | - | |
3599 | | - | |
3600 | | - | |
3601 | | - | |
3602 | | - | |
3603 | | - | |
3604 | 3564 | | |
3605 | 3565 | | |
3606 | 3566 | | |
| |||
3623 | 3583 | | |
3624 | 3584 | | |
3625 | 3585 | | |
3626 | | - | |
| 3586 | + | |
3627 | 3587 | | |
3628 | 3588 | | |
3629 | 3589 | | |
3630 | | - | |
| 3590 | + | |
3631 | 3591 | | |
3632 | 3592 | | |
3633 | 3593 | | |
| |||
3704 | 3664 | | |
3705 | 3665 | | |
3706 | 3666 | | |
3707 | | - | |
3708 | | - | |
3709 | | - | |
3710 | | - | |
3711 | | - | |
3712 | | - | |
3713 | | - | |
3714 | | - | |
3715 | | - | |
3716 | | - | |
3717 | | - | |
3718 | | - | |
3719 | | - | |
3720 | | - | |
3721 | | - | |
3722 | | - | |
3723 | | - | |
3724 | | - | |
3725 | | - | |
3726 | | - | |
3727 | | - | |
3728 | | - | |
3729 | 3667 | | |
3730 | 3668 | | |
3731 | 3669 | | |
| |||
3746 | 3684 | | |
3747 | 3685 | | |
3748 | 3686 | | |
3749 | | - | |
3750 | | - | |
3751 | | - | |
3752 | | - | |
3753 | | - | |
3754 | | - | |
3755 | | - | |
3756 | | - | |
3757 | | - | |
3758 | | - | |
3759 | 3687 | | |
3760 | 3688 | | |
3761 | 3689 | | |
| |||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
944 | 944 | | |
945 | 945 | | |
946 | 946 | | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
947 | 954 | | |
948 | 955 | | |
949 | 956 | | |
| |||
3203 | 3210 | | |
3204 | 3211 | | |
3205 | 3212 | | |
3206 | | - | |
| 3213 | + | |
| 3214 | + | |
| 3215 | + | |
| 3216 | + | |
3207 | 3217 | | |
3208 | 3218 | | |
3209 | 3219 | | |
| |||
3876 | 3886 | | |
3877 | 3887 | | |
3878 | 3888 | | |
3879 | | - | |
| 3889 | + | |
| 3890 | + | |
| 3891 | + | |
| 3892 | + | |
| 3893 | + | |
3880 | 3894 | | |
3881 | 3895 | | |
3882 | 3896 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| 76 | + | |
76 | 77 | | |
77 | 78 | | |
78 | 79 | | |
| |||
1613 | 1614 | | |
1614 | 1615 | | |
1615 | 1616 | | |
| 1617 | + | |
| 1618 | + | |
| 1619 | + | |
| 1620 | + | |
| 1621 | + | |
| 1622 | + | |
| 1623 | + | |
| 1624 | + | |
| 1625 | + | |
| 1626 | + | |
| 1627 | + | |
| 1628 | + | |
1616 | 1629 | | |
1617 | 1630 | | |
1618 | 1631 | | |
| |||
4653 | 4666 | | |
4654 | 4667 | | |
4655 | 4668 | | |
4656 | | - | |
| 4669 | + | |
| 4670 | + | |
| 4671 | + | |
| 4672 | + | |
4657 | 4673 | | |
4658 | 4674 | | |
4659 | 4675 | | |
| |||
5600 | 5616 | | |
5601 | 5617 | | |
5602 | 5618 | | |
| 5619 | + | |
5603 | 5620 | | |
5604 | 5621 | | |
5605 | 5622 | | |
| |||
0 commit comments