fragment refactoring (#4072)

* fragment refactoring

* jar

* jar?

firmware/console/binary/FragmentEntry.cpp

@@ -8,40 +8,35 @@
 #include "pch.h"
 #include "FragmentEntry.h"
 
-/**
- * copy dataLength of fragmented outputs starting at dataOffset into destination starting at zero
- */
-void copyRange(uint8_t *destination,
-		FragmentEntry *fragments, int fragmentsCount,
-		size_t dataOffset, size_t dataLength) {
+void copyRange(uint8_t* destination, FragmentList src, size_t skip, size_t size) {
 	int fragmentIndex = 0;
 
-	// scroll to starting fragment
-	while (dataOffset > fragments[fragmentIndex].size && fragmentIndex <= fragmentsCount) {
-		dataOffset -= fragments[fragmentIndex].size;
-		fragmentIndex ++;
+	// Find which fragment to start - skip any full fragments smaller than `skip` parameter
+	while (skip > src.fragments[fragmentIndex].size && fragmentIndex <= src.count) {
+		skip -= src.fragments[fragmentIndex].size;
+		fragmentIndex++;
 	}
 
 	int destinationIndex = 0;
 
-	while (dataLength > 0) {
-		if (fragmentIndex > fragmentsCount) {
+	while (size > 0) {
+		if (fragmentIndex >= src.count) {
 			// somehow we are past the end of fragments - fill with zeros
-			memset(destination + destinationIndex, 0, dataLength);
+			memset(destination + destinationIndex, 0, size);
 			return;
 		}
 
-		int copyNowSize = minI(dataLength, fragments[fragmentIndex].size - dataOffset);
-		const uint8_t *fromBase = fragments[fragmentIndex].data;
-		if (fromBase == nullptr) {
+		int copyNowSize = minI(size, src.fragments[fragmentIndex].size - skip);
+		const uint8_t* fromBase = src.fragments[fragmentIndex].data;
+		if (!fromBase) {
 			// we have no buffer for this fragment - fill with zeroes
 			memset(destination + destinationIndex, 0, copyNowSize);
 		} else {
-			memcpy(destination + destinationIndex, fromBase + dataOffset, copyNowSize);
+			memcpy(destination + destinationIndex, fromBase + skip, copyNowSize);
 		}
 		destinationIndex += copyNowSize;
-		dataOffset = 0;
-		dataLength -= copyNowSize;
+		skip = 0;
+		size -= copyNowSize;
 		fragmentIndex++;
 	}
 }

firmware/console/binary/FragmentEntry.h

@@ -8,22 +8,21 @@
 #pragma once
 
 struct FragmentEntry {
-	FragmentEntry(const uint8_t *data, size_t size) {
-		init(data, size);
+	template <typename TData>
+	FragmentEntry(const TData* data)
+		: data(reinterpret_cast<const uint8_t*>(data))
+		, size(sizeof(TData))
+	{
 	}
 
-	FragmentEntry() {
-	}
-
-	const uint8_t *data = nullptr;
-	size_t size = 0;
-
-	void init(const uint8_t *data, size_t size) {
-		this->data = data;
-		this->size = size;
-	}
+	const uint8_t* const data;
+	const size_t size;
 };
 
-void copyRange(uint8_t *destination,
-		FragmentEntry *fragments, int fragmentsCount,
-		size_t dataOffset, size_t dataLength);
+struct FragmentList {
+	const FragmentEntry* fragments;
+	const size_t count;
+};
+
+// copy `size` of fragmented outputs in to destination, skipping the first `skip` bytes
+void copyRange(uint8_t* destination, FragmentList src, size_t skip, size_t size);

firmware/console/binary/tunerstudio.cpp

@@ -839,7 +839,6 @@ void startTunerStudioConnectivity(void) {
 //	char (*__kaboom)[sizeof(persistent_config_s)] = 1;
 
 	memset(&tsState, 0, sizeof(tsState));
-	initFragments();
 
 	addConsoleAction("tsinfo", printTsStats);
 	addConsoleAction("reset_ts", resetTs);

firmware/console/binary/tunerstudio.h

@@ -38,9 +38,7 @@ const void * getStructAddr(live_data_e structId);
 #include "thread_controller.h"
 #include "thread_priority.h"
 
-void initFragments();
-int getFragmentsCount();
-FragmentEntry *getFragments();
+FragmentList getFragments();
 
 void updateTunerStudioState();
 

firmware/console/binary/tunerstudio_commands.cpp

@@ -53,9 +53,7 @@ void TunerStudio::cmdOutputChannels(TsChannelBase* tsChannel, uint16_t offset, u
 	/**
 	 * collect data from all models
 	 */
-	copyRange(scratchBuffer + 3,
-			getFragments(), getFragmentsCount(),
-			offset, count);
+	copyRange(scratchBuffer + 3, getFragments(), offset, count);
 
 	tsChannel->crcAndWriteBuffer(TS_RESPONSE_OK, count);
 }

java_tools/configuration_definition/src/main/java/com/rusefi/ldmp/UsagesReader.java

@@ -96,18 +96,7 @@ public class UsagesReader {
 
         LinkedHashMap<?, ?> liveDocs = (LinkedHashMap) data.get("Usages");
 
-        fragmentsContent.append("static FragmentEntry fragments[" + liveDocs.size() + "];\n\n");
-
-        fragmentsContent.append("int getFragmentsCount() {\n" +
-                "\treturn " + liveDocs.size() + ";\n" +
-                "}\n" +
-                "\n" +
-                "FragmentEntry *getFragments() {\n" +
-                "\treturn fragments;\n" +
-                "}\n\n" +
-                "void initFragments() {\n");
-
-        int index = 0;
+        fragmentsContent.append("static const FragmentEntry fragments[] = {\n");
 
         for (Map.Entry entry : liveDocs.entrySet()) {
             String name = (String) entry.getKey();
@@ -122,17 +111,18 @@ public class UsagesReader {
             enumContent.append(enumName + ",\n");
 
             fragmentsContent
-                    .append("\tfragments[")
-                    .append(index++)
-                    .append("].init(")
-                    .append("(const uint8_t *)getStructAddr(")
-                    .append(enumName)
-                    .append("), sizeof(")
+                    .append("\treinterpret_cast<const ")
                     .append(type)
-                    .append("));\n");
+                    .append("*>(getStructAddr(")
+                    .append(enumName)
+                    .append(")),\n");
         }
         enumContent.append("} live_data_e;\n");
-        fragmentsContent.append("};\n");
+
+        fragmentsContent
+            .append("};\n\n")
+            .append("FragmentList getFragments() {\n\treturn { fragments, efi::size(fragments) };\n}\n");
+
     }
 
     private void writeFiles() throws IOException {

unit_tests/tests/test_scattered_outputs.cpp

@@ -1,40 +1,89 @@
 #include "pch.h"
 #include "FragmentEntry.h"
 
-static uint8_t buffer10[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
-static uint8_t buffer5[] = {11, 12, 13, 14, 15};
-
-static FragmentEntry fragments[] = {
-		FragmentEntry(buffer10, sizeof(buffer10)),
-		FragmentEntry(buffer5, sizeof(buffer5)),
-		FragmentEntry(nullptr, sizeof(5)),
+struct obj1 {
+	const uint8_t x[10] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
 };
 
+static_assert(sizeof(obj1) == 10);
+
+struct obj2 {
+	const uint8_t x[5] = { 11, 12, 13, 14, 15 };
+};
+
+static_assert(sizeof(obj2) == 5);
+
+obj1 buffer10;
+obj2 buffer5;
+
+static FragmentEntry fragmentBuffer[] = {
+		&buffer10,
+		&buffer5,
+		reinterpret_cast<obj2*>(0),	// null fragment for fun
+		&buffer10
+};
+
+static uint8_t buffer[120];
+static void resetBuffer() {
+	memset(buffer, 0xFF, sizeof(buffer));
+}
+
 TEST(outputs, fragments) {
-	ASSERT_EQ(3, efi::size(fragments));
+	static_assert(4 == efi::size(fragmentBuffer));
+
+	FragmentList fragments{ fragmentBuffer, efi::size(fragmentBuffer) };
+
+	resetBuffer();
 
-	uint8_t buffer[120];
 	{
+		// Check overlap between first and second fragments
 		uint8_t expected[] = {9, 10, 11, 12, 13};
-		copyRange(buffer, fragments, efi::size(fragments), 8, 5);
+		copyRange(buffer, fragments, 8, 5);
 		EXPECT_TRUE( 0 == std::memcmp(buffer, expected, sizeof(expected)));
 	}
 
+	resetBuffer();
+
 	{
+		// Check partial of only second fragment
 		uint8_t expected[] = {13, 14, 15};
-		copyRange(buffer, fragments, efi::size(fragments), 12, 3);
+		copyRange(buffer, fragments, 12, 3);
 		EXPECT_TRUE( 0 == std::memcmp(buffer, expected, sizeof(expected)));
 	}
 
+	resetBuffer();
+
 	{
+		// Check overlap between second fragment and null fragment
 		uint8_t expected[] = {15, 0, 0};
-		copyRange(buffer, fragments, efi::size(fragments), 14, 3);
+		copyRange(buffer, fragments, 14, 3);
 		EXPECT_TRUE( 0 == std::memcmp(buffer, expected, sizeof(expected)));
 	}
 
+	resetBuffer();
+
 	{
+		// Check overlap between null fragment and 4th fragment
+		uint8_t expected[] = {0, 1, 2};
+		copyRange(buffer, fragments, 19, 3);
+		EXPECT_TRUE( 0 == std::memcmp(buffer, expected, sizeof(expected)));
+	}
+
+	resetBuffer();
+
+	{
+		// Check overlap between last fragment and off-end of buffer (off-buffer should give 0s)
+		uint8_t expected[] = {9, 10, 0, 0};
+		copyRange(buffer, fragments, 28, 4);
+		EXPECT_TRUE( 0 == std::memcmp(buffer, expected, sizeof(expected)));
+	}
+
+	resetBuffer();
+
+	{
+		// Check way off the end of the buffer
 		uint8_t expected[] = {0, 0, 0};
-		copyRange(buffer, fragments, efi::size(fragments), 114, 3);
+		copyRange(buffer, fragments, 1000, 3);
 		EXPECT_TRUE( 0 == std::memcmp(buffer, expected, sizeof(expected)));
 	}
 }