Repository navigation
Conversation
… a function besides MSP checkPwmTimerConflicts() kept outputs off the pins of UART2 to UART8 but not UART1. On NEXUSX and VANTAC_RF007 the pads labelled AUX and SBUS are both UART1 and two timer outputs, and SPRACINGF7DUAL has the same on PWM 3 and 4: with a receiver on UART1, a motor or servo could be assigned to the receiver's pin and drive nothing once the receiver took it back. UART1 gets MSP by default next to the VCP, and MSP opens before the outputs and has always given them these pins, so MSP alone still does; any other function on UART1 now keeps them, as on UART2 to UART8.
|
ⓘ 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 QodoPrevent PWM outputs from claiming UART1 pins used by other functions
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link |
|
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 #12131 251 targets built. Find your board's
|
1 participant
Problem
checkPwmTimerConflicts()keeps motor and servo outputs off the pins of a serial port that has a function, but it checks UART2 to UART8 and the soft serial ports, not UART1. Three targets have timer outputs on UART1's pins:When UART1 has a function and the mixer needs enough outputs to reach those pads, both claim the pin, and whichever is set up last keeps it. A function opened after the outputs, such as the receiver, GPS or telemetry, takes the pin back: the motor or servo the Configurator shows on that pad drives nothing. One opened before them, such as DJI HD OSD or SmartPort master, loses it and doesn't work. On NEXUSX the pad labelled SBUS is UART1 RX, so a receiver wired where the label says, with 9 outputs in the mixer, leaves S9 dead without any warning.
Change
UART1 is checked like UART2 to UART8, with one difference: MSP alone doesn't count. On a board with a USB VCP, UART1 gets MSP by default unless the target gives it another function (
pgResetFn_serialConfig(), and some targets'targetConfiguration()). MSP opens before the outputs, so it has always given these pins up to them: on that default, AUX/SBUS and PWM 3/4 work as outputs today. Counting MSP would remove those outputs from every default configuration, and on SPRACINGF7DUAL PWM 3 and 4 are the only servo pads by default. That's probably why UART1 was left out when the check was written (#4705, 2019), two years after the MSP default (74e786a, 2017).So the outputs are kept with UART1 on MSP alone or off, and skipped as soon as UART1 gets any other function. The code can't tell the default MSP from MSP chosen on purpose, so a device that talks MSP on UART1 still loses these pins to the outputs, as today. As on UART2 to UART8, a function that uses one pin only (an SBUS receiver is receive-only) takes both pads. That is the one setup that works today and changes on update: an SBUS receiver on the SBUS pad with 8 outputs in the mixer, the last one on AUX. After this the mixer is one output short and the board reports it and won't arm, until the mixer or the port changes.
On NEXUSX this makes the table in
docs/boards/NEXUSX.mdhold for every function except MSP. @Raffi1202 noticed in #11925 that the firmware didn't do what the page says; that PR changes the page to the current behaviour, and the AUX/SBUS rows would need "except MSP" with this one. Happy to adjust either way.Testing
MSP2_INAV_OUTPUT_MAPPING_EXT2:pwm_mapping.cis not in the host build, so there is no unit test of the check itself.Not tested on a NEXUSX, VANTAC_RF007 or SPRACINGF7DUAL: I don't have one. If someone with a NEXUSX can try a receiver on the SBUS pad with 9 outputs in the mixer, that would confirm it on the board the docs describe.
It depends on nothing else of mine, and merges cleanly with the open PRs that also change
pwm_mapping.c(#12087, #12128, #12129).