arduino-libraries / MIDIUSB

A MIDI library over USB, based on PluggableUSB
GNU Lesser General Public License v2.1
480 stars 89 forks source link

When using PlatformIO I get warning about -Wnarrowing #91

Open EdgarBarranco opened 1 year ago

EdgarBarranco commented 1 year ago

I am using Visual Studio Code 1.72.2 and PlatformIO 6.1.4. The board I am using is the Sparkfun ProMicro16 based on the 32u4. When compiling the example MIDIUSB_write.ino, I get the following warnings:

Processing micro (platform: atmelavr; board: sparkfun_promicro16; framework: arduino)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/atmelavr/sparkfun_promicro16.html
PLATFORM: Atmel AVR (4.0.0) > SparkFun Pro Micro 5V/16MHz
HARDWARE: ATMEGA32U4 16MHz, 2.50KB RAM, 28KB Flash
DEBUG: Current (simavr) On-board (simavr)
PACKAGES:
 - framework-arduino-avr @ 5.1.0
 - tool-avrdude @ 1.60300.200527 (6.3.0)
 - toolchain-atmelavr @ 1.70300.191015 (7.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 7 compatible libraries
Scanning dependencies...
Dependency Graph
|-- MIDIUSB @ 1.0.5
Building in release mode
Compiling .pio\build\micro\liba61\MIDIUSB\MIDIUSB.cpp.o
Compiling .pio\build\micro\src\main.cpp.o
src\main.cpp: In function 'void noteOn(byte, byte, byte)':
src\main.cpp:11:42: warning: narrowing conversion of '(int)(144 | ((unsigned char)((int)channel)))' from 'int' to 'uint8_t {aka unsigned char}' inside { } [-Wnarrowing]
   midiEventPacket_t noteOn = {0x09, 0x90 | channel, pitch, velocity};
                                     ~~~~~^~~~~~~~~
src\main.cpp: In function 'void noteOff(byte, byte, byte)':
src\main.cpp:16:43: warning: narrowing conversion of '(int)(128 | ((unsigned char)((int)channel)))' from 'int' to 'uint8_t {aka unsigned char}' inside { } [-Wnarrowing]
   midiEventPacket_t noteOff = {0x08, 0x80 | channel, pitch, velocity};
                                      ~~~~~^~~~~~~~~
src\main.cpp: In function 'void controlChange(byte, byte, byte)':
src\main.cpp:30:41: warning: narrowing conversion of '(int)(176 | ((unsigned char)((int)channel)))' from 'int' to 'uint8_t {aka unsigned char}' inside { } [-Wnarrowing]
   midiEventPacket_t event = {0x0B, 0xB0 | channel, control, value};
                                    ~~~~~^~~~~~~~~
Linking .pio\build\micro\firmware.elf
Checking size .pio\build\micro\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=         ]   8.3% (used 213 bytes from 2560 bytes)
Flash: [==        ]  15.9% (used 4562 bytes from 28672 bytes)
Building .pio\build\micro\firmware.hex

Doing a little reading about these warnings I see that in the c99 standards ISO/IEC 9899:1999 section "6.5.11 Bitwise exclusive OR operator" under constrains says "Each of the operands shall have integer type."

Digging through the code, the file MIDIUSB_Defs.h on lines 6 through 9 declares each element of the struct midiEventPacket_t as "uint8_t" which is fine in this case because of the range it should have, but the type should be uint16_t to comply with the standard. Making the following changes to this file clear the warnings:

#pragma once
#include <stdint.h>

typedef struct
{
    uint16_t header;
    uint16_t byte1;
    uint16_t byte2;
    uint16_t byte3;
} midiEventPacket_t;

In my case, these changes do not affect the functionality of the code. I only have this board with the 32u4 to test, so I don't know if it works on other boards, but I don't see why not.

I don't see these warnings using the Arduino IDE 1.8.19 but this is because they added "-Wno-error=narrowing" to the compilation flags which hides the warning issues, but does not solve the problem.

fab672000 commented 1 year ago

No,in short: the proper way to do it is to do the the contrary , and only use a cast in this function of the int value to an uint8 value is needed. The midi event packet maps to a USB 32-bit dword (4x8bits) packet and can't change because it is part of the USB specification. this warning can easily disappear by casting the int to an uint8_t internally to the USB midi packing data code (we don't need or want to expose USB protocol specifics at the user level.

On Sun, Oct 23, 2022 at 12:20 AM Edgar Barranco - K2UN < @.***> wrote:

I am using Visual Studio Code 1.72.2 and PlatformIO 6.1.4. The board I am using is the Sparkfun ProMicro16 based on the 32u4. When compiling the example MIDIUSB_write.ino, I get the following warnings:

Processing micro (platform: atmelavr; board: sparkfun_promicro16; framework: arduino)-------------------------------------------------------------------------------------------------------------------------------------------------------------------------Verbose mode can be enabled via -v, --verbose optionCONFIGURATION: https://docs.platformio.org/page/boards/atmelavr/sparkfun_promicro16.htmlPLATFORM: Atmel AVR (4.0.0) > SparkFun Pro Micro 5V/16MHzHARDWARE: ATMEGA32U4 16MHz, 2.50KB RAM, 28KB FlashDEBUG: Current (simavr) On-board (simavr)PACKAGES:

  • framework-arduino-avr @ 5.1.0
  • tool-avrdude @ 1.60300.200527 (6.3.0)
  • toolchain-atmelavr @ 1.70300.191015 (7.3.0)LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldfLDF Modes: Finder ~ chain, Compatibility ~ softFound 7 compatible librariesScanning dependencies...Dependency Graph|-- MIDIUSB @ 1.0.5Building in release modeCompiling .pio\build\micro\liba61\MIDIUSB\MIDIUSB.cpp.oCompiling .pio\build\micro\src\main.cpp.osrc\main.cpp: In function 'void noteOn(byte, byte, byte)':src\main.cpp:11:42: warning: narrowing conversion of '(int)(144 | ((unsigned char)((int)channel)))' from 'int' to 'uint8_t {aka unsigned char}' inside { } [-Wnarrowing] midiEventPacket_t noteOn = {0x09, 0x90 | channel, pitch, velocity};
    
    midiEventPacket_t noteOff = {0x08, 0x80 | channel, pitch, velocity};
                                      ~~~~~^~~~~~~~~src\main.cpp: In function 'void controlChange(byte, byte, byte)':src\main.cpp:30:41: warning: narrowing conversion of '(int)(176 | ((unsigned char)((int)channel)))' from 'int' to 'uint8_t {aka unsigned char}' inside { } [-Wnarrowing]
    midiEventPacket_t event = {0x0B, 0xB0 | channel, control, value};
                                    ~~~~~^~~~~~~~~Linking .pio\build\micro\firmware.elfChecking size .pio\build\micro\firmware.elfAdvanced Memory Usage is available via "PlatformIO Home > Project Inspect"RAM:   [=         ]   8.3% (used 213 bytes from 2560 bytes)Flash: [==        ]  15.9% (used 4562 bytes from 28672 bytes)Building .pio\build\micro\firmware.hex

Doing a little reading about these warnings I see that in the c99 standards ISO/IEC 9899:1999 section "6.5.11 Bitwise exclusive OR operator" under constrains says "Each of the operands shall have integer type."

Digging through the code, the file MIDIUSB_Defs.h on lines 6 through 9 declares each element of the struct midiEventPacket_t as "uint8_t" which is fine in this case because of the range it should have, but the type should be uint16_t to comply with the standard. Making the following changes to this file clear the warnings:

pragma once

include

typedef struct { uint16_t header; uint16_t byte1; uint16_t byte2; uint16_t byte3; } midiEventPacket_t;

In my case, these changes do not affect the functionality of the code. I only have this board with the 32u4 to test, so I don't know if it works on other boards, but I don't see why not.

I don't see these warnings using the Arduino IDE 1.8.19 but this is because they added "-Wno-error=narrowing" to the compilation flags which hides the warning issues, but does not solve the problem.

— Reply to this email directly, view it on GitHub https://github.com/arduino-libraries/MIDIUSB/issues/91, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOXKZNZBR77XVM552R64DDWETDKJANCNFSM6AAAAAARMEKPLE . You are receiving this because you are subscribed to this thread.Message ID: @.***>