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

Redefined define with lolin_s3_mini #10822

Closed
1 task done
BCsabaEngine opened this issue Jan 7, 2025 · 16 comments · Fixed by #10841
Closed
1 task done

Redefined define with lolin_s3_mini #10822

BCsabaEngine opened this issue Jan 7, 2025 · 16 comments · Fixed by #10841
Assignees
Labels
IDE: PlaformIO Issue relates to PlatformIO IDE Type: Bug 🐛 All bugs Type: 3rd party Boards

Comments

@BCsabaEngine
Copy link

Board

ESp32 Lolin S3 mini

Device Description

default

Hardware Configuration

default

Version

v3.1.0

IDE Name

VScode, PlatformIO

Operating System

macOS

Flash frequency

80Mhz

PSRAM enabled

yes

Upload speed

115200

Description

I got a warning (and can build without -Werror, but I like this option) when use a board that contains RGB led:

.platformio/packages/framework-arduinoespressif32/variants/lolin_s3_mini/pins_arduino.h:17: error: "RGB_BUILTIN_LED_COLOR_ORDER" redefined

.platformio/packages/framework-arduinoespressif32/cores/esp32/esp32-hal-rgb-led.h:15

The cause of the error is pins_arduino.h for the lolin_s3_mini variant.

How can I include these files in proper order to make the #ifndef works?

Sketch

Any sketch

Debug Message

No debug, compile warning

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@BCsabaEngine BCsabaEngine added the Status: Awaiting triage Issue is waiting for triage label Jan 7, 2025
@SuGlider SuGlider added IDE: PlaformIO Issue relates to PlatformIO IDE Type: 3rd party Boards Type: Question Only question and removed Status: Awaiting triage Issue is waiting for triage labels Jan 8, 2025
@SuGlider
Copy link
Collaborator

SuGlider commented Jan 8, 2025

I think that @Jason2866 may help with it.

@Jason2866
Copy link
Collaborator

@BCsabaEngine Please post your Platformio.ini

@Jason2866
Copy link
Collaborator

@SuGlider I think the reason for is here

#define RGB_BUILTIN_LED_COLOR_ORDER LED_COLOR_ORDER_RGB

@BCsabaEngine
Copy link
Author

Here is my platformio:

[env:default]
platform = https://github.com/pioarduino/platform-espressif32/releases/download/stable/platform-espressif32.zip
board = lolin_s3_mini
framework = arduino
board_build.partitions = min_spiffs.csv
upload_speed = 921600
monitor_speed = 115200
custom_master_version = 1.5.0
build_flags = 
	-DCORE_DEBUG_LEVEL=0
	; -DEMULATE_ALLPANELS
	-DMASTER_VERSION=${this.custom_master_version}
	-DARDUINO_USB_MODE=1
	-DARDUINO_USB_CDC_ON_BOOT=1
	-DARDUINO_USB_MSC_ON_BOOT=0
	-DARDUINO_USB_DFU_ON_BOOT=0
	-DNDEBUG
	-Wall
	-Werror
	-Wextra
	-Wno-error=deprecated-enum-enum-conversion
	-Wlogical-op
	-Wnonnull
	-Wunused-parameter
	-Wuninitialized
	-Wno-free-nonheap-object
extra_scripts = pre:pre_build.py
lib_deps = 
	SPI
	Wire
	adafruit/Adafruit BusIO
	adafruit/Adafruit GFX Library
	adafruit/Adafruit SSD1306
	adafruit/Adafruit INA219
	bblanchon/ArduinoJson
	https://github.com/BCsabaEngine/TerminalWindow
	https://github.com/me-no-dev/ESPAsyncWebServer
check_tool = clangtidy
check_skip_packages = yes
check_flags =
  clangtidy: --checks=*,-llvmlibc-*,-modernize-macro-to-enum,-*braces*,-*casting*

@BCsabaEngine
Copy link
Author

@SuGlider I think the reason for is here

#define RGB_BUILTIN_LED_COLOR_ORDER LED_COLOR_ORDER_RGB

Yes I think an #ifndef missing here

@SuGlider
Copy link
Collaborator

SuGlider commented Jan 10, 2025

@BCsabaEngine - Where is it redefined?
Arduino Core doesn't define it. It is defined only within lolin_s3_mini board pins_arduino.h file.

Is PlatformIO redefining it in the configuration?

@SuGlider
Copy link
Collaborator

It already has the #ifndef guard in the code:

#ifndef RGB_BUILTIN_LED_COLOR_ORDER
#define RGB_BUILTIN_LED_COLOR_ORDER LED_COLOR_ORDER_GRB // default WS2812B color order
#endif

@SuGlider
Copy link
Collaborator

It has been added in #10128

@Jason2866
Copy link
Collaborator

I guess that the core file is first compiled and after that the variant header file. So the redefine happens

@SuGlider
Copy link
Collaborator

I guess that the core file is first compiled and after that the variant header file. So the redefine happens

But *.h is never compiled. It must be included by some C/CPP file.

@SuGlider
Copy link
Collaborator

When there is a -DRGB_BUILTIN_LED_COLOR_ORDER=XXX in the gcc command line, it may cause such error.

@SuGlider
Copy link
Collaborator

SuGlider commented Jan 10, 2025

I've searched Arduino code. There are just 3 places where RGB_BUILTIN_LED_COLOR_ORDER appears:

in variants/lolin_s3_min/pins_arduino.h as a direct #define
in cores/esp32/esp32-hal-rgb-led.h as a guarded #define - therefore, it should not cause the issue
in cores/esp32/esp32-hal-rgb-led.c in the code by rgbLedWrite()

@SuGlider
Copy link
Collaborator

SuGlider commented Jan 10, 2025

I guess that the core file is first compiled and after that the variant header file. So the redefine happens

The only way is when cores/esp32/esp32-hal-rgb-led.h is included before variants/lolin_s3_min/pins_arduino.h

Let me check it...

@SuGlider
Copy link
Collaborator

Ok... found. Arduino.h includes esp32-hal.h that includes esp32-hal-rgb-led.h and latter it includes pins_arduino.h.
This sequence generates the issue.

The order is wrong. It must be in the reverse order.

@SuGlider SuGlider added Type: Bug 🐛 All bugs and removed Type: Question Only question labels Jan 10, 2025
@SuGlider SuGlider self-assigned this Jan 10, 2025
@SuGlider
Copy link
Collaborator

@BCsabaEngine @Jason2866 - Please test PR #10841 and let me know. Thanks.

@BCsabaEngine
Copy link
Author

BCsabaEngine commented Jan 11, 2025

Thank you for detect problem properly. I have tested the PR, I adopted to my codebase (not a oneline example app), and I confirm, this solves the situation

Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDE: PlaformIO Issue relates to PlatformIO IDE Type: Bug 🐛 All bugs Type: 3rd party Boards
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants