very big update. Reorganized the code, added a command processor, tests.

controller/tea_poor/lib/Arduino/ArduinoEnvironment.h

@@ -0,0 +1,15 @@
+// Arduino environment
+#ifndef ARDUINO_ENVIRONMENT_H
+#define ARDUINO_ENVIRONMENT_H
+
+#include <IEnvironment.h>
+#include <Arduino.h>
+
+class ArduinoEnvironment : public IEnvironment {
+  public:
+    unsigned long time() const override {
+      return millis();
+    }
+};
+
+#endif // ARDUINO_ENVIRONMENT_H

controller/tea_poor/lib/CommandProcessor/CommandProcessor.cpp

@@ -0,0 +1,58 @@
+#include "CommandProcessor.h"
+#include <cstring>
+#include <sstream>
+
+bool isValidIntNumber(const char *str, const int maxValue, const int minValue=0) {
+  const int len = strlen(str);
+  if (len < 1) return false;
+  // check that string contains only digits
+  // first character can be '-' for negative numbers
+  if ((str[0] != '-') && !isdigit(str[0])) return false;
+  for (int i = 1; i < len; i++) {
+    if (!isdigit(str[i])) return false;
+  }
+  // check that number is in range
+  const int value = atoi(str);
+  if (value < minValue) return false;
+  if (maxValue <= value) return false;
+  return true;
+}
+
+std::string CommandProcessor::status() {
+  std::stringstream response;
+  response << "{";
+  // send water threshold
+  response << "\"water threshold\": " << _waterPumpSafeThreshold << ", ";
+  // send water pump status
+  const auto waterPumpStatus = _waterPump->status();
+  const auto now = _env->time();
+  const auto timeLeft = waterPumpStatus.isRunning ? waterPumpStatus.stopTime - now : 0;
+  response 
+    << "\"pump\": {"
+    << "  \"running\": " << (waterPumpStatus.isRunning ? "true, " : "false, ")
+    << "  \"time left\": " << timeLeft
+    << "}";
+  // end of water pump status
+  ///////////////////////////////////
+  // send remote control status
+  // response << "\"remote control\": " << remoteControl.asJSONString();
+  // end of JSON
+  response << "}";
+
+  return response.str();
+}
+
+std::string CommandProcessor::pour_tea(const char *milliseconds) {
+  if (!isValidIntNumber(milliseconds, _waterPumpSafeThreshold)) {
+    // send error message as JSON
+    return std::string("{ \"error\": \"invalid milliseconds value\" }");
+  }
+  // start pouring tea
+  _waterPump->start( atoi(milliseconds), _env->time() );
+  return status();
+}
+
+std::string CommandProcessor::stop() {
+  _waterPump->stop();
+  return status();
+}

controller/tea_poor/lib/CommandProcessor/CommandProcessor.h

@@ -0,0 +1,30 @@
+// CommandProcessor class definition
+#ifndef COMMANDPROCESSOR_H
+#define COMMANDPROCESSOR_H
+
+#include <string>
+#include <IWaterPumpSchedulerAPI.h>
+#include <IEnvironment.h>
+
+// This class is used to process incoming commands
+class CommandProcessor {
+  public:
+    CommandProcessor(
+      int waterPumpSafeThreshold,
+      const IEnvironmentPtr env,
+      const IWaterPumpSchedulerAPIPtr waterPump
+    ) : 
+      _waterPumpSafeThreshold(waterPumpSafeThreshold),
+      _env(env),
+      _waterPump(waterPump)
+    {}
+
+    std::string status();
+    std::string pour_tea(const char *milliseconds);
+    std::string stop();
+  private:
+    const int _waterPumpSafeThreshold;
+    const IEnvironmentPtr _env;
+    const IWaterPumpSchedulerAPIPtr _waterPump;
+};
+#endif // COMMANDPROCESSOR_H

controller/tea_poor/lib/WaterPump/WaterPumpScheduler.cpp → controller/tea_poor/lib/WaterPumpScheduler/WaterPumpScheduler.cpp

@@ -29,9 +29,6 @@ void WaterPumpScheduler::tick(unsigned long currentTimeMs) {
   }
 }
 
