Reviewable example of refactoring for testing: Li710 Driver

.github/workflows/manual-flash.yaml

@@ -41,7 +41,7 @@ jobs:
 
       - name: Download latest firmware artifact
         if: github.event.inputs.use_latest_artifact == 'yes'
-        uses: dawidd6/action-download-artifact@v3
+        uses: dawidd6/action-download-artifact@v6
         with:
           workflow: manual-compile.yaml
           name: firmware-binary

.github/workflows/manual-upload.yaml

@@ -45,7 +45,7 @@ jobs:
 
       - name: Download release artifacts
         if: github.event.inputs.use_release_artifact == 'yes'
-        uses: dawidd6/action-download-artifact@v3
+        uses: dawidd6/action-download-artifact@v6
         with:
           workflow: manual-release.yaml
           name: release-artifacts

.github/workflows/pr-workflow.yaml

@@ -3,7 +3,7 @@ name: Pull Request Workflow
 on:
   # Runs on PR open
   pull_request:
-    types: [opened, reopened, synchronize]
+    types: [opened, reopened, synchronize, edited]
     branches:
       - master  # or 'main' depending on your main branch
 

.gitmodules

@@ -131,3 +131,9 @@
 [submodule "test/external/fff"]
 	path = test/external/fff
 	url = https://github.com/meekrosoft/fff.git
+[submodule "lib/FlightControl-platform-dependencies"]
+	path = lib/FlightControl-platform-dependencies
+	url = https://github.com/gemsiot/FlightControl-platform-dependencies
+[submodule "lib/FlightControl-hardware-dependencies"]
+	path = lib/FlightControl-hardware-dependencies
+	url = https://github.com/gemsiot/FlightControl-hardware-dependencies

CMakeLists.txt

@@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.14)
 project(FlightControl_Demo_Tests)
 
 # Specify C++ standard
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 # Add test directory

src/FlightControl_Demo.cpp

@@ -59,6 +59,9 @@ int configurePowerSave(int desiredPowerSaveMode);
 #include <vector>
 #include <memory>
 
+#include "hardware/SDI12TalonAdapter.h"
+#include "platform/ParticleTimeProvider.h"
+
 const String firmwareVersion = "2.9.11";
 const String schemaVersion = "2.2.9";
 
@@ -75,6 +78,8 @@ I2CTalon i2c(0, 0x21); //Instantiate I2C talon with alt - null port and hardware
 SDI12Talon sdi12(0, 0x14); //Instantiate SDI12 talon with alt - null port and hardware v1.4
 PCAL9535A ioAlpha(0x20);
 PCAL9535A ioBeta(0x21);
+SDI12TalonAdapter realSdi12(sdi12);
+ParticleTimeProvider realTimeProvider;
 
 String globalNodeID = ""; //Store current node ID
 
@@ -114,7 +119,7 @@ TDR315H soil2(sdi12, 0, 0); //Instantiate soil sensor with default ports and unk
 TDR315H soil3(sdi12, 0, 0); //Instantiate soil sensor with default ports and unknown version, pass over SDI12 Talon interface 
 Hedorah gas(0, 0, 0x10); //Instantiate CO2 sensor with default ports and v1.0 hardware
 // T9602 humidity(0, 0, 0x00); //Instantiate Telair T9602 with default ports and version v0.0 
-LI710 et(sdi12, 0, 0); //Instantiate ET sensor with default ports and unknown version, pass over SDI12 Talon interface 
+LI710 et(realTimeProvider, realSdi12, 0, 0); //Instantiate ET sensor with default ports and unknown version, pass over SDI12 Talon interface 
 BaroVue10 campPressure(sdi12, 0, 0x00); // Instantiate Barovue10 with default ports and v0.0 hardware
 
 const uint8_t numSensors = 7; //Number must match the number of objects defined in `sensors` array
@@ -1077,7 +1082,7 @@ int wakeSensors()
 	for(int s = 0; s < numSensors; s++) {
 		if(sensors[s]->getTalonPort() != 0) {
 			logger.enableData(sensors[s]->getTalonPort(), true); //Turn on data for given port
-			sensors[s]->wake(); //Wake each sensor
+			sensors[s]->wake(realTimeProvider); //Wake each sensor
 			logger.enableData(sensors[s]->getTalonPort(), false); //Turn data back off for given port
 		}
 	}

src/hardware/SDI12TalonAdapter.h

