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

Unused variable warning on ESP8266 #381

Open
nyanpasu64 opened this issue Mar 15, 2023 · 3 comments
Open

Unused variable warning on ESP8266 #381

nyanpasu64 opened this issue Mar 15, 2023 · 3 comments

Comments

@nyanpasu64
Copy link

nyanpasu64 commented Mar 15, 2023

Describe the bug
On platforms other than ESP32, I get an unused variable warning in the SSD1306Wire constructor, since only the ARDUINO_ARCH_ESP32 branch references the _i2cBus parameter.

To Reproduce
Steps to reproduce the behavior:

  1. Enable warnings in Arduino. V1 instructions: Preferences > Compiler warnings > All.
  2. Build an ESP8266 (not ESP32) project including SSD1306Wire.h.

Sample code
Provide a MCVE below.

#include "SSD1306Wire.h"

Expected behavior
No compiler warning. Perhaps mark the parameter [[maybe_unused]] (since C++17), or add a (void)_i2cBus statement, either in the #if !defined(ARDUINO_ARCH_ESP32) branch or globally?

Screenshots
If applicable, add screenshots to help explain your problem.

SSD1306Wire(uint8_t _address, int _sda = -1, int _scl = -1, OLEDDISPLAY_GEOMETRY g = GEOMETRY_128_64, HW_I2C _i2cBus = I2C_ONE, int _frequency = 700000) {
setGeometry(g);
this->_address = _address;
this->_sda = _sda;
this->_scl = _scl;
#if !defined(ARDUINO_ARCH_ESP32)
this->_wire = &Wire;
#else
this->_wire = (_i2cBus==I2C_ONE) ? &Wire : &Wire1;
#endif

In file included from /home/nyanpasu64/Documents (Shared)/gbs-c/gbs-control/gbs-control.ino:22:
/home/nyanpasu64/Arduino/libraries/esp8266-oled-ssd1306-master/src/SSD1306Wire.h: In constructor 'SSD1306Wire::SSD1306Wire(uint8_t, int, int, OLEDDISPLAY_GEOMETRY, HW_I2C, int)':
/home/nyanpasu64/Arduino/libraries/esp8266-oled-ssd1306-master/src/SSD1306Wire.h:70:114: warning: unused parameter '_i2cBus' [-Wunused-parameter]
   70 |     SSD1306Wire(uint8_t _address, int _sda = -1, int _scl = -1, OLEDDISPLAY_GEOMETRY g = GEOMETRY_128_64, HW_I2C _i2cBus = I2C_ONE, int _frequency = 700000) {
      |                                                                                                           ~~~~~~~^~~~~~~~~~~~~~~~~

Versions (please complete the following information):

  • Library: e2e063a (though unchanged in master f96fd6a)
  • Platform: ESP8266 Arduino 3.1.1

Additional context
Add any other context about the problem here.

@marcelstoer
Copy link
Member

@benoitm974 what is your take on this? I ask for your opinion as you contributed the code in question back then.

@nyanpasu64
Copy link
Author

I actually feel the "unused arguments" warning is too noisy and turn it off in my personal projects, but it's difficult to set custom compiler flags on Arduino IDE v1, and reducing the warning level turns off other useful warnings as well. In this case I think (void)_i2cBus isn't too ugly of a change to make.

In any case, should the _i2cBus argument even exist outside of ESP32? Though removing the argument on some platforms can break code on a platform-dependent basis, which is even worse than leaving it in.

Should the HW_I2C type only define I2C_TWO on ESP32, and otherwise be a one-value enum to prevent users from passing in I2C_TWO on ESP8266 which is silently ignored? Or is it better to leave I2C_TWO in?

@benoitm974
Copy link
Contributor

benoitm974 commented Mar 26, 2023

@marcelstoer Thanks for the head up and kind attention on this contribution.

Since this code is executed at init only why don't we secure it by adding a test/assert in case someone is trying to use a second wire on another ESP32 platform with no 2nd Wire?

#if !defined(ARDUINO_ARCH_ESP32) //assert on trying to use a second wire on non-ESP32 platform. assert(_i2cBus!=I2C_ONE) this->_wire = &Wire; #else this->_wire = (_i2cBus==I2C_ONE) ? &Wire : &Wire1; #endif

Benoit

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