Loco and function names fixed and tests passing

DCC-EX · Dec 10, 2024 · 63a9a77 · 63a9a77
1 parent 9b23e5d
commit 63a9a77
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 50 deletions.
diff --git a/src/DCCEXLoco.cpp b/src/DCCEXLoco.cpp
@@ -37,6 +37,9 @@ Loco *Loco::_first = nullptr;
 Loco::Loco(int address, LocoSource source) {
   _address = address;
   _source = source;
+  for (int i = 0; i < MAX_FUNCTIONS; i++) {
+    _functionNames[i] = nullptr;
+  }
   _direction = Forward;
   _speed = 0;
   _name = nullptr;
@@ -60,6 +63,9 @@ Loco::Loco(int address, bool inRoster) : _address(address) {
   } else {
     _source = LocoSource::LocoSourceEntry;
   }
+  for (int i = 0; i < MAX_FUNCTIONS; i++) {
+    _functionNames[i] = nullptr;
+  }
   _direction = Forward;
   _speed = 0;
   _name = nullptr;
@@ -91,56 +97,55 @@ Direction Loco::getDirection() { return (Direction)_direction; }
 LocoSource Loco::getSource() { return (LocoSource)_source; }
 
 void Loco::setupFunctions(char *functionNames) {
-  // Important note:
-  // The functionNames string is modified in place.
-  //   console->print(F("Splitting \""));
-  //   console->print(functionNames);
-  //   console->println(F("\""));
-  char *t = functionNames;
-  int fkey = 0;
-
-  while (*t) {
-    bool momentary = false;
-    if (*t == '*') {
-      momentary = true;
-      t++;
-    }
-    char *fName = t; // function name starts here
-    while (*t) {     // loop completes at end of name ('/' or 0)
-      if (*t == '/') {
-        // found end of name
-        *t = '\0'; // mark name ends here
-        t++;
-        break;
-      }
-      t++;
+  if (functionNames == nullptr) {
+    return;
+  }
+  // Copy functionNames so fNames can be freed later
+  char *fNames = (char *)malloc(strlen(functionNames) + 1);
+  if (fNames == nullptr) {
+    return;
+  }
+  strcpy(fNames, functionNames);
+  int fIndex = 0;
+  int fNamesLength = strlen(fNames);
+  int fNameStart = 0;
+  // Free any existing function names
+  for (int i = 0; i < MAX_FUNCTIONS; i++) {
+    if (_functionNames[i] != nullptr) {
+      free(_functionNames[i]);
+      _functionNames[i] = nullptr;
     }
-
-    // At this point we have a function key
-    // int fkey = function number 0....
-    // bool momentary = is it a momentary
-    // fName = pointer to the function name
-    if (fkey < MAX_FUNCTIONS) {
-      _functionNames[fkey] = fName;
-      if (momentary) {
-        _momentaryFlags |= 1 << fkey;
+  }
+  for (int i = 0; i <= fNamesLength; i++) {
+    if (fNames[i] == '/' || fNames[i] == '\0') { // Check if it's the end of the name
+      if (fIndex < MAX_FUNCTIONS) {              // Make sure it's a sane index
+        if (_functionNames[fIndex] != nullptr) { // If it exists already, free it
+          free(_functionNames[fIndex]);
+        }
+        bool momentary = false;
+        if (fNames[fNameStart] == '*') { // If * it's momentary, and skip to next index for name
+          momentary = true;
+          fNameStart++;
+        }
+        int nameLength = i - fNameStart;                         // Calculate the length of the name
+        _functionNames[fIndex] = (char *)malloc(nameLength + 1); // Allocate mem
+        if (_functionNames[fIndex] != nullptr) {
+          strncpy(_functionNames[fIndex], &fNames[fNameStart], nameLength); // Copy name
+          _functionNames[fIndex][nameLength] = '\0';                        // Null terminate it
+        }
+        if (momentary) {
+          _momentaryFlags |= 1 << fIndex;
+        } else {
+          _momentaryFlags &= ~(1 << fIndex);
+        }
+        fIndex++;
       } else {
-        _momentaryFlags &= ~(1 << fkey);
+        break;
       }
-    }
-    //  Serial.print("Function ");
-    //  Serial.print(fkey);
-    //  Serial.print(momentary ? F("  Momentary ") : F(""));
-    //  Serial.print(" ");
-    //  Serial.println(fName);
-    //  Serial.println(_functionNames[fkey]);
-    fkey++;
-  }
-  if (fkey < MAX_FUNCTIONS) {
-    for (int i = fkey; i < MAX_FUNCTIONS; i++) {
-      _functionNames[i] = nullptr;
+      fNameStart = i + 1; // Move to the next name
     }
   }
+  free(fNames);
 }
 
 bool Loco::isFunctionOn(int function) { return _functionStates & 1 << function; }
@@ -174,6 +179,12 @@ Loco::~Loco() {
     _name = nullptr;
   }
 
+  for (int i = 0; i < MAX_FUNCTIONS; i++) {
+    if (_functionNames[i]) {
+      free(_functionNames[i]);
+    }
+  }
+
   if (Loco::getFirst() == this) {
     Loco::setFirst(_next);
   } else {

diff --git a/tests/Loco.cpp b/tests/Loco.cpp
@@ -5,7 +5,7 @@ TEST_F(DCCEXProtocolTest, createLoco) {
   // Create an individual loco
   Loco *loco100 = new Loco(100, false);
   loco100->setName("Loco 100");
-  // loco100->setupFunctions("Lights/*Horn/Bell///Idiot on tracks");
+  loco100->setupFunctions("Lights/*Horn/Bell///Idiot on tracks");
 
   // Check address is 1, name is correct, LocoSource correct, and no next
   EXPECT_EQ(loco100->getAddress(), 100);
@@ -14,10 +14,10 @@ TEST_F(DCCEXProtocolTest, createLoco) {
   EXPECT_EQ(loco100->getNext(), nullptr);
 
   // Check our functions
-  // EXPECT_FALSE(loco100->isFunctionMomentary(0));
-  // EXPECT_TRUE(loco100->isFunctionMomentary(1));
-  // EXPECT_STREQ(loco100->getFunctionName(2), "Bell");
-  // EXPECT_STREQ(loco100->getFunctionName(5), "Idiot on tracks");
+  EXPECT_FALSE(loco100->isFunctionMomentary(0));
+  EXPECT_TRUE(loco100->isFunctionMomentary(1));
+  EXPECT_STREQ(loco100->getFunctionName(2), "Bell");
+  EXPECT_STREQ(loco100->getFunctionName(5), "Idiot on tracks");
 
   // Clean this Loco up
   delete loco100;