Merge pull request ExpressLRS#3331 from valeriyvan/fifo-skip

Fix bug in FIFO skip() leading to internal state corruption, add tests

Author: pkendall64

src/lib/FIFO/FIFO.h

@@ -335,7 +335,8 @@ class FIFO
      */
     ICACHE_RAM_ATTR void skip(const uint16_t len)
     {
-        numElements -= std::min((uint32_t)len, numElements);
-        head = (head + len) % FIFO_SIZE;
+        uint32_t skipCount = std::min((uint32_t)len, numElements);
+        numElements -= skipCount;
+        head = (head + skipCount) % FIFO_SIZE;
     }
 };

src/test/test_fifo/test_fifo.cpp

@@ -58,6 +58,107 @@ void test_fifo_ensure()
         TEST_ASSERT_EQUAL(10, f.pop()); // and that all the bytes in the head packet are what we expect
 }
 
+void test_fifo_skip_basic()
+{
+    f.flush();
+
+    // Add 5 elements
+    for(int i = 0; i < 5; i++) {
+        f.push(i + 10);
+    }
+    TEST_ASSERT_EQUAL(5, f.size());
+
+    // Skip 3 elements
+    f.skip(3);
+    TEST_ASSERT_EQUAL(2, f.size());
+
+    // Remaining elements should be correct
+    TEST_ASSERT_EQUAL(13, f.pop());
+    TEST_ASSERT_EQUAL(14, f.pop());
+    TEST_ASSERT_EQUAL(0, f.size());
+}
+
+void test_fifo_skip_overflow()
+{
+    f.flush();
+
+    // Add 5 elements
+    for(int i = 0; i < 5; i++) {
+        f.push(i + 10);
+    }
+    TEST_ASSERT_EQUAL(5, f.size());
+
+    // Try to skip 20 elements (but only 5 exist)
+    // This should skip all 5 and leave FIFO empty
+    f.skip(20);
+    TEST_ASSERT_EQUAL(0, f.size());
+
+    // FIFO should be in consistent state - can add new elements
+    f.push(99);
+    TEST_ASSERT_EQUAL(1, f.size());
+    TEST_ASSERT_EQUAL(99, f.pop());
+    TEST_ASSERT_EQUAL(0, f.size());
+}
+
+void test_fifo_skip_wraparound()
+{
+    f.flush();
+
+    // Fill buffer near capacity and advance head by popping
+    for(int i = 0; i < 200; i++) {
+        f.push(i);
+        f.pop();  // Advance head to near end of buffer
+    }
+
+    // Add elements that will wrap around
+    f.push(100);
+    f.push(101);
+    f.push(102);
+    TEST_ASSERT_EQUAL(3, f.size());
+
+    // Skip more elements than exist with wrap-around
+    f.skip(10);
+    TEST_ASSERT_EQUAL(0, f.size());
+
+    // Should be able to add elements normally
+    f.push(200);
+    f.push(201);
+    TEST_ASSERT_EQUAL(2, f.size());
+    TEST_ASSERT_EQUAL(200, f.pop());
+    TEST_ASSERT_EQUAL(201, f.pop());
+}
+
+void test_fifo_skip_zero()
+{
+    f.flush();
+
+    // Add elements
+    f.push(10);
+    f.push(11);
+    TEST_ASSERT_EQUAL(2, f.size());
+
+    // Skip zero elements should not change anything
+    f.skip(0);
+    TEST_ASSERT_EQUAL(2, f.size());
+    TEST_ASSERT_EQUAL(10, f.pop());
+    TEST_ASSERT_EQUAL(11, f.pop());
+}
+
+void test_fifo_skip_empty()
+{
+    f.flush();
+    TEST_ASSERT_EQUAL(0, f.size());
+
+    // Skip on empty FIFO should do nothing
+    f.skip(10);
+    TEST_ASSERT_EQUAL(0, f.size());
+
+    // Should still be able to add elements
+    f.push(42);
+    TEST_ASSERT_EQUAL(1, f.size());
+    TEST_ASSERT_EQUAL(42, f.pop());
+}
+
 // Unity setup/teardown
 void setUp() {}
 void tearDown() {}
@@ -68,6 +169,11 @@ int main(int argc, char **argv)
     RUN_TEST(test_fifo_pop_wrap);
     RUN_TEST(test_fifo_popBytes_wrap);
     RUN_TEST(test_fifo_ensure);
+    RUN_TEST(test_fifo_skip_basic);
+    RUN_TEST(test_fifo_skip_overflow);
+    RUN_TEST(test_fifo_skip_wraparound);
+    RUN_TEST(test_fifo_skip_zero);
+    RUN_TEST(test_fifo_skip_empty);
     UNITY_END();
 
     return 0;