diff --git a/firmware/controllers/system/timer/event_queue.cpp b/firmware/controllers/system/timer/event_queue.cpp index e8a7bde985..e3987546bc 100644 --- a/firmware/controllers/system/timer/event_queue.cpp +++ b/firmware/controllers/system/timer/event_queue.cpp @@ -72,6 +72,51 @@ bool EventQueue::insertTask(scheduling_s *scheduling, efitime_t timeX, action_s } } +void EventQueue::remove(scheduling_s* scheduling) { +#if EFI_UNIT_TEST + assertListIsSorted(); +#endif /* EFI_UNIT_TEST */ + + // Special case: empty list, nothing to do + if (!head) { + return; + } + + // Special case: is the item to remove at the head? + if (scheduling == head) { + head = head->nextScheduling_s; + scheduling->nextScheduling_s = nullptr; + scheduling->action = {}; + } else { + auto prev = head; // keep track of the element before the one to remove, so we can link around it + auto current = prev->nextScheduling_s; + + // Find our element + while (current && current != scheduling) { + prev = current; + current = current->nextScheduling_s; + } + + // Walked off the end, not present, nothing more to do + if (!current) { + return; + } + + efiAssertVoid(OBD_PCM_Processor_Fault, current == scheduling, "current not equal to scheduling"); + + // Link around the removed item + prev->nextScheduling_s = current->nextScheduling_s; + + // Clean the item to remove + current->nextScheduling_s = nullptr; + current->action = {}; + } + +#if EFI_UNIT_TEST + assertListIsSorted(); +#endif /* EFI_UNIT_TEST */ +} + /** * On this layer it does not matter which units are used - us, ms ot nt. * @@ -207,9 +252,7 @@ scheduling_s *EventQueue::getElementAtIndexForUnitText(int index) { return current; index--; } -#if EFI_UNIT_TEST - firmwareError(OBD_PCM_Processor_Fault, "getForUnitText: null"); -#endif /* EFI_UNIT_TEST */ + return NULL; } diff --git a/firmware/controllers/system/timer/event_queue.h b/firmware/controllers/system/timer/event_queue.h index 24d4a7298f..ca7bb0dcd2 100644 --- a/firmware/controllers/system/timer/event_queue.h +++ b/firmware/controllers/system/timer/event_queue.h @@ -50,6 +50,7 @@ public: * O(size) - linear search in sorted linked list */ bool insertTask(scheduling_s *scheduling, efitime_t timeX, action_s action); + void remove(scheduling_s* scheduling); int executeAll(efitime_t now); bool executeOne(efitime_t now); diff --git a/unit_tests/tests/test_signal_executor.cpp b/unit_tests/tests/test_signal_executor.cpp index 6c3f066989..0152c66d8f 100644 --- a/unit_tests/tests/test_signal_executor.cpp +++ b/unit_tests/tests/test_signal_executor.cpp @@ -69,7 +69,7 @@ static void orderCallback(void *a) { prevValue = value; } -TEST(misc, testSignalExecutor3) { +TEST(EventQueue, simple) { EventQueue eq; scheduling_s s1; @@ -82,7 +82,7 @@ TEST(misc, testSignalExecutor3) { eq.executeAll(100); } -TEST(misc, testSignalExecutor) { +TEST(EventQueue, complex) { EventQueue eq; ASSERT_EQ(eq.getNextEventTime(0), unexpected); scheduling_s s1; @@ -152,3 +152,64 @@ TEST(misc, testSignalExecutor) { ASSERT_EQ(2, callbackCounter); } + +class EventQueueRemoveTest : public ::testing::Test { +protected: + EventQueue dut; + scheduling_s s1, s2, s3; + + void SetUp() override { + dut.insertTask(&s1, 100, callback); + dut.insertTask(&s2, 200, callback); + dut.insertTask(&s3, 300, callback); + + // Check that things are assembled as we think + ASSERT_EQ(&s1, dut.getElementAtIndexForUnitText(0)); + ASSERT_EQ(&s2, dut.getElementAtIndexForUnitText(1)); + ASSERT_EQ(&s3, dut.getElementAtIndexForUnitText(2)); + ASSERT_EQ(nullptr, dut.getElementAtIndexForUnitText(3)); + } +}; + +TEST_F(EventQueueRemoveTest, removeHead) { + // Remove the element at the head + dut.remove(&s1); + + // Check that it's gone + ASSERT_EQ(&s2, dut.getElementAtIndexForUnitText(0)); + ASSERT_EQ(&s3, dut.getElementAtIndexForUnitText(1)); + ASSERT_EQ(nullptr, dut.getElementAtIndexForUnitText(2)); +} + +TEST_F(EventQueueRemoveTest, removeMiddle) { + // Remove the element in the middle + dut.remove(&s2); + + // Check that it's gone + ASSERT_EQ(&s1, dut.getElementAtIndexForUnitText(0)); + ASSERT_EQ(&s3, dut.getElementAtIndexForUnitText(1)); + ASSERT_EQ(nullptr, dut.getElementAtIndexForUnitText(2)); +} + +TEST_F(EventQueueRemoveTest, removeEnd) { + // Remove the element at the end + dut.remove(&s3); + + // Check that it's gone + ASSERT_EQ(&s1, dut.getElementAtIndexForUnitText(0)); + ASSERT_EQ(&s2, dut.getElementAtIndexForUnitText(1)); + ASSERT_EQ(nullptr, dut.getElementAtIndexForUnitText(2)); +} + +TEST_F(EventQueueRemoveTest, removeNotPresent) { + scheduling_s s4; + + // Remove an element not already in the list - shouldn't fail + EXPECT_NO_THROW(dut.remove(&s4)); + + // Check that the list didn't change + ASSERT_EQ(&s1, dut.getElementAtIndexForUnitText(0)); + ASSERT_EQ(&s2, dut.getElementAtIndexForUnitText(1)); + ASSERT_EQ(&s3, dut.getElementAtIndexForUnitText(2)); + ASSERT_EQ(nullptr, dut.getElementAtIndexForUnitText(3)); +}