More variant.h cleanup. LED_NOTIFICATION, remove dead code, etc #9477
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the variant.h cleanup effort by standardizing LED naming conventions and removing dead code across multiple hardware variants. The main focus is renaming PIN_LED2 to LED_NOTIFICATION for devices using it for notification purposes, and simplifying conditional compilation in core files.
Changes:
- Renamed
PIN_LED2toLED_NOTIFICATIONacross 20+ nRF52840 and ESP32S3 variants - Removed unused LED definitions and initialization code from variant files
- Simplified external notification configuration in NodeDB.cpp to use
LED_NOTIFICATIONmacro instead of device-specific conditionals - Updated main.cpp to use
LED_NOTIFICATIONinstead ofUSER_LED - Improved
LED_STATE_OFFdefinition in configuration.h to automatically derive fromLED_STATE_ON - Added variant.cpp files for two variants with early LED initialization
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| variants/stm32/milesight_gs301/variant.cpp | Added early init for USER_LED (should use LED_NOTIFICATION) |
| variants/stm32/milesight_gs301/platformio.ini | Added build_src_filter to include variant.cpp |
| variants/rp2040/rak11310/pins_arduino.h | Renamed PIN_LED2 to LED_NOTIFICATION |
| variants/nrf52840/*/variant.h (20+ files) | Renamed PIN_LED2 to LED_BLUE, added LED_NOTIFICATION defines, removed dead LED defines |
| variants/nrf52840/*/variant.cpp (15 files) | Removed PIN_LED2 initialization code |
| variants/esp32s3/*/variant.h (3 files) | Renamed PIN_LED2 to LED_NOTIFICATION |
| variants/esp32/diy/9m2ibr_aprs_lora_tracker/variant.cpp | Added early init for USER_LED (should use LED_NOTIFICATION) |
| variants/esp32/diy/9m2ibr_aprs_lora_tracker/platformio.ini | Added build_src_filter to include variant.cpp |
| variants/nrf52840/nrf52.ini | Updated Adafruit nRF52 framework commit hash |
| src/mesh/NodeDB.cpp | Simplified external notification defaults using LED_NOTIFICATION macro |
| src/main.cpp | Changed USER_LED to LED_NOTIFICATION |
| src/configuration.h | Made LED_STATE_OFF automatically derive from LED_STATE_ON |
| src/Power.cpp | Added LED_NOTIFICATION to shutdown sequence |
| pinMode(USER_LED, OUTPUT); | ||
| digitalWrite(USER_LED, HIGH ^ LED_STATE_ON); |
There was a problem hiding this comment.
The variant.cpp file references USER_LED, but according to the PR changes in main.cpp (line 357-359), USER_LED has been renamed to LED_NOTIFICATION. This variant.cpp should use LED_NOTIFICATION instead for consistency with the rest of the codebase.
There was a problem hiding this comment.
These defines were not being used for notifications. I don't know these devices at all, so don't know if they should be or not. May want to keep the name or pick another one. Open to suggestions.
| pinMode(USER_LED, OUTPUT); | ||
| digitalWrite(USER_LED, HIGH ^ LED_STATE_ON); |
There was a problem hiding this comment.
The variant.cpp file references USER_LED, but according to the PR changes in main.cpp (line 357-359), USER_LED has been renamed to LED_NOTIFICATION. This variant.cpp should use LED_NOTIFICATION instead for consistency with the rest of the codebase.
| pinMode(USER_LED, OUTPUT); | |
| digitalWrite(USER_LED, HIGH ^ LED_STATE_ON); | |
| pinMode(LED_NOTIFICATION, OUTPUT); | |
| digitalWrite(LED_NOTIFICATION, HIGH ^ LED_STATE_ON); |
Rename PIN_LED2 to LED_NOTIFICATION for the devices using it for that purpose.
Drop dead and unused defines
General cleanup