ISSUE: Works on NRF52840 but apparently not Arduino boards

[TheAngryRaven]

Problem

Werks On My Machine(tm)

Description

So there seems to be a slight problem with the library when running on an actual arduino.

I had originally built this for the Seeed NRF52840 | Amazon Link (NOT A REFERRAL)

This seems to cause a problem with my "fancy" internal debugging system, the use of std:swap, and possibly some issues with my use of floats. I have produced a small patch that manages to stop the errors, but now produces a "Not enough memory" error on various other arduino boards.

I hope this helps some users who were having issues, this is the first time I have developed a publicly exposed library for a microcontroller so bare with me while I attempt to solve the issue.

If you are suffering from a similar issue, please comment below with your arduino IDE version, and the microcontroller you are attempting to compile for.

Temp Solution

If you have half of a brain you could apply this simple patch and try to help me out.

UPDATE: reducing the logging points to 50 allows it to compile for an "Arduino Leonardo", do with this information as you will, I will investigate further.

nrfToArduino.patch

DovesLapTimer.h

diff --git a/src/DovesLapTimer.h b/src/DovesLapTimer.h
index e3359b9..da473cb 100644
--- a/src/DovesLapTimer.h
+++ b/src/DovesLapTimer.h
@@ -10,10 +10,10 @@
 
 #ifndef _DOVES_LAP_TIMER_H
 #define _DOVES_LAP_TIMER_H
-#include <cfloat>
+// #include <cfloat>
 #include <math.h>
 #include "Arduino.h"
-#include <algorithm>
+// #include <algorithm>
 
 using TRITYPE = double;
 
@@ -271,18 +271,18 @@ public:
   #endif
 
 private:
-  template<typename... Args>
-  void debug_print(Args&&... args) {
-    if(_serial) {
-      _serial->print(std::forward<Args>(args)...);
-    }
-  }
-  template<typename... Args>
-  void debug_println(Args&&... args) {
-    if(_serial) {
-      _serial->println(std::forward<Args>(args)...);
-    }
-  }
+  // template<typename... Args>
+  // void debug_print(Args&&... args) {
+  //   if(_serial) {
+  //     _serial->print(std::forward<Args>(args)...);
+  //   }
+  // }
+  // template<typename... Args>
+  // void debug_println(Args&&... args) {
+  //   if(_serial) {
+  //     _serial->println(std::forward<Args>(args)...);
+  //   }
+  // }
 
   #ifndef DOVES_UNIT_TEST
   /**
@@ -372,7 +372,7 @@ private:
 
   #ifndef DOVES_UNIT_TEST
   // Number of GPS coordinates to store in the buffer for interpolation
-  static const int crossingPointBufferSize = 500;
+  static const int crossingPointBufferSize = 50;
 
   crossingPointBufferEntry crossingPointBuffer[crossingPointBufferSize];
   int crossingPointBufferIndex = 0;

DovesLapTimer.cpp

diff --git a/src/DovesLapTimer.cpp b/src/DovesLapTimer.cpp
index f9e94ef..7854565 100644
--- a/src/DovesLapTimer.cpp
+++ b/src/DovesLapTimer.cpp
@@ -8,8 +8,12 @@
 
 #include "DovesLapTimer.h"
 
-#define debugln debug_println
-#define debug debug_print
+// #define debugln debug_println
+// #define debug debug_print
+
+void dummy_debug(...) {}
+#define debug dummy_debug
+#define debugln dummy_debug
 
 DovesLapTimer::DovesLapTimer(double crossingThresholdMeters, Stream *debugSerial) {
   this->crossingThresholdMeters = crossingThresholdMeters;
@@ -215,9 +219,26 @@ bool DovesLapTimer::isObtuseTriangle(double lat1, double lon1, double lat2, doub
   double c = haversine(lat2, lon2, lat3, lon3);
 
   // Sort the sides in ascending order
-  if (a > b) std::swap(a, b);
-  if (b > c) std::swap(b, c);
-  if (a > b) std::swap(a, b);
+  // if (a > b) std::swap(a, b);
+  // if (b > c) std::swap(b, c);
+  // if (a > b) std::swap(a, b);
+  if (a > b) {
+      int temp = a;
+      a = b;
+      b = temp;
+  }
+
+  if (b > c) {
+      int temp = b;
+      b = c;
+      c = temp;
+  }
+
+  if (a > b) {
+      int temp = a;
+      a = b;
+      b = temp;
+  }
 
   // listen... this has been a long debugging session
   if ( a + b <= c ) {
@@ -341,7 +362,8 @@ void DovesLapTimer::interpolateCrossingPoint(double& crossingLat, double& crossi
   // Variables to store the best pair of points
   int bestIndexA = -1;
   int bestIndexB = -1;
-  double bestSumDistances = DBL_MAX;
+  // double bestSumDistances = DBL_MAX;
+  double bestSumDistances = 10000.0;
 
   // Iterate through the crossingPointBuffer, comparing the sum of distances from the start/finish line of each pair of consecutive points
   for (int i = 0; i < numPoints - 1; i++) {

[TheAngryRaven]

Possible Solution

Installing the ArxTypeTraits Arduino library manages to fix some of these issues.
The following patch is still needed to allow the code to work on some boards.

This would both FIX the problem, AND also allow the internal debugger to function if requested!

DovesLapTimer.h

diff --git a/src/DovesLapTimer.h b/src/DovesLapTimer.h
index e3359b9..fc131c2 100644
--- a/src/DovesLapTimer.h
+++ b/src/DovesLapTimer.h
@@ -10,10 +10,9 @@
 
 #ifndef _DOVES_LAP_TIMER_H
 #define _DOVES_LAP_TIMER_H
-#include <cfloat>
 #include <math.h>
 #include "Arduino.h"
-#include <algorithm>
+#include "ArxTypeTraits.h"
 
 using TRITYPE = double;
 
@@ -372,7 +371,7 @@ private:
 
   #ifndef DOVES_UNIT_TEST
   // Number of GPS coordinates to store in the buffer for interpolation
-  static const int crossingPointBufferSize = 500;
+  static const int crossingPointBufferSize = 50;
 
   crossingPointBufferEntry crossingPointBuffer[crossingPointBufferSize];
   int crossingPointBufferIndex = 0;

Concerns

1. Removing the cfloat header causes build failure for NRF52840
2. Limiting the crossing buffer size to 50 is a little worrisome, more investigation will be needed.

Possible solution for cfloat

We are really only calling the library to set a default high value during a comparison.
Is this a viable solution?

  // Variables to store the best pair of points
  int bestIndexA = -1;
  int bestIndexB = -1;
- double bestSumDistances = DBL_MAX;
+ double bestSumDistances = 10000.0;

Final concerns before merging changes

1. Many of the Arduinos lack the memory to hold the GPS data test to verify these changes are valid
   a. Realworld testing will be required
2. Forcing another library to be required

[TheAngryRaven]

With a little more tinkering, i might have a more viable solution.,

Still more research to be done.

dynamicFix.patch

Note: Reset master and apply this patch only!

This should in theory keep the functionality the same for the NRF52840, and drop the required crossing point buffer for the chips with less ram.

Change #4 NEEDS to be done for me to feel secure about merging this change.

This patch is expirimental

DovesLapTimer.h

diff --git a/src/DovesLapTimer.h b/src/DovesLapTimer.h
index e3359b9..d67d239 100644
--- a/src/DovesLapTimer.h
+++ b/src/DovesLapTimer.h
@@ -10,10 +10,14 @@
 
 #ifndef _DOVES_LAP_TIMER_H
 #define _DOVES_LAP_TIMER_H
-#include <cfloat>
-#include <math.h>
-#include "Arduino.h"
-#include <algorithm>
+
+#if __has_include("cfloat")
+  #include <cfloat>
+#else
+  #define DBL_MAX 100000.0;
+#endif
+
+#include "ArxTypeTraits.h"
 
 using TRITYPE = double;
 
@@ -264,7 +268,12 @@ public:
   double catmullRom(double p0, double p1, double p2, double p3, double t);
   void interpolateCrossingPoint(double& crossingLat, double& crossingLng, unsigned long& crossingTime, double& crossingOdometer, double pointALat, double pointALng, double pointBLat, double pointBLng);
 
-  static const int crossingPointBufferSize = 300;
+  #if ((RAMEND - RAMSTART) > 3000 )
+    static const int crossingPointBufferSize = 200;
+  #else
+    static const int crossingPointBufferSize = 50;
+  #endif
+
   crossingPointBufferEntry crossingPointBuffer[crossingPointBufferSize];
   int crossingPointBufferIndex = 0;
   bool crossingPointBufferFull = false;
@@ -371,8 +380,12 @@ private:
   const double radiusEarth = 6371.0 * 1000;
 
   #ifndef DOVES_UNIT_TEST
-  // Number of GPS coordinates to store in the buffer for interpolation
-  static const int crossingPointBufferSize = 500;
+  // UNTESTED: Hotfix for low memory systems
+  #if ((RAMEND - RAMSTART) > 3000 )
+    static const int crossingPointBufferSize = 200;
+  #else
+    static const int crossingPointBufferSize = 50;
+  #endif
 
   crossingPointBufferEntry crossingPointBuffer[crossingPointBufferSize];
   int crossingPointBufferIndex = 0;

[TheAngryRaven]

Pushed possible solutions, be on your toes when running on underpowered hardware though.

Can possibly open again if people are still suffering from problems, I believe the latest code should work on an arduino MEGA now, but not sure if it has the power to process the library and log to an SD card.