feat(P070): Enhance NeoPixel Clock functionality by Capitanguex · Pull Request #5516 · letscontrolit/ESPEasy

Capitanguex · 2026-03-19T10:32:07Z

Improved NeoPixel Clock plugin with new features:
- Added predefined color selection for hands/marks and hourly lightshow.
- Implemented automatic brightness control via BH1750 lux sensor.
- Introduced selectable LED ring mode: 60-LED (standard) and 16/12/24-LED.
Refactored P070_data_struct to manage new configurations and features:
- Added support for RGBW LEDs and various lightshow modes.
- Implemented auto brightness adjustment based on lux readings.
- Enhanced clock update logic to include lightshow functionality.
Updated web interface for configuration options related to LED modes, brightness, colors, and lightshow settings.

- Updated platformio.ini to change default environment for custom ESP8266. - Improved NeoPixel Clock plugin with new features: - Added predefined color selection for hands/marks and hourly lightshow. - Implemented automatic brightness control via BH1750 lux sensor. - Introduced selectable LED ring mode: 60-LED (standard) or 24-LED. - Refactored P070_data_struct to manage new configurations and features: - Added support for RGBW LEDs and various lightshow modes. - Implemented auto brightness adjustment based on lux readings. - Enhanced clock update logic to include lightshow functionality. - Updated web interface for configuration options related to LED modes, brightness, colors, and lightshow settings. - Improved memory management for NeoPixelBus_wrapper instances.

TD-er · 2026-03-19T10:41:48Z

I notice quite a lot of seemingly unrelated docs changes.
Is that a merge gone wrong as I think that's related to the changes Ton made just a few days ago?

TD-er · 2026-03-19T10:46:11Z

src/src/PluginStructs/P070_data_struct.cpp

+  auto_brightness       = PCONFIG(6);
+  lightshow_mode        = PCONFIG(7);
+  lightshow_duration    = PCONFIG(8);
+  led_type              = PCONFIG(9);


PCONFIG can only go upto 7.
With these you're accessing unrelated values (likely in the next task)

TD-er · 2026-03-19T10:50:06Z

src/src/PluginStructs/P070_data_struct.cpp

