Merge pull request #207 from henrygab/fix_compile_warnings3

Improve build script & fix more warnings

.github/workflows/githubci.yml

@@ -49,11 +49,15 @@ jobs:
         arduino-cli core update-index --additional-urls $BSP_URL        
         arduino-cli core install arduino:samd --additional-urls $BSP_URL
         arduino-cli core install adafruit:samd --additional-urls $BSP_URL
-        # Repalce release BSP with our code
+        # Replace release BSP with our code
         BSP_VERSION=`eval ls $HOME/$BSP_PATH`
         rm -r $HOME/$BSP_PATH/*
         ln -s $GITHUB_WORKSPACE $HOME/$BSP_PATH/$BSP_VERSION
         arduino-cli lib install $LIB_DEPS
 
     - name: Build examples
       run: python3 extras/build_all.py ${{ matrix.arduino-platform }}
+
+    # How to mark this as allowed-to-fail?
+    - name: Build examples (-Wall)
+      run: python3 extras/build_all.py --all_warnings --warnings_do_not_cause_job_failure

cores/arduino/SERCOM.h

@@ -248,7 +248,7 @@ class SERCOM
 		uint32_t getFreqRef(void) { return freqRef; };
 #else
 		// The equivalent SAMD21 dummy functions...
-		void setClockSource(__attribute__((unused)) int8_t idx, __attribute__((unused)) SercomClockSource src, __attribute__((unused)) bool core) { };
+		void setClockSource(int8_t idx, SercomClockSource src, bool core) { (void)idx; (void)src; (void)core; };
 		SercomClockSource getClockSource(void) { return SERCOM_CLOCK_SOURCE_FCPU; };
 		uint32_t getFreqRef(void) { return F_CPU; };
 #endif

cores/arduino/WInterrupts.c

@@ -250,8 +250,9 @@ void detachInterrupt(uint32_t pin)
  * External Interrupt Controller NVIC Interrupt Handler
  */
 #if defined(__SAMD51__)
