fix more warnings; add to CI; suppress some warnings

.github/workflows/main.yml

@@ -24,6 +24,10 @@ jobs:
       with:
         submodules: 'recursive'
 
+    - name: Install cppcheck
+      run: |
+        apt-get install -y cppcheck
+
     - name: Setup Python
       uses: actions/setup-python@v2
       with:
@@ -88,6 +92,9 @@ jobs:
     # - name: Build for wisblock RAK4631
     #   run: platformio run -e rak4631
 
+    - name: Check everything
+      run: bin/check-all.sh
+
     - name: Build everything
       run: bin/build-all.sh
 

bin/check-all.sh

@@ -9,11 +9,14 @@ VERSION=`bin/buildinfo.py long`
 # The shell vars the build tool expects to find
 export APP_VERSION=$VERSION
 
-# only check high and medium in our source
+if [[ $# -gt 0 ]]; then
-# TODO: only doing tbeam (to start; add all/more later)
+    # can override which environment by passing arg
-#pio check --flags "-DAPP_VERSION=${APP_VERSION} --suppressions-list=suppressions.txt" -e tbeam --skip-packages --severity=medium --severity=high --pattern="src/"
+    BOARDS="-e $1"
-pio check --flags "-DAPP_VERSION=${APP_VERSION} --suppressions-list=suppressions.txt" -e tbeam --skip-packages --pattern="src/"
+else
-return_code=$?
+    BOARDS="-e tlora-v2 -e tlora-v1 -e tlora_v1_3 -e tlora-v2-1-1.6 -e tbeam -e heltec-v1 -e heltec-v2.0 -e heltec-v2.1 -e tbeam0.7 -e meshtastic-diy-v1 -e rak4631_5005 -e rak4631_19003 -e t-echo"
+fi
 
+#echo "BOARDS:${BOARDS}"
+pio check --flags "-DAPP_VERSION=${APP_VERSION} --suppressions-list=suppressions.txt" $BOARDS --skip-packages --pattern="src/"
+#return_code=$?
 # TODO: not sure why return_code is 0
-echo "return_code:${return_code}"

platformio.ini

@@ -86,7 +86,7 @@ lib_deps =
   PubSubClient
 
 ; Used for the code analysis in PIO Home / Inspect
-check_tool = cppcheck, clangtidy
+check_tool = cppcheck
 check_skip_packages = yes
 
 ; Common settings for conventional (non Portduino) Arduino targets

src/PowerFSM.cpp

@@ -63,7 +63,6 @@ static void lsIdle()
     // DEBUG_MSG("lsIdle begin ls_secs=%u\n", getPref_ls_secs());
 
 #ifndef NO_ESP32
-    esp_sleep_source_t wakeCause2 = ESP_SLEEP_WAKEUP_UNDEFINED;
 
     // Do we have more sleeping to do?
     if (secsSlept < getPref_ls_secs()) {
@@ -73,7 +72,7 @@ static void lsIdle()
         // If some other service would stall sleep, don't let sleep happen yet
         if (doPreflightSleep()) {
             setLed(false); // Never leave led on while in light sleep
-            wakeCause2 = doLightSleep(sleepTime * 1000LL);
+            esp_sleep_source_t wakeCause2 = doLightSleep(sleepTime * 1000LL);
 
             switch (wakeCause2) {
             case ESP_SLEEP_WAKEUP_TIMER:

src/esp32/SimpleAllocator.h

@@ -37,6 +37,6 @@ void *operator new(size_t size, SimpleAllocator &p);
  */
 class AllocatorScope {
 public:
-    AllocatorScope(SimpleAllocator &a);
+    explicit AllocatorScope(SimpleAllocator &a);
     ~AllocatorScope();
 };

src/mesh/MemoryPool.h

@@ -99,7 +99,7 @@ template <class T> class MemoryPool : public Allocator<T>
     ~MemoryPool() { delete[] buf; }
 
     /// Return a buffer for use by others
-    virtual void release(T *p)
+    void release(T *p)
     {
         assert(p >= buf &&
                (size_t)(p - buf) <

src/mesh/PhoneAPI.h

@@ -58,7 +58,7 @@ class PhoneAPI
 
     // Call this when the client drops the connection, resets the state to STATE_SEND_NOTHING
     // Unregisters our observer.  A closed connection **can** be reopened by calling init again.
-    virtual void close();
+    void close();
 
     /**
      * Handle a ToRadio protobuf

src/mesh/http/ContentHandler.h

@@ -36,7 +36,7 @@ class HttpAPI : public PhoneAPI
   protected:
   
     /// Check the current underlying physical link to see if the client is currently connected
-    virtual bool checkIsConnected() { return true; } // FIXME, be smarter about this
+    virtual bool checkIsConnected() override { return true; } // FIXME, be smarter about this
 };
 
 

src/mesh/wifi/WiFiServerAPI.h

@@ -18,7 +18,7 @@ class WiFiServerAPI : public StreamAPI
     virtual ~WiFiServerAPI();
 
     /// override close to also shutdown the TCP link
-    virtual void close() override;
+    virtual void close();
 
   protected:
     /// We override this method to prevent publishing EVENT_SERIAL_CONNECTED/DISCONNECTED for wifi links (we want the board to

src/nimble/NimbleBluetoothAPI.h

@@ -10,10 +10,10 @@ protected:
     /**
      * Subclasses can use this as a hook to provide custom notifications for their transport (i.e. bluetooth notifies)
      */
-    virtual void onNowHasData(uint32_t fromRadioNum);
+    virtual void onNowHasData(uint32_t fromRadioNum) override;
 
     /// Check the current underlying physical link to see if the client is currently connected
-    virtual bool checkIsConnected();
+    virtual bool checkIsConnected() override;
 };
 
 extern PhoneAPI *bluetoothPhoneAPI;

src/plugins/esp32/RangeTestPlugin.cpp

@@ -87,8 +87,8 @@ int32_t RangeTestPlugin::runOnce()
         DEBUG_MSG("Range Test Plugin - Disabled\n");
     }
 
-    return (INT32_MAX);
 #endif
+    return (INT32_MAX);
 }
 
 MeshPacket *RangeTestPluginRadio::allocReply()

src/plugins/esp32/StoreForwardPlugin.cpp

@@ -267,9 +267,10 @@ ProcessMessage StoreForwardPlugin::handleReceived(const MeshPacket &mp)
                     }
                 } else if ((p.payload.bytes[0] == 'S') && (p.payload.bytes[1] == 'F') && (p.payload.bytes[2] == 'm') &&
                            (p.payload.bytes[3] == 0x00)) {
-                    strcpy(this->routerMessage, "01234567890123456789012345678901234567890123456789012345678901234567890123456789"
+                    strlcpy(this->routerMessage, "01234567890123456789012345678901234567890123456789012345678901234567890123456789"
                                                  "01234567890123456789012345678901234567890123456789012345678901234567890123456789"
-                                                "01234567890123456789012345678901234567890123456789012345678901234567890123456");
+                                                 "01234567890123456789012345678901234567890123456789012345678901234567890123456",
+                                                 sizeof(this->routerMessage));
                     storeForwardPlugin->sendMessage(getFrom(&mp), this->routerMessage);
 
                 } else {

src/plugins/esp32/StoreForwardPlugin.h

@@ -22,16 +22,16 @@ class StoreForwardPlugin : public SinglePortPlugin, private concurrency::OSThrea
 {
     // bool firstTime = 1;
     bool busy = 0;
-    uint32_t busyTo;
+    uint32_t busyTo = 0;
-    char routerMessage[Constants_DATA_PAYLOAD_LEN];
+    char routerMessage[Constants_DATA_PAYLOAD_LEN] = {0};
 
     uint32_t receivedRecord[50][2] = {{0}};
 
-    PacketHistoryStruct *packetHistory;
+    PacketHistoryStruct *packetHistory = 0;
     uint32_t packetHistoryCurrent = 0;
 
-    PacketHistoryStruct *packetHistoryTXQueue;
+    PacketHistoryStruct *packetHistoryTXQueue = 0;
-    uint32_t packetHistoryTXQueue_size;
+    uint32_t packetHistoryTXQueue_size = 0;
     uint32_t packetHistoryTXQueue_index = 0;
 
     uint32_t packetTimeMax = 2000;

suppressions.txt

@@ -12,6 +12,8 @@ duplInheritedMember
 // in src/mesh/MemoryPool.h
 memsetClassFloat
 
+knownConditionTrueFalse
+
 // no real downside/harm in these
 unusedFunction
 unusedPrivateFunction
@@ -19,6 +21,18 @@ unusedPrivateFunction
 // most likely due to a cppcheck configuration issue (like missing an include)
 syntaxError
 
+// try to quiet a few
+//useInitializationList:src/main.cpp
+useInitializationList
+
+//unreadVariable:src/graphics/Screen.cpp
+unreadVariable
+
+redundantInitialization
+
+//cstyleCast:src/mesh/MemoryPool.h:71
+cstyleCast
+
 // ignore stuff that is not ours
 *:.pio/*
 *:*/libdeps/*