From ba5b844f038401bbfa9dd606631ffeae6410d7b6 Mon Sep 17 00:00:00 2001
From: Pascal <engelerp@phys.ethz.ch>
Date: Mon, 12 Jun 2023 15:32:21 +0200
Subject: [PATCH] Fixed lots of bugs, now the new firmware compiles

---
 .../drivers/communicator/communicator.cpp     | 55 +++++++++++--------
 .../drivers/communicator/communicator.hpp     | 15 ++---
 .../drivers/pid_controller/pid_controller.cpp | 31 +++++++----
 .../drivers/pid_controller/pid_controller.hpp | 10 ++--
 firmware/drivers/ptimer/ptimer.cpp            |  3 +-
 firmware/drivers/ptimer/ptimer.hpp            |  2 +-
 firmware/drivers/utility/utility.cpp          |  1 +
 firmware/drivers/utility/utility.hpp          |  3 +-
 firmware/pid_controller/pid_controller.ino    | 10 ++--
 9 files changed, 78 insertions(+), 52 deletions(-)

diff --git a/firmware/drivers/communicator/communicator.cpp b/firmware/drivers/communicator/communicator.cpp
index df399f3..7ff3c51 100644
--- a/firmware/drivers/communicator/communicator.cpp
+++ b/firmware/drivers/communicator/communicator.cpp
@@ -1,12 +1,23 @@
-#include <communicator.cpp>
+#include <communicator.hpp>
+#include <cstdio> //snprintf
 #include <utility.hpp>
 #include <ptimer.hpp>
 #include <pid_controller.hpp>
+#include <cstdlib> //std::strtod
+#include <algorithm> //This is necessary to fix a bug that cost me 4h.
+
+/*
+    As I tracked down, a (standard) string library calls std::min() in some functions (like compare).
+    The arduino library device_usb.h defines a min() macro.
+    Because <algorithm> wasn't included anywhere in this compilation unit, but device_usb.h is, these calls to std::min() resolve to the device_usb.hpp version.
+    This, however, doesn't work, and the compiler throws an error concerning this macro.
+    Holy fucking shit.
+*/
 
 unsigned long Communicator::timeout_us_ = 20000; //timeout 20ms
-char Communicator::outbuf[256]; //out buffer
+char Communicator::outbuf_[256]; //out buffer
 char Communicator::cmd_ = 'X'; //command received
-std::vector<char> Communicator::arg_ = std::vector<char>('\0',256); //arguments received
+std::vector<char> Communicator::arg_ = std::vector<char>(256, '\0'); //arguments received
 
 bool Communicator::communicate(){
     if(!serialCharAvailable_()){
@@ -16,7 +27,7 @@ bool Communicator::communicate(){
         cmd_ = getSerialChar_();
         if(isSetCommand_()){
             //read argument
-            char latest = cmd;
+            char latest = cmd_;
             size_t i = 0;
             PTimer::start();
             while(latest != ';' && i < arg_.size()-10 && PTimer::elapsed() < timeout_us_){
@@ -24,7 +35,7 @@ bool Communicator::communicate(){
                     latest = getSerialChar_();
                     arg_[i++] = latest;
                     if(latest == ';'){
-                        arg[i-1] = '\0';
+                        arg_[i-1] = '\0';
                     }
                     PTimer::start();
                 }
@@ -47,12 +58,12 @@ bool Communicator::communicate(){
             if(cmd_ == 't'){ //getTemperature
                 snprintf(   outbuf_,
                             255,
-                            "%f,%f,%f,%f,%f;", 
-                            Temperatures.temperatures[0],
-                            Temperatures.temperatures[1],
-                            Temperatures.temperatures[2],
-                            Temperatures.temperatures[3],
-                            Temperatures.temperatures[4]
+                            "%f,%f,%f,%f,%f;",
+                            Temperatures::temperatures[0],
+                            Temperatures::temperatures[1],
+                            Temperatures::temperatures[2],
+                            Temperatures::temperatures[3],
+                            Temperatures::temperatures[4]
                             );
                 sendBuf_();
                 return true;
@@ -107,11 +118,11 @@ bool Communicator::communicate(){
     }
 }
 
-bool Communicator::serialCharAvailable_() const{
+bool Communicator::serialCharAvailable_(){
     return SERCHAN.available() > 0;
 }
 
-bool Communicator::isSetCommand_() const{
+bool Communicator::isSetCommand_(){
     return  cmd_ == 'P' ||
             cmd_ == 'I' ||
             cmd_ == 'D' ||
@@ -119,7 +130,7 @@ bool Communicator::isSetCommand_() const{
             cmd_ == 'S';
 }
 
-bool Communicator::isGetCommand_() const{
+bool Communicator::isGetCommand_(){
     return  cmd_ == 't' ||
             cmd_ == 'p' ||
             cmd_ == 'v' ||
@@ -127,11 +138,11 @@ bool Communicator::isGetCommand_() const{
             cmd_ == 's';
 }
 
-bool Communicator::isNopCommand_() const{
+bool Communicator::isNopCommand_(){
     return  cmd_ == '?';
 }
 
-char Communicator::getSerialChar_() const{
+char Communicator::getSerialChar_(){
     int newChar = SERCHAN.read();
     if(newChar < 0 || newChar >= 128){//invalid char
         return 'X';
@@ -141,21 +152,21 @@ char Communicator::getSerialChar_() const{
     }
 }
 
-void Communicator::sendBuf_() const{
+void Communicator::sendBuf_(){
     SERCHAN.write(outbuf_);
 }
 
-void Communicator::handleTimeout_() const{
+void Communicator::handleTimeout_(){
     snprintf(outbuf_, 255, "!");
     sendBuf_();
 }
 
-void Communicator::handleInputOverflow_() const{
+void Communicator::handleInputOverflow_(){
     snprintf(outbuf_, 255, "!");
     sendBuf_();
 }
 
-void Communicator::handleSetCommand_() const{
+void Communicator::handleSetCommand_(){
     char* endptr = arg_.data()+250;
     double darg = std::strtod(arg_.data(), &endptr);
     if(arg_.data() == endptr){ //error
@@ -194,13 +205,13 @@ void Communicator::handleSetCommand_() const{
     }
 }
 
-void Communicator::handleNopCommand_() const{
+void Communicator::handleNopCommand_(){
     snprintf(outbuf_, 255, ";");
     sendBuf_();
     return;
 }
 
-void Communicator::handleInvalidCommand_() const{
+void Communicator::handleInvalidCommand_(){
     snprintf(outbuf_, 255, "!");
     sendBuf_();
     return;
diff --git a/firmware/drivers/communicator/communicator.hpp b/firmware/drivers/communicator/communicator.hpp
index ec2e69d..560722f 100644
--- a/firmware/drivers/communicator/communicator.hpp
+++ b/firmware/drivers/communicator/communicator.hpp
@@ -1,6 +1,7 @@
 #ifndef COMMUNICATOR_HPP_INCLUDED
 #define COMMUNICATOR_HPP_INCLUDED
 #include <vector>
+#include <algorithm>
 
 #define SERCHAN SerialUSB
 
@@ -10,13 +11,13 @@ public:
 
 private:
     /*Function Members*/
-    static bool serialCharAvailable_() const;
-    static bool isSetCommand_() const;
-    static bool isGetCommand_() const;
-    static bool isNopCommand_() const;
+    static bool serialCharAvailable_();
+    static bool isSetCommand_();
+    static bool isGetCommand_();
+    static bool isNopCommand_();
 
-    static char getSerialChar_() const;
-    static void sendBuf_() const;
+    static char getSerialChar_();
+    static void sendBuf_();
 
     static void handleTimeout_();
     static void handleInputOverflow_();
@@ -30,6 +31,6 @@ private:
     static char outbuf_[256];
     static char cmd_;
     static std::vector<char> arg_;
-}
+};
 
 #endif
diff --git a/firmware/drivers/pid_controller/pid_controller.cpp b/firmware/drivers/pid_controller/pid_controller.cpp
index 3ed79ba..b19b34a 100644
--- a/firmware/drivers/pid_controller/pid_controller.cpp
+++ b/firmware/drivers/pid_controller/pid_controller.cpp
@@ -2,21 +2,32 @@
 #include <algorithm>
 #include <ltc6992.hpp>
 
+/*Arduino isn't C++ standard compliant, need to fix this myself*/
+namespace std {
+  void __throw_bad_alloc()
+  {
+    SerialUSB.println("Unable to allocate memory");
+  }
+
+  void __throw_length_error( char const*e )
+  {
+    SerialUSB.print("Length Error :");
+    SerialUSB.println(e);
+  }
+}
+
 //define statics
 double Pid_controller::setpoint_=0.; //target temperature in °C
 double Pid_controller::output_=0.05; //output to LTC6992
-std::vector<double> Pid_controller::pid_vec_ = {0.,0.,0.}; //current values of P I D
+std::vector<double> Pid_controller::pid_vec_ = {0., 0., 0.}; //current values of P I D
 std::vector<double> Pid_controller::kpid_vec_ = {0., 0., 0.}; //coefficients Kp Ki Kd
 double Pid_controller::I_reset_ = 0.; //reset value of I
 double Pid_controller::I_reset_errsq_ = 0.; //value of error*error above which the I-term is held in reset
 
 
-Pid_controller::
-
-
 void Pid_controller::init(double setpoint, std::vector<double> kpid, double I_reset, double I_reset_errsq){
     setpoint_ = setpoint;
-    kpid_ = kpid;
+    kpid_vec_ = kpid;
     I_reset_ = I_reset;
     I_reset_errsq_ = I_reset_errsq;
 }
@@ -62,22 +73,22 @@ void Pid_controller::set_setpoint(double temperature){
     setpoint_ = temperature;
 }
 
-std::vector<double> Pid_controller::get_kpid() const{
+std::vector<double> Pid_controller::get_kpid(){
     return kpid_vec_;
 }
 
-std::vector<double> Pid_controller::get_pid() const{
+std::vector<double> Pid_controller::get_pid(){
     return pid_vec_;
 }
 
-double Pid_controller::get_output() const{
+double Pid_controller::get_output(){
     return output_;
 }
 
-double Pid_controller::get_I_reset() const{
+double Pid_controller::get_I_reset(){
     return I_reset_;
 }
 
-double Pid_controller::get_setpoint() const{
+double Pid_controller::get_setpoint(){
     return setpoint_;
 }
diff --git a/firmware/drivers/pid_controller/pid_controller.hpp b/firmware/drivers/pid_controller/pid_controller.hpp
index 95a89c5..4c5bd04 100644
--- a/firmware/drivers/pid_controller/pid_controller.hpp
+++ b/firmware/drivers/pid_controller/pid_controller.hpp
@@ -15,11 +15,11 @@ public:
     static void set_I_reset_errsq(double I_reset_errsq);
     static void set_setpoint(double temperature);
 
-    static std::vector<double> get_kpid() const;
-    static std::vector<double> get_pid() const;
-    static double get_output() const;
-    static double get_I_reset() const;
-    static double get_setpoint() const;
+    static std::vector<double> get_kpid();
+    static std::vector<double> get_pid();
+    static double get_output();
+    static double get_I_reset();
+    static double get_setpoint();
 
 private:
     static double setpoint_; //target temperature in °C
diff --git a/firmware/drivers/ptimer/ptimer.cpp b/firmware/drivers/ptimer/ptimer.cpp
index 09c510c..883fc26 100644
--- a/firmware/drivers/ptimer/ptimer.cpp
+++ b/firmware/drivers/ptimer/ptimer.cpp
@@ -1,4 +1,5 @@
 #include <ptimer.hpp>
+#include <Arduino.h>
 
 unsigned long PTimer::start_t_ = 0;
 
@@ -7,5 +8,5 @@ void PTimer::start(){
 }
 
 unsigned long PTimer::elapsed(){
-    return static_cast<unsigned long>(micros - start_t_);;
+    return static_cast<unsigned long>(micros() - start_t_);;
 }
diff --git a/firmware/drivers/ptimer/ptimer.hpp b/firmware/drivers/ptimer/ptimer.hpp
index de4f843..1674c9f 100644
--- a/firmware/drivers/ptimer/ptimer.hpp
+++ b/firmware/drivers/ptimer/ptimer.hpp
@@ -4,7 +4,7 @@ class PTimer{
 public:
     static void start();
     //returns the elapsed time in microseconds
-    static unsigned long elapsed() const;
+    static unsigned long elapsed();
 private:
     static unsigned long start_t_;
 };
diff --git a/firmware/drivers/utility/utility.cpp b/firmware/drivers/utility/utility.cpp
index c2b7c5e..a8da28b 100644
--- a/firmware/drivers/utility/utility.cpp
+++ b/firmware/drivers/utility/utility.cpp
@@ -1,6 +1,7 @@
 #include <utility.hpp>
 #include <ad7124_4.hpp>
 #include <ad7124_registers.hpp>
+#include <spi_selecta.hpp>
 
 std::array<double,5> Temperatures::temperatures = {0., 0., 0., 0., 0.};
 
diff --git a/firmware/drivers/utility/utility.hpp b/firmware/drivers/utility/utility.hpp
index c413546..081a094 100644
--- a/firmware/drivers/utility/utility.hpp
+++ b/firmware/drivers/utility/utility.hpp
@@ -1,6 +1,7 @@
 #ifndef UTILITY_HPP_INCLUDED
 #define UTILITY_HPP_INCLUDED
 #include <ad7124_4.hpp>
+#include <array>
 
 struct Adc_data{
     bool valid;
@@ -8,7 +9,7 @@ struct Adc_data{
     uint32_t data;
 };
 
-Adc_Data readAdc(Ad7124_4* device);
+Adc_data readAdc(Ad7124_4* device);
 
 struct Temperatures{
     /*The 0th temperature shall always be the loop, the others the out-of-loops*/
diff --git a/firmware/pid_controller/pid_controller.ino b/firmware/pid_controller/pid_controller.ino
index 9b83ecf..c8fe31c 100644
--- a/firmware/pid_controller/pid_controller.ino
+++ b/firmware/pid_controller/pid_controller.ino
@@ -10,6 +10,8 @@
 #include <communicator.hpp>
 
 
+
+
 #define SPIRATE 84000000/32
 
 #define nCS_0 4
@@ -23,8 +25,8 @@ SPISettings spisettings = SPISettings(SPIRATE, MSBFIRST, SPI_MODE3);
 Ad7124_4 adc_1(nCS_1, &spisettings);
 Ad7124_4 adc_2(nCS_2, &spisettings);
 /*ADC Data Storage*/
-Adc_data adc1_data;
-Adc_data adc2_data;
+Adc_data adc1Data;
+Adc_data adc2Data;
 
 
 /*Set up Timer Interrupt Infrastructure*/
@@ -36,8 +38,6 @@ void handlerPid(){
 }
 
 void setup() {
-    Temperatures::init();
-
     SerialUSB.begin(9600);
 
     Ltc6992::init();
@@ -125,7 +125,7 @@ void loop() {
     //ADC 2
     adc2Data = readAdc(&adc_2);
     if(adc2Data.valid && adc2Data.channel < 4u){
-        Temperatures[adc2Data.channel+1] = data_to_temperature(adc2Data.data);
+        Temperatures::temperatures[adc2Data.channel+1] = data_to_temperature(adc2Data.data);
     }
 
     /*Receive Commands*/
-- 
GitLab