-void InterruptHandler(uint32_t i)
+void InterruptHandler(uint32_t unused_i)
 {
+  (void)unused_i;
   // Calling the routine directly from -here- takes about 1us
   // Depending on where you are in the list it will take longer
 

cores/arduino/math_helper.c

@@ -165,18 +165,18 @@ uint32_t arm_compare_fixed_q15(q15_t *pIn, q15_t * pOut, uint32_t numSamples)
 {
   uint32_t i; 
   int32_t diff;
-  uint32_t diffCrnt;
+  uint32_t diffCrnt = 0;
   uint32_t maxDiff = 0;
 
   for (i = 0; i < numSamples; i++)
   {
-  	diff = pIn[i] - pOut[i];
-  	diffCrnt = (diff > 0) ? diff : -diff;
+    diff = pIn[i] - pOut[i];
+    diffCrnt = (uint32_t)( (diff > 0) ? diff : -diff );
 
-	if(diffCrnt > maxDiff)
-	{
-		maxDiff = diffCrnt;
-	}	
+    if(diffCrnt > maxDiff)
+    {
+      maxDiff = diffCrnt;
+    }
   }
 
   return(maxDiff);
@@ -194,18 +194,18 @@ uint32_t arm_compare_fixed_q31(q31_t *pIn, q31_t * pOut, uint32_t numSamples)
 {
   uint32_t i; 
   int32_t diff;
-  uint32_t diffCrnt;
+  uint32_t diffCrnt = 0;
   uint32_t maxDiff = 0;
 
   for (i = 0; i < numSamples; i++)
   {
-  	diff = pIn[i] - pOut[i];
-  	diffCrnt = (diff > 0) ? diff : -diff;
+    diff = pIn[i] - pOut[i];
+    diffCrnt = (uint32_t)( (diff > 0) ? diff : -diff );
 
-	if(diffCrnt > maxDiff)
-	{
-		maxDiff = diffCrnt;
-	}
+    if(diffCrnt > maxDiff)
+    {
+      maxDiff = diffCrnt;
+    }
   }
 
   return(maxDiff);

extras/build_all.py

@@ -3,37 +3,51 @@
 import sys
 import subprocess
 import time
+import argparse
+
+FQBN_PREFIX='adafruit:samd:adafruit_'
+
+
+parser = argparse.ArgumentParser(
+    description='python wrapper for adafruit arduino CI workflows',
+    allow_abbrev=False
+    )
+parser.add_argument(
+    '--all_warnings', '--Wall',
+    action='store_true',
+    help='build with all warnings enabled (`--warnings all`)',
+    )
+parser.add_argument(
+    '--warnings_do_not_cause_job_failure',
+    action='store_true',
+    help='failed builds will be listed as failed, but not cause job to exit with an error status',
+    )
+parser.add_argument(
+    'build_boards',
+    metavar='board',
+    nargs='*',
+    help='list of boards to be built -- Note that the fqbn is created by prepending "{}"'.format(FQBN_PREFIX),
+    default= [ 'metro_m0', 'metro_m4', 'circuitplayground_m0' ]
+    )
+args = parser.parse_args()
 
-all_warnings = False
 exit_status = 0
 success_count = 0
 fail_count = 0
 skip_count = 0
-
 build_format = '| {:22} | {:30} | {:9} '
 build_separator = '-' * 80
 
-default_boards = [ 'metro_m0', 'metro_m4', 'circuitplayground_m0']
-
-build_boards = []
-
-# build all variants if input not existed
-if len(sys.argv) > 1:
-    build_boards.append(sys.argv[1])
-else:
-    build_boards = default_boards
-
-def errorOutputFilter(line):
+def errorOutputFilter(line: str):
     if len(line) == 0:
         return False
     if line.isspace(): # Note: empty string does not match here!
         return False
     # TODO: additional items to remove?
     return True
 
-
-def build_examples(variant):
-    global exit_status, success_count, fail_count, skip_count, build_format, build_separator
+def build_examples(variant: str):
+    global args, exit_status, success_count, fail_count, skip_count, build_format, build_separator
 
     print('\n')
     print(build_separator)
@@ -42,7 +56,7 @@ def build_examples(variant):
     print((build_format + '| {:6} |').format('Library', 'Example', 'Result', 'Time'))
     print(build_separator)
 
-    fqbn = "adafruit:samd:adafruit_{}".format(variant)
+    fqbn = "{}{}".format(FQBN_PREFIX, variant)
 
     for sketch in glob.iglob('libraries/**/*.ino', recursive=True):
         start_time = time.monotonic()
@@ -58,14 +72,14 @@ def build_examples(variant):
             # TODO - preferably, would have STDERR show up in **both** STDOUT and STDERR.
             #        preferably, would use Python logging handler to get both distinct outputs and one merged output
             #        for now, split STDERR when building with all warnings enabled, so can detect warning/error output.
-            if all_warnings:
+            if args.all_warnings:
                 build_result = subprocess.run("arduino-cli compile --warnings all --fqbn {} {}".format(fqbn, sketch), shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             else:
                 build_result = subprocess.run("arduino-cli compile --warnings default --fqbn {} {}".format(fqbn, sketch), shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
 
             # get stderr into a form where len(warningLines) indicates a true warning was output to stderr
             warningLines = [];
-            if all_warnings and build_result.stderr:
+            if args.all_warnings and build_result.stderr:
                 tmpWarningLines = build_result.stderr.decode("utf-8").splitlines()
                 warningLines = list(filter(errorOutputFilter, (tmpWarningLines)))
 
@@ -74,7 +88,8 @@ def build_examples(variant):
                 success = "\033[31mfailed\033[0m   "
                 fail_count += 1
             elif len(warningLines) != 0:
-                exit_status = -1
+                if not args.warnings_do_not_cause_job_failure:
+                    exit_status = -1
                 success = "\033[31mwarnings\033[0m "
                 fail_count += 1
             else:
@@ -98,7 +113,7 @@ def build_examples(variant):
 
 build_time = time.monotonic()
 
-for board in build_boards:
+for board in args.build_boards:
     build_examples(board)
 
 print(build_separator)

libraries/Adafruit_ZeroDMA/examples/zerodma_memcpy/zerodma_memcpy.ino

@@ -17,6 +17,7 @@ volatile bool transfer_is_done = false; // Done yet?
 
 // Callback for end-of-DMA-transfer
 void dma_callback(Adafruit_ZeroDMA *dma) {
+  (void)dma; // avoid compiler warning about unused function parameter
   transfer_is_done = true;
 }
 

libraries/Adafruit_ZeroDMA/examples/zerodma_spi1/zerodma_spi1.ino

@@ -19,6 +19,7 @@ volatile bool transfer_is_done = false; // Done yet?
 
 // Callback for end-of-DMA-transfer
 void dma_callback(Adafruit_ZeroDMA *dma) {
+  (void)dma; // avoid compiler warning about unused parameter
   transfer_is_done = true;
 }
 

libraries/Adafruit_ZeroDMA/examples/zerodma_spi2/zerodma_spi2.ino

@@ -33,6 +33,7 @@ volatile bool transfer_is_done = true; // Done yet?
 
 // Callback for end-of-DMA-transfer
 void dma_callback(Adafruit_ZeroDMA *dma) {
+  (void)dma; // avoid compiler warning about unused parameter
   transfer_is_done = true;
 }
 

libraries/SPI/SPI.h

@@ -149,7 +149,7 @@ class SPIClass {
 #else
   // On SAMD21, this compiles to nothing, so user code doesn't need to
   // check and conditionally compile lines for different architectures.
-  void setClockSource(__attribute__((unused)) SercomClockSource clk) { };
+  void setClockSource(SercomClockSource clk) { (void)clk; };
 #endif // end __SAMD51__
 
   private:

libraries/Servo/src/samd/Servo.cpp

@@ -174,6 +174,7 @@ static inline void resetTC (Tc* TCx)
 
 static void _initISR(Tc *tc, uint8_t channel, uint32_t id, IRQn_Type irqn, uint8_t gcmForTimer, uint8_t intEnableBit)
 {
+    (void)id;
     // Select GCLK0 as timer/counter input clock source
 #if defined(__SAMD51__)
     int idx = gcmForTimer;           // see datasheet Table 14-9
@@ -265,6 +266,7 @@ static void initISR(timer16_Sequence_t timer)
 
 static void finISR(timer16_Sequence_t timer)
 {
+  (void)timer;
 #if defined (_useTimer1)
     // Disable the match channel interrupt request
     TC_FOR_TIMER1->COUNT16.INTENCLR.reg = INTENCLR_BIT_FOR_TIMER_1;

libraries/USBHost/examples/ADKTerminalTest/ADKTerminalTest.ino

@@ -37,7 +37,7 @@ void setup(void)
   while (!SERIAL_PORT_MONITOR); // Wait for serial port to connect - used on Leonardo, Teensy and other boards with built-in USB CDC serial connection
   SERIAL_PORT_MONITOR.println("\r\nADK demo start");
 
-  if (usb.Init() == -1)
+  if (usb.Init() == (uint32_t)-1)
 	SERIAL_PORT_MONITOR.println("OSC did not start.");
 
   delay(20);

libraries/USBHost/examples/KeyboardController/KeyboardController.ino

@@ -83,8 +83,7 @@ void setup()
 {
   SerialDebug.begin( 115200 );
   SerialDebug.println("USB Host Keyboard Controller Program started");
-
-  if (usb.Init() == -1)
+  if (usb.Init() == (uint32_t)-1)
 	  SerialDebug.println("USB Host did not start.");
 
   SerialDebug.println("USB Host started");

libraries/USBHost/examples/MouseController/MouseController.ino

@@ -91,7 +91,7 @@ void setup()
   SerialDebug.begin( 115200 );
   SerialDebug.println("USB Host Mouse Controller Program started");
 
-  if (usb.Init() == -1)
+  if (usb.Init() == (uint32_t)-1)
     SerialDebug.println("USB Host did not start.");
 
   SerialDebug.println("USB Host started");

libraries/USBHost/examples/USB_desc/USB_desc.ino

@@ -59,8 +59,8 @@ void setup()
   SerialDebug.println("Starting USB Descriptor test");
 
   SerialDebug.println("Initializing USB");
-  if (usb.Init() == -1)
-      SerialDebug.println("USBhost did not start.");
+  if (usb.Init() == (uint32_t)-1)
+    SerialDebug.println("USBhost did not start.");
 
   delay( 20 );
 
@@ -161,6 +161,7 @@ byte getdevdescr( byte addr, byte &num_conf )
 
 void printhubdescr(uint8_t *descrptr, uint8_t addr)
 {
+    (void)addr;
     HubDescriptor  *pHub = (HubDescriptor*) descrptr;
     uint8_t        len = *((uint8_t*)descrptr);
 
@@ -209,17 +210,19 @@ byte getconfdescr( byte addr, byte conf )
 {
   uint8_t buf[ BUFSIZE ];
   uint8_t* buf_ptr = buf;
-  byte rcode;
+  byte rcode; // FIXME -- code does not actually check return code (no error handling!)
   byte descr_length;
   byte descr_type;
   uint16_t total_length;
+  // FIXME -- no check of return code from usb.getConfDescr()
   rcode = usb.getConfDescr( addr, 0, 4, conf, buf );  //get total length
   LOBYTE( total_length ) = buf[ 2 ];
   HIBYTE( total_length ) = buf[ 3 ];
   if( total_length > sizeof(buf)) {    //check if total length is larger than buffer
     printProgStr(Conf_Trunc_str);
     total_length = sizeof(buf);
   }
+  // FIXME -- no check of return code from usb.getConfDescr()
   rcode = usb.getConfDescr( addr, 0, total_length, conf, buf ); //get the whole descriptor
   while( buf_ptr < buf + total_length ) {  //parsing descriptors
     descr_length = *( buf_ptr );