Code optimization

- Remove excess millis usage
- Remove thread dispatch time
- Replace 'unsigned char' type by 'byte'
- Optimize drawing
- Replace 'int' by 'bool' for pin states
- Align types usage

Arduino/eScooterControl/accelerationcontrol.cpp

@@ -32,6 +32,7 @@
 
 const int AcceleratorSensorStep = 10;
 const int AcceleratorStep = 21;//From 0 to 4095 for 2s with update each 10ms
+const int MaxLevel = 10;
 
 const PROGMEM unsigned short AccelerationLevelTable[11] = {
   0,
@@ -47,9 +48,6 @@ const PROGMEM unsigned short AccelerationLevelTable[11] = {
   4095
 };
 
-const int AcceleratorSensorDivider = AcceleratorSensorDiff * AcceleratorSensorStep;
-
-const int MaxLevel = 100/AcceleratorSensorStep;
 
 AccelerationControl::AccelerationControl() : m_stopState(HIGH)
   , m_acceleratorMinValue(0)
@@ -62,7 +60,7 @@ AccelerationControl::AccelerationControl() : m_stopState(HIGH)
   Wire.begin();
   pinMode(AcceleratorSensorPin, INPUT);
   pinMode(StopSensorPin, INPUT_PULLUP);
-  attachInterrupt(digitalPinToInterrupt(StopSensorPin), [](){
+  attachInterrupt(digitalPinToInterrupt(StopSensorPin), []() {
     AccelerationControl::instance()->stop();
   }, CHANGE);
 
@@ -71,7 +69,7 @@ AccelerationControl::AccelerationControl() : m_stopState(HIGH)
 }
 
 
-void AccelerationControl::setAccelerationLevel(const int level)
+void AccelerationControl::setAccelerationLevel(const byte level)
 {
   if (level == m_accelerationLevel) {
     if (m_cruiseLevel < m_accelerationLevel
@@ -89,7 +87,7 @@ void AccelerationControl::setAccelerationLevel(const int level)
   m_accelerationLevel = level;
 }
 
-void AccelerationControl::dispatch(unsigned long)
+void AccelerationControl::dispatch()
 {  
   if (m_stopState == HIGH) {
     setAccelerationLevel(readAcceleratorData());
@@ -105,20 +103,18 @@ void AccelerationControl::stop() {
   }
 }
 
-int AccelerationControl::readAcceleratorData()
+byte AccelerationControl::readAcceleratorData()
 {
-  int value = analogRead(AcceleratorSensorPin);
+  unsigned int value = analogRead(AcceleratorSensorPin);
   if (value < m_acceleratorMinValue) {
     m_acceleratorMinValue = value; //Andjust min value accordingly
   }
 //  Serial.print("Accelerator sensor: ");
 //  Serial.print(value);
 //  Serial.print(" ");
-  int outValue = round(100.0*(value - m_acceleratorMinValue)/AcceleratorSensorDivider);
+  byte outValue = round(MaxLevel * float(value - m_acceleratorMinValue) / AcceleratorSensorDiff);
   if (outValue > MaxLevel) {
     outValue = MaxLevel;
-  } else if (outValue < 0) {
-    outValue = 0;
   }
 //  Serial.println(outValue);
   return outValue;
@@ -128,12 +124,12 @@ void AccelerationControl::updateAcceleration() {
 //  Serial.print("Acceleration level: ");
 //  Serial.println(m_accelerationLevel);
 
-  int level = m_accelerationLevel;
+  byte level = m_accelerationLevel;
   if (m_cruiseLevel > 0) {//At cruise control
     level = m_cruiseLevel;
   }
 
-  Display::instance()->drawAccelerationLevel(level);
+  Display::instance()->setAcceleration(level);
 
   m_expectedAcceleration = pgm_read_word(&AccelerationLevelTable[level]);
 //  Serial.print("Expected acceleration: ");
@@ -149,11 +145,6 @@ void AccelerationControl::dispatchAcceleration()
   if (m_actualAcceleration > m_expectedAcceleration) {
     //No need to step down, set acceleration immediatelly
     m_actualAcceleration = m_expectedAcceleration;
-//    if (m_actualAcceleration > AcceleratorStep) {
-//      m_actualAcceleration -= AcceleratorStep;
-//    } else {
-//      m_actualAcceleration = 0;
-//    }
   } else if (m_actualAcceleration < m_expectedAcceleration) {
     m_actualAcceleration += AcceleratorStep;
     if (m_actualAcceleration > 0x0fff) {

Arduino/eScooterControl/accelerationcontrol.h

@@ -25,30 +25,37 @@
 
 #pragma once
 
+#if defined(ARDUINO) && ARDUINO >= 100
+  #include <Arduino.h>
+#else
+  #include <WProgram.h>
+#endif
+
 #include "singleton.h"
 
 class AccelerationControl : public Singleton<AccelerationControl>
 {
 public:
-  void dispatch(unsigned long);
+  void dispatch();
   void dispatchAcceleration();
 private:
   AccelerationControl();
   friend class Singleton;
 
-  void setAccelerationLevel(int level);
-  int readAcceleratorData();
+  void setAccelerationLevel(const byte level);
+  byte readAcceleratorData();
   void updateAcceleration();
   
   void stop();
 
-  unsigned int m_stopState;
+  bool m_stopState;
+  
+  unsigned int m_acceleratorMinValue;
+  byte m_accelerationLevel;
   
-  int m_acceleratorMinValue;
-  unsigned int m_accelerationLevel;
   unsigned short m_expectedAcceleration;
   unsigned short m_actualAcceleration;
 
   unsigned long m_cruiseTime;
-  unsigned int m_cruiseLevel;
+  byte m_cruiseLevel;
 };

Arduino/eScooterControl/buttoncontrol.cpp

@@ -43,18 +43,17 @@ ButtonControl::ButtonControl() : m_triggers(0)
 }
 
 void ButtonControl::dispatch() {
-  int state = digitalRead(ButtonPin);
+  unsigned long currentTime = millis();
+  bool state = digitalRead(ButtonPin);
   if (m_previousState == HIGH && state == LOW) {
     if (m_triggers == 0) {
-      m_triggerTime = millis();
+      m_triggerTime = currentTime;
     }
     ++m_triggers;
   }
 
   m_previousState = state;
 
-  unsigned long currentTime = millis();
-
   if ((currentTime - m_triggerTime) >= ReactionTime) {
     if (m_triggers == 1 && state == LOW) {
 //      Serial.println("Power off");

Arduino/eScooterControl/buttoncontrol.h

@@ -25,6 +25,12 @@
 
 #pragma once
 
+#if defined(ARDUINO) && ARDUINO >= 100
+  #include <Arduino.h>
+#else
+  #include <WProgram.h>
+#endif
+
 #include "singleton.h"
 
 class ButtonControl : public Singleton<ButtonControl>
@@ -37,8 +43,8 @@ private:
   friend class Singleton;
   ButtonControl();
 
-  unsigned int m_triggers;
+  byte m_triggers;
   unsigned long m_triggerTime;
-  int m_previousState;
-  int m_ledState;
+  bool m_previousState;
+  bool m_ledState;
 };

Arduino/eScooterControl/display.cpp

@@ -186,6 +186,9 @@ Display::Display() : mLedControl(DisplayDataPin, DisplayClkPin, DisplayCsPin,2)
   B00000000,
   B00000000,
   B00000000}
+  , m_acceleration(0)
+  , m_battery(0)
+  , m_speed(0)
 {
   mLedControl.shutdown(0, false);
   mLedControl.setIntensity(0, 0);
@@ -196,31 +199,30 @@ Display::Display() : mLedControl(DisplayDataPin, DisplayClkPin, DisplayCsPin,2)
   mLedControl.clearDisplay(1);  
 }
 
-void Display::drawAccelerationLevel(unsigned char level)
+void Display::drawAccelerationLevel()
 {
-  if (level > 10) {
-    level = 10;
+  if (m_acceleration > 10) {
+    m_acceleration = 10;
   }
   
-  for (int i = 0; i < 6; i++) {
+  for (byte i = 0; i < 6; ++i) {
     mDisplayBuffer[8 + i] = 0;
-    if (level > 0) {
-      byte value = pgm_read_byte(&(AcceleratorLevel[level - 1][i]));
-      mDisplayBuffer[8 + i] = value;
+    if (m_acceleration > 0) {
+      mDisplayBuffer[8 + i] = pgm_read_byte(&(AcceleratorLevel[m_acceleration - 1][i]));
     }
   }
 }
 
 
-void Display::drawSpeed(unsigned char number)
+void Display::drawSpeed()
 {
-  if (number > 99) {
-    number = 99;
+  if (m_speed > 99) {
+    m_speed = 99;
   }
-  unsigned char tens = number / 10;
-  unsigned char ones = number % 10;
+  byte tens = m_speed / 10;
+  byte ones = m_speed % 10;
 
-  for (int i = 0; i < 5; i++) {
+  for (byte i = 0; i < 5; ++i) {
     mDisplayBuffer[i] = 0;
     byte tensDigit = pgm_read_byte(&(Digit[tens][i]));
     byte onesDigit = pgm_read_byte(&(Digit[ones][i]));
@@ -228,17 +230,17 @@ void Display::drawSpeed(unsigned char number)
   }
 }
 
-void Display::drawBatteryLevel(unsigned char level)
+void Display::drawBatteryLevel()
 { 
-  if (level > 5) {
-    level = 5;
+  if (m_battery > 5) {
+    m_battery = 5;
   }
 
   mDisplayBuffer[6] = 0;
   mDisplayBuffer[7] = 0;
 
   byte batteryLevelChar = pgm_read_byte(&(BatteryLevel));
-  for (unsigned char i = 0; i < level; ++i) {
+  for (byte i = 0; i < m_battery; ++i) {
     mDisplayBuffer[6] |= batteryLevelChar << i;
     mDisplayBuffer[7] |= batteryLevelChar << (i + 1);
   }
@@ -246,12 +248,11 @@ void Display::drawBatteryLevel(unsigned char level)
 
 void Display::updateDisplayBuffer()
 {
-  for(int i = 0; i < 8; i++) {
+  drawAccelerationLevel();
+  drawSpeed();
+  drawBatteryLevel();
+  for(byte i = 0; i < 8; ++i) {
     mLedControl.setColumn(1, i, mDisplayBuffer[i]);    
-  }
-
-  
-  for(int i = 0; i < 8; i++) {
     mLedControl.setColumn(0, i, mDisplayBuffer[i + 8]);    
   }
 }

Arduino/eScooterControl/display.h

@@ -31,15 +31,32 @@
 class Display : public Singleton<Display>
 {
 public:
-  void drawAccelerationLevel(unsigned char level);
-  void drawSpeed(unsigned char number);
-  void drawBatteryLevel(unsigned char level);
   void updateDisplayBuffer();
 
+  void setAcceleration(byte level) {
+    m_acceleration = level;
+  }
+
+  void setSpeed(byte speed) {
+    m_speed = speed;
+  }
+
+  void setBatteryLevel(byte level) {
+    m_battery = level;
+  }
+
 private:
   Display();
   friend class Singleton;
 
+  void drawAccelerationLevel();
+  void drawSpeed();
+  void drawBatteryLevel();
+  
   byte mDisplayBuffer[16];
   LedControl mLedControl;
+
+  byte m_acceleration;
+  byte m_battery;
+  byte m_speed;
 };

Arduino/eScooterControl/eScooterControl.ino

@@ -28,7 +28,6 @@
 #include "accelerationcontrol.h"
 #include "buttoncontrol.h"
 
-/* we always wait a bit between updates of the display */
 const unsigned long AcceleratorPedalUpdateTime = 200;
 const unsigned long AcceleratorUpdateTime = 10;
 const unsigned long DisplayUpdateTime = 250;
@@ -49,11 +48,11 @@ void setup() {
 
   delay(1000);
   AccelerationControl::instance();
-  gAcceleratorPedalThread.assignCallback([](unsigned long time){
-    AccelerationControl::instance()->dispatch(time);
+  gAcceleratorPedalThread.assignCallback([](){
+    AccelerationControl::instance()->dispatch();
   });
 
-  gAcceleratorThread.assignCallback([](unsigned long){
+  gAcceleratorThread.assignCallback([](){
     AccelerationControl::instance()->dispatchAcceleration();
   });
 
@@ -62,12 +61,12 @@ void setup() {
 
   delay(500);
   Display::instance();
-  gDisplayThread.assignCallback([](unsigned long){
+  gDisplayThread.assignCallback([](){
     Display::instance()->updateDisplayBuffer();
   });
 
-  gBatteryThread.assignCallback([](unsigned long){
-    Display::instance()->drawBatteryLevel(fakeBatteryLevel);
+  gBatteryThread.assignCallback([](){
+    Display::instance()->setBatteryLevel(fakeBatteryLevel);
     fakeBatteryLevel++;
     if (fakeBatteryLevel > 5) {
       fakeBatteryLevel = 0;
@@ -75,7 +74,7 @@ void setup() {
   });
 
   ButtonControl::instance();
-  gButtonThread.assignCallback([](unsigned long){
+  gButtonThread.assignCallback([](){
     ButtonControl::instance()->dispatch();
   });
 //  Serial.println("Init complete");

Arduino/eScooterControl/pinconfig.h

@@ -33,8 +33,8 @@
 
 //Two pins used by Wire library for I2C bus
 //Are not used in code directly. Keep here as reminder.
-const int I2CSDA = A4;
-const int I2CSCL = A5;
+const int I2CSDAPin = A4;
+const int I2CSCLPin = A5;
 
 const int ButtonPin = 4;
 const int PowerPin = 5;
@@ -42,7 +42,7 @@ const int PowerPin = 5;
 const int LedPin = 6;
 
 const int HallSensorPin = 2;
-const int PulsePerCircle = 45;
+const byte PulsePerCircle = 45;
 
 const int StopSensorPin = 3;
 
@@ -53,8 +53,8 @@ const int DisplayClkPin = 12;
 const int AcceleratorSensorPin = A0;
 
 //Align accelerator trigger values
-const int AcceleratorSensorDiff = 548;
+const unsigned int AcceleratorSensorDiff = 548;
 
-const int CruiseTime = 3000; //ms
+const unsigned long CruiseTime = 3000; //ms
 
 const uint8_t AcceleratorAddress = 0x60;

Arduino/eScooterControl/singleton.h

@@ -35,7 +35,7 @@ public:
   
 protected:
   Singleton() = default;
-
+  virtual ~Singleton() = default;
 private:
   Singleton(const Singleton&) = delete;
   Singleton(const Singleton&&) = delete;

Arduino/eScooterControl/speedometer.cpp

@@ -58,7 +58,7 @@ void Speedometer::attachToDisplay(Display *display) {
 void Speedometer::incrementHallCounter()
 {
   mHallCounter++;
-  int currentTick = mHallCounter % PulsePerCircle;
+  byte currentTick = mHallCounter % PulsePerCircle;
   if (currentTick == 0) {
     mHallCounter = 0;
     unsigned long currentTime = millis();
@@ -66,7 +66,7 @@ void Speedometer::incrementHallCounter()
     mLastHallTime = currentTime;
   
     if (mDisplay != nullptr) {
-      mDisplay->drawSpeed(mMomentSpeed);
+      mDisplay->setSpeed(mMomentSpeed);
     }
   }
 }

Arduino/eScooterControl/speedometer.h

@@ -24,6 +24,12 @@
  */
 #pragma once
 
+#if defined(ARDUINO) && ARDUINO >= 100
+  #include <Arduino.h>
+#else
+  #include <WProgram.h>
+#endif
+
 #include "singleton.h"
 
 class Display;
@@ -37,9 +43,9 @@ private:
   Speedometer();
   friend class Singleton;
   
-  unsigned int mMomentSpeed;
+  byte mMomentSpeed;
   unsigned long mLastHallTime;
-  unsigned int mHallCounter;//TODO: Not used for now, need for moment speed correction
+  byte mHallCounter;//TODO: Not used for now, need for moment speed correction
   
   Display *mDisplay;
 };

Arduino/eScooterControl/thread.cpp

@@ -24,7 +24,7 @@
  */
 #include "thread.h"
 
-unsigned char Thread::sThreadSlots = 0;
+byte Thread::sThreadSlots = 0;
 Thread* Thread::sThreadPool[8] = { nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr };
 
 Thread::Thread(unsigned long delay) : mDelay(delay)
@@ -44,13 +44,13 @@ void Thread::dispatchThread()
   unsigned long time = millis();
   unsigned long delay = time - mLastCallTime;
   if (delay >= mDelay) {
-    mCallback(delay);
+    mCallback();
     mLastCallTime = time;
   }
 }
 
 void Thread::dispatch() {
-  for (unsigned char i = 0; i < sThreadSlots; i++) {
+  for (byte i = 0; i < sThreadSlots; i++) {
     if (sThreadPool[i] != nullptr) {
       sThreadPool[i]->dispatchThread();
     }

Arduino/eScooterControl/thread.h

@@ -30,7 +30,7 @@
   #include <WProgram.h>
 #endif
 
-typedef void (*threadCallback)(unsigned long);
+typedef void (*threadCallback)();
 
 class Thread {
 public:
@@ -49,6 +49,6 @@ private:
   threadCallback mCallback;
   unsigned long mLastCallTime;
   
-  static unsigned char sThreadSlots;
+  static byte sThreadSlots;
   static Thread* sThreadPool[8];
 };