INAV Configurator can not open some menus via the bridge

[TByte007]

After fixing the send issue I can open most menus in INAV Configurator but some like "Mixer", "Outputs" and "Modes" for example just show "Waiting for data ..." and never opens up. I can open much bigger (data wise) menus like "Programming" without a problem. I can see on the bridge's web that the serial is receiving data and I can see with Wireshark that INAV Configurator is supposed to be receiving it too. But that "Waiting" is going on forever. Can you point me in the right direction to find what the problem could be and can you tell me how to enable the debug log because setting with menuconfig "Default log verbosity (Debug)" doesn't seem to work.

Thank you.

[seeul8er]

https://github.com/DroneBridge/ESP32/blob/71422b15ac54e336c97a02bb82c952d59e15cc88/main/main.c#L424 sets the debug level and overwrites the one set via menuconfig. I am not sure how much of a help that will be.

Maybe the MSP parser is not fit to parse the latest MSP protocol. Seems like the configurator is waiting for some kind of packet that it never receives. All MSP packets are sent the moment the packet was received via UART.

[TByte007]

This turned out to be somewhat more complicated. There were 3 basic problems after fixing it both INAV Configurator and MWP work perfectly:

1. DroneBridge was ignoring the UNSUPPORTED / ERROR packets - the ones marked with "!" instead of "<" or ">" and that was leading to basically locking some apps or app menus.
2. After a problematic packet of any kind the counter of the bytes from the UART (serial_read_bytes) was not reset leading to crashes of the ESP32.
3. The size of the input buffer on the Ground Control (INAV, MWP and so on) side is 512 and not 192 as on the Flight Controller side. And that was forcing the bridge to ignore the larger (Jumbo) frames especially the ones with text in it.

Sorry that i didn't create a pull request for this. I'm attaching the patch and pasting it here to evaluate it : esp32db.patch

diff --git a/main/db_esp32_control.c b/main/db_esp32_control.c
index 2daa82a..d6516ce 100644
--- a/main/db_esp32_control.c
+++ b/main/db_esp32_control.c
@@ -137,7 +137,7 @@ void send_to_all_clients(int tcp_clients[], struct udp_conn_list_t *n_udp_conn_l
 void write_to_uart(const char data_buffer[], const size_t data_length) {
     int written = uart_write_bytes(UART_NUM, data_buffer, data_length);
     if (written > 0)
-        ESP_LOGD(TAG, "Wrote %i bytes", written);
+        ESP_LOGD(TAG, "Wrote %i bytes to UART", written);
     else
         ESP_LOGE(TAG, "Error writing to UART %s", esp_err_to_name(errno));
 }
@@ -158,8 +158,8 @@ void parse_msp_ltm(int tcp_clients[], struct udp_conn_list_t *udp_connection, ui
             if (parse_msp_ltm_byte(db_msp_ltm_port, serial_byte)) {
                 msp_message_buffer[(*serial_read_bytes - 1)] = serial_byte;
                 if (db_msp_ltm_port->parse_state == MSP_PACKET_RECEIVED) {
-                    *serial_read_bytes = 0;
                     send_to_all_clients(tcp_clients, udp_connection, msp_message_buffer, *serial_read_bytes);
+                    *serial_read_bytes = 0;
                 } else if (db_msp_ltm_port->parse_state == LTM_PACKET_RECEIVED) {
                     memcpy(&ltm_frame_buffer[ltm_frames_in_buffer_pnt], db_msp_ltm_port->ltm_frame_buffer,
                            (db_msp_ltm_port->ltm_payload_cnt + 4));
@@ -174,6 +174,8 @@ void parse_msp_ltm(int tcp_clients[], struct udp_conn_list_t *udp_connection, ui
                         *serial_read_bytes = 0;
                     }
                 }
+            } else { // XXX Leads to crashes of the ESP32 without it!
+                *serial_read_bytes = 0;
             }
         }
     }
diff --git a/main/msp_ltm_serial.c b/main/msp_ltm_serial.c
index c6e4a83..053ad91 100644
--- a/main/msp_ltm_serial.c
+++ b/main/msp_ltm_serial.c
@@ -68,7 +68,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                     break;
                 default:
                     msp_ltm_port->parse_state = IDLE;
-                    break;
+                    return false;
             }
             break;

@@ -100,7 +100,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                     break;
                 default:
                     msp_ltm_port->parse_state = IDLE;
-                    break;
+                    return false;
             }
             msp_ltm_port->ltm_frame_buffer[2] = new_byte;
             msp_ltm_port->ltm_payload_cnt = 0;
@@ -125,7 +125,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                     break;
                 default:
                     msp_ltm_port->parse_state = IDLE;
-                    break;
+                    return false;
             }
             break;

@@ -135,28 +135,31 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                 msp_ltm_port->parse_state = LTM_PACKET_RECEIVED;
             } else {
                 msp_ltm_port->parse_state = IDLE;
+                return false;
             }
             break;

         case MSP_HEADER_M:
