[lvgl] Ensure final animation frame completes before restart (#19642)

Co-authored-by: clydebarrow <2366188+clydebarrow@users.noreply.github.com>

esphome/components/lvgl/animation.h

@@ -4,6 +4,7 @@
 #ifdef USE_LVGL_ANIMATION
 #include "lvgl_esphome.h"
 #include "esphome/core/hal.h"
+#include "esphome/core/helpers.h"
 
 namespace esphome::lvgl {
 
@@ -110,17 +111,8 @@ template<size_t DATA_SIZE, bool AUTO_START = false> class LvAnimation : public C
   }
 
   void start() {
-    if (this->state_ > AnimationState::STOPPED)
-      this->stop();
-    if (this->duration_ == 0)
+    if (!this->prepare_())
       return;
-    // evaluate any lambdas
-    for (size_t i = 0; i != DATA_SIZE; i++) {
-      this->data_from_[i] = this->from_[i].value();
-      this->data_to_[i] = this->to_[i].value();
-    }
-    this->start_time_ = millis();
-    this->state_ = AnimationState::STARTED;
     this->loop();
     this->start_callback_.call();
   }
@@ -143,29 +135,26 @@ template<size_t DATA_SIZE, bool AUTO_START = false> class LvAnimation : public C
     if (this->state_ == AnimationState::STOPPED)
       return;
     uint32_t elapsed = millis() - this->start_time_;
-    float progress = static_cast<float>(elapsed) / static_cast<float>(this->duration_);
+    float progress = clamp_at_most(elapsed / static_cast<float>(this->duration_), 1.0f);
     switch (this->state_) {
       case AnimationState::STARTED:
         if (elapsed < this->start_delay_)
           return;
         this->state_ = AnimationState::RUNNING;
         this->start_time_ = millis();
+        elapsed = 0;
         progress = 0.0f;
         break;
       case AnimationState::RUNNING:
-        if (progress >= 1.0f) {
-          progress = 1.0f;
-          this->stop();
-          if (this->loop_)
-            this->start();
-        }
         break;
       default:
         return;
     }
 
+    // state here is RUNNING.
     for (auto *timing : this->timings_) {
-      progress = timing->map_progress(progress);
+      // avoid overshooting
+      progress = clamp_at_most(timing->map_progress(progress), 1.0f);
     }
     lv_coord_t data[DATA_SIZE];
     for (size_t i = 0; i != DATA_SIZE; i++) {
@@ -173,6 +162,12 @@ template<size_t DATA_SIZE, bool AUTO_START = false> class LvAnimation : public C
           roundf(this->data_from_[i] + static_cast<lv_coord_t>(this->data_to_[i] - this->data_from_[i]) * progress));
     }
     this->update_callback_(data);
+    if (elapsed >= this->duration_) {
+      this->stop();
+      // Restart without drawing the first frame now, so the final frame is not overwritten before it is shown
+      if (this->loop_ && this->prepare_())
+        this->start_callback_.call();
+    }
   }
 
   float get_setup_priority() const override { return setup_priority::PROCESSOR - 20.0; }
@@ -187,6 +182,22 @@ template<size_t DATA_SIZE, bool AUTO_START = false> class LvAnimation : public C
   template<typename F> void add_on_stop_callback(F &&callback) { this->stop_callback_.add(std::forward<F>(callback)); }
 
  protected:
+  // Arms a new run. Returns false if there is nothing to run.
+  bool prepare_() {
+    if (this->state_ > AnimationState::STOPPED)
+      this->stop();
+    if (this->duration_ == 0)
+      return false;
+    // evaluate any lambdas
+    for (size_t i = 0; i != DATA_SIZE; i++) {
+      this->data_from_[i] = this->from_[i].value();
+      this->data_to_[i] = this->to_[i].value();
+    }
+    this->start_time_ = millis();
+    this->state_ = AnimationState::STARTED;
+    return true;
+  }
+
   void (*const update_callback_)(const lv_coord_t *data);
   LazyCallbackManager<void()> start_callback_{};
   LazyCallbackManager<void()> stop_callback_{};

tests/components/lvgl/__init__.py

@@ -0,0 +1,15 @@
+import esphome.codegen as cg
+from esphome.components.lvgl import LVGL_VERSION
+from esphome.types import ConfigType
+from tests.testing_helpers import ComponentManifestOverride
+
+
+def override_manifest(manifest: ComponentManifestOverride) -> None:
+    # The real to_code needs a display and widgets. The unit tests only need the LVGL
+    # headers and the animation code.
+    async def to_code_testing(config: ConfigType) -> None:
+        cg.add_library("lvgl/lvgl", LVGL_VERSION)
+        cg.add_build_flag("-DLV_CONF_SKIP=1")
+        cg.add_define("USE_LVGL_ANIMATION")
+
+    manifest.to_code = to_code_testing

tests/components/lvgl/animation_test.cpp

@@ -0,0 +1,184 @@
+#include <gtest/gtest.h>
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <vector>
+#include "esphome/components/lvgl/animation.h"
+
+namespace esphome::lvgl::testing {
+
+namespace {
+
+// The update callback is a plain function pointer, so it cannot capture; results go to a global.
+std::vector<lv_coord_t> updates;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+
+void record_update(const lv_coord_t *data) { updates.push_back(data[0]); }
+
+// Exposes the running state so tests can wait for the animation to finish.
+class TestAnimation : public LvAnimation<1> {
+ public:
+  TestAnimation(lv_coord_t from, lv_coord_t to, uint32_t duration_ms, uint32_t start_delay_ms = 0)
+      : LvAnimation<1>(record_update, {TemplatableValue<lv_coord_t>(from)}, {TemplatableValue<lv_coord_t>(to)}) {
+    this->set_duration(duration_ms);
+    this->set_start_delay(start_delay_ms);
+    this->add_on_start_callback([this]() { this->start_count++; });
+    this->add_on_stop_callback([this]() { this->stop_count++; });
+  }
+
+  bool is_running() const { return this->state_ != AnimationState::STOPPED; }
+
+  // Returns true if the animation stopped before the timeout.
+  bool run_until_stopped(uint32_t timeout_ms = 1000) {
+    const uint32_t begin = millis();
+    while (this->is_running() && millis() - begin < timeout_ms) {
+      this->loop();
+      delay(1);
+    }
+    return !this->is_running();
+  }
+
+  void run_for(uint32_t duration_ms) {
+    const uint32_t begin = millis();
+    while (millis() - begin < duration_ms) {
+      this->loop();
+      delay(1);
+    }
+  }
+
+  int start_count{0};
+  int stop_count{0};
+};
+
+lv_coord_t max_update() {
+  lv_coord_t result = updates.front();
+  for (auto value : updates)
+    result = std::max(result, value);
+  return result;
+}
+
+}  // namespace
+
+class LvAnimationTest : public ::testing::Test {
+ protected:
+  void SetUp() override { updates.clear(); }
+};
+
+TEST_F(LvAnimationTest, LinearRunsToEndAndStopsOnce) {
+  TestAnimation anim(0, 100, 20);
+  anim.start();
+  ASSERT_TRUE(anim.run_until_stopped());
+  EXPECT_EQ(updates.front(), 0);
+  EXPECT_EQ(updates.back(), 100);
+  EXPECT_EQ(anim.start_count, 1);
+  EXPECT_EQ(anim.stop_count, 1);
+
+  // Nothing more happens once stopped.
+  const size_t update_count = updates.size();
+  anim.run_for(10);
+  EXPECT_EQ(updates.size(), update_count);
+  EXPECT_EQ(anim.stop_count, 1);
+}
+
+TEST_F(LvAnimationTest, ZeroDurationDoesNotStart) {
+  TestAnimation anim(0, 100, 0);
+  anim.start();
+  EXPECT_FALSE(anim.is_running());
+  EXPECT_EQ(anim.start_count, 0);
+  EXPECT_TRUE(updates.empty());
+}
+
+TEST_F(LvAnimationTest, StartDelayHoldsBackUpdates) {
+  TestAnimation anim(0, 100, 20, 30);
+  anim.start();
+  EXPECT_TRUE(anim.is_running());
+  EXPECT_TRUE(updates.empty());
+  ASSERT_TRUE(anim.run_until_stopped());
+  EXPECT_EQ(updates.back(), 100);
+}
+
+// Round trip maps the end of the duration back to the start value, so completion must not depend on the
+// mapped value reaching 1.0.
+TEST_F(LvAnimationTest, RoundTripStopsAtStartValue) {
+  LvAnimationTimingRoundTrip timing(0.0f);
+  TestAnimation anim(0, 100, 20);
+  anim.add_timing(&timing);
+  anim.start();
+  ASSERT_TRUE(anim.run_until_stopped());
+  EXPECT_EQ(updates.back(), 0);
+  EXPECT_EQ(anim.stop_count, 1);
+}
+
+// The pause maps to 1.0 in the middle of the duration; the animation must still play the return leg.
+TEST_F(LvAnimationTest, RoundTripWithPausePlaysReturnLeg) {
+  LvAnimationTimingRoundTrip timing(0.5f);
+  TestAnimation anim(0, 100, 40);
+  anim.add_timing(&timing);
+  anim.start();
+  ASSERT_TRUE(anim.run_until_stopped());
+  EXPECT_EQ(max_update(), 100);
+  EXPECT_EQ(updates.back(), 0);
+  EXPECT_EQ(anim.stop_count, 1);
+}
+
+TEST_F(LvAnimationTest, GravityStops) {
+  LvAnimationTimingGravity timing(0.5f, 0.5f);
+  TestAnimation anim(0, 100, 20);
+  anim.add_timing(&timing);
+  anim.start();
+  ASSERT_TRUE(anim.run_until_stopped());
+  EXPECT_EQ(anim.stop_count, 1);
+}
+
+TEST_F(LvAnimationTest, EaseInOutEndsAtTarget) {
+  LvAnimationTimingEaseInOut timing(1.0f);
+  TestAnimation anim(0, 100, 20);
+  anim.add_timing(&timing);
+  anim.start();
+  ASSERT_TRUE(anim.run_until_stopped());
+  EXPECT_EQ(updates.back(), 100);
+}
+
+TEST_F(LvAnimationTest, LoopRestartsOnlyAfterReachingEnd) {
+  TestAnimation anim(0, 100, 20);
+  anim.set_loop(true);
+  anim.start();
+  anim.run_for(100);
+  anim.stop();
+
+  EXPECT_GE(anim.start_count, 2);
+  // Each restart is a drop in value, and must follow a completed cycle that reached the end value.
+  for (size_t i = 1; i < updates.size(); i++) {
+    if (updates[i] < updates[i - 1])
+      EXPECT_EQ(updates[i - 1], 100) << "update " << i << " restarted before the end was reached";
+  }
+}
+
+// The first frame of the next cycle must not be drawn in the same pass as the final frame of the last one.
+TEST_F(LvAnimationTest, LoopRestartKeepsFinalFrame) {
+  TestAnimation anim(0, 100, 20);
+  anim.set_loop(true);
+  anim.start();
+  const uint32_t begin = millis();
+  while (millis() - begin < 100) {
+    const size_t before = updates.size();
+    anim.loop();
+    ASSERT_LE(updates.size() - before, 1u);
+    delay(1);
+  }
+  anim.stop();
+  EXPECT_GE(anim.start_count, 2);
+}
+
+TEST_F(LvAnimationTest, LoopWithRoundTripKeepsCycling) {
+  LvAnimationTimingRoundTrip timing(0.0f);
+  TestAnimation anim(0, 100, 20);
+  anim.add_timing(&timing);
+  anim.set_loop(true);
+  anim.start();
+  anim.run_for(100);
+  EXPECT_GE(anim.start_count, 2);
+  EXPECT_GE(anim.stop_count, 1);
+  anim.stop();
+}
+
+}  // namespace esphome::lvgl::testing