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

Build is "noisy" when Arduino Compiler Warnings = All selected. #18

Closed
g0uus opened this issue Oct 15, 2018 · 2 comments
Closed

Build is "noisy" when Arduino Compiler Warnings = All selected. #18

g0uus opened this issue Oct 15, 2018 · 2 comments

Comments

@g0uus
Copy link

g0uus commented Oct 15, 2018

If the Arduino Compiler Warnings preference is set to All, a number of unnecessary warnings are generated.

It is always good practice to build C/C++ programs with the highest compiler warning level enabled (and in fact with with a compiler flag to treat warnings as errors, but we don't easily have the level in the Arduino IDE without editing configuration files.

Unfortunately, several warnings come from the core EEPROM library, however, there are also a few from DMXSerial2 that we could fix.

Firstly the constants

  • SERIAL_8N1
  • SERIAL_8N2
  • SERIAL_8E1
  • SERIAL_8E2
    Are preceded by a comment that they are already defined in HardwareSerial.h.
    However, each one generates a long warning message like

C:\Users\grh\Google Drive\Arduino\libraries\DmxSerial2\src\DMXSerial2.cpp:146:0: warning: "SERIAL_8N1" redefined

#define SERIAL_8N1 ((0<<USBSn) | (0<<UPMn0) | (3<<UCSZn0))

^

In file included from C:\Users\grh\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.21\cores\arduino/Arduino.h:232:0,

            from C:\Users\grh\Google Drive\Arduino\libraries\DmxSerial2\src\DMXSerial2.cpp:18:

C:\Users\grh\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.21\cores\arduino/HardwareSerial.h:71:0: note: this is the location of the previous definition

#define SERIAL_8N1 0x06

^
Further, only two values

  • SERIAL_8E1
  • SERIAL_8N2
    are actually used.
    My suggestion would be to remove (or comment) the unused values SERIAL_8N1 and SERIAL_8E2 immediately removing two warnings.
    For the other two constants we have options -
  • If we can guarantee that the HardwareSerial.h header will always be included, we could simply remove (or comment) the new definitions, alternatively, we can protect them ussing
#ifndef SERIAL_8N2
#   define  SERIAL_8N2  ((1<<USBSn) | (0<<UPMn0) | (3<<UCSZn0))
#endif
#ifndef SERIAL_8E1
#   define  SERIAL_8E1  ((0<<USBSn) | (2<<UPMn0) | (3<<UCSZn0))
#endif

Assuming of course that we trust the environment, if not we would need to #undef the values if already defined before #defineing them.

There are a couple of other warnings - I will continue in another issue!

Graham

@peternewman
Copy link
Contributor

Hi Graham,

The best way to get these in is probably to open some pull requests. You can do test driven development ( https://en.wikipedia.org/wiki/Test-driven_development ) using Travis (enable yours here: https://travis-ci.com/g0uus/DmxSerial2 ). So add the compiler warnings to Travis first, then make the changes to the code so they pass. You'll need to get it to skip the EEPROM ones somehow.

@mathertel
Copy link
Owner

I can confirm and will do some changes. See commit 5fab4db.

Thanks for the suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants