Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler warnings #64

Closed
thijstriemstra opened this issue Mar 21, 2024 · 18 comments · Fixed by #67
Closed

Compiler warnings #64

thijstriemstra opened this issue Mar 21, 2024 · 18 comments · Fixed by #67
Assignees
Labels
enhancement New feature or request

Comments

@thijstriemstra
Copy link
Contributor

Using platformio:

[env:pico]
platform = raspberrypi
board = pico
framework = arduino

lib_deps =
  robtillaart/I2C_EEPROM@^1.8.2

seeing some warnings while compiling my project:

.pio\libdeps\pico\I2C_EEPROM\I2C_eeprom.cpp: In member function 'bool I2C_eeprom::writeBlockVerify(uint16_t, const uint8_t*, uint16_t)':
.pio\libdeps\pico\I2C_EEPROM\I2C_eeprom.cpp:206:11: warning: variable length array 'data' is used [-Wvla]
  206 |   uint8_t data[length];
      |           ^~~~
.pio\libdeps\pico\I2C_EEPROM\I2C_eeprom.cpp: In member function 'bool I2C_eeprom::setBlockVerify(uint16_t, uint8_t, uint16_t)':
.pio\libdeps\pico\I2C_EEPROM\I2C_eeprom.cpp:216:11: warning: variable length array 'data' is used [-Wvla]
  216 |   uint8_t data[length];
      |           ^~~~
.pio\libdeps\pico\I2C_EEPROM\I2C_eeprom.cpp: In member function 'bool I2C_eeprom::updateBlockVerify(uint16_t, const uint8_t*, uint16_t)':
.pio\libdeps\pico\I2C_EEPROM\I2C_eeprom.cpp:239:11: warning: variable length array 'data' is used [-Wvla]
  239 |   uint8_t data[length];
      |           ^~~~
@RobTillaart RobTillaart self-assigned this Mar 21, 2024
@RobTillaart RobTillaart added the enhancement New feature or request label Mar 21, 2024
@RobTillaart
Copy link
Owner

Analysis

data[length] is used as a buffer to verify / compare.

Possible solution

Maybe a _verifyBlock(addr, data, length) that verifies while reading. Could work per byte, uses less memory and fails faster?

RobTillaart added a commit that referenced this issue Mar 23, 2024
@RobTillaart
Copy link
Owner

@thijstriemstra
Created a develop branch for testing

  • added verifyBlock() for readBlockVerify() and writeBlockVerify
  • that did not fit the setBlockVerify so I solver that with dynamic memory for now.

Can you check if the warnings still exist (or are replaced with others)?

RobTillaart added a commit that referenced this issue Mar 23, 2024
@thijstriemstra
Copy link
Contributor Author

Can you check if the warnings still exist (or are replaced with others)?

I'll give it a try. Might be a good idea to setup a platformio builder and make it fail on warnings like this.

@RobTillaart
Copy link
Owner

That is a good idea,
Do you know if there is a GitHub workflow that I could use?
Then I could roll it out quite easily for all libraries.

@thijstriemstra
Copy link
Contributor Author

See https://docs.platformio.org/en/latest/integration/ci/github-actions.html

so something like:

name: PlatformIO CI

  on: [push]

  jobs:
    build:
      runs-on: ubuntu-latest
      strategy:
        matrix:
          example: [path/to/test/file.c, examples/file.ino, path/to/test/directory]

      steps:
        - uses: actions/checkout@v4
        - uses: actions/cache@v3
          with:
            path: |
              ~/.cache/pip
              ~/.platformio/.cache
            key: ${{ runner.os }}-pio
        - uses: actions/setup-python@v5
          with:
            python-version: '3.11'
        - name: Install PlatformIO Core
          run: pip install --upgrade platformio

        - name: Build PlatformIO examples
          run: pio ci --board=<ID_1> --board=<ID_2> --board=<ID_N>
          env:
            PLATFORMIO_CI_SRC: ${{ matrix.example }}

@thijstriemstra
Copy link
Contributor Author

And to make it fail on warnings add -Werror (see https://docs.platformio.org/en/latest/projectconf/sections/env/options/build/build_flags.html):

Make all warnings into hard errors. With this option, if any source code triggers warnings, the compilation will be aborted.

So:

- name: Run PlatformIO
  run: pio ci path/to/test/file.c --board=<ID_1> --board=<ID_2> --board=<ID_N>
  env:
    PLATFORMIO_BUILD_FLAGS: -D -Werror

(untested)

RobTillaart added a commit that referenced this issue Mar 23, 2024
@RobTillaart
Copy link
Owner

Created an issue for this in a smaller library to investigate and get it to work

@thijstriemstra
Copy link
Contributor Author

thijstriemstra commented Mar 23, 2024

Cool! The only disadvantage i can see is compiler warnings in third party libraries/frameworks, that'll fail the build as well (I think). Narrowing warnings down to lib only code might also be possible but not sure/tested.

@RobTillaart
Copy link
Owner

OK, I will try to find out coming days/weeks (quite busy with other projects) how to get the platformIO build running.

@RobTillaart
Copy link
Owner

Got something building with green results 😎

yaml file is not portable or engineered (yet) but there is a starting point.

@RobTillaart
Copy link
Owner

The -Werror fails upon the Arduino core libs like HWSerial, so that switch will not be used...

Building in release mode
Compiling .pio/build/uno/src/A1301_autoMidPoint.ino.cpp.o
Compiling .pio/build/uno/lib954/A1301/A1301.cpp.o
Archiving .pio/build/uno/libFrameworkArduinoVariant.a
<command-line>:0:1: error: macro names must be identifiers
Compiling .pio/build/uno/FrameworkArduino/CDC.cpp.o
<command-line>:0:1: error: macro names must be identifiers
Indexing .pio/build/uno/libFrameworkArduinoVariant.a
<command-line>:0:1: error: macro names must be identifiers
Compiling .pio/build/uno/FrameworkArduino/HardwareSerial.cpp.o
<command-line>:0:1: error: macro names must be identifiers
*** [.pio/build/uno/src/A1301_autoMidPoint.ino.cpp.o] Error 1
*** [.pio/build/uno/lib954/A1301/A1301.cpp.o] Error 1
*** [.pio/build/uno/FrameworkArduino/CDC.cpp.o] Error 1
*** [.pio/build/uno/FrameworkArduino/HardwareSerial.cpp.o] Error 1
========================== [FAILED] Took 0.[65](https://github.com/RobTillaart/A1301/actions/runs/8403259599/job/23013574092#step:7:66) seconds ==========================

@RobTillaart
Copy link
Owner

compiling all examples by coding a hard list works

to be continued later.

@RobTillaart
Copy link
Owner

Notes to me:
current test code A1301 repo

name: PlatformIO CI

on: [push, pull_request]

jobs:
  build:
    runs-on: ${{ matrix.os }}
    #  added timeout
    timeout-minutes: 20
    strategy:
      matrix:
        os: [ubuntu-latest]
        example: [
                   examples/A1301_autoMidPoint/A1301_autoMidPoint.ino,
                   examples/A1301_demo/A1301_demo.ino,
                   examples/A1301_determineNoise/A1301_determineNoise.ino,
                   examples/A1301_performance/A1301_performance.ino,
                   examples/A1301_plotter/A1301_plotter.ino,
                   examples/A1301_test/A1301_test.ino,
                   examples/A1301_test_saturation/A1301_test_saturation.ino
                  ]
 
    steps:
      - uses: actions/checkout@v4
      - uses: actions/cache@v3
        with:
          path: |
            ~/.cache/pip
            ~/.platformio/.cache
          key: ${{ runner.os }}-pio
      - uses: actions/setup-python@v5
        with:
          python-version: '3.11'
      - name: Install PlatformIO Core
        run: pip install --upgrade platformio
      - name: Download library
        run: |
          wget https://github.com/RobTillaart/A1301/archive/master.zip -O /tmp/master.zip
          unzip /tmp/master.zip -d /tmp
      - name: Build PlatformIO examples
        run: pio ci --lib "." --lib="/tmp/A1301-master" --board=uno
        env:
          PLATFORMIO_CI_SRC: ${{ matrix.example }}
          #  PLATFORMIO_BUILD_FLAGS: -D -Wall  # too much errors in core Arduino.

hardcoded are

  • examples to compile
  • repository to checkout
  • branch to checkout

These should ideally be created / derived from the branch pushed.
Some options to get it done:

  • maintain manually
  • python script generating the yml file
  • python script that runs on VM and do the matrix
  • do magic in yml file

@RobTillaart
Copy link
Owner

RobTillaart commented Mar 24, 2024

After reading

I could fix the repository and correct branch (first it was hardcoded master while it had to be a develop branch).

      - name: Download library
        run: |
          wget https://github.com/${{github.repository}}/archive/${{github.ref_name}}.zip -O /tmp/${{github.ref_name}}.zip
          unzip /tmp/${{github.ref_name}}.zip -d /tmp/${{github.actor}}
      - name: Build PlatformIO examples
        run: pio ci --lib "." --lib="/tmp/${{github.repository}}-${{github.ref_name}}" --board=uno
        env:
          PLATFORMIO_CI_SRC: ${{ matrix.example }}
          #  PLATFORMIO_BUILD_FLAGS: -D -Wall  # too much 

Only the list of examples files need to be generated,.
Maintaining that list manually is doable as the list does not change often.

Note: strategy of matrix job is set up before the download and build starts. (need to read more)

@RobTillaart
Copy link
Owner

Did a test with ESP32DEV instead of UNO, also warnings in the ESP32 specific files.
So I doubt if the Wall option will work

Compiling .pio/build/esp32dev/FrameworkArduino/libb64/cdecode.c.o
/home/runner/.platformio/packages/framework-arduinoespressif32/cores/esp32/esp32-hal-uart.c: In function 'uartSetPins':
/home/runner/.platformio/packages/framework-arduinoespressif32/cores/esp32/esp32-hal-uart.c:153:9: warning: 'return' with no value, in function returning non-void
         return;
         ^~~~~~

This bug is already reported at - espressif/arduino-esp32#8641

@RobTillaart
Copy link
Owner

@thijstriemstra

I'll give it a try. Might be a good idea to setup a platformio builder and make it fail on warnings like this.

Did you have time to try the develop branch?

@thijstriemstra
Copy link
Contributor Author

thijstriemstra commented Mar 27, 2024

After switching to:

[env:pico]
platform = https://github.com/maxgerhardt/platform-raspberrypi.git
board = rpipico
framework = arduino
monitor_speed = 115200
; board can use both Arduino cores -- we select Arduino-Pico here
board_build.core = earlephilhower
; Flash Size: 2MB (Sketch: 1.5MB, FS: 0.5MB)
board_build.filesystem_size = 0.5m

the compile errors seem to be gone, and since I'll be using earlephilhower (since platformio raspberrypi is not supported by platformio labs and basically abandoned) I'll leave it up to you to implement your suggested fix or not.

Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/raspberrypi/rpipico.html
PLATFORM: Raspberry Pi RP2040 (1.11.0+sha.60d6ae8) > Pico
HARDWARE: RP2040 133MHz, 264KB RAM, 2MB Flash
DEBUG: Current (blackmagic) External (blackmagic, cmsis-dap, jlink, pico-debug, picoprobe, raspberrypi-swd)
PACKAGES:
 - framework-arduinopico @ 1.30700.0+sha.88ccf0c
 - tool-rp2040tools @ 1.0.2
 - toolchain-rp2040-earlephilhower @ 5.120300.240125 (12.3.0)
Flash size: 2.00MB
Sketch size: 1.50MB
Filesystem size: 0.50MB
Maximium Sketch size: 1568768 EEPROM start: 0x101ff000 Filesystem start: 0x1017f000 Filesystem end: 0x101ff000       
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 56 compatible libraries
Scanning dependencies...
Dependency Graph
|-- I2C_LCD @ 0.2.1
|-- I2C_EEPROM @ 1.8.2
|-- DHT sensor library @ 1.4.6
|-- Adafruit Unified Sensor @ 1.1.14
|-- SPI @ 1.0
|-- Wire @ 1.0
Building in release mode
Compiling .pio\build\pico\FrameworkArduinoBootloader\boot2_w25q080_2_padded_checksum.S.o
Compiling .pio\build\pico\src\DryBoxDisplay.cpp.o
Compiling .pio\build\pico\src\WRKeyState.cpp.o
Compiling .pio\build\pico\src\main.cpp.o
Generating linkerscript C:\Users\Thijs\projects\drybox\.pio\build\pico/memmap_default.ld
Compiling .pio\build\pico\lib785\Wire\Wire.cpp.o
Compiling .pio\build\pico\libf9e\I2C_LCD\I2C_LCD.cpp.o
Compiling .pio\build\pico\lib002\I2C_EEPROM\I2C_eeprom.cpp.o
Compiling .pio\build\pico\lib0c1\Adafruit Unified Sensor\Adafruit_Sensor.cpp.o
Compiling .pio\build\pico\libcb5\DHT sensor library\DHT.cpp.o
Compiling .pio\build\pico\libcb5\DHT sensor library\DHT_U.cpp.o
Archiving .pio\build\pico\lib785\libWire.a
Archiving .pio\build\pico\lib002\libI2C_EEPROM.a
Compiling .pio\build\pico\lib91d\SPI\SPI.cpp.o
Archiving .pio\build\pico\lib0c1\libAdafruit Unified Sensor.a
Compiling .pio\build\pico\FrameworkArduino\BluetoothDebug.cpp.o
Compiling .pio\build\pico\FrameworkArduino\Bootsel.cpp.o
Compiling .pio\build\pico\FrameworkArduino\CoreMutex.cpp.o
Compiling .pio\build\pico\FrameworkArduino\FS.cpp.o
Archiving .pio\build\pico\libf9e\libI2C_LCD.a
Compiling .pio\build\pico\FrameworkArduino\PIOProgram.cpp.o
Compiling .pio\build\pico\FrameworkArduino\RP2040Support.cpp.o
Compiling .pio\build\pico\FrameworkArduino\RP2040USB.cpp.o
Archiving .pio\build\pico\libcb5\libDHT sensor library.a
Compiling .pio\build\pico\FrameworkArduino\SerialPIO.cpp.o
Compiling .pio\build\pico\FrameworkArduino\SerialUART.cpp.o
Compiling .pio\build\pico\FrameworkArduino\SerialUSB.cpp.o
Compiling .pio\build\pico\FrameworkArduino\StackThunk.cpp.o
Compiling .pio\build\pico\FrameworkArduino\Tone.cpp.o
Compiling .pio\build\pico\FrameworkArduino\WMath.cpp.o
Archiving .pio\build\pico\lib91d\libSPI.a
Compiling .pio\build\pico\FrameworkArduino\_freertos.cpp.o
Compiling .pio\build\pico\FrameworkArduino\api\Common.cpp.o
Compiling .pio\build\pico\FrameworkArduino\api\IPAddress.cpp.o
Compiling .pio\build\pico\FrameworkArduino\api\PluggableUSB.cpp.o
Compiling .pio\build\pico\FrameworkArduino\api\Print.cpp.o
Compiling .pio\build\pico\FrameworkArduino\api\Stream.cpp.o
Compiling .pio\build\pico\FrameworkArduino\api\String.cpp.o
Compiling .pio\build\pico\FrameworkArduino\cyw43_wrappers.cpp.o
Compiling .pio\build\pico\FrameworkArduino\delay.cpp.o
Compiling .pio\build\pico\FrameworkArduino\libb64\cdecode.cpp.o
Compiling .pio\build\pico\FrameworkArduino\libb64\cencode.cpp.o
Compiling .pio\build\pico\FrameworkArduino\lock.cpp.o
Compiling .pio\build\pico\FrameworkArduino\lwip_wrap.cpp.o
Compiling .pio\build\pico\FrameworkArduino\main.cpp.o
Compiling .pio\build\pico\FrameworkArduino\malloc-lock.cpp.o
Compiling .pio\build\pico\FrameworkArduino\posix.cpp.o
Compiling .pio\build\pico\FrameworkArduino\sdkoverride\att_db.c.o
Compiling .pio\build\pico\FrameworkArduino\sdkoverride\btstack_flash_bank.cpp.o
Compiling .pio\build\pico\FrameworkArduino\sdkoverride\hids_device.c.o
Compiling .pio\build\pico\FrameworkArduino\sdkoverride\pico_bootsel_via_double_reset.c.o
Compiling .pio\build\pico\FrameworkArduino\stdlib_noniso.cpp.o
Compiling .pio\build\pico\FrameworkArduino\wiring_analog.cpp.o
Compiling .pio\build\pico\FrameworkArduino\wiring_digital.cpp.o
Compiling .pio\build\pico\FrameworkArduino\wiring_private.cpp.o
Compiling .pio\build\pico\FrameworkArduino\wiring_pulse.cpp.o
Compiling .pio\build\pico\FrameworkArduino\wiring_shift.cpp.o
Archiving .pio\build\pico\libFrameworkArduino.a
Linking .pio\build\pico\firmware.elf
Generating UF2 image
elf2uf2 ".pio\build\pico\firmware.elf" ".pio\build\pico\firmware.uf2"
Retrieving maximum program size .pio\build\pico\firmware.elf
Flash size: 2.00MB
Sketch size: 1.50MB
Filesystem size: 0.50MB
Maximium Sketch size: 1568768 EEPROM start: 0x101ff000 Filesystem start: 0x1017f000 Filesystem end: 0x101ff000       
Checking size .pio\build\pico\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=         ]   5.5% (used 14480 bytes from 262144 bytes)
Flash: [          ]   5.0% (used 78288 bytes from 1568768 bytes)
Building .pio\build\pico\firmware.bin
Building .pio\build\pico\firmware.bin.signed

@RobTillaart
Copy link
Owner

OK, I will merge the develop branch today

  • adds verifyBlock(memoryAddress, buffer, length)
  • updates build-CI scripts

@RobTillaart RobTillaart linked a pull request Mar 28, 2024 that will close this issue
RobTillaart added a commit that referenced this issue Mar 28, 2024
- Fix #64, compiler warning.
- add **verifyBlock(memoryAddress, buffer, length)**
- add example **I2C_eeprom_verifyBlock.ino**
- update GitHub actions
- update keywords.txt
- update examples
- update readme.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants