From 50934e6840995154109ab7d532dbb61af0377f90 Mon Sep 17 00:00:00 2001
From: Frank <91616163+softhack007@users.noreply.github.com>
Date: Sat, 2 Nov 2024 18:16:51 +0100
Subject: [PATCH] adressing some review comments

* keep FRAMETIME_FIXED as a fixed value
* remove WLED_FPS_SLOW and FRAMETIME_FIXED_SLOW
* explicit test "(_targetFps != FPS_UNLIMITED)" for debug messages
* don't modify _lastServiceShow in show()
* test for "fps == FPS_UNLIMITED" explicitly, so we could pick a different
 magic number later
---
 wled00/FX.h       | 13 ++++---------
 wled00/FX_fcn.cpp | 12 +++++-------
 2 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/wled00/FX.h b/wled00/FX.h
index acdb62c81..1f0df9da3 100644
--- a/wled00/FX.h
+++ b/wled00/FX.h
@@ -44,20 +44,15 @@
 
 /* Not used in all effects yet */
 #define WLED_FPS         42
+#define FRAMETIME_FIXED  (1000/WLED_FPS)
 #define FRAMETIME        strip.getFrameTime()
 #if defined(ARDUINO_ARCH_ESP32) && !defined(CONFIG_IDF_TARGET_ESP32C3)    // all ESP32 except -C3(very slow, untested)
-  #define FRAMETIME_FIXED  (strip.getFrameTime() < 10 ? 12 : 24)            // allow faster FRAMETIME_FIXED when target FPS >= 100  
   #define MIN_SHOW_DELAY   (max(2, (_frametime*5)/8))                       // supports higher framerates and better animation control -- 5/8 = 62%
   // used to initialize for strip attributes:
-  #define WLED_FPS_SLOW    42
-  #define FRAMETIME_FIXED_SLOW  (24)                                        // 1000/42 = 24ms
 #else
-  #define FRAMETIME_FIXED  (1000/WLED_FPS)
   #define MIN_SHOW_DELAY   (_frametime < 16 ? 8 : 15)                       // legacy MIN_SHOW_DELAY - creates more idle loops, but reduces framerates
-  #define WLED_FPS_SLOW     WLED_FPS
-  #define FRAMETIME_FIXED_SLOW FRAMETIME_FIXED
 #endif
-#define FPS_UNLIMITED    119
+#define FPS_UNLIMITED    120
 
 /* each segment uses 82 bytes of SRAM memory, so if you're application fails because of
   insufficient memory, decreasing MAX_NUM_SEGMENTS may help */
@@ -737,8 +732,8 @@ class WS2812FX {  // 96 bytes
       _length(DEFAULT_LED_COUNT),
       _brightness(DEFAULT_BRIGHTNESS),
       _transitionDur(750),
-      _targetFps(WLED_FPS_SLOW),
-      _frametime(FRAMETIME_FIXED_SLOW),
+      _targetFps(WLED_FPS),
+      _frametime(FRAMETIME_FIXED),
       _cumulativeFps(2),
       _isServicing(false),
       _isOffRefreshRequired(false),
diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp
index 5856ad786..3972dad2c 100644
--- a/wled00/FX_fcn.cpp
+++ b/wled00/FX_fcn.cpp
@@ -1314,7 +1314,7 @@ void WS2812FX::service() {
 
   #if defined(ARDUINO_ARCH_ESP32) && !defined(CONFIG_IDF_TARGET_ESP32C3)
   if (elapsed < 2) return;                                                       // keep wifi alive
-  if ( !_triggered && (_targetFps < FPS_UNLIMITED) && (_targetFps > 0)) {
+  if ( !_triggered && (_targetFps != FPS_UNLIMITED) && (_targetFps > 0)) {
     if (elapsed < MIN_SHOW_DELAY) return;                                        // WLEDMM too early for service
   }
   #else  // legacy
@@ -1325,7 +1325,7 @@ void WS2812FX::service() {
 
   _isServicing = true;
   _segment_index = 0;
-  unsigned speedLimit = (_targetFps < FPS_UNLIMITED) ? (0.85f * FRAMETIME) : 1;      // lower limit for effect frametime
+  unsigned speedLimit = (_targetFps != FPS_UNLIMITED) ? (0.85f * FRAMETIME) : 1;      // lower limit for effect frametime
 
   for (segment &seg : _segments) {
     if (_suspend) return; // immediately stop processing segments if suspend requested during service()
@@ -1389,7 +1389,7 @@ void WS2812FX::service() {
   _triggered = false;
 
   #ifdef WLED_DEBUG
-  if (millis() - nowUp > _frametime*2) DEBUG_PRINTF_P(PSTR("Slow effects %u/%d.\n"), (unsigned)(millis()-nowUp), (int)_frametime);
+  if ((_targetFps != FPS_UNLIMITED) && (millis() - nowUp > _frametime)) DEBUG_PRINTF_P(PSTR("Slow effects %u/%d.\n"), (unsigned)(millis()-nowUp), (int)_frametime);
   #endif
   if (doShow) {
     yield();
@@ -1398,7 +1398,7 @@ void WS2812FX::service() {
     _lastServiceShow = nowUp; // correct timestamp, for better FPS control
   }
   #ifdef WLED_DEBUG
-  if (millis() - nowUp > _frametime*2) DEBUG_PRINTF_P(PSTR("Slow strip %u/%d.\n"), (unsigned)(millis()-nowUp), (int)_frametime);
+  if ((_targetFps != FPS_UNLIMITED) && (millis() - nowUp > _frametime)) DEBUG_PRINTF_P(PSTR("Slow strip %u/%d.\n"), (unsigned)(millis()-nowUp), (int)_frametime);
   #endif
 }
 
@@ -1430,7 +1430,6 @@ void WS2812FX::show() {
   if (diff > 0) fpsCurr = 1000 / diff;
   _cumulativeFps = (3 * _cumulativeFps + fpsCurr +2) >> 2;   // "+2" for proper rounding (2/4 = 0.5)
   _lastShow = showNow;
-  _lastServiceShow = showNow;
 }
 
 /**
@@ -1453,8 +1452,7 @@ uint16_t WS2812FX::getFps() const {
 void WS2812FX::setTargetFps(uint8_t fps) {
   if (fps > 0 && fps <= 120) _targetFps = fps;
   _frametime = 1000 / _targetFps;
-  if (_frametime < 1) _frametime = 1;           // better safe than sorry
-  if (fps >= FPS_UNLIMITED) _frametime = 3;     // unlimited mode
+  if (fps == FPS_UNLIMITED) _frametime = 3;     // unlimited mode
 }
 
 void WS2812FX::setMode(uint8_t segid, uint8_t m) {