@@ -0,0 +1,54 @@
+// src/hardware/SDI12TalonAdapter.h
+
+#ifndef SDI12_TALON_ADAPTER_H
+#define SDI12_TALON_ADAPTER_H
+
+#include "ISDI12Talon.h"
+#include "SDI12Talon.h"
+
+/**
+ * @brief Adapter implementing ISDI12Talon interface
+ * 
+ * Adapts the concrete SDI12Talon class to the ISDI12Talon interface
+ * for dependency injection and testing purposes.
+ */
+class SDI12TalonAdapter : public ISDI12Talon {
+public:
+    /**
+     * @brief Constructor that takes a reference to a concrete SDI12Talon
+     * @param talon The concrete SDI12Talon implementation to delegate to
+     */
+    SDI12TalonAdapter(SDI12Talon& talon) : talon(talon) {}
+    ~SDI12TalonAdapter() override = default;
+
+    // SDI12 communication methods
+    int getAddress() override { return talon.getAddress(); }
+    String sendCommand(String command) override { return talon.sendCommand(command); }
+    String command(String commandStr, int address) override { return talon.command(commandStr, address); }
+    int startMeasurment(int Address) override { return talon.startMeasurment(Address); }
+    int startMeasurmentIndex(int index, int Address) override { return talon.startMeasurmentIndex(index, Address); }
+    String continuousMeasurmentCRC(int Measure, int Address) override { return talon.continuousMeasurmentCRC(Measure, Address); }
+    bool testCRC(String message) override { return talon.testCRC(message); }
+
+    // Port management methods
+    int enableData(uint8_t port, bool state) override { return talon.enableData(port, state); }
+    int enablePower(uint8_t port, bool state) override { return talon.enablePower(port, state); }
+    void disableDataAll() override { talon.disableDataAll(); }
+    uint8_t getNumPorts() override { return talon.getNumPorts(); }
+
+    // Sensor interrogation
+    bool isPresent() override { return talon.isPresent(); }
+
+    // Error handling and state reporting
+    //int throwError(uint32_t error) override { return talon.throwError(error); }
+    String getSensorPortString() override { return talon.getSensorPortString(); }
+    String getTalonPortString() override { return talon.getTalonPortString(); }
+    uint8_t getSensorPort() override { return talon.getSensorPort(); }
+    uint8_t getTalonPort() override { return talon.getTalonPort(); }
+    int restart() override { return talon.restart(); }
+
+private:
+    SDI12Talon& talon; // Reference to the concrete implementation
+};
+
+#endif // SDI12_TALON_ADAPTER_H

src/platform/ParticleTimeProvider.cpp

@@ -0,0 +1,24 @@
+// src/platform/ParticleTimeProvider.cpp
+#include "ParticleTimeProvider.h"
+
+// Use :: prefix for global scope functions like millis() and delay()
+// to avoid potential name conflicts if methods had same name.
+
+int ParticleTimeProvider::year() { return Time.year(); }
+int ParticleTimeProvider::year(time_t t) { return Time.year(t); }
+int ParticleTimeProvider::month() { return Time.month(); }
+int ParticleTimeProvider::month(time_t t) { return Time.month(t); }
+int ParticleTimeProvider::day() { return Time.day(); }
+int ParticleTimeProvider::day(time_t t) { return Time.day(t); }
+int ParticleTimeProvider::hour() { return Time.hour(); }
+int ParticleTimeProvider::hour(time_t t) { return Time.hour(t); }
+int ParticleTimeProvider::minute() { return Time.minute(); }
+int ParticleTimeProvider::minute(time_t t) { return Time.minute(t); }
+int ParticleTimeProvider::second() { return Time.second(); }
+int ParticleTimeProvider::second(time_t t) { return Time.second(t); }
+time_t ParticleTimeProvider::now() { return Time.now(); }
+void ParticleTimeProvider::setTime(time_t t) { Time.setTime(t); }
+bool ParticleTimeProvider::isValid() { return Time.isValid(); }
+void ParticleTimeProvider::zone(float GMT_Offset) { Time.zone(GMT_Offset); }
+uint32_t ParticleTimeProvider::millis() { return ::millis(); }
+void ParticleTimeProvider::delay(uint32_t ms) { ::delay(ms); }

src/platform/ParticleTimeProvider.h

@@ -0,0 +1,38 @@
+// src/platform/ParticleTimeProvider.h
+#ifndef PARTICLE_TIME_PROVIDER_H
+#define PARTICLE_TIME_PROVIDER_H
+
+#include "ITimeProvider.h" // Include the interface definition
+#include "Particle.h" // Include the actual Particle header HERE
+
+/**
+ * @brief Concrete implementation of ITimeProvider using Particle API.
+ */
+class ParticleTimeProvider : public ITimeProvider {
+public:
+    // Constructor/Destructor (often default is fine)
+    ParticleTimeProvider() = default;
+    ~ParticleTimeProvider() override = default;
+
+    // Implement methods from ITimeProvider
+    int year() override;
+    int year(time_t t) override;
+    int month() override;
+    int month(time_t t) override;
+    int day() override;
+    int day(time_t t) override;
+    int hour() override;
+    int hour(time_t t) override;
+    int minute() override;
+    int minute(time_t t) override;
+    int second() override;
+    int second(time_t t) override;
+    time_t now() override;
+    void setTime(time_t t) override;
+    bool isValid() override;
+    void zone(float GMT_Offset) override;
+    uint32_t millis() override;
+    void delay(uint32_t ms) override;
+};
+
+#endif // PARTICLE_TIME_PROVIDER_H

test/CMakeLists.txt

@@ -1,33 +1,42 @@
 # FlightControl_Demo/test/CMakeLists.txt
+#allow debugging
+set(CMAKE_BUILD_TYPE Debug)
+
 # Add GoogleTest
 add_subdirectory(external/googletest)
 
 # Define the TESTING preprocessor macro for all test builds
 add_compile_definitions(TESTING)
 
-# IMPORTANT: Make sure mocks are included first to override system headers
-include_directories(BEFORE mocks)
-
-# Include FFF headers
-include_directories(external/fff)
+# Set C++ standard
+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
-# Include Google Test headers
+# Include Google Test and Google Mock headers
 include_directories(external/googletest/googletest/include)
+include_directories(external/googletest/googlemock/include)
+
+# Find all driver directories and add them to the include path
+file(GLOB DRIVER_DIRS ${CMAKE_SOURCE_DIR}/lib/*/src)
+
+# Create a library for the mocks
+file(GLOB MOCK_SOURCES 
+    ${CMAKE_CURRENT_SOURCE_DIR}/mocks/*.cpp
+)
 
-# Create a mocks library
-add_library(mocks STATIC
-    mocks/MockWire.cpp
-    mocks/MockArduino.cpp
-    mocks/MockPCAL9535A.cpp
-    mocks/MockMCP79412.cpp
-    mocks/MockSPI.cpp
-    mocks/MockPCA9634.cpp
-    mocks/MockSHT4x.cpp
-    mocks/MockVEML3328.cpp
-    mocks/MockPAC1934.cpp
-    mocks/MockMXC6655.cpp
-    mocks/MockBMA456.cpp
-    mocks/MockGNSS.cpp
+# Add a library of mocks - if there are no source files yet, use an interface library
+if(MOCK_SOURCES)
+    add_library(mocks ${MOCK_SOURCES})
+else()
+    add_library(mocks INTERFACE)
+endif()
+
+# Add header include directories for mocks - mock directory MUST be first
+target_include_directories(mocks BEFORE INTERFACE
+    ${CMAKE_CURRENT_SOURCE_DIR}/mocks  # Mocks FIRST
+    ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_SOURCE_DIR}/src
+    ${DRIVER_DIRS}  # Add all driver directories
 )
 
 # Specify test executable
@@ -36,27 +45,13 @@ add_executable(unit_tests
     main.cpp
 
     # Kestrel tests
-    unit/Driver_-_Kestrel/Driver_-_KestrelSetupTests.cpp
-    unit/Driver_-_Kestrel/Driver_-_KestrelFunctionTests.cpp
+    #unit/Driver_-_Kestrel/KestrelTest.cpp
+    #${CMAKE_SOURCE_DIR}/lib/Driver_-_Kestrel/src/Kestrel.cpp
+
+    # Li710 tests
+    unit/Driver_-_Li710/Li710Test.cpp
+    ${CMAKE_SOURCE_DIR}/lib/Driver_-_Li710/src/Li710.cpp
 )
 
 # Link against mocks and GoogleTest
-target_link_libraries(unit_tests mocks gtest gtest_main)
-
-# Find all driver directories and add them to the include path
-file(GLOB DRIVER_DIRS ${CMAKE_SOURCE_DIR}/lib/*/src)
-
-# Add header include directories - mock directory MUST be first
-target_include_directories(mocks BEFORE PUBLIC 
-    ${CMAKE_CURRENT_SOURCE_DIR}/mocks  # Mocks FIRST
-    ${CMAKE_CURRENT_SOURCE_DIR}
-    ${CMAKE_SOURCE_DIR}/src
-    ${DRIVER_DIRS}  # Add all driver directories
-)
-
-target_include_directories(unit_tests BEFORE PRIVATE
-    ${CMAKE_CURRENT_SOURCE_DIR}/mocks  # Mocks FIRST
-    ${CMAKE_CURRENT_SOURCE_DIR}
-    ${CMAKE_SOURCE_DIR}/src
-    ${DRIVER_DIRS}  # Add all driver directories
-)
+target_link_libraries(unit_tests mocks gtest gtest_main gmock)

test/main.cpp

@@ -1,8 +1,5 @@
 // FlightControl_Demo/test/main.cpp
 #include "gtest/gtest.h"
-#include "fff.h"
-
-DEFINE_FFF_GLOBALS;
 
 int main(int argc, char **argv) {
     ::testing::InitGoogleTest(&argc, argv);