-WaterPumpScheduler::WaterPumpStatus WaterPumpScheduler::status() {
-  return {
-    _waterPump->isRunning(),
-    _stopTime
-  };
+WaterPumpStatus WaterPumpScheduler::status() {
+  return WaterPumpStatus(_waterPump->isRunning(), _stopTime);
 }

controller/tea_poor/lib/WaterPump/WaterPumpScheduler.h → controller/tea_poor/lib/WaterPumpScheduler/WaterPumpScheduler.h

@@ -1,12 +1,13 @@
 #ifndef WATERPUMPSCHEDULER_H
 #define WATERPUMPSCHEDULER_H
 
-#include "IWaterPump.h"
+#include <IWaterPump.h>
+#include <IWaterPumpSchedulerAPI.h>
 
 // This class is responsible for scheduling water pump
 // It is used to make sure that water pump is running for a limited time
 // It is also ensuring that water pump is stopped if not needed
-class WaterPumpScheduler {
+class WaterPumpScheduler : public IWaterPumpSchedulerAPI {
 private:
   IWaterPumpPtr _waterPump;
   unsigned long _stopTime = 0;
@@ -18,16 +19,12 @@ class WaterPumpScheduler {
   ~WaterPumpScheduler();
 
   void setup();
-  void stop();
   // for simplicity and testability we are passing current time as parameter
-  void start(unsigned long runTimeMs, unsigned long currentTimeMs);
   void tick(unsigned long currentTimeMs);
 
-  // pump status
-  struct WaterPumpStatus {
-    bool isRunning;
-    unsigned long stopTime;
-  };
-  WaterPumpStatus status();
+  // Public API
+  void start(unsigned long runTimeMs, unsigned long currentTimeMs) override;
+  void stop() override;
+  WaterPumpStatus status() override;
 };
 #endif

controller/tea_poor/lib/interfaces/IEnvironment.h

@@ -0,0 +1,13 @@
+#ifndef IENVIRONMENT_H
+#define IENVIRONMENT_H
+
+#include <memory>
+
+class IEnvironment {
+  public:
+    virtual unsigned long time() const = 0;
+    virtual ~IEnvironment() {}
+};
+
+typedef std::shared_ptr<IEnvironment> IEnvironmentPtr;
+#endif // IENVIRONMENT_H

controller/tea_poor/lib/interfaces/IWaterPumpSchedulerAPI.h

@@ -0,0 +1,30 @@
+// IWaterPumpSchedulerAPI interface
+#ifndef IWATERPUMPSCHEDULERAPI_H
+#define IWATERPUMPSCHEDULERAPI_H
+
+#include <memory>
+// pump status
+struct WaterPumpStatus {
+public:
+  bool isRunning;
+  unsigned long stopTime;
+  // copy constructor
+  WaterPumpStatus(const WaterPumpStatus &other) {
+    isRunning = other.isRunning;
+    stopTime = other.stopTime;
+  }
+  WaterPumpStatus(bool isRunning, unsigned long stopTime) : isRunning(isRunning), stopTime(stopTime) {}
+  // default constructor
+  WaterPumpStatus() : isRunning(false), stopTime(0) {}
+};
+
+class IWaterPumpSchedulerAPI {
+public:
+  virtual ~IWaterPumpSchedulerAPI() {}
+  virtual void stop() = 0;
+  virtual void start(unsigned long runTimeMs, unsigned long currentTimeMs) = 0;
+  virtual WaterPumpStatus status() = 0;
+};
+
+using IWaterPumpSchedulerAPIPtr = std::shared_ptr<IWaterPumpSchedulerAPI>;
+#endif

controller/tea_poor/platformio.ini

@@ -28,9 +28,7 @@ build_flags =
 	-Wall	-Wextra	-Wunused
 	-static -static-libgcc -static-libstdc++
 ; ignore libraries that are only for the Arduino
-lib_ignore =
-  RemoteControl
-	WaterPumpController
+lib_ignore = Arduino
 test_ignore =  test_uno_r4_wifi
 
 [platformio]

controller/tea_poor/src/main.cpp

@@ -3,94 +3,56 @@
 #include <WaterPumpController.h>
 #include <WaterPumpScheduler.h>
 #include <RemoteControl.h>
+#include <CommandProcessor.h>
 #include "secrets.h"
 
 #include <sstream>
+#include <ArduinoEnvironment.h>
+
+IEnvironmentPtr env = std::make_shared<ArduinoEnvironment>();
+
 // Setting up water pump
-WaterPumpScheduler waterPump(
+auto waterPump = std::make_shared<WaterPumpScheduler>(
   std::make_shared<WaterPumpController>(
     WATER_PUMP_DIRECTION_PIN, WATER_PUMP_BRAKE_PIN, WATER_PUMP_POWER_PIN
   )
 );
-// Just for safety reasons, we don't want to pour tea for too long
-// Their is no reason to make it configurable and add unnecessary complexity
-const int WATER_PUMP_SAFE_THRESHOLD = 10 * 1000;
 
 // setting up remote control
 RemoteControl remoteControl(WIFI_SSID, WIFI_PASSWORD);
 
-void _sendSystemStatus(std::ostream& response) {
-  response << "{";
-  // send water threshold
-  response << "\"water threshold\": " << WATER_PUMP_SAFE_THRESHOLD << ",";
-  // send water pump status
-  const auto waterPumpStatus = waterPump.status();
-  const unsigned long timeLeft = 
-    waterPumpStatus.isRunning ?
-    waterPumpStatus.stopTime - millis() :
-    0;
-  response 
-    << "\"pump\": {"
-    << "  \"running\": " << (waterPumpStatus.isRunning ? "true, " : "false, ")
-    << "  \"time left\": " << timeLeft
-    << "}";
-  // end of water pump status
-  ///////////////////////////////////
-  // send remote control status
-  response << "\"remote control\": " << remoteControl.asJSONString();
-  // end of JSON
-  response << "}";
-}
-
-bool isValidIntNumber(const char *str, const int maxValue, const int minValue=0) {
-  if (strlen(str) < 1) return false;
-  const int value = atoi(str);
-  if (value < minValue) return false;
-  if (maxValue <= value) return false;
-  return true;
-}
-
-void pour_tea(const char *milliseconds, std::ostream &res) {
-  if (!isValidIntNumber(milliseconds, WATER_PUMP_SAFE_THRESHOLD)) {
-    // send error message as JSON
-    res << "{ \"error\": \"invalid milliseconds value\" }";
-    return;
-  }
-  // start pouring tea
-  waterPump.start( atoi(milliseconds), millis() );
-  _sendSystemStatus(res);
-}
+// build command processor
+CommandProcessor commandProcessor(
+  WATER_PUMP_SAFE_THRESHOLD,
+  env,
+  waterPump
+);
 
 void setup() {
   Serial.begin(9600);
-  waterPump.setup();
-  // TODO: find a way to remove redundant code with string streams
+  waterPump->setup();
   remoteControl.setup([](Application &app) {
     app.get("/pour_tea", [](Request &req, Response &res) {
       char milliseconds[64];
       req.query("milliseconds", milliseconds, 64);
 
-      std::stringstream response;
-      pour_tea(milliseconds, response);
-      res.println(response.str().c_str());
+      const auto response = commandProcessor.pour_tea(milliseconds);
+      res.print(response.c_str());
     });
     // stop water pump
     app.get("/stop", [](Request &req, Response &res) {
-      waterPump.stop();
-      std::stringstream response;
-      _sendSystemStatus(response);
-      res.println(response.str().c_str());
+      const auto response = commandProcessor.stop();
+      res.print(response.c_str());
     });
     // get system status
     app.get("/status", [](Request &req, Response &res) {
-      std::stringstream response;
-      _sendSystemStatus(response);
-      res.println(response.str().c_str());
+      const auto response = commandProcessor.status();
+      res.print(response.c_str());
     });
   });
 }
 
 void loop() {
-  waterPump.tick(millis());
+  waterPump->tick(millis());
   remoteControl.process();
 };

controller/tea_poor/src/secrets.h.example

@@ -11,4 +11,8 @@ const int WATER_PUMP_DIRECTION_PIN = 12;
 const int WATER_PUMP_BRAKE_PIN = 9;
 const int WATER_PUMP_POWER_PIN = 3;
 
+// Just for safety reasons, we don't want to pour tea for too long
+// Their is no reason to make it configurable and add unnecessary complexity
+const int WATER_PUMP_SAFE_THRESHOLD = 10 * 1000;
+
 #endif // SECRETS_H

controller/tea_poor/test/test_native/main.cpp

@@ -0,0 +1,13 @@
+#include <gtest/gtest.h>
+
+// include tests
+#include "tests/WaterPumpScheduler_test.h"
+#include "tests/CommandProcessor_test.h"
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  int result = RUN_ALL_TESTS(); // Intentionally ignoring the return value
+  (void)result; // Silence unused variable warning
+  // Always return zero-code and allow PlatformIO to parse results
+  return 0;
+}