Conversation
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoAdd custom Tramp power levels and AUX pit mode
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1. Signal loss does not stop pit changes
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
1496112 to
ec6ea3b
Compare
…g the VTX Three defects in this pull request, found while going back over it. **The documented mode ID was wrong.** docs/VTx.md told the reader to assign permanent mode ID 69 and gave "aux 2 69 4 1800 2100" as the example. 69 is BOXAUTOSPEED; VTX PIT MODE is 72 (fc_msp_box.c:120). Following the instructions armed an autonomous speed mode on the switch instead of pit mode. Corrected in both places. **The switch never appeared.** initActiveBoxIds() gated BOXVTXPITMODE on feature(FEATURE_VTX). That flag has exactly two references in the whole tree: its own enum entry and that check. No target lists it in DEFAULT_FEATURES and COMMON_DEFAULT_FEATURES is (FEATURE_TX_PROF_SEL) alone, so on a stock configuration the mode was offered on no board at all. The gate is dropped; BOXVTXPITMODE is now added under USE_VTX_CONTROL the same way the camera boxes below it are, since VTX control is enabled by assigning a serial port function rather than by that flag. **Pit mode could starve channel and power indefinitely.** The IDLE dispatcher is a strict priority chain and pit mode was first in it, while every status response unconditionally restored the request whenever the reported state did not match. A Tramp-compatible device that will not enter pit mode - common, many only allow pit at power-up - therefore kept the highest-priority bit set forever, and no frequency or power command was ever sent again. Only a link timeout broke the loop, so a healthy link made it permanent. Frequency and power are now serviced before pit mode, and the requeue is bounded by VTX_PITMODE_MAX_RETRIES. The budget is refilled in impl_SetPitMode() only when the requested state actually changes, because io/vtx.c re-issues the same request roughly twice a second and would otherwise defeat the cap.
|
Went back over this one against the sources and found three defects, two of them raised by the bot review above and one it did not mention. All three are fixed in The documented mode ID was wrong, and dangerously so
Following the instructions as written put an autonomous speed mode on that switch instead of pit mode. Corrected in both places. The switch never appeared on any board
So on a stock configuration the new mode was offered nowhere. The gate is dropped and the box is added under Pit mode could starve channel and power for goodThis is the bot's first finding, and it is real and introduced here. The if (vtxState.request.pitModeRequested && vtxState.state.pitMode != vtxState.request.pitMode) {
vtxState.updateReqMask |= VTX_UPDATE_REQ_PITMODE;
}A Tramp-compatible device that will not enter pit mode — common, many only allow pit at power-up — therefore keeps the highest-priority bit set forever, and no frequency or power command is ever sent again. Only a link timeout clears it, so a healthy link makes the starvation permanent. Frequency and power are now serviced before pit mode, and the requeue is bounded by On the bot's other two findings"A saved power level can be ignored" is real but pre-existing, not introduced here. "Some transmitters ignore the pit switch" is also pre-existing driver behaviour: Not builtThere is no ARM toolchain on the machine this was written on, and INAV does not build natively on Windows — |
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #11887 250 targets built. Find your board's
|
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit f01c881 |
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 2671dd7 |
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 43e8b72 |
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 5c51bee |
MSP_BOXIDS and the activity bitmask follow the order of the active box list. A configurator that does not know VTX PIT MODE drops it from its mode list and maps the remaining activity bits by position, so a box in the middle shifted the active indicator of every later mode by one (CAMERA 1-3, USER1-4, TURTLE, MIXER PROFILE, ...). Appending it last, as IN FLIGHT MENU and THRUST REVERSE were, keeps older configurators right.
2 participants
Adds configurable Tramp power levels and an AUX pit-mode switch for #9841. Valid consecutive ascending values in vtx_tramp_power_a through e replace the fixed table after device capability detection. Advertised levels and labels reflect the reported or explicitly overridden power limit; duplicate capped levels are collapsed. Existing VTX settings migrate from PG version 2.
VTX PIT MODE uses permanent mode ID 73 and is offered only by drivers supporting pit control. Tramp cancels queued pit-entry requests on arming and rotates fairly through frequency, power and pit commands, so an ignored command cannot starve the others. Pit mismatch retries stop after three attempts; reconnect renews the budget. Blackbox records the third mode word.
The CMS power selector refreshes when device capabilities arrive or change and clamps the displayed index before rendering or saving. An overlarge saved setting remains unchanged until the user explicitly saves a valid selection.
Validation: 36 host tests passed (20 Tramp, 6 CMS, 9 AUX pit-mode, 1 settings migration), including startup metadata, ignored commands, arming before dispatch, reconnect and changing CMS capabilities. SPEEDYBEEF405V4 and MATEKH743 builds passed. Current CI results are tracked in the PR checks. Physical VTX testing is outstanding. Configuration, limits and migration are documented in docs/VTx.md.