Skip to content

MZTC thermal camera integration, rebased onto next-10.1 - #12172

Open
wdunn001 wants to merge 2 commits into
iNavFlight:next-10.1from
wdunn001:mztc-next-10.1
Open

wdunn001 wants to merge 2 commits into
iNavFlight:next-10.1from
wdunn001:mztc-next-10.1

Conversation

@wdunn001

Copy link
Copy Markdown

Adds support for the Mass Zero thermal camera (MZTC).

This supersedes #11837. That PR superseded #11005. Both of those branches have
fallen behind next-10.1 and no longer apply. #11837 is currently the only
open PR against next-10.1 that GitHub reports as conflicting. I opened a fresh
PR because the head of #11837 lives on a fork I cannot push to.

What it adds

The driver speaks the camera's serial protocol over a dedicated serial function.
Configuration lives in a new parameter group. Camera state reaches the
configurator over MSP and the CLI. A new OSD element reports camera status. Zoom
and palette are exposed as in-flight adjustments. A mode switch triggers the
camera's calibration routine.

USE_MZTC stays off 512 KB targets to preserve flash space. Targets above that
compile it in. SITL builds it regardless, since SITL has no flash constraint.
Nothing changes on a target that compiles the camera out.

Rebase notes

The work is squashed onto next-10.1 at d02085d. Two places needed attention.

src/main/target/common.h carried the only real code conflict. Upstream added
RP2350 to the USE_TELEMETRY_SBUS2 gate in the same hunk as the USE_MZTC
block. Both changes are kept.

The generated MSP docs were reset to their next-10.1 content and rebuilt with
src/utils/gen_msp_docs.sh, since the previous branch predated both the doc
regeneration and the move of those generators into src/utils. That rebuild also
picks up upstream enums the committed docs had not yet captured. The new entries
cover srxl2CalPhase_e, gyroDataReady_e, navFwWpTurnMode_e and
BOXTHRUSTREVERSE. Those are unrelated to this feature. They are the generator
catching up.
docs/Settings.md was rebuilt with src/utils/update_cli_docs.py.

Every identifier this PR claims was checked against the current base. Serial
function bit 28 was already reserved upstream for this camera by name.
PG_MZTC_CAMERA_CONFIG takes 1048, above the previous highest allocation.
The MSP2 codes occupy 0x2240 through 0x2249, clear of upstream's highest INAV
code at 0x2234. BOXMZTCCALIBRATE sits at 64, above BOXTHRUSTREVERSE.

Testing

ninja check passes 664 of 664 tests, including 44 cases covering this driver.
The SITL build is clean under -DWARNINGS_AS_ERRORS=ON with no warnings.

This has not been flight tested against the current base. The hardware testing
behind the earlier revisions is described in #11005 and #11837.

Ray Morris did the review fixes and the earlier conflict resolution against
maintenance-10.x that this branch carries forward.

Add support for the Mass Zero thermal camera. The driver speaks the
camera's serial protocol on a dedicated serial function. Configuration
lives in a new parameter group. Camera state reaches the configurator
over MSP and the CLI. A new OSD element reports camera status. Zoom and
palette are exposed as in-flight adjustments. A mode switch triggers the
camera's calibration routine.

USE_MZTC stays off 512 KB targets to preserve flash space. SITL builds
the feature regardless, since SITL has no flash constraint.

This replaces the history of PR iNavFlight#11837. That branch carried two merges
of maintenance-10.x and no longer applied to the current base. The work
is squashed onto next-10.1 at d02085d.

The generated MSP docs were rebuilt with src/utils/gen_msp_docs.sh. That
rebuild also picks up upstream enums the committed docs had not yet
captured, including srxl2CalPhase_e, gyroDataReady_e, navFwWpTurnMode_e
and BOXTHRUSTREVERSE. docs/Settings.md was rebuilt with
src/utils/update_cli_docs.py.

Ray Morris did the review fixes and the earlier conflict resolution
against maintenance-10.x that this commit carries forward.
@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Integrate MassZero thermal camera control into INAV

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Add UART-based MassZero camera control with persistent image settings, presets, and connection
 monitoring.
• Expose camera controls and status through MSP, CLI, OSD, and in-flight switches.
• Limit firmware support by flash size; add protocol tests, setup documentation, and bench
 utilities.
Diagram

graph TD
    HOST["CLI and MSP"] --> CFG["Camera settings"] --> DRIVER["MZTC driver"] --> UART["Camera UART"] --> CAMERA["MassZero camera"]
    RC["RC controls"] --> DRIVER
    DRIVER --> OSD["OSD status"]
    DRIVER --> HOST
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extend an existing camera-control abstraction
  • ➕ Could reuse common camera-control entry points for future devices.
  • ➖ The MassZero wire protocol, image presets, and link diagnostics require device-specific behavior; broadening a shared abstraction increases scope and regression risk.

Recommendation: Keep the dedicated driver and integrate it through INAV's existing serial-function, parameter-group, scheduler, MSP, RC, and OSD mechanisms. A generic camera abstraction is worth revisiting if more devices need the same controls, but does not simplify this integration now.

Files changed (32) +3938 / -57

Enhancement (18) +1977 / -2
mztc_camera.hDefine camera settings, status, and protocol limits +189/-0

Define camera settings, status, and protocol limits

• Introduces the configuration and status models, presets and setting enums, wire-framing constants, validation limits, parameter-group declaration, and driver API.

src/main/config/mztc_camera.h

cli.cAdd camera status and control commands +296/-0

Add camera status and control commands

• Adds conditional CLI commands for status, presets, image settings, calibration, reconnection, camera-flash save, factory defaults, and vignetting correction.

src/main/fc/cli.c

fc_init.cInitialize the camera driver at boot +6/-0

Initialize the camera driver at boot

• Calls camera initialization when USE_MZTC is enabled.

src/main/fc/fc_init.c

fc_msp.cExpose camera configuration and control over MSP V2 +152/-0

Expose camera configuration and control over MSP V2

• Adds config and status reads plus validated configuration and action commands. Serializes fields individually and rejects invalid full-config requests before applying them.

src/main/fc/fc_msp.c

fc_msp_box.cRegister the thermal calibration mode +19/-0

Register the thermal calibration mode

• Adds a calibration box, makes it available when a camera UART is assigned, and reports its active state through MSP.

src/main/fc/fc_msp_box.c

fc_tasks.cSchedule periodic camera updates +9/-0

Schedule periodic camera updates

• Registers a low-priority 10 Hz task for connection management, configuration delivery, and switch handling.

src/main/fc/fc_tasks.c

rc_adjustments.cImplement in-flight zoom and palette stepping +40/-0

Implement in-flight zoom and palette stepping

• Adds bounded step adjustments that send new values to the camera, update stored settings, and log successful changes.

src/main/fc/rc_adjustments.c

rc_adjustments.hAssign thermal adjustment identifiers +4/-0

Assign thermal adjustment identifiers

• Defines conditional adjustment IDs 61 and 62 for camera zoom and palette.

src/main/fc/rc_adjustments.h

rc_modes.hAssign the thermal calibration box identifier +3/-0

Assign the thermal calibration box identifier

• Adds BOXMZTCCALIBRATE at box ID 64 when the feature is compiled in.

src/main/fc/rc_modes.h

mztc_camera.cImplement the MassZero serial camera driver +987/-0

Implement the MassZero serial camera driver

• Adds packet framing and validation, response decoding, probing and reconnects, connection-quality tracking, preset application, configuration delivery, and camera actions. Keeps serial receive handling separate from the scheduled configuration burst.

src/main/io/mztc_camera.c

mztc_camera.hExpose the camera device-ID accessor +35/-0

Expose the camera device-ID accessor

• Provides a driver-facing header and an accessor for identity bytes reported by the camera.

src/main/io/mztc_camera.h

osd.cRender and position camera status on the OSD +11/-0

Render and position camera status on the OSD

• Connects the thermal status formatter to OSD drawing and assigns the new item a default position.

src/main/io/osd.c

osd.hDeclare the camera OSD item +3/-0

Declare the camera OSD item

• Adds a conditional OSD_MZTC_STATUS identifier to the OSD item enumeration.

src/main/io/osd.h

mztc_camera_osd.cFormat thermal camera link state +72/-0

Format thermal camera link state

• Maps driver states to a short IR status string and blinks the display for offline or error states.

src/main/io/osd/mztc_camera_osd.c

mztc_camera_osd.hDeclare the thermal OSD formatter +44/-0

Declare the thermal OSD formatter

• Defines the formatter interface and output-buffer size for the camera status item.

src/main/io/osd/mztc_camera_osd.h

serial.hActivate the reserved camera serial function +3/-2

Activate the reserved camera serial function

• Uses reserved serial-function bit 28 for MZTC when the feature is enabled.

src/main/io/serial.h

msp_mztc.hDefine MZTC MSP V2 commands and payloads +101/-0

Define MZTC MSP V2 commands and payloads

• Allocates command codes 0x2240–0x2249 and documents the configuration, status, and control payload layouts.

src/main/msp/msp_mztc.h

scheduler.hDeclare the camera scheduler task +3/-0

Declare the camera scheduler task

• Adds a conditional scheduler task identifier for periodic MZTC processing.

src/main/scheduler/scheduler.h

Tests (2) +823 / -0
CMakeLists.txtRegister camera driver unit tests +4/-0

Register camera driver unit tests

• Builds the MZTC test with its driver dependencies and enables the feature definition for that test.

src/test/unit/CMakeLists.txt

mztc_camera_unittest.ccTest camera framing, settings, and driver behavior +819/-0

Test camera framing, settings, and driver behavior

• Tests captured packet framing, malformed input, configuration validation and presets, receive dispatch, calibration-switch behavior, and transmitted setting values using stubbed serial I/O.

src/test/unit/mztc_camera_unittest.cc

Documentation (5) +728 / -55
Inflight Adjustments.mdList thermal camera adjustment functions +4/-0

List thermal camera adjustment functions

• Documents zoom and palette adjustment IDs 61 and 62, and fills in the preceding adjustment entries.

docs/Inflight Adjustments.md

MassZero_Thermal_Camera.mdDocument camera setup and operation +346/-0

Document camera setup and operation

• Explains supported builds, UART and video wiring, settings, presets, CLI and MSP commands, in-flight controls, OSD status, troubleshooting, and safety limitations. Clarifies that INAV controls the camera but receives no thermal image pixels.

docs/MassZero_Thermal_Camera.md

Settings.mdDocument generated MZTC settings +137/-0

Document generated MZTC settings

• Adds generated reference entries for the camera's preset, image, palette, shutter, zoom, mirror, and correction-interval settings.

docs/Settings.md

inav_enums.jsonRegenerate MSP enum data +107/-26

Regenerate MSP enum data

• Adds MZTC adjustment, mode, OSD, and serial-function identifiers. Regeneration also captures unrelated enum changes already present on the base branch.

docs/development/msp/inav_enums.json

inav_enums_ref.mdRegenerate human-readable MSP enum reference +134/-29

Regenerate human-readable MSP enum reference

• Documents the new MZTC identifiers and reflects unrelated upstream enum additions and renumbering picked up by regeneration.

docs/development/msp/inav_enums_ref.md

Other (7) +410 / -0
CMakeLists.txtInclude camera modules in firmware sources +7/-0

Include camera modules in firmware sources

• Registers the camera configuration, driver, OSD formatter, and MSP header with the main build.

src/main/CMakeLists.txt

parameter_group_ids.hAllocate the camera parameter-group ID +5/-0

Allocate the camera parameter-group ID

• Assigns PG_MZTC_CAMERA_CONFIG ID 1048 and conditionally extends the INAV parameter-group range when MZTC is compiled in.

src/main/config/parameter_group_ids.h

settings.yamlDefine persistent camera settings and lookups +97/-0

Define persistent camera settings and lookups

• Adds the conditional camera parameter-group settings, enum lookup tables, defaults, and numeric bounds used by generated settings interfaces.

src/main/fc/settings.yaml

common.hGate camera support by available flash +6/-0

Gate camera support by available flash

• Enables USE_MZTC for targets above 512 KB of flash and for SITL, leaving smaller firmware builds without the feature.

src/main/target/common.h

test_thermal_camera.pyAdd a direct serial camera probe utility +86/-0

Add a direct serial camera probe utility

• Provides a pyserial script for sending identity, initialization, and brightness commands to a camera on a configured COM port.

src/utils/test_thermal_camera.py

thermal_bridge.pyAdd a serial-to-SITL TCP bridge +83/-0

Add a serial-to-SITL TCP bridge

• Forwards traffic bidirectionally between a camera COM port and a SITL TCP endpoint, with brief byte logging.

src/utils/thermal_bridge.py

thermal_bridge_debug.pyAdd a diagnostic camera-to-SITL bridge +126/-0

Add a diagnostic camera-to-SITL bridge

• Provides verbose bidirectional bridge logging and sends a model probe before forwarding traffic.

src/utils/thermal_bridge_debug.py

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. The camera never opens its UART ✓ Resolved
Description
TASK_MZTC_CAMERA defines mztcUpdate but is never enabled in fcTasksInit(). The scheduler
initially queues only the system task, so the update loop that opens the assigned UART and probes
the camera never runs on hardware or SITL.
Code

src/main/fc/fc_tasks.c[R721-725]

+    [TASK_MZTC_CAMERA] = {
+        .taskName = "MZTC",
+        .taskFunc = mztcUpdate,
+        .desiredPeriod = TASK_PERIOD_HZ(10),
+        .staticPriority = TASK_PRIORITY_LOW,
Evidence
The new task is defined, but task initialization does not enable it; the scheduler adds only
TASK_SYSTEM by default, while UART opening occurs in the camera update callback.

src/main/fc/fc_tasks.c[388-515]
src/main/scheduler/scheduler.c[194-205]
src/main/scheduler/scheduler.c[232-235]
src/main/io/mztc_camera.c[543-555]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The camera update task is defined but never queued, so no UART opens or probe runs.
## Fix Focus Areas
- src/main/fc/fc_tasks.c[388-515]
- src/main/fc/fc_tasks.c[720-726]
## Recommended Fix
Enable TASK_MZTC_CAMERA in fcTasksInit under USE_MZTC and verify the assigned port opens during normal boot.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Pilots cannot select thermal calibration ✓ Resolved
Description
initActiveBoxIds() calls mztcIsEnabled() before mztcInit() sets the driver's initialized flag.
At boot the check is therefore false even with a camera UART assigned, and the sole active-box
initialization omits the calibration mode from the modes exposed over MSP.
Code

src/main/fc/fc_msp_box.c[R282-284]

+    if (mztcIsEnabled()) {
+        ADD_ACTIVE_BOX(BOXMZTCCALIBRATE);
+    }
Evidence
Boot calls mspFcInit before mztcInit; mspFcInit builds the active-box list, and mztcIsEnabled
requires mztcInitialized.

src/main/fc/fc_init.c[624-631]
src/main/fc/fc_msp.c[5417-5420]
src/main/io/mztc_camera.c[617-620]
src/main/fc/fc_msp_box.c[163-190]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The calibration mode is filtered out while active boxes are initialized because the camera driver has not initialized yet.
## Fix Focus Areas
- src/main/fc/fc_init.c[624-631]
- src/main/fc/fc_msp_box.c[278-285]
## Recommended Fix
Initialize the camera before the active-box list is built, or make the box-availability check depend directly on the configured serial assignment rather than driver initialization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. CLI image settings leave the camera unchanged ✓ Resolved
Description
mztcUpdate() watches only the preset byte for generic settings changes, not changes to the other
mztc_* fields. Setting brightness, palette, zoom, mirror, denoise, shutter mode, or correction
interval through the documented set interface changes the stored value but sends nothing while the
link stays connected.
Code

src/main/io/mztc_camera.c[R601-605]

+    // A preset written through the settings framework or "set mztc_preset"
+    // only stores a byte. Applying it here is what makes every path agree.
+    if (mztcConfig()->preset != mztcLastAppliedPreset) {
+        mztcSetPreset((mztcPreset_e)mztcConfig()->preset);
+    }
Evidence
The settings group exposes the individual fields; the update loop sends only a pending burst and
compares only preset, while camera transmission is performed elsewhere.

src/main/fc/settings.yaml[4836-4912]
src/main/io/mztc_camera.c[579-605]
src/main/io/mztc_camera.c[853-879]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Generic CLI setting writes bypass camera setters and only preset changes are detected.
## Fix Focus Areas
- src/main/io/mztc_camera.c[579-605]
- src/main/fc/settings.yaml[4836-4912]
## Recommended Fix
Detect changes to the complete camera configuration or hook setting updates into a driver API that queues the affected camera commands.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (1)
4. Every thermal camera CLI subcommand just prints status ✓ Resolved
Description
The CLI dispatcher walks cmdTable linearly and accepts the first entry whose name is a prefix of
the input when the next character is not alphanumeric; _ is not alphanumeric (sl_isalnum), and
the mztc entry is listed before all mztc_* entries. As a result mztc_preset 3,
mztc_calibrate, mztc_zoom 2, mztc_vignetting, mztc_save, mztc_defaults etc. all dispatch
to cliMztc, which ignores its argument and prints status, so none of the documented CLI controls
ever reach the camera.
Code

src/main/fc/cli.c[R5407-5408]

+    CLI_COMMAND_DEF("mztc", "MassZero Thermal Camera status", NULL, cliMztc),
+    CLI_COMMAND_DEF("mztc_calibrate", "trigger a manual shutter / flat field correction", NULL, cliMztcCalibrate),
Evidence
cliProcess matches with `!sl_strncasecmp(cliBuffer, cmd->name, strlen(cmd->name)) &&
!sl_isalnum(cliBuffer[strlen(cmd->name)])` and breaks on the first match; sl_isalnum only accepts
digits and letters, so mztc_calibrate matches the earlier mztc entry.

src/main/fc/cli.c[5577-5585]
src/main/common/string_light.c[24-27]
src/main/fc/cli.c[5406-5419]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `mztc` CLI command shadows every `mztc_*` command because the linear prefix match treats `_` as a terminator and `mztc` appears first in cmdTable.
## Fix Focus Areas
- src/main/fc/cli.c[5406-5419]
- src/main/fc/cli.c[5577-5585]
## Recommended Fix
Rename the status command (e.g. `mztc_status`) or move the `mztc` entry after all `mztc_*` entries so longer names are matched first; also move the whole block to its alphabetical spot after `motor`/`msc` entries to keep the table sorted as the comment requires. Update docs/MassZero_Thermal_Camera.md accordingly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. Factory reset loses its shutter interval ✓ Resolved
Description
mztcRestoreDefaults() resets several local fields but leaves ffc_interval and preset unchanged
after sending the factory-reset command. Status and settings can therefore retain the old preset and
interval, and the next reconnect sends the stale interval back to the camera.
Code

src/main/io/mztc_camera.c[R932-935]

+    cfg->palette_mode = SETTING_MZTC_PALETTE_MODE_DEFAULT;
+    cfg->zoom_level = SETTING_MZTC_ZOOM_LEVEL_DEFAULT;
+    cfg->mirror_mode = SETTING_MZTC_MIRROR_MODE_DEFAULT;
+    cfg->auto_shutter = SETTING_MZTC_AUTO_SHUTTER_DEFAULT;
Evidence
The reset template supplies defaults for both omitted fields; restore-defaults does not assign
either one, and the reconnect configuration sends the locally stored interval.

src/main/io/mztc_camera.c[159-171]
src/main/io/mztc_camera.c[915-937]
src/main/io/mztc_camera.c[859-869]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Factory reset leaves the locally stored preset and shutter interval unchanged and reconnect retransmits the stale interval.
## Fix Focus Areas
- src/main/io/mztc_camera.c[915-937]
- src/main/io/mztc_camera.c[159-171]
## Recommended Fix
Reset all fields represented in the parameter-group defaults, including preset and ffc_interval, when restoring camera defaults.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. A failed calibration stays on the display ✓ Resolved
Description
mztcTriggerCalibration() immediately sets the status to calibrating, but the error-reply branch of
mztcHandlePacket() does not clear it or mark calibration failed. If the camera rejects that
command while continuing to answer periodic probes, the OSD keeps showing calibration rather than
returning to a usable state or showing the failure.
Code

src/main/io/mztc_camera.c[R643-645]

+    if (mztcSendPacket(MZTC_CLASS_SHUTTER, MZTC_SUB_MANUAL_SHUTTER, MZTC_FLAG_WRITE, NULL, 0)) {
+        mztcStatus.status = MZTC_STATUS_CALIBRATING;
+        mztcLastCalibrationTime = millis();
Evidence
Calibration sets the state; only a successful manual-shutter reply restores it, while error replies
merely set a communication flag and subsequent healthy probes prevent the link timeout.

src/main/io/mztc_camera.c[342-352]
src/main/io/mztc_camera.c[381-419]
src/main/io/mztc_camera.c[573-590]
src/main/io/osd/mztc_camera_osd.c[39-49]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A rejected manual shutter command leaves the status stuck in calibrating while the link remains healthy.
## Fix Focus Areas
- src/main/io/mztc_camera.c[342-352]
- src/main/io/mztc_camera.c[381-419]
- src/main/io/mztc_camera.c[637-650]
## Recommended Fix
Recognize a manual-shutter error reply and update calibration error and status; provide a bounded fallback if no matching reply arrives.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Changing camera ports keeps the old UART ✓ Resolved
Description
mztcUpdate() checks the configured serial function only when its port pointer is null. Removing or
moving the assignment while a link is active leaves the old UART open and probed, with connection
status still based on that stale handle.
Code

src/main/io/mztc_camera.c[R553-555]

+    if (mztcSerialPort == NULL) {
+        mztcTryOpenPort(now);
+        return;
Evidence
The configured function is looked up only during opening, whereas connected state depends only on
the existing handle and connection flag.

src/main/io/mztc_camera.c[481-515]
src/main/io/mztc_camera.c[543-575]
src/main/io/mztc_camera.c[612-620]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The driver never notices a changed or removed camera port assignment once its UART is open.
## Fix Focus Areas
- src/main/io/mztc_camera.c[481-515]
- src/main/io/mztc_camera.c[543-575]
## Recommended Fix
Remember the opened port identifier and compare it with the current assignment during updates; close and reopen when the assignment changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (7)
8. Onboard menus cannot show camera status ✓ Resolved
Description
OSD_MZTC_STATUS starts without OSD_VISIBLE_FLAG and has no corresponding entry in the CMS
OSD-items menu. A pilot configuring the onboard OSD without an external configurator cannot enable
or position the new, initially hidden status item.
Code

src/main/io/osd.c[R4597-4599]

+#ifdef USE_MZTC
+    osdLayoutsConfig->item_pos[0][OSD_MZTC_STATUS] = OSD_POS(1, 4);
+#endif
Evidence
Rendering requires the visible flag, but the default position omits it and the CMS list of
selectable items ends without the new status item.

src/main/io/osd.c[1818-1825]
src/main/io/osd.c[4597-4599]
src/main/cms/cms_menu_osd.c[173-331]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new status item is hidden by default and absent from the onboard menu used to enable OSD elements.
## Fix Focus Areas
- src/main/io/osd.c[4597-4599]
- src/main/cms/cms_menu_osd.c[173-331]
## Recommended Fix
Add a USE_MZTC-guarded status entry to the CMS OSD-items menu so pilots can enable and position it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. The bridge can drop serial bytes ✓ Resolved
Description
forward_data() passes each serial chunk to socket.send() once without handling a partial write,
and the debug bridge makes the same assumption. Under TCP backpressure, an unsent suffix is
discarded and SITL receives a truncated camera packet.
Code

src/utils/thermal_bridge.py[R30-33]

+                if hasattr(destination, 'write'):  # Serial destination
+                    destination.write(data)
+                else:  # Socket destination
+                    destination.send(data)
Evidence
The standard bridge makes one socket send and moves to the next serial read; the debug bridge prints
the returned byte count but likewise never retries the remainder.

src/utils/thermal_bridge.py[25-33]
src/utils/thermal_bridge_debug.py[23-32]
src/main/drivers/serial_tcp.c[178-188]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Both bridge variants discard any bytes that a single socket send does not accept.
## Fix Focus Areas
- src/utils/thermal_bridge.py[25-33]
- src/utils/thermal_bridge_debug.py[23-32]
## Recommended Fix
Use sendall for serial-to-TCP forwarding in both scripts, or retry until the entire chunk is sent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. The bridge stays running after SITL exits ✓ Resolved
Description
The TCP-to-serial worker exits on socket EOF, but the serial-to-TCP worker has no shared shutdown
condition and main() waits for both workers. When SITL disconnects while the serial port remains
open, the bridge does not complete cleanup or become ready for a new connection.
Code

src/utils/thermal_bridge.py[R72-75]

+        # Wait for threads to finish
+        t1.join()
+        t2.join()
+        
Evidence
The socket reader breaks on EOF while the serial reader continues its unconditional loop; main joins
both threads before reaching its cleanup block. The debug variant has the same lifecycle.

src/utils/thermal_bridge.py[17-40]
src/utils/thermal_bridge.py[61-80]
src/utils/thermal_bridge_debug.py[37-58]
src/utils/thermal_bridge_debug.py[101-122]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Socket EOF ends only one forwarding worker and leaves the bridge waiting for the other indefinitely.
## Fix Focus Areas
- src/utils/thermal_bridge.py[17-40]
- src/utils/thermal_bridge.py[61-80]
- src/utils/thermal_bridge_debug.py[18-58]
- src/utils/thermal_bridge_debug.py[101-122]
## Recommended Fix
Share a shutdown signal between workers, close or interrupt the remaining endpoint on EOF, and let main clean up or reconnect.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. A saved preset is never applied after reboot or reconnect ✓ Resolved
Description
mztcResetLinkState() and mztcInit() set mztcLastAppliedPreset to the stored preset, while
mztcUpdate() checks for changes only after its disconnected early return. If a generic `set
mztc_preset = X` is stored while disconnected, reconnecting—or rebooting before a tick applies
it—leaves the preset’s image fields at their old values even though the stored and reported preset
is X.
Code

src/main/io/mztc_camera.c[435]

+    mztcLastAppliedPreset = mztcConfig()->preset;
Evidence
Disconnected updates return at lines 558–571 before the preset comparison at lines 603–605 can
expand the selection into image values. Reopening the port calls mztcResetLinkState(), which
copies the stored preset into the comparison marker; mztcInit() does the same at boot, so neither
path recognizes the pending change.

src/main/io/mztc_camera.c[426-439]
src/main/io/mztc_camera.c[527-531]
src/main/io/mztc_camera.c[553-605]
src/main/io/mztc_camera.c[426-438]
src/main/io/mztc_camera.c[553-570]
src/main/io/mztc_camera.c[601-605]
src/main/io/mztc_camera.c[708-737]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A preset selected through generic configuration while disconnected can remain unexpanded because link reset or initialization marks it as already applied.
## Fix Focus Areas
- src/main/io/mztc_camera.c[426-439]
- src/main/io/mztc_camera.c[527-531]
- src/main/io/mztc_camera.c[553-605]
## Recommended Fix
Do not overwrite `mztcLastAppliedPreset` in `mztcResetLinkState`; update it when `mztcSetPreset` applies the preset. Apply a pending preset to the config even while disconnected, or check for it before `mztcUpdate()` returns early. Preserve the intended boot-time no-reapply behavior without treating an unexpanded saved selection as applied, and test a generic CLI preset change made during disconnection.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Configurator config writes do not reach the camera ✓ Resolved
Description
The MSP2_SET_MZTC_CONFIG handler copies the validated candidate into *mztcConfigMutable()
without setting mztcConfigurationPending, so an already-connected camera keeps its previous image
settings even though MSP acknowledges the update. If the payload also changes preset, the next
mztcUpdate tick calls mztcSetPreset and overwrites the palette and image values the host
supplied.
Code

src/main/fc/fc_msp.c[4186]

+            *mztcConfigMutable() = candidate;
Evidence
The MSP handler only assigns the validated struct. The driver sends stored settings to the camera
through mztcSendConfiguration when mztcConfigurationPending is set, but the handler does not set
that flag; the update loop instead checks for a preset mismatch and re-applies the preset,
overwriting explicit fields.

src/main/fc/fc_msp.c[4164-4190]
src/main/io/mztc_camera.c[579-582]
src/main/io/mztc_camera.c[601-605]
src/main/io/mztc_camera.c[708-738]
src/main/fc/fc_msp.c[4164-4189]
src/main/io/mztc_camera.c[579-605]
src/main/io/mztc_camera.c[708-737]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MSP2_SET_MZTC_CONFIG` stores a validated full configuration without scheduling transmission to the camera. When the payload changes the preset, the next update can also overwrite its explicit palette and image values.
## Fix Focus Areas
- src/main/fc/fc_msp.c[4164-4190]
- src/main/io/mztc_camera.c[579-605]
## Recommended Fix
Add a driver API, such as `mztcApplyConfig(const mztcConfig_t*)`, that accepts the validated full configuration, stores it, sets `mztcLastAppliedPreset` to the candidate's preset so its explicit values take precedence, and sets `mztcConfigurationPending = true` even when the preset is unchanged. Call that API from the MSP handler instead of assigning the struct directly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. Link quality stays at 87% while probes go unanswered ✓ Resolved
Description
mztcSendProbe() tries to slide the eight-probe window by subtracting mztcProbesAnswered / 8
rather than expiring the oldest probe outcome, so the count stops decreasing as soon as it falls
below eight. After one missed reply leaves seven answered probes, further unanswered probes keep
connection_quality at 87% instead of letting it fall as the camera stops responding, until the
connection times out.
Code

src/main/io/mztc_camera.c[R460-466]

+    if (mztcProbesSent < MZTC_QUALITY_WINDOW) {
+        mztcProbesSent++;
+    } else {
+        // Slide the window so quality tracks recent traffic. Without this it
+        // would average everything since the port opened.
+        mztcProbesAnswered -= mztcProbesAnswered / MZTC_QUALITY_WINDOW;
+    }
Evidence
The send path caps the probe denominator at eight but stores only aggregate counters, so it cannot
identify and expire the oldest outcome. Integer division subtracts one only when
mztcProbesAnswered is eight; below eight it subtracts zero, leaving seven answered probes reported
as 7 * 100 / 8, or 87%, despite subsequent unanswered probes.

src/main/io/mztc_camera.c[454-477]
src/main/io/mztc_camera.c[376-379]
docs/MassZero_Thermal_Camera.md[338-338]
src/main/io/mztc_camera.c[121-123]
src/main/io/mztc_camera.c[454-476]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The probe-quality window uses an aggregate answered count that cannot expire the oldest outcome. After the first missed reply lowers the count below eight, integer division stops its decay and quality can remain at 87% through further unanswered probes.
## Fix Focus Areas
- src/main/io/mztc_camera.c[121-123]
- src/main/io/mztc_camera.c[376-379]
- src/main/io/mztc_camera.c[454-477]
## Recommended Fix
Replace the aggregate decay with a `uint8_t` bitmask history of the last eight probe outcomes. Shift the history left when sending each probe to add an unanswered slot, set bit 0 on a `READ_MODEL` reply, and calculate quality as `popcount(history & mask) * 100 / min(sent, 8)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


14. Status message layout differs on camera-enabled builds ✓ Resolved
Description
Adding BOXMZTCCALIBRATE = 64 raises CHECKBOX_ITEM_COUNT to 65, so boxBitmask_t grows from two
to three 32-bit words. MSP2_INAV_STATUS and MSP_ACTIVEBOXES write sizeof(boxBitmask_t), so on
USE_MZTC targets the payload grows by 4 bytes and the trailing mixer-profile byte moves; blackbox
also logs only bits[0..1], so the new mode is never recorded.
Code

src/main/fc/rc_modes.h[R92-94]

+#ifdef USE_MZTC
+    BOXMZTCCALIBRATE = 64,
+#endif
Evidence
BITARRAY_DECLARE allocates (bits+31)/32 words; 65 bits need 3 words. fc_msp.c writes
sizeof(mspBoxModeFlags) before the mixer profile byte. blackbox only copies bits[0] and bits[1].

src/main/common/bitarray.h[31-31]
src/main/fc/rc_modes.h[95-99]
src/main/fc/fc_msp.c[587-605]
src/main/blackbox/blackbox.c[1514-1515]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Crossing 64 box IDs changes MSP2_INAV_STATUS/MSP_ACTIVEBOXES payload size on USE_MZTC targets only and drops the new mode from blackbox.
## Fix Focus Areas
- src/main/fc/rc_modes.h[89-99]
- src/main/fc/fc_msp.c[587-605]
- src/main/blackbox/blackbox.c[1514-1515]
## Recommended Fix
Verify the configurator parses activeModes by length (and that mixerProfile is read from the end), document the size change in the MSP reference, and extend blackbox rcModeFlags logging if needed; alternatively coordinate with maintainers before being the first box beyond 63.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

15. Preset stays reported after manual image changes ✓ Resolved
Description
mztcSetPalette, mztcSetZoom, mztcSetImageParams and mztcSetDenoising (and set mztc_* on
owned fields) never reset preset to MZTC_PRESET_CUSTOM. The docs state that adjusting any
preset-owned setting returns to CUSTOM, so mztc, MSP status and the configurator keep showing e.g.
SEARCH while the camera runs hand-tuned values.
Code

src/main/io/mztc_camera.c[R747-749]

+    if (mztcSendPacket(MZTC_CLASS_IMAGE, MZTC_SUB_PSEUDO_COLOR, MZTC_FLAG_WRITE, &value, 1)) {
+        mztcConfigMutable()->palette_mode = value;
+        return true;
Evidence
None of the setter functions touch cfg->preset; the docs claim otherwise.

src/main/io/mztc_camera.c[740-812]
docs/MassZero_Thermal_Camera.md[100-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Manual changes to preset-owned fields leave the preset reported as a non-CUSTOM value, contrary to docs.
## Fix Focus Areas
- src/main/io/mztc_camera.c[740-812]
## Recommended Fix
In mztcSetPalette, mztcSetImageParams and mztcSetDenoising, after a successful write, set cfg->preset, mztcStatus.preset and mztcLastAppliedPreset to MZTC_PRESET_CUSTOM (zoom is not preset-owned, so leave it).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


16. Bridge scripts hardcode a developer's COM port and IP ✓ Resolved
Description
thermal_bridge.py and thermal_bridge_debug.py hardcode the serial port COM13 and the private
address 172.18.102.124, which they label as localhost, without command-line overrides. When the
serial port or SITL listener address differs, users must edit the scripts to connect, making them
read as developer-specific debugging artifacts in src/utils.
Code

src/utils/thermal_bridge.py[R12-15]

+SERIAL_PORT = 'COM13'  # Your thermal camera port
+SERIAL_BAUD = 115200
+TCP_HOST = '172.18.102.124'  # Connect to localhost (SITL)
+TCP_PORT = 5762
Evidence
Both scripts define fixed module-level connection values, including the same private IP despite
calling it localhost. SITL resolves its TCP listener address locally and supports configurable TCP
base ports, so the hardcoded destination need not match a running listener.

src/utils/thermal_bridge.py[11-15]
src/utils/thermal_bridge_debug.py[12-16]
src/main/drivers/serial_tcp.c[59-72]
src/main/target/SITL/target.c[505-536]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Both bridge scripts hardcode a developer-specific serial port and private IP address, preventing users with different serial devices or SITL listener addresses from connecting without source edits.
## Fix Focus Areas
- src/utils/thermal_bridge.py[11-15]
- src/utils/thermal_bridge_debug.py[12-16]
## Recommended Fix
Use `argparse` in both scripts to accept `--serial`, `--baud`, `--host`, and `--port`, with `127.0.0.1` and `5762` as the host and port defaults. Alternatively, merge the scripts with a `--debug` flag while retaining those connection options, or drop the scripts from the PR.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can import rules and skills only from the repositories your team follows

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/main/fc/fc_tasks.c
Comment thread src/main/fc/fc_msp_box.c
Comment thread src/main/io/mztc_camera.c Outdated
Comment thread src/main/io/mztc_camera.c
Comment thread src/main/io/mztc_camera.c
Comment thread src/main/fc/fc_msp.c Outdated
Comment thread src/main/io/mztc_camera.c Outdated
Comment thread src/main/io/mztc_camera.c
Comment thread src/main/fc/rc_modes.h
Comment thread src/utils/thermal_bridge.py Outdated
Three findings were fatal to the feature as shipped. The scheduler never
enabled TASK_MZTC_CAMERA. The UART therefore never opened on hardware or
on SITL. The CLI dispatcher takes the first prefix match whose next
character is not alphanumeric. An underscore is not alphanumeric. The
mztc status entry therefore swallowed every mztc_* command. The driver
also initialized after the box list was built. The THERMAL CALIBRATE
mode was never offered. The task is now enabled at init. The mztc entry
sits after the mztc_* entries with a comment explaining why. The driver
initializes before mspFcInit().

The driver now keeps a snapshot of the configuration it last sent. A
generic "set mztc_*" write is detected against that snapshot and
transmitted. A hand change to a preset-owned field moves the selection
to CUSTOM. The documentation always promised that behaviour.
MSP2_SET_MZTC_CONFIG goes through a new mztcApplyConfig(). The apply
queues the transmission and stores the preset byte as a label. It never
expands the preset over the explicit values in the same payload. A
preset stored while the link is down survives the link reset and is
applied. Factory reset also restores the preset and the correction
interval.

The connection quality window is a bitmask of the last eight probe
outcomes. The old arithmetic could not expire the oldest outcome. It
pinned quality at 87 percent through a dead link. A rejected manual
shutter command, or no reply at all within five seconds, returns the
status from calibrating to ready and raises the calibration error flag.
The update loop also notices when the Ports tab moves or removes the
camera's UART and drops the stale handle.

The OSD element gets a CMS entry. A pilot without a configurator can now
enable and place it. The bench bridge script takes its serial port and
endpoint from the command line, retries partial socket writes, and
shares one shutdown signal between both workers. The debug variant is
folded into it behind --verbose.

The box bitmask crossing 64 entries grows MSP2_INAV_STATUS and
MSP_ACTIVEBOXES by 4 bytes on camera builds. The configurator reads both
by length. The growth and the blackbox stance are documented in
docs/MassZero_Thermal_Camera.md.

The rebase onto next-10.1 had reset msp_messages.json to the upstream
copy. That file is a hand-maintained input to gen_msp_md.py. No generator
recreates its entries. The reset therefore lost the ten MZTC messages.
They are restored. README.md is regenerated from the result.
@wdunn001

Copy link
Copy Markdown
Author

All sixteen review findings are addressed in f7c0c3c.

Three were fatal to the feature as shipped. The scheduler never enabled the camera task. The mztc CLI entry shadowed every mztc_* command through the dispatcher's prefix match. The driver initialized after the box list was built. That hid the THERMAL CALIBRATE mode. All three are fixed.

The driver now keeps a snapshot of the configuration it last transmitted. Generic set mztc_* writes and MSP2_SET_MZTC_CONFIG payloads reach the camera through it. Hand changes to preset-owned fields move the selection to CUSTOM, as the docs promise. Factory reset restores every stored field. A rejected or unanswered shutter command returns the status from calibrating to ready. Connection quality is now an eight-probe bitmask window and decays properly. The update loop notices a moved or removed Ports tab assignment. The OSD element has a CMS menu entry. The bench bridge script takes its endpoints from the command line. It retries partial writes. Both workers share one shutdown signal.

The box bitmask crosses 64 entries on camera builds. The Configurator already reads MSP2_INAV_STATUS and MSP_ACTIVEBOXES by length. The growth is documented in docs/MassZero_Thermal_Camera.md along with the blackbox stance.

Nine unit tests cover the fixes. The suite passes 673 of 673 and SITL builds clean under -DWARNINGS_AS_ERRORS=ON.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant