Fix arming disable flag and OSD warning buffer size (#14057)

* Fix missing arming disable flag

* Add test for array length

* Fix array init && add getter

* armingDisableFlagNames is no longer external

* Update array

* Thanks ledvinap

* Revert designated initializers

* Increase OSD Warnings buffer size

* Remove standard libraries

* Refactor

* Update pos

* Restore comment

* Micro optimize

* Extract condition

* Another optimization

* Use cmpTimeUs (not cmp32)

* Revert STATIC ASSERT string change

* Adjust esc sensor warning

* I like this

* Optimize intended minimum width

* revert Optimize intended minimum width

* Is dshot telemetry and esc telemetry exclusive

* Fix conflict

* OSD - WARNINGS_PREFERRED_SIZE

* OSD - Return NONE for no flag

src/main/cli/cli.c

@@ -4892,9 +4892,9 @@ if (buildKey) {
     cliPrint("Arming disable flags:");
     armingDisableFlags_e flags = getArmingDisableFlags();
     while (flags) {
-        const int bitpos = ffs(flags) - 1;
+        const armingDisableFlags_e flag = 1 << (ffs(flags) - 1);
-        flags &= ~(1 << bitpos);
+        flags &= ~flag;
-        cliPrintf(" %s", armingDisableFlagNames[bitpos]);
+        cliPrintf(" %s", getArmingDisableFlagName(flag));
     }
     cliPrintLinefeed();
 }

src/main/fc/runtime_config.c

@@ -20,6 +20,7 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include <string.h>
 
 #include "platform.h"
 
@@ -32,7 +33,8 @@ uint16_t flightModeFlags = 0;
 
 static uint32_t enabledSensors = 0;
 
-// Must be no longer than OSD_WARNINGS_MAX_SIZE (11) to be displayed fully in OSD
+// Name must be no longer than OSD_WARNINGS_MAX_SIZE
+// try to keep names within OSD_WARNINGS_PREFFERED_SIZE
 const char *armingDisableFlagNames[]= {
     "NOGYRO",
     "FAILSAFE",
@@ -64,6 +66,7 @@ const char *armingDisableFlagNames[]= {
     "POS_HOLD_SW",
     "ARM_SWITCH",
 };
+STATIC_ASSERT(ARRAYLEN(armingDisableFlagNames) == ARMING_DISABLE_FLAGS_COUNT, armingDisableFlagNames_size_mismatch);
 
 static armingDisableFlags_e armingDisableFlags = 0;
 
@@ -87,6 +90,17 @@ armingDisableFlags_e getArmingDisableFlags(void)
     return armingDisableFlags;
 }
 
+// return name for given flag
+// will return first name (LSB) if multiple bits are passed
+const char *getArmingDisableFlagName(armingDisableFlags_e flag)
+{
+    if (!flag) {
+        return "NONE";
+    }
+    unsigned idx = ffs(flag & -flag) - 1;   // use LSB if there are multiple bits set
+    return idx < ARRAYLEN(armingDisableFlagNames) ? armingDisableFlagNames[idx] : "UNKNOWN";
+}
+
 /**
  * Enables the given flight mode.  A beep is sounded if the flight mode
  * has changed.  Returns the new 'flightModeFlags' value.

src/main/fc/runtime_config.h

@@ -73,12 +73,11 @@ typedef enum {
 
 #define ARMING_DISABLE_FLAGS_COUNT (LOG2(ARMING_DISABLED_ARM_SWITCH) + 1)
 
-extern const char *armingDisableFlagNames[ARMING_DISABLE_FLAGS_COUNT];
-
 void setArmingDisabled(armingDisableFlags_e flag);
 void unsetArmingDisabled(armingDisableFlags_e flag);
 bool isArmingDisabled(void);
 armingDisableFlags_e getArmingDisableFlags(void);
+const char *getArmingDisableFlagName(armingDisableFlags_e flag);
 
 typedef enum {
     ANGLE_MODE      = (1 << 0),

src/main/msp/msp.c

@@ -1301,7 +1301,7 @@ case MSP_NAME:
         {
             bool isBlinking;
             uint8_t displayAttr;
-            char warningsBuffer[OSD_FORMAT_MESSAGE_BUFFER_SIZE];
+            char warningsBuffer[OSD_WARNINGS_MAX_SIZE + 1];
 
             renderOsdWarning(warningsBuffer, &isBlinking, &displayAttr);
             const uint8_t warningsLen = strlen(warningsBuffer);
@@ -1311,9 +1311,7 @@ case MSP_NAME:
             }
             sbufWriteU8(dst, displayAttr);  // see displayPortSeverity_e
             sbufWriteU8(dst, warningsLen);  // length byte followed by the actual characters
-            for (unsigned i = 0; i < warningsLen; i++) {
+            sbufWriteData(dst, warningsBuffer, warningsLen);
-                sbufWriteU8(dst, warningsBuffer[i]);
-            }
             break;
         }
 #endif

src/main/osd/osd.c

@@ -453,7 +453,7 @@ void pgResetFn_osdElementConfig(osdElementConfig_t *osdElementConfig)
     for (unsigned i = 1; i <= OSD_PROFILE_COUNT; i++) {
         profileFlags |= OSD_PROFILE_FLAG(i);
     }
-    osdElementConfig->item_pos[OSD_WARNINGS] = OSD_POS((midCol - 6), (midRow + 3)) | profileFlags;
+    osdElementConfig->item_pos[OSD_WARNINGS] = OSD_POS((midCol - OSD_WARNINGS_PREFFERED_SIZE / 2), (midRow + 3)) | profileFlags;
 
     // Default to old fixed positions for these elements
     osdElementConfig->item_pos[OSD_CROSSHAIRS]         = OSD_POS((midCol - 2), (midRow - 1));

src/main/osd/osd_warnings.c

@@ -73,7 +73,7 @@ void renderOsdWarning(char *warningText, bool *blinking, uint8_t *displayAttr)
     const timeUs_t currentTimeUs = micros();
 
     static timeUs_t armingDisabledUpdateTimeUs;
-    static unsigned armingDisabledDisplayIndex;
+    static armingDisableFlags_e armingDisabledDisplayFlag = 0;
 
     warningText[0] = '\0';
     *displayAttr = DISPLAYPORT_SEVERITY_NORMAL;
@@ -82,34 +82,34 @@ void renderOsdWarning(char *warningText, bool *blinking, uint8_t *displayAttr)
     // Cycle through the arming disabled reasons
     if (osdWarnGetState(OSD_WARNING_ARMING_DISABLE)) {
         if (IS_RC_MODE_ACTIVE(BOXARM) && isArmingDisabled()) {
-            const armingDisableFlags_e armSwitchOnlyFlag = 1 << (ARMING_DISABLE_FLAGS_COUNT - 1);
             armingDisableFlags_e flags = getArmingDisableFlags();
 
             // Remove the ARMSWITCH flag unless it's the only one
-            if ((flags & armSwitchOnlyFlag) && (flags != armSwitchOnlyFlag)) {
+            if (flags != ARMING_DISABLED_ARM_SWITCH) {
-                flags -= armSwitchOnlyFlag;
+                flags &= ~ARMING_DISABLED_ARM_SWITCH;
             }
 
             // Rotate to the next arming disabled reason after a 0.5 second time delay
-            // or if the current flag is no longer set
+            // or if the current flag is no longer set or if just starting
-            if ((currentTimeUs - armingDisabledUpdateTimeUs > 5e5) || !(flags & (1 << armingDisabledDisplayIndex))) {
+            if (cmpTimeUs(currentTimeUs, armingDisabledUpdateTimeUs) > 500000
-                if (armingDisabledUpdateTimeUs == 0) {
+                || (flags & armingDisabledDisplayFlag) == 0) {
-                    armingDisabledDisplayIndex = ARMING_DISABLE_FLAGS_COUNT - 1;
-                }
                 armingDisabledUpdateTimeUs = currentTimeUs;
 
-                do {
+                armingDisableFlags_e flag = armingDisabledDisplayFlag << 1;         // next bit to try or 0
-                    if (++armingDisabledDisplayIndex >= ARMING_DISABLE_FLAGS_COUNT) {
+                armingDisableFlags_e flagsRemaining = flags & ~(flag - 1);          // clear all bits <= flag; clear all bits when flag == 0
-                        armingDisabledDisplayIndex = 0;
+                flag = flagsRemaining & -flagsRemaining;                            // LSB in remaining bits
-                    }
+                if (!flag) {
-                } while (!(flags & (1 << armingDisabledDisplayIndex)));
+                    // no bit is set above flag (or flag was 0), try again with all bits
+                    flag = flags & -flags;
+                }
+                armingDisabledDisplayFlag = flag;                                   // store for next iteration
             }
 
-            tfp_sprintf(warningText, "%s", armingDisableFlagNames[armingDisabledDisplayIndex]);
+            tfp_sprintf(warningText, "%s", getArmingDisableFlagName(armingDisabledDisplayFlag));
             *displayAttr = DISPLAYPORT_SEVERITY_WARNING;
             return;
         } else {
-            armingDisabledUpdateTimeUs = 0;
+            armingDisabledDisplayFlag = 0;                                          // start from LSB next time
         }
     }
 
@@ -278,60 +278,55 @@ void renderOsdWarning(char *warningText, bool *blinking, uint8_t *displayAttr)
 
 #ifdef USE_ESC_SENSOR
     // Show warning if we lose motor output, the ESC is overheating or excessive current draw
-    if (featureIsEnabled(FEATURE_ESC_SENSOR) && osdWarnGetState(OSD_WARNING_ESC_FAIL)) {
+    if (featureIsEnabled(FEATURE_ESC_SENSOR) && osdWarnGetState(OSD_WARNING_ESC_FAIL) && ARMING_FLAG(ARMED)) {
-        char escWarningMsg[OSD_FORMAT_MESSAGE_BUFFER_SIZE];
+        char* p = warningText;
-        unsigned pos = 0;
+        strcpy(p, "ESC");
+        p += strlen("ESC");
 
-        const char *title = "ESC";
+        bool escWarning = false;
-
+        for (unsigned i = 0; i < getMotorCount() && p < warningText + OSD_WARNINGS_MAX_SIZE - 1; i++) {
-        // center justify message
-        while (pos < (OSD_WARNINGS_MAX_SIZE - (strlen(title) + getMotorCount())) / 2) {
-            escWarningMsg[pos++] = ' ';
-        }
-
-        strcpy(escWarningMsg + pos, title);
-        pos += strlen(title);
-
-        unsigned i = 0;
-        unsigned escWarningCount = 0;
-        while (i < getMotorCount() && pos < OSD_FORMAT_MESSAGE_BUFFER_SIZE - 1) {
             escSensorData_t *escData = getEscSensorData(i);
-            const char motorNumber = '1' + i;
             // if everything is OK just display motor number else R, T or C
-            char warnFlag = motorNumber;
+            if (osdConfig()->esc_current_alarm != ESC_CURRENT_ALARM_OFF
-            if (ARMING_FLAG(ARMED) && osdConfig()->esc_rpm_alarm != ESC_RPM_ALARM_OFF && (uint32_t)erpmToRpm(escData->rpm) <= (uint32_t)osdConfig()->esc_rpm_alarm) {
+                && escData->current >= osdConfig()->esc_current_alarm) {
-                warnFlag = 'R';
+                *p++ = 'C';
+                escWarning = true;
+            } else if (osdConfig()->esc_temp_alarm != ESC_TEMP_ALARM_OFF
+                       && escData->temperature >= osdConfig()->esc_temp_alarm) {
+                *p++ = 'T';
+                escWarning = true;
+            } else if (osdConfig()->esc_rpm_alarm != ESC_RPM_ALARM_OFF
+                       && erpmToRpm(escData->rpm) <= osdConfig()->esc_rpm_alarm) {
+                *p++ = 'R';
+                escWarning = true;;
+            } else {   // no error, display motor number
+                *p++ = '0' + (i + 1) % 10; // 123..9012..
             }
-            if (osdConfig()->esc_temp_alarm != ESC_TEMP_ALARM_OFF && escData->temperature >= osdConfig()->esc_temp_alarm) {
-                warnFlag = 'T';
-            }
-            if (ARMING_FLAG(ARMED) && osdConfig()->esc_current_alarm != ESC_CURRENT_ALARM_OFF && escData->current >= osdConfig()->esc_current_alarm) {
-                warnFlag = 'C';
-            }
-
-            escWarningMsg[pos++] = warnFlag;
-
-            if (warnFlag != motorNumber) {
-                escWarningCount++;
-            }
-
-            i++;
         }
 
-        escWarningMsg[pos] = '\0';
+        *p++ = 0;  // terminate string
-
+        if (escWarning) {
-        if (escWarningCount > 0) {
+            const int msgLen = strlen(warningText);
-            tfp_sprintf(warningText, "%s", escWarningMsg);
+            const int minMsgLen = OSD_WARNINGS_PREFFERED_SIZE;           // intended minimum width
+            if (msgLen < minMsgLen - 1) {
+                // message is short, center it within minMsgLen
+                const int offset = (minMsgLen - msgLen) / 2;
+                memmove(warningText + offset, warningText, msgLen + 1);  // copy including '\0'
+                memset(warningText, ' ', offset);                        // left padding with spaces
+            }
             *displayAttr = DISPLAYPORT_SEVERITY_WARNING;
             *blinking = true;
             return;
+        } else {
+            // no warning, erase generated message and continue
+            warningText[0] = '\0';
         }
     }
 #endif // USE_ESC_SENSOR
 
 #if defined(USE_DSHOT) && defined(USE_DSHOT_TELEMETRY)
     // Show esc error
-    if (osdWarnGetState(OSD_WARNING_ESC_FAIL)) {
+    if (motorConfig()->dev.useDshotTelemetry && osdWarnGetState(OSD_WARNING_ESC_FAIL) && ARMING_FLAG(ARMED)) {
         uint32_t dshotEscErrorLengthMotorBegin;
         uint32_t dshotEscErrorLength = 0;
 
@@ -354,7 +349,7 @@ void renderOsdWarning(char *warningText, bool *blinking, uint8_t *displayAttr)
             warningText[dshotEscErrorLength++] = '0' + k + 1;
 
             // Add esc warnings
-            if (ARMING_FLAG(ARMED) && osdConfig()->esc_rpm_alarm != ESC_RPM_ALARM_OFF
+            if (osdConfig()->esc_rpm_alarm != ESC_RPM_ALARM_OFF
                     && isDshotMotorTelemetryActive(k)
                     && (dshotTelemetryState.motorState[k].telemetryData[DSHOT_TELEMETRY_TYPE_eRPM] * 100 * 2 / motorConfig()->motorPoleCount) <= osdConfig()->esc_rpm_alarm) {
                 warningText[dshotEscErrorLength++] = 'R';
@@ -364,7 +359,7 @@ void renderOsdWarning(char *warningText, bool *blinking, uint8_t *displayAttr)
                     && dshotTelemetryState.motorState[k].telemetryData[DSHOT_TELEMETRY_TYPE_TEMPERATURE] >= osdConfig()->esc_temp_alarm) {
                 warningText[dshotEscErrorLength++] = 'T';
             }
-            if (ARMING_FLAG(ARMED) && osdConfig()->esc_current_alarm != ESC_CURRENT_ALARM_OFF
+            if (osdConfig()->esc_current_alarm != ESC_CURRENT_ALARM_OFF
                     && (dshotTelemetryState.motorState[k].telemetryTypes & (1 << DSHOT_TELEMETRY_TYPE_CURRENT)) != 0
                     && dshotTelemetryState.motorState[k].telemetryData[DSHOT_TELEMETRY_TYPE_CURRENT] >= osdConfig()->esc_current_alarm) {
                 warningText[dshotEscErrorLength++] = 'C';

src/main/osd/osd_warnings.h

@@ -20,11 +20,11 @@
 
 #pragma once
 
-#define OSD_WARNINGS_MAX_SIZE 12
+#define OSD_WARNINGS_MAX_SIZE 31         // buffer size excluding terminating '\0'
-#define OSD_FORMAT_MESSAGE_BUFFER_SIZE (OSD_WARNINGS_MAX_SIZE + 1)
+#define OSD_WARNINGS_PREFFERED_SIZE 12   // normal size for OSD_WARNINGS (centered on OSD)
 
 extern const char CRASHFLIP_WARNING[];
 
-STATIC_ASSERT(OSD_FORMAT_MESSAGE_BUFFER_SIZE <= OSD_ELEMENT_BUFFER_LENGTH, osd_warnings_size_exceeds_buffer_size);
+STATIC_ASSERT(OSD_WARNINGS_MAX_SIZE + 1 <= OSD_ELEMENT_BUFFER_LENGTH, osd_warnings_size_exceeds_buffer_size);
 
 void renderOsdWarning(char *warningText, bool *blinking, uint8_t *displayAttr);