-            if (new_byte == '>') {
+            if (new_byte == '>' || new_byte == '!') {
                 msp_ltm_port->offset = 0;
                 msp_ltm_port->checksum1 = 0;
                 msp_ltm_port->checksum2 = 0;
                 msp_ltm_port->parse_state = MSP_HEADER_V1;
             } else {
                 msp_ltm_port->parse_state = IDLE;
+                return false;
             }
             break;

         case MSP_HEADER_X:
-            if (new_byte == '>') {
+            if (new_byte == '>' || new_byte == '!') {
                 msp_ltm_port->offset = 0;
                 msp_ltm_port->checksum2 = 0;
                 msp_ltm_port->mspVersion = MSP_V2_NATIVE;
                 msp_ltm_port->parse_state = MSP_HEADER_V2_NATIVE;
             } else {
                 msp_ltm_port->parse_state = IDLE;
+                return false;
             }
             break;

@@ -165,15 +168,15 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
             msp_ltm_port->checksum1 ^= new_byte;
             if (msp_ltm_port->offset == sizeof(mspHeaderV1_t)) {
                 mspHeaderV1_t *hdr = (mspHeaderV1_t *) &msp_ltm_port->inBuf[0];
-                // Check incoming buffer size limit
-                if (hdr->size > MSP_PORT_INBUF_SIZE) {
-                    msp_ltm_port->parse_state = IDLE;
-                } else if (hdr->cmd == MSP_V2_FRAME_ID) {
+                // No need to check incoming buffer size limit for V1
+                // because the buffer is 512 which is bigger than the INTMAX for byte
+                if (hdr->cmd == MSP_V2_FRAME_ID) {
                     if (hdr->size >= sizeof(mspHeaderV2_t) + 1) {
                         msp_ltm_port->mspVersion = MSP_V2_OVER_V1;
                         msp_ltm_port->parse_state = MSP_HEADER_V2_OVER_V1;
                     } else {
                         msp_ltm_port->parse_state = IDLE;
+                        return false;
                     }
                 } else {
                     msp_ltm_port->dataSize = hdr->size;
@@ -198,6 +201,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                 msp_ltm_port->parse_state = MSP_PACKET_RECEIVED;
             } else {
                 msp_ltm_port->parse_state = IDLE;
+                return false;
             }
             break;

@@ -210,6 +214,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                 msp_ltm_port->dataSize = hdrv2->size;
                 if (hdrv2->size > MSP_PORT_INBUF_SIZE) {
                     msp_ltm_port->parse_state = IDLE;
+                    return false;
                 } else {
                     msp_ltm_port->cmdMSP = hdrv2->cmd;
                     msp_ltm_port->cmdFlags = hdrv2->flags;
@@ -236,6 +241,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                 msp_ltm_port->parse_state = MSP_CHECKSUM_V1;
             } else {
                 msp_ltm_port->parse_state = IDLE;
+                return false;
             }
             break;

@@ -246,6 +252,7 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                 mspHeaderV2_t *hdrv2 = (mspHeaderV2_t *) &msp_ltm_port->inBuf[0];
                 if (hdrv2->size > MSP_PORT_INBUF_SIZE) {
                     msp_ltm_port->parse_state = IDLE;
+                    return false;
                 } else {
                     msp_ltm_port->dataSize = hdrv2->size;
                     msp_ltm_port->cmdMSP = hdrv2->cmd;
@@ -271,8 +278,9 @@ bool parse_msp_ltm_byte(msp_ltm_port_t *msp_ltm_port, uint8_t new_byte) {
                 msp_ltm_port->parse_state = MSP_PACKET_RECEIVED;
             } else {
                 msp_ltm_port->parse_state = IDLE;
+                return false;
             }
             break;
     }
     return true;
-}
\ No newline at end of file
+}
diff --git a/main/msp_ltm_serial.h b/main/msp_ltm_serial.h
index e2dacb3..8c958ff 100644
--- a/main/msp_ltm_serial.h
+++ b/main/msp_ltm_serial.h
@@ -23,7 +23,7 @@

 #define MSP_MAX_HEADER_SIZE 9
 #define MSP_V2_FRAME_ID 255
-#define MSP_PORT_INBUF_SIZE 192
+#define MSP_PORT_INBUF_SIZE 512
 #define MAX_MSP_PORT_COUNT 3
 #define MSP_PORT_DATAFLASH_BUFFER_SIZE 4096
 #define MSP_PORT_OUTBUF_SIZE 512

[seeul8er]

Wow thank you! Quiet a bit of investigation and work! I will look into it and then include it with the next release.

[TByte007]

Btw I asked the devs of INAV about the size of the frame and what the real size should be and they said that the MSP_PORT_INBUF_SIZE (192) is a legacy number and have not been assessed recently and have to be. He said and I quote: "MSP2_INAV_LOGIC_CONDITIONS currently returns 896 bytes, so with an non-FLASHFS FC, you will smash the stack bigtime, kill the FC (or just crash the SITL)."

So I might have been to conservative with MSP_PORT_INBUF_SIZE 512 but it's a start and it works :) But after they reassess it it will be better to set a larger value.

[seeul8er]

Thank you for your support and the implementation update. The issue should be fixed with the new v1.5 release. Since I do not have a flashed iNAV fc by hand I was not able to test it.

[TByte007]

Thank you for your support and the implementation update. The issue should be fixed with the new v1.5 release. Since I do not have a flashed iNAV fc by hand I was not able to test it.

It mostly works but there are things like MWP that don't work because the bridge is ignoring the '!' marked packets and the apps need those to be able to "fish" which features are supported/unsupported on the Flight controller.

you should change those two lines: https://github.com/DroneBridge/ESP32/blob/5486b1490dc575a16b6342a89f71426b17262a12/main/msp_ltm_serial.c#L142 https://github.com/DroneBridge/ESP32/blob/5486b1490dc575a16b6342a89f71426b17262a12/main/msp_ltm_serial.c#L153

to if (new_byte == '>' || new_byte == '!') { for those apps to work.

[seeul8er]

@TByte007 Damn seems I have missed that. Due to changes I had to apply the changes manually. I'll fix it. Thank you!

[TByte007]

@TByte007 Damn seems I have missed that. Due to changes I had to apply the changes manually. I'll fix it. Thank you!

Sorry for that. I'll do a proper PR if I find something else which needs fixing.

[seeul8er]

Should be fixed with v2.0 RC1: https://github.com/DroneBridge/ESP32/releases/tag/v2.0RC1