+  if (led_type == P070_LED_TYPE_RGBW) {
+    // If all channels equal → pure white via W channel
+    if (r == g && g == b) {
+      Plugin_070_pixels->setPixelColor(pos, Plugin_070_pixels->Color(0, 0, 0, r));


There is no check for the pointer to be valid.
If the function can be called from anywhere without checking this pointer, then please add a check for it in this function.

TD-er · 2026-03-19T10:54:17Z

src/src/PluginStructs/P070_data_struct.cpp

+  for (taskIndex_t i = 0; i < TASKS_MAX; i++) {
+    if (Settings.TaskDeviceEnabled[i]) {
+      LoadTaskSettings(i);
+      if (String(ExtraTaskSettings.TaskDeviceName).equalsIgnoreCase(F(P070_LUX_TASK_NAME))) {


This is a really strange way of checking for a task.
You could check for pluginID, which doesn't even require to load Task Settings. (which is an expensive operation)
Also we do have task names cached so actually nowhere else the LoadTaskSettings has to be called anymore (I think...)

Anyway, the taskname is not guaranteed to be the same as the default once set. Better check for the PluginID as that remains constant and is 70 for this plugin.

TD-er · 2026-03-19T10:55:39Z

src/src/PluginStructs/P070_data_struct.cpp

+  const uint8_t bright = brightness > 0 ? brightness : 128;
+
+  // -----------------------------------------------------------------------
+  // Mode 1: Colour-chase (Lauflicht)


Please use English also in the comments.

TD-er · 2026-03-19T10:58:13Z

src/src/PluginStructs/P070_data_struct.cpp

+    const uint8_t trail = 3;
+    for (uint8_t t = 0; t <= trail; t++) {
+      int pos = (int)lightshow_step - (int)t;
+      if (pos < 0) { pos += number_leds; }


This still will fail when number_leds < trial.
Simple fix is to change the if by a while

TD-er · 2026-03-19T11:00:50Z

src/src/PluginStructs/P070_data_struct.cpp

+  // Each "flash cycle" = 2 seconds: sec 0 flash-on, sec 1 flash-off+hands
+  // -----------------------------------------------------------------------
+  if (lightshow_mode == P070_LIGHTSHOW_FLASH) {
+    const uint8_t cycle = sec % 2; // 0 = flash all, 1 = show hands briefly


Not sure how 'expensive' the modulo operator is, but since you're essentially checking for 0 or 1, you can also do:

const uint8_t cycle = sec & 1;

TD-er · 2026-03-19T11:07:42Z

src/_P070_NeoPixel_Clock.ino

+      Device[deviceCount].SendDataOption     = false;
+      Device[deviceCount].TimerOption        = false;
+      Device[deviceCount].GlobalSyncOption   = false;
+      Device[deviceCount].ExitTaskBeforeSave = false;


Ton recently reduced all these by no longer explicitly setting the flags to their defaults in order to reduce the build size.
So please check which ones are now set to their defaults.

Also this part was quite explicitly made to operate on the result of Device[++deviceCount]

auto& dev = Device[++deviceCount];

TD-er · 2026-03-19T11:08:31Z

src/_P070_NeoPixel_Clock.ino

-      success     = true;
-      break;
-    }
-    # endif // if FEATURE_MQTT_DISCOVER


Why is this section removed?
It looks like you started working on a rather old version of the plugin code and didn't take in recent changes.

TD-er · 2026-03-19T11:09:53Z

platformio.ini


 ;default_envs = normal_ESP32c6_4M316k_LittleFS_CDC
-; default_envs = custom_ESP8266_4M1M
+; default_envs = custom_274_ESP8266_4M1M


Please remove this from the PR as it is not related to the code changes

tonhuisman · 2026-03-19T12:50:00Z

src/src/PluginStructs/P070_data_struct.cpp

+  if (Plugin_070_pixels != nullptr) {
+    delete Plugin_070_pixels;
+    Plugin_070_pixels = nullptr;
+  }


delete does that null-check himself, no need to waste an if on it (== smaller code, every byte is important in our sometimes quite cramped builds)

tonhuisman · 2026-03-19T12:50:18Z

src/src/PluginStructs/P070_data_struct.cpp

+  if (Plugin_070_pixels != nullptr) {
+    delete Plugin_070_pixels;
+    Plugin_070_pixels = nullptr;
+  }


tonhuisman · 2026-03-19T14:38:55Z

src/_P070_NeoPixel_Clock.ino

 // #######################################################################################################

 /** Changelog:
- * 2025-01-12 tonhuisman: Add support for MQTT AutoDiscovery (not supported for Neopixel)


Can you please keep original changelog items in place?

tonhuisman · 2026-03-19T14:40:08Z

src/_P070_NeoPixel_Clock.ino

+ * 2024-xx-xx: Add automatic brightness control via BH1750 lux sensor (PCONFIG(6))
+ * 2024-xx-xx: Add selectable LED ring mode: 60-LED (standard) or 24-LED via web interface (PCONFIG(5))


Your changes are new to us, would be better to date them accordingly, so 2026-03-19, I'd say

tonhuisman · 2026-03-19T14:44:31Z

tools/pio/pre_custom_esp32.py

@@ -40,6 +41,7 @@
    "-DUSES_P052",  # SenseAir
    "-DUSES_P056",  # SDS011-Dust
    "-DUSES_P059",  # Encoder
+    "-DUSES_P070",  # Neopixel Clock
    "-DUSES_P080",  # Dallas iButton


Please do not commit these pre_custom files.

tonhuisman · 2026-03-19T14:44:46Z

tools/pio/pre_custom_esp82xx.py

@@ -51,7 +54,7 @@
 #   "-DUSES_P095",  # TFT ILI9341
 #    "-DUSES_P106",  # BME680
 #    "-DUSES_P107",  # SI1145 UV index
-
+#    "-DUSES_P131",  # NeoPixel Matrix
    "-DUSES_P146",  # Cache Reader


tonhuisman · 2026-03-19T21:25:08Z

src/_P070_NeoPixel_Clock.ino

+      {
+        String radioHtml = F("<input type='radio' name='led_mode' value='0'");
+        if (PCONFIG(5) == P070_LED_MODE_60) { radioHtml += F(" checked"); }
+        radioHtml += F("> 60 LEDs&nbsp;&nbsp;&nbsp;");
+        radioHtml += F("<input type='radio' name='led_mode' value='1'");
+        if (PCONFIG(5) == P070_LED_MODE_24) { radioHtml += F(" checked"); }
+        radioHtml += F("> 24 LEDs&nbsp;&nbsp;&nbsp;");
+        radioHtml += F("<input type='radio' name='led_mode' value='2'");
+        if (PCONFIG(5) == P070_LED_MODE_16) { radioHtml += F(" checked"); }
+        radioHtml += F("> 16 LEDs&nbsp;&nbsp;&nbsp;");
+        radioHtml += F("<input type='radio' name='led_mode' value='3'");
+        if (PCONFIG(5) == P070_LED_MODE_12) { radioHtml += F(" checked"); }
+        radioHtml += F("> 12 LEDs");
+        addRowLabel(F("LED Ring size"));
+        addHtml(radioHtml);
+      }


Instead of collecting a lot of html string in memory and then sending it out all at the end, you'd better just use addHtml() directly, that's much more efficient.
The same goes for other places where you use this same technique.

tonhuisman · 2026-03-19T21:27:50Z

src/_P070_NeoPixel_Clock.ino

+      addFormCheckBox(F("Auto brightness (BH1750)"), F("auto_brightness"), PCONFIG(6));
+      addFormNote(F("Automatically adjust brightness via BH1750 sensor (Task name must be 'BH1750')"));


How to use addTaskSelect() can be found in plugin P021 - Regulator - Level Control.

tonhuisman · 2026-03-19T21:31:56Z

src/_P070_NeoPixel_Clock.ino

+        lsHtml += F("> Aus &nbsp;&nbsp;&nbsp;");
+        lsHtml += F("<input type='radio' name='lightshow_mode' value='1'");
+        if (PCONFIG(7) == P070_LIGHTSHOW_CHASE) { lsHtml += F(" checked"); }
+        lsHtml += F("> Lauflicht (Farbe zuf&auml;llig) &nbsp;&nbsp;&nbsp;");


Please use English terms. Any translation can be done by the webbrowser, if desired. Aus, Lauflicht, Zeiger-Blink etc. won't mean much to non-German speaking people.

tonhuisman · 2026-03-19T21:36:20Z

docs/source/Plugin/_plugin_sets_overview.repl

This file, while present in .gitignore, doesn't seem to be ignored by git (for unknown reasons). It doesn't make much sense to be committed, as it is re-generated during the Github merge-build, when the documentation is published to ReadTheDocs.

tonhuisman · 2026-03-19T22:10:57Z

src/_P070_NeoPixel_Clock.ino

+    html += F("' value='");
+    html += String(i);
+    html += F("'");
+    if (currentValue == i) { html += F(" checked"); }
+    html += F("> ");


When migrating this to addHtml():

No need to convert int to String, there are addHtml() overloads that take numeric values

A single-character flash-string (using the F() macro) is quite expensive, using a char ('\'') is more efficient

There is also an addHtml() overload that takes 2 char arguments to easily handle line 59

NB: Using F-Strings for other textual parts is preferred, like you already have.

tonhuisman · 2026-03-19T22:13:45Z

src/_P070_NeoPixel_Clock.ino


+// Helper: emit a row of color radio buttons for one hand/element
+static void addColorRadioRow(const __FlashStringHelper *label,
+                             const char               *fieldName,


fieldName can either be a const __FlashStringHelper * (or const String & if you have to assemble a name where this function is called), avoiding the use of regular C-strings in the code.

tonhuisman · 2026-03-19T22:18:21Z

src/_P070_NeoPixel_Clock.ino

-
+// A clock that uses a strip/ring of 60 or 24 WS2812 NeoPixel LEDs as display for a classic clock.
+// Hand/mark colors are configurable. An optional hourly colour-chase lightshow is available.
+// Brightness can be set manually or automatically via a BH1750 lux sensor (Task name: "BH1750").


Any light-sensor that reports a brightness value in Lux should be usable here, right? Then you might need to be able to select the Task value also. That's also used in P021, using addTaskValueSelect(), so similar code can be used for this plugin.

tonhuisman · 2026-03-19T22:28:55Z

src/_P070_NeoPixel_Clock.ino

      PCONFIG(3) = getFormItemInt(F("offset"));
      PCONFIG(4) = isFormItemChecked(F("thick_12_mark"));
+      PCONFIG(5) = new_led_mode;
+      PCONFIG(6) = isFormItemChecked(F("auto_brightness"));
+      PCONFIG(7) = getFormItemInt(F("lightshow_mode"));
+      PCONFIG(8) = getFormItemInt(F("lightshow_duration"));
+      PCONFIG(9) = new_led_type;


Boolean and small numeric values need only a few bits to be stored, so can be combined in a 16 or 32 bit PCONFIG setting, examples can be found in f.e. P159, P165 and P176.
There are support-functions to get/set 2, 3, 4, 8 and 16 bit values, and bools of course.

That should avoid you needing PCONFIG(8) and up.
NB: Be aware that PCONFIG_LONG() and PCONFIG_ULONG() use the same storage space (in a union), so you can only use one or the other!

Capitanguex · 2026-03-20T12:01:30Z

Thanks for all these suggestions. I’ll revise it (though that will take some time), make sure everything is in proper English, and then resubmit it later.

TD-er requested changes Mar 19, 2026

View reviewed changes

tonhuisman reviewed Mar 19, 2026

View reviewed changes

Capitanguex closed this Apr 1, 2026

		* 2024-xx-xx: Add automatic brightness control via BH1750 lux sensor (PCONFIG(6))
		* 2024-xx-xx: Add selectable LED ring mode: 60-LED (standard) or 24-LED via web interface (PCONFIG(5))

		addFormCheckBox(F("Auto brightness (BH1750)"), F("auto_brightness"), PCONFIG(6));
		addFormNote(F("Automatically adjust brightness via BH1750 sensor (Task name must be 'BH1750')"));

Conversation

Capitanguex commented Mar 19, 2026

Uh oh!

TD-er commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Capitanguex commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants