add & test remove from linked list (#3199)

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;
 }
 

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);

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));
+}