Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion companion/src/apppreferencesdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ void AppPreferencesDialog::initSettings()
}
});

connect(ui->chkDelDecompress, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(ui->chkDelDecompress, &QCheckBox::stateChanged, [=](const int checked) {
if (!checked) {
if (ui->chkDecompressDirUseDwnld->isChecked()) {
ui->chkDelDownloads->setEnabled(false);
Expand Down
2 changes: 1 addition & 1 deletion companion/src/firmwares/sourcenumref.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ SourceNumRefEditor::SourceNumRefEditor(int & srcNumValue, QCheckBox * chkUseSour
lock(false)
{
if (chkUseSource)
connect(chkUseSource, &QCheckBox::checkStateChanged, this, &SourceNumRefEditor::chkUseSourceChanged);
connect(chkUseSource, &QCheckBox::stateChanged, this, &SourceNumRefEditor::chkUseSourceChanged);

if (sbxValue) {
sbxValue->setMinimum(minValue);
Expand Down
2 changes: 1 addition & 1 deletion companion/src/mdichild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,7 @@ void MdiChild::writeSettings(StatusDialog * status, bool toRadio)

QCheckBox *cb = new QCheckBox(tr("Do not show this message again"));
msgbox.setCheckBox(cb);
connect(cb, &QCheckBox::checkStateChanged, [=](const int &state){ g.confirmWriteModelsAndSettings(!state); });
connect(cb, &QCheckBox::stateChanged, [=](const int &state){ g.confirmWriteModelsAndSettings(!state); });
if (msgbox.exec() == QMessageBox::Abort)
return;
}
Expand Down
2 changes: 1 addition & 1 deletion companion/src/modeledit/colorcustomscreens.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ QGridLayout * addOptionsLayout(T & persistentData, int optioncnt, QString title)
val = QString("%1").arg(zovt.value.signedValue);
break;
case ZOV_String:
val = QString("%1").arg(zovt.value.stringValue);
val = QString("%1").arg(QString::fromStdString(zovt.value.stringValue));
break;
default:
val = QString("%1").arg(zovt.value.unsignedValue);
Expand Down
2 changes: 1 addition & 1 deletion companion/src/shared/curveimagewidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void CurveImageWidget::draw()

QImage image = curveImage->get();
if (index < 0)
image = image.flipped(Qt::Horizontal);
image = image.mirrored(true, false);
setPixmap(QPixmap::fromImage(image.scaled(height(), width())));

delete curveImage;
Expand Down
16 changes: 8 additions & 8 deletions companion/src/thirdparty/qcustomplot/qcustomplot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20642,9 +20642,9 @@ void QCPColorScaleAxisRectPrivate::draw(QCPPainter *painter)
// Qt 6.9.0 fix for depreciated function
// painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(mirrorHorz, mirrorVert));
if (mirrorHorz)
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.flipped(Qt::Horizontal));
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(true, false));
if (mirrorVert)
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.flipped(Qt::Vertical));
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(false, true));
Comment on lines 20644 to +20647

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix mirrored draw logic regression in gradient rendering.

Line 20644-20647 changes behavior: when both flags are true, the second draw overwrites the first (so both-axis mirror is lost), and when both are false nothing is drawn. Keep a single draw call with both flags applied once.

Proposed fix
-  if (mirrorHorz)
-    painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(true, false));
-  if (mirrorVert)
-    painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(false, true));
+  QImage img = mGradientImage;
+  if (mirrorHorz)
+    img = img.mirrored(true, false);
+  if (mirrorVert)
+    img = img.mirrored(false, true);
+  painter->drawImage(rect().adjusted(0, -1, 0, -1), img);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (mirrorHorz)
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.flipped(Qt::Horizontal));
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(true, false));
if (mirrorVert)
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.flipped(Qt::Vertical));
painter->drawImage(rect().adjusted(0, -1, 0, -1), mGradientImage.mirrored(false, true));
QImage img = mGradientImage;
if (mirrorHorz)
img = img.mirrored(true, false);
if (mirrorVert)
img = img.mirrored(false, true);
painter->drawImage(rect().adjusted(0, -1, 0, -1), img);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@companion/src/thirdparty/qcustomplot/qcustomplot.cpp` around lines 20644 -
20647, The two separate conditional draws cause overwrites and lost
double-mirroring; replace them with a single draw that applies both mirror flags
at once. In the code around mirrorHorz/mirrorVert, compute the image to draw
using mGradientImage.mirrored(mirrorHorz, mirrorVert) (or mGradientImage when
neither flag is set) and call painter->drawImage(rect().adjusted(0, -1, 0, -1),
<that image>) exactly once so both-axis mirroring and the no-mirror case work
correctly.

// end fix
QCPAxisRect::draw(painter);
}
Expand Down Expand Up @@ -26687,9 +26687,9 @@ void QCPColorMap::updateLegendIcon(Qt::TransformationMode transformMode, const Q
// mLegendIcon = QPixmap::fromImage(mMapImage.mirrored(mirrorX, mirrorY)).scaled(thumbSize, Qt::KeepAspectRatio, transformMode);
QImage img = mMapImage;
if (mirrorX)
img = img.flipped(Qt::Horizontal);
img = img.mirrored(true, false);
if (mirrorY)
img = img.flipped(Qt::Vertical);
img = img.mirrored(false, true);
mLegendIcon = QPixmap::fromImage(img).scaled(thumbSize, Qt::KeepAspectRatio, transformMode);
// end fix
}
Expand Down Expand Up @@ -26923,9 +26923,9 @@ void QCPColorMap::draw(QCPPainter *painter)
//localPainter->drawImage(imageRect, mMapImage.mirrored(mirrorX, mirrorY));
QImage img = mMapImage;
if (mirrorX)
img = img.flipped(Qt::Horizontal);
img = img.mirrored(true, false);
if (mirrorY)
img = img.flipped(Qt::Vertical);
img = img.mirrored(false, true);
localPainter->drawImage(imageRect, img);
// end fix
if (mTightBoundary)
Expand Down Expand Up @@ -30357,9 +30357,9 @@ void QCPItemPixmap::updateScaledPixmap(QRect finalRect, bool flipHorz, bool flip
if (flipHorz || flipVert) {
QImage img = mScaledPixmap.toImage();
if (flipHorz)
img = img.flipped(Qt::Horizontal);
img = img.mirrored(true, false);
if (flipVert)
img = img.flipped(Qt::Vertical);
img = img.mirrored(false, true);
mScaledPixmap = QPixmap::fromImage(img);
}
// end fix
Expand Down
6 changes: 3 additions & 3 deletions companion/src/updates/updateoptionsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ UpdateOptionsDialog::UpdateOptionsDialog(QWidget * parent, UpdateInterface * ifa
leSubFolders << leSubFolder;
layout3->addWidget(leSubFolder, 1, 2, 1, 2);

connect(chkDownload, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(chkDownload, &QCheckBox::stateChanged, [=](const int checked) {
if (!checked) {
chkDecompress->setChecked(false);
chkCopy->setChecked(false);
Expand All @@ -199,7 +199,7 @@ UpdateOptionsDialog::UpdateOptionsDialog(QWidget * parent, UpdateInterface * ifa
}
});

connect(chkDecompress, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(chkDecompress, &QCheckBox::stateChanged, [=](const int checked) {
if (!checked) {
chkCopy->setChecked(false);
chkInstall->setChecked(false);
Expand All @@ -210,7 +210,7 @@ UpdateOptionsDialog::UpdateOptionsDialog(QWidget * parent, UpdateInterface * ifa
}
});

connect(chkCopy, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(chkCopy, &QCheckBox::stateChanged, [=](const int checked) {
cboCopyFilterType->setEnabled(checked ? (processes & UpdateInterface::UPDFLG_CopyDest) && (!locked) : checked);
leCopyFilter->setEnabled(checked ? (processes & UpdateInterface::UPDFLG_CopyDest) && (!locked) : checked);
leSubFolder->setEnabled(checked ? (processes & UpdateInterface::UPDFLG_CopyDest) && (!locked) : checked);
Expand Down
6 changes: 3 additions & 3 deletions companion/src/updates/updatesdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ UpdatesDialog::UpdatesDialog(QWidget * parent, UpdateFactories * factories) :
ui->chkDelDecompress->setChecked(g.updDelDecompress());
ui->leDownloadDir->setText(g.downloadDir());

connect(ui->chkDecompressDirUseDwnld, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(ui->chkDecompressDirUseDwnld, &QCheckBox::stateChanged, [=](const int checked) {
if (!checked) {
ui->leDecompressDir->setText(g.decompressDir());
ui->leDecompressDir->setEnabled(true);
Expand All @@ -75,7 +75,7 @@ UpdatesDialog::UpdatesDialog(QWidget * parent, UpdateFactories * factories) :
ui->chkDecompressDirUseDwnld->setChecked(!ui->chkDecompressDirUseDwnld->isChecked());
ui->chkDecompressDirUseDwnld->setChecked(g.decompressDirUseDwnld());

connect(ui->chkUpdateDirUseSD, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(ui->chkUpdateDirUseSD, &QCheckBox::stateChanged, [=](const int checked) {
if (!checked) {
ui->leUpdateDir->setText(g.updateDir());
ui->leUpdateDir->setEnabled(true);
Expand Down Expand Up @@ -122,7 +122,7 @@ UpdatesDialog::UpdatesDialog(QWidget * parent, UpdateFactories * factories) :
}
});

connect(ui->chkDelDecompress, &QCheckBox::checkStateChanged, [=](const int checked) {
connect(ui->chkDelDecompress, &QCheckBox::stateChanged, [=](const int checked) {
if (!checked) {
if (ui->chkDecompressDirUseDwnld->isChecked()) {
ui->chkDelDownloads->setEnabled(false);
Expand Down
208 changes: 208 additions & 0 deletions radio/src/cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ static void cliReceiveData(uint8_t* buf, uint32_t len)
// Assumes it is called from ISR...
static void cliDefaultRx(uint8_t *buf, uint32_t len)
{
// FlapLink: When CRSF trainer mode is active, binary CRSF data is still
// forwarded to the stream buffer. The CLI task will check flaplinkTrainerActive
// and route bytes to the CRSF parser instead of CLI text processing.

BaseType_t xHigherPriorityTaskWoken = pdFALSE;
xStreamBufferSendFromISR(cliRxBuffer, buf, len, &xHigherPriorityTaskWoken);
portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
Expand Down Expand Up @@ -1486,6 +1490,200 @@ void printAudioVars()
}
#endif

/* ====== FlapLink Ground Station Commands ====== */
/* These commands are always available (no DEBUG required) to support
the FlapLink ground station HTML interface communication via USB VCP. */

#include "trainer.h"
#include "telemetry/crossfire.h"
#include "telemetry/telemetry_sensors.h"

// CRSF frame reception state for trainer input via USB VCP
static constexpr uint8_t FLAPLINK_CRSF_MAX_BUF = 64;
static uint8_t flaplinkCrsfBuf[FLAPLINK_CRSF_MAX_BUF];
static uint8_t flaplinkCrsfBufLen = 0;
static bool flaplinkTrainerActive = false;

// CRSF CRC8 lookup table (same as crossfire.cpp)
static const uint8_t crsfCrc8Table[256] = {
0x00, 0xD5, 0x7F, 0xAA, 0xFE, 0x2B, 0x81, 0x54, 0x29, 0xFC, 0x56, 0x83, 0xD7, 0x02, 0xA8, 0x7D,
0x52, 0x87, 0x2D, 0xF8, 0xAC, 0x79, 0xD3, 0x06, 0x7B, 0xAE, 0x04, 0xD1, 0x85, 0x50, 0xFA, 0x2F,
0xA4, 0x71, 0xDB, 0x0E, 0x5A, 0x8F, 0x25, 0xF0, 0x8D, 0x58, 0xF2, 0x27, 0x73, 0xA6, 0x0C, 0xD9,
0xF6, 0x23, 0x89, 0x5C, 0x08, 0xDD, 0x77, 0xA2, 0xDF, 0x0A, 0xA0, 0x75, 0x21, 0xF4, 0x5E, 0x8B,
0x9D, 0x48, 0xE2, 0x37, 0x63, 0xB6, 0x1C, 0xC9, 0xB4, 0x61, 0xCB, 0x1E, 0x4A, 0x9F, 0x35, 0xE0,
0xCF, 0x1A, 0xB0, 0x65, 0x31, 0xE4, 0x4E, 0x9B, 0xE6, 0x33, 0x99, 0x4C, 0x18, 0xCD, 0x67, 0xB2,
0x39, 0xEC, 0x46, 0x93, 0xC7, 0x12, 0xB8, 0x6D, 0x10, 0xC5, 0x6F, 0xBA, 0xEE, 0x3B, 0x91, 0x44,
0x6B, 0xBE, 0x14, 0xC1, 0x95, 0x40, 0xEA, 0x3F, 0x42, 0x97, 0x3D, 0xE8, 0xBC, 0x69, 0xC3, 0x16,
0xE3, 0x36, 0x9C, 0x49, 0x1D, 0xC8, 0x62, 0xB7, 0xCA, 0x1F, 0xB5, 0x60, 0x34, 0xE1, 0x4B, 0x9E,
0xB1, 0x64, 0xCE, 0x1B, 0x4F, 0x9A, 0x30, 0xE5, 0x98, 0x4D, 0xE7, 0x32, 0x66, 0xB3, 0x19, 0xCC,
0x47, 0x92, 0x38, 0xED, 0xB9, 0x6C, 0xC6, 0x13, 0x6E, 0xBB, 0x11, 0xC4, 0x90, 0x45, 0xEF, 0x3A,
0x15, 0xC0, 0x6A, 0xBF, 0xEB, 0x3E, 0x94, 0x41, 0x3C, 0xE9, 0x43, 0x96, 0xC2, 0x17, 0xBD, 0x68,
0x75, 0xA0, 0x0A, 0xDF, 0x8B, 0x5E, 0xF4, 0x21, 0x5C, 0x89, 0x23, 0xF6, 0xA2, 0x77, 0xDD, 0x08,
0x27, 0xF2, 0x58, 0x8D, 0xD9, 0x0C, 0xA6, 0x73, 0x0E, 0xDB, 0x71, 0xA4, 0xF0, 0x25, 0x8F, 0x5A,
0xD1, 0x04, 0xAE, 0x7B, 0x2F, 0xFA, 0x50, 0x85, 0xF8, 0x2D, 0x87, 0x52, 0x06, 0xD3, 0x79, 0xAC,
0x83, 0x56, 0xFC, 0x29, 0x7D, 0xA8, 0x02, 0xD7, 0xAA, 0x7F, 0xD5, 0x00, 0x54, 0x81, 0x2B, 0xFE
};

static uint8_t flaplinkCrsfCrc8(const uint8_t* ptr, uint8_t len)
{
uint8_t crc = 0;
for (uint8_t i = 0; i < len; i++) {
crc = crsfCrc8Table[crc ^ *ptr++];
}
return crc;
}

// Process a received CRSF frame from ground station
static void flaplinkProcessCrsfFrame(const uint8_t* frame, uint8_t len)
{
// frame[0] = address, frame[1] = length, frame[2] = type, ... frame[len-1] = CRC
if (len < 4) return;

uint8_t type = frame[2];

// Verify CRC (covers type + payload, i.e., frame[2..len-2])
uint8_t expectedCrc = flaplinkCrsfCrc8(&frame[2], len - 3);
if (expectedCrc != frame[len - 1]) return;

if (type == 0x16) {
// CRSF Channel frame - decode 16 channels into trainerInput
// Same logic as processCrossfireTelemetryFrame CHANNELS_ID
#define FLAPLINK_CRSF_CH_BITS 11
#define FLAPLINK_CRSF_CH_MASK ((1 << FLAPLINK_CRSF_CH_BITS) - 1)
#define FLAPLINK_CRSF_CH_CENTER 0x3E0

uint8_t byteIdx = 3;
uint32_t inputbits = 0;
uint8_t inputbitsavailable = 0;
int16_t* pulses = trainerInput;

for (int i = 0; i < 16; i++) {
while (inputbitsavailable < FLAPLINK_CRSF_CH_BITS) {
inputbits |= (uint32_t)(frame[byteIdx++]) << inputbitsavailable;
inputbitsavailable += 8;
}
*pulses++ = ((int32_t)(inputbits & FLAPLINK_CRSF_CH_MASK) - FLAPLINK_CRSF_CH_CENTER) * 5 / 8;
inputbitsavailable -= FLAPLINK_CRSF_CH_BITS;
inputbits >>= FLAPLINK_CRSF_CH_BITS;
}

trainerResetTimer();
}
Comment on lines +1548 to +1571

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add length validation before decoding channel data.

The channel decoding reads 22 bytes starting at frame[3] but doesn't verify that the frame's length field indicates sufficient payload. If a malformed frame with type 0x16 but a short length byte passes CRC validation (unlikely but possible), byteIdx will exceed len, causing a buffer over-read.

🛡️ Proposed fix
   if (type == 0x16) {
     // CRSF Channel frame - decode 16 channels into trainerInput
+    // Channel frame requires 22 bytes payload (16 channels * 11 bits)
+    // Length field = type(1) + payload(22) + crc(1) = 24
+    if (frame[1] < 24) return;
+
     `#define` FLAPLINK_CRSF_CH_BITS 11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/cli.cpp` around lines 1548 - 1571, Before decoding CRSF channel
data for type 0x16, validate the frame length to ensure there are 22 bytes
available starting at frame[3] (total minimal length = 3 + 22 = 25); if the
length (len) is too short, skip decoding and bail out (e.g., return or break) to
avoid buffer over-read. Add this check just before the block that defines
FLAPLINK_CRSF_CH_BITS/CH_MASK and uses byteIdx, inputbits and trainerInput, and
ensure trainerResetTimer() is only called after successful decoding.

}

// Feed raw bytes from USB VCP into CRSF frame parser
static void flaplinkFeedCrsfData(const uint8_t* data, uint8_t len)
{
if (!flaplinkTrainerActive) return;

for (uint8_t i = 0; i < len; i++) {
uint8_t byte = data[i];

if (flaplinkCrsfBufLen == 0) {
// Looking for sync address byte
if (byte == 0xEE || byte == 0xEC || byte == 0xEA || byte == 0xC8) {
flaplinkCrsfBuf[0] = byte;
flaplinkCrsfBufLen = 1;
}
} else if (flaplinkCrsfBufLen == 1) {
// Length byte
if (byte >= 2 && byte <= 62) {
flaplinkCrsfBuf[1] = byte;
flaplinkCrsfBufLen = 2;
} else {
flaplinkCrsfBufLen = 0; // invalid, reset
}
} else {
// Collecting payload + CRC
flaplinkCrsfBuf[flaplinkCrsfBufLen++] = byte;
uint8_t totalLen = 2 + flaplinkCrsfBuf[1]; // addr + len + payload
if (flaplinkCrsfBufLen >= totalLen) {
// Complete frame received
flaplinkProcessCrsfFrame(flaplinkCrsfBuf, flaplinkCrsfBufLen);
flaplinkCrsfBufLen = 0;
} else if (flaplinkCrsfBufLen >= FLAPLINK_CRSF_MAX_BUF) {
// Buffer overflow, reset
flaplinkCrsfBufLen = 0;
}
}
}
}

