consider static analysis feedback

Halbelunge · Jan 8, 2021 · 7cfef14 · 7cfef14
1 parent 0f1f60a
commit 7cfef14
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 29 deletions.
diff --git a/src/config.cpp b/src/config.cpp
@@ -136,7 +136,7 @@ void ObsConfig::makeSureSystemDefaultsAreSet() {
   }
   ensureSet(data, PROPERTY_PORTAL_URL, "https://openbikesensor.hlrs.de");
   ensureSet(data, PROPERTY_PORTAL_TOKEN, "5e8f2f43e7e3b3668ca13151");
-  ensureSet(data, PROPERTY_GPS_FIX, GPS::FIX_POS);
+  ensureSet(data, PROPERTY_GPS_FIX, (int) Gps::WaitFor::FIX_POS);
   ensureSet(data, PROPERTY_DISPLAY_CONFIG, DisplaySimple);
   if (getProperty<int>(PROPERTY_DISPLAY_CONFIG) == 0) {
     data[PROPERTY_DISPLAY_CONFIG] = DisplaySimple;
@@ -371,13 +371,13 @@ void ObsConfig::parseOldJsonDocument(DynamicJsonDocument &doc) {
 
   uint16_t gpsConfig = doc["GPSConfig"];
   if (gpsConfig & GPSOptions::ValidTime) {
-    setProperty(0, PROPERTY_GPS_FIX, GPS::FIX_TIME);
+    setProperty(0, PROPERTY_GPS_FIX, (int) Gps::WaitFor::FIX_TIME);
   } else if (gpsConfig & GPSOptions::ValidLocation) {
-    setProperty(0, PROPERTY_GPS_FIX, GPS::FIX_POS);
+    setProperty(0, PROPERTY_GPS_FIX, (int) Gps::WaitFor::FIX_POS);
   } else if (gpsConfig & GPSOptions::NumberSatellites) {
     setProperty(0, PROPERTY_GPS_FIX, doc["satsForFix"] | 4);
   } else {
-    setProperty(0, PROPERTY_GPS_FIX, GPS::FIX_NO_WAIT);
+    setProperty(0, PROPERTY_GPS_FIX, (int) Gps::WaitFor::FIX_NO_WAIT);
   }
   setProperty(0, PROPERTY_CONFIRMATION_TIME_SECONDS, doc["confirmationTimeWindow"] | 5);
   setProperty(0, PROPERTY_PRIVACY_CONFIG, doc["privacyConfig"] | AbsolutePrivacy);

diff --git a/src/configServer.cpp b/src/configServer.cpp
@@ -854,9 +854,9 @@ void startServer(ObsConfig *obsConfig) {
                  theObsConfig->getProperty<bool>(ObsConfig::PROPERTY_SIM_RA) ? "checked" : "");
 
     int gpsFix = theObsConfig->getProperty<int>(ObsConfig::PROPERTY_GPS_FIX);
-    html.replace("{fixPos}", gpsFix == GPS::FIX_POS || gpsFix > 0 ? "selected" : "");
-    html.replace("{fixTime}", gpsFix == GPS::FIX_TIME ? "selected" : "");
-    html.replace("{fixNoWait}", gpsFix == GPS::FIX_NO_WAIT ? "selected" : "");
+    html.replace("{fixPos}", gpsFix == (int) Gps::WaitFor::FIX_POS || gpsFix > 0 ? "selected" : "");
+    html.replace("{fixTime}", gpsFix == (int) Gps::WaitFor::FIX_TIME ? "selected" : "");
+    html.replace("{fixNoWait}", gpsFix == (int) Gps::WaitFor::FIX_NO_WAIT ? "selected" : "");
 
     const uint privacyConfig = (uint) theObsConfig->getProperty<int>(
       ObsConfig::PROPERTY_PRIVACY_CONFIG);

diff --git a/src/gps.cpp b/src/gps.cpp
@@ -24,9 +24,6 @@
 /* Value is in the past (just went by at the time of writing). */
 static const time_t PAST_TIME = 1606672131;
 
-Gps::Gps() {
-}
-
 void Gps::begin() {
   mSerial.begin(9600, SERIAL_8N1, 16, 17);
   // TODO: Increase speed etc.
@@ -137,7 +134,7 @@ void Gps::handleNewTxtData() {
       mMessage += mTxtMessage.value();
     }
     if (String(mTxtCount.value()) == String(mTxtSeq.value())) {
-      for (String message : mMessages) {
+      for (String& message : mMessages) {
         if (message == mMessage) {
           mMessage.clear();
           break;
@@ -155,7 +152,6 @@ void Gps::handleNewTxtData() {
 }
 
 bool Gps::isInsidePrivacyArea() {
-  TinyGPSLocation location = location;
   // quite accurate haversine formula
   // consider using simplified flat earth calculation to save time
 
@@ -246,22 +242,22 @@ PrivacyArea Gps::newPrivacyArea(double latitude, double longitude, int radius) {
 bool Gps::hasState(int state, SSD1306DisplayDevice *display) {
   bool result = false;
   switch (state) {
-    case FIX_POS:
+    case (int) WaitFor::FIX_POS:
       if (sentencesWithFix() > 0) {
         log_d("Got location...");
         display->showTextOnGrid(2, 4, "Got location");
         result = true;
       }
       break;
-    case FIX_TIME:
+    case (int) WaitFor::FIX_TIME:
       if (time.isValid()
                      && !(time.second() == 00 && time.minute() == 00 && time.hour() == 00)) {
         log_d("Got time...");
         display->showTextOnGrid(2, 4, "Got time");
         result = true;
       }
       break;
-    case FIX_NO_WAIT:
+    case (int) WaitFor::FIX_NO_WAIT:
       log_d("GPS, no wait");
       display->showTextOnGrid(2, 4, "GPS, no wait");
       result = true;
@@ -290,10 +286,10 @@ void Gps::showWaitStatus(SSD1306DisplayDevice *display) {
              time.hour(), time.minute(), time.second(), satellites.value());
     satellitesString = String(timeStr);
   }
-  displayTest->showTextOnGrid(2, 5, satellitesString);
+  display->showTextOnGrid(2, 5, satellitesString);
 }
 
-bool Gps::moduleIsAlive() {
+bool Gps::moduleIsAlive() const {
   return passedChecksum() > 0;
 }
 
@@ -311,13 +307,13 @@ double Gps::getSpeed() {
   return theSpeed;
 }
 
-const String Gps::getHdopAsString() {
+String Gps::getHdopAsString() {
   return String(hdop.hdop(), 2);
 }
 
-const String Gps::getMessages() {
+String Gps::getMessages() const {
   String theGpsMessage = "";
-  for (String msg : mMessages) {
+  for (const String& msg : mMessages) {
     theGpsMessage += "<br/>";
     theGpsMessage += msg;
   }

diff --git a/src/gps.h b/src/gps.h
@@ -30,43 +30,40 @@
 // ??
 class SSD1306DisplayDevice;
 
-
+/*
 namespace GPS {
   const int FIX_NO_WAIT = 0;
   const int FIX_TIME = -1;
   const int FIX_POS = -2;
-}
+} */
 
 class Gps : public TinyGPSPlus {
   public:
-    enum WaitFor {
+    enum class WaitFor {
       FIX_NO_WAIT = 0,
       FIX_TIME = -1,
       FIX_POS = -2,
     };
-    Gps();
     void begin();
     /* read and process data from serial. */
     void handle();
     /* Returns the current time - GPS time if available, system time otherwise. */
     time_t currentTime();
     bool hasState(int state, SSD1306DisplayDevice *display);
     /* Returns true if valid communication with the gps module was possible. */
-    bool moduleIsAlive();
+    bool moduleIsAlive() const;
     bool isInsidePrivacyArea();
     uint8_t getValidSatellites();
     void showWaitStatus(SSD1306DisplayDevice *display);
     /* Returns current speed, negative value means unknown speed. */
     double getSpeed();
-    const String getHdopAsString();
-    const String getMessages();
+    String getHdopAsString();
+    String getMessages() const;
     static PrivacyArea newPrivacyArea(double latitude, double longitude, int radius);
 
 
   private:
     HardwareSerial mSerial = HardwareSerial(1); // but uses uart 2 ports
-
-    // FIXME: HELP! how to do this?
     TinyGPSCustom mTxtCount = TinyGPSCustom(*this, "GPTXT", 1);
     TinyGPSCustom mTxtSeq = TinyGPSCustom(*this, "GPTXT", 2);
     TinyGPSCustom mTxtSeverity = TinyGPSCustom(*this, "GPTXT", 3);