From 35adb5ba067cc1c0ec40c80b55d321dab8a9e23a Mon Sep 17 00:00:00 2001 From: Bruce Luckcuck Date: Mon, 4 Jan 2021 12:20:02 -0500 Subject: [PATCH] Only register CMS displayPort for SRXL and CRSF when appropriate Previous logic was always registering CRSL and SRXL as CMS displayPort devices regardless of whether the user was actually using that type of radio and telemetry. The problem this caused is that if the user accidentally (or intentionally) used the CMS invoke stick command while already in the CMS on the MAX7456 the logic interprets this to mean "switch to the next registered displayPort device". So in this case the CMS would appear to exit based on what the user could see. But in reality it's still active and simply switched to the next (nonexistent) device. The user is then stuck and can't arm because the `CMS` arming disabled will be active and they have no way to interact with the CMS. They can technically blindly do the CMS stick command 2 more times to get back to the MAX7456, but how would they know that? So this change only registers the CRSF and SRXL displayPorts when the user has selected those receiver types and enabled telemetry. This is actually only a partial solution since it prevents registering the devices when they're impossible to work, but doesn't do anything for the actual CRSF or SRXL user that may be using equipment not capable of displaying the CMS. So it's still possible for them to get stuck in this situation. But I don't see any reasonable way to prevent this. Perhaps it's time to rethink this "Switch to next CMS device" logic? It's something that nobody really knows about and seems to be more trouble than value. Maybe there are some edge cases where it's useful like using the OLED dashboard along with a MAX7456, but that's a rare use-case. Or possibly we need some explicit control where the user has to enable the particular displayPort device to be eligible for CMS? --- src/main/cms/cms.c | 2 +- src/main/io/displayport_crsf.c | 20 +++++++++++++++++--- src/main/io/displayport_srxl.c | 24 ++++++++++++++++++------ 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/main/cms/cms.c b/src/main/cms/cms.c index e188413603..d53228afb2 100644 --- a/src/main/cms/cms.c +++ b/src/main/cms/cms.c @@ -92,7 +92,7 @@ int menuChainBack; bool cmsDisplayPortRegister(displayPort_t *pDisplay) { - if (cmsDeviceCount >= CMS_MAX_DEVICE) { + if (!pDisplay || cmsDeviceCount >= CMS_MAX_DEVICE) { return false; } diff --git a/src/main/io/displayport_crsf.c b/src/main/io/displayport_crsf.c index 376752ad83..4ac97c713a 100644 --- a/src/main/io/displayport_crsf.c +++ b/src/main/io/displayport_crsf.c @@ -26,13 +26,20 @@ #if defined(USE_CRSF_CMS_TELEMETRY) #include "cms/cms.h" + #include "common/maths.h" #include "common/printf.h" #include "common/time.h" + +#include "config/feature.h" + #include "drivers/display.h" #include "drivers/time.h" + #include "io/displayport_crsf.h" +#include "rx/rx.h" + #define CRSF_DISPLAY_PORT_OPEN_DELAY_MS 400 #define CRSF_DISPLAY_PORT_CLEAR_DELAY_MS 45 @@ -198,9 +205,16 @@ bool crsfDisplayPortIsReady(void) displayPort_t *displayPortCrsfInit() { - crsfDisplayPortSetDimensions(CRSF_DISPLAY_PORT_ROWS_MAX, CRSF_DISPLAY_PORT_COLS_MAX); - displayInit(&crsfDisplayPort, &crsfDisplayPortVTable); - return &crsfDisplayPort; + if (featureIsEnabled(FEATURE_TELEMETRY) + && featureIsEnabled(FEATURE_RX_SERIAL) + && (rxConfig()->serialrx_provider == SERIALRX_CRSF)) { + + crsfDisplayPortSetDimensions(CRSF_DISPLAY_PORT_ROWS_MAX, CRSF_DISPLAY_PORT_COLS_MAX); + displayInit(&crsfDisplayPort, &crsfDisplayPortVTable); + return &crsfDisplayPort; + } else { + return NULL; + } } #endif diff --git a/src/main/io/displayport_srxl.c b/src/main/io/displayport_srxl.c index da4390d8c7..9422cd9f4d 100644 --- a/src/main/io/displayport_srxl.c +++ b/src/main/io/displayport_srxl.c @@ -25,10 +25,15 @@ #include "platform.h" #if defined (USE_SPEKTRUM_CMS_TELEMETRY) && defined (USE_CMS) && defined(USE_TELEMETRY_SRXL) +#include "cms/cms.h" + #include "common/utils.h" +#include "config/feature.h" + #include "drivers/display.h" -#include "cms/cms.h" + +#include "rx/rx.h" #include "telemetry/srxl.h" @@ -140,11 +145,18 @@ static const displayPortVTable_t srxlVTable = { displayPort_t *displayPortSrxlInit() { - srxlDisplayPort.device = NULL; - displayInit(&srxlDisplayPort, &srxlVTable); - srxlDisplayPort.rows = SPEKTRUM_SRXL_TEXTGEN_BUFFER_ROWS; - srxlDisplayPort.cols = SPEKTRUM_SRXL_TEXTGEN_BUFFER_COLS; - return &srxlDisplayPort; + if (featureIsEnabled(FEATURE_TELEMETRY) + && featureIsEnabled(FEATURE_RX_SERIAL) + && ((rxConfig()->serialrx_provider == SERIALRX_SRXL) || (rxConfig()->serialrx_provider == SERIALRX_SRXL2))) { + + srxlDisplayPort.device = NULL; + displayInit(&srxlDisplayPort, &srxlVTable); + srxlDisplayPort.rows = SPEKTRUM_SRXL_TEXTGEN_BUFFER_ROWS; + srxlDisplayPort.cols = SPEKTRUM_SRXL_TEXTGEN_BUFFER_COLS; + return &srxlDisplayPort; + } else { + return NULL; + } } #endif