// CLI command: outputs - print channel output values (always available for ground station)
static int cliOutputs(const char** argv)
{
for (int i = 0; i < MAX_OUTPUT_CHANNELS; i++) {
cliSerialPrint("outputs[%d] = %04d", i, (int)channelOutputs[i]);
}
return 0;
}

// CLI command: trainer - control ground station trainer mode
// Usage:
// trainer crsf on - start receiving CRSF frames from USB VCP
// trainer crsf off - stop receiving CRSF frames
// trainer status - show trainer status
static int cliTrainer(const char** argv)
{
if (!strcmp(argv[1], "crsf")) {
if (!strcmp(argv[2], "on") || !strcmp(argv[2], "1")) {
flaplinkTrainerActive = true;
flaplinkCrsfBufLen = 0;
cliSerialPrint("FLAPLINK: CRSF trainer mode ON");
} else if (!strcmp(argv[2], "off") || !strcmp(argv[2], "0")) {
flaplinkTrainerActive = false;
flaplinkCrsfBufLen = 0;
// Reset trainer inputs to center
for (int i = 0; i < MAX_TRAINER_CHANNELS; i++) {
trainerInput[i] = 0;
}
cliSerialPrint("FLAPLINK: CRSF trainer mode OFF");
} else {
cliSerialPrint("Usage: trainer crsf on|off");
}
} else if (!strcmp(argv[1], "status")) {
cliSerialPrint("FLAPLINK trainer: %s", flaplinkTrainerActive ? "CRSF ON" : "OFF");
cliSerialPrint("Trainer mode: %d", (int)g_model.trainerData.mode);
if (flaplinkTrainerActive) {
cliSerialPrint("CH1=%d CH2=%d CH3=%d CH4=%d",
(int)trainerInput[0], (int)trainerInput[1],
(int)trainerInput[2], (int)trainerInput[3]);
}
} else {
cliSerialPrint("Usage: trainer crsf on|off | trainer status");
}
return 0;
}
Comment on lines +1626 to +1656

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against null argv elements to prevent crashes.

If the user types trainer without arguments, argv[1] is NULL (from memset at line 2132), causing strcmp(argv[1], "crsf") to crash. Similarly, trainer crsf without on/off leaves argv[2] as NULL.

🛡️ Proposed fix
 static int cliTrainer(const char** argv)
 {
-  if (!strcmp(argv[1], "crsf")) {
-    if (!strcmp(argv[2], "on") || !strcmp(argv[2], "1")) {
+  if (argv[1] && !strcmp(argv[1], "crsf")) {
+    if (argv[2] && (!strcmp(argv[2], "on") || !strcmp(argv[2], "1"))) {
       flaplinkTrainerActive = true;
       flaplinkCrsfBufLen = 0;
       cliSerialPrint("FLAPLINK: CRSF trainer mode ON");
-    } else if (!strcmp(argv[2], "off") || !strcmp(argv[2], "0")) {
+    } else if (argv[2] && (!strcmp(argv[2], "off") || !strcmp(argv[2], "0"))) {
       flaplinkTrainerActive = false;
       flaplinkCrsfBufLen = 0;
       // Reset trainer inputs to center
       for (int i = 0; i < MAX_TRAINER_CHANNELS; i++) {
         trainerInput[i] = 0;
       }
       cliSerialPrint("FLAPLINK: CRSF trainer mode OFF");
     } else {
       cliSerialPrint("Usage: trainer crsf on|off");
     }
-  } else if (!strcmp(argv[1], "status")) {
+  } else if (argv[1] && !strcmp(argv[1], "status")) {
     cliSerialPrint("FLAPLINK trainer: %s", flaplinkTrainerActive ? "CRSF ON" : "OFF");
     cliSerialPrint("Trainer mode: %d", (int)g_model.trainerData.mode);
     if (flaplinkTrainerActive) {
       cliSerialPrint("CH1=%d CH2=%d CH3=%d CH4=%d",
                      (int)trainerInput[0], (int)trainerInput[1],
                      (int)trainerInput[2], (int)trainerInput[3]);
     }
   } else {
     cliSerialPrint("Usage: trainer crsf on|off | trainer status");
   }
   return 0;
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/cli.cpp` around lines 1626 - 1656, The cliTrainer function
dereferences argv[1] and argv[2] without null checks which crashes if the user
supplies fewer args; update cliTrainer to guard accesses by either adding an
argc parameter or explicitly checking argv[1] and argv[2] != NULL before calling
strcmp, and return a usage/error message when missing; ensure the branches that
use flaplinkTrainerActive, flaplinkCrsfBufLen and trainerInput remain unchanged
except gated by these null checks so the code never reads argv[1]/argv[2] when
they are NULL.


// CLI command: telemetry - output telemetry data for ground station
// Usage:
// telemetry on - start streaming telemetry frames
// telemetry off - stop streaming
// telemetry once - output one frame of all sensor values
static int cliTelemetry(const char** argv)
{
if (!strcmp(argv[1], "on") || !strcmp(argv[1], "1")) {
cliSerialPrint("FLAPLINK: telemetry streaming ON (not yet implemented - use CRSF telemetry)");
} else if (!strcmp(argv[1], "off") || !strcmp(argv[1], "0")) {
cliSerialPrint("FLAPLINK: telemetry streaming OFF");
} else if (!strcmp(argv[1], "once")) {
// Output current telemetry values in text format
cliSerialPrint("FLAPLINK TELEMETRY DUMP:");
for (int i = 0; i < MAX_TELEMETRY_SENSORS; i++) {
TelemetrySensor& sensor = g_model.telemetrySensors[i];
if (sensor.isAvailable()) {
TelemetryItem& item = telemetryItems[i];
cliSerialPrint("sensor[%d] id=%d val=%d", i, (int)sensor.id, (int)item.value);
}
}
} else {
cliSerialPrint("Usage: telemetry on|off|once");
}
return 0;
}
Comment on lines +1663 to +1683

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Same null-pointer risk as cliTrainer.

Calling telemetry without arguments leaves argv[1] as NULL, causing a crash.

🛡️ Proposed fix
 static int cliTelemetry(const char** argv)
 {
-  if (!strcmp(argv[1], "on") || !strcmp(argv[1], "1")) {
+  if (!argv[1] || argv[1][0] == '\0') {
+    cliSerialPrint("Usage: telemetry on|off|once");
+  } else if (!strcmp(argv[1], "on") || !strcmp(argv[1], "1")) {
     cliSerialPrint("FLAPLINK: telemetry streaming ON (not yet implemented - use CRSF telemetry)");
   } else if (!strcmp(argv[1], "off") || !strcmp(argv[1], "0")) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/cli.cpp` around lines 1663 - 1683, The cliTelemetry function reads
argv[1] without checking for NULL, causing a crash when telemetry is invoked
with no arguments; update cliTelemetry to first verify argv[1] is non-NULL (and
if NULL, print the usage message and return) before any strcmp calls, and keep
the existing "on|off|once" handling intact; also ensure when iterating sensors
you still guard access by sensor.isAvailable() as currently done (references:
cliTelemetry, g_model.telemetrySensors, telemetryItems, MAX_TELEMETRY_SENSORS).


/* ====== End FlapLink Ground Station Commands ====== */

#if defined(DEBUG)

#include "hal/switch_driver.h"
Expand Down Expand Up @@ -1857,6 +2055,9 @@ const CliCommand cliCommands[] = {
#endif
{ "reboot", cliReboot, "[wdt]" },
{ "set", cliSet, "<what> <value>" },
{ "outputs", cliOutputs, "" }, // FlapLink: print channel outputs
{ "trainer", cliTrainer, "crsf on|off | status" }, // FlapLink: CRSF trainer via USB
{ "telemetry", cliTelemetry, "on|off|once" }, // FlapLink: telemetry output
#if defined(ENABLE_SERIAL_PASSTHROUGH)
{ "serialpassthrough", cliSerialPassthrough, "<port type> [<port number>] [<baudrate>]"},
#endif
Expand Down Expand Up @@ -1972,6 +2173,13 @@ static void cliTask()
continue;
}

// FlapLink: When CRSF trainer mode is active, feed bytes to CRSF parser
// instead of CLI text processing
if (flaplinkTrainerActive) {
flaplinkFeedCrsfData(&c, 1);
continue;
}

switch(c) {
case CHAR_NEWPAGE:
// clear screen
Expand Down