From 41e527645735299e84fcae39078ad2daa785f9dd Mon Sep 17 00:00:00 2001 From: Jan Delgado Date: Sun, 10 Nov 2024 15:58:28 +0100 Subject: [PATCH] Make JLed::Update(uint32_t t) public (#130) * make JLed::Update(uint32_t t) public allowing minor optimizations and simpler tests * optionally install build tools using devbox * upgrade cpplint * fix linter findings * don't run CI jobs concurrently --- .github/workflows/test.yml | 7 ++- CHANGELOG.md | 5 ++ README.md | 8 ++- devbox.json | 18 ++++++ devbox.lock | 114 +++++++++++++++++++++++++++++++++++++ library.json | 2 +- library.properties | 2 +- src/esp32_hal.h | 2 +- src/jled_base.cpp | 4 +- src/jled_base.h | 37 ++++++------ src/pico_hal.h | 5 +- test/test_jled.cpp | 35 ++++-------- 12 files changed, 187 insertions(+), 52 deletions(-) create mode 100644 devbox.json create mode 100644 devbox.lock diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 27f737f..32425ee 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,6 +7,11 @@ on: - master name: run tests + +concurrency: + group: ${{ github.head_ref }} + cancel-in-progress: true + jobs: lint: runs-on: ubuntu-latest @@ -21,7 +26,7 @@ jobs: - name: linter run: | - pip install cpplint + pip install cpplint==2.0.0 make lint test: diff --git a/CHANGELOG.md b/CHANGELOG.md index 7de1e52..45708db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # JLed changelog (github.com/jandelgado/jled) +## [2024-09-21] 4.14 + +* new: make `Jled::Update(unit32_t t)` public, allowing optimizations and + simplified tests + ## [2023-09-10] 4.13.1 * fix: `Update()` sometimes returning wrong state (https://github.com/jandelgado/jled/issues/122) diff --git a/README.md b/README.md index e4495a3..39f01b6 100644 --- a/README.md +++ b/README.md @@ -170,7 +170,7 @@ hardware abstraction, which might scale it to the resolution used by the actual device (e.g. 10 bits for an ESP8266). Finally the brightness value is written out to the configure GPIO. -``` +```text ┌───────────┐ ┌────────────┐ ┌─────────┐ ┌────────┐ ┌─────────┐ ┌────────┐ │ Evaluate │ │ Scale to │ │ Low │YES │ Invert │ │Scale for│ │Write to│ │ effect(t) ├───►│ [min, max] ├───►│ active? ├───►│ signal ├───►│Hardware ├───►│ GPIO │ @@ -410,8 +410,10 @@ specified by `DelayAfter()` method. ##### Update -Call `Update()` periodically to update the state of the LED. `Update` returns -`true` if the effect is active, and `false` when it finished. +Call `Update()` or `Update(uint32_t t)` periodically to update the state of the +LED. `Update` returns `true` if the effect is active, and `false` when it +finished. `Update()` is a shortcut to call `Update(uint32_t t)` with the +current time. ##### IsRunning diff --git a/devbox.json b/devbox.json new file mode 100644 index 0000000..5dfc81c --- /dev/null +++ b/devbox.json @@ -0,0 +1,18 @@ +{ + "packages": [ + "python@3.11", + "lcov@1.16", + "pipx", + "cpplint@2.0.0" + ], + "shell": { + "init_hook": [ + "echo 'Welcome to devbox!' > /dev/null" + ], + "scripts": { + "test": [ + "echo \"Error: no test specified\" && exit 1" + ] + } + } +} diff --git a/devbox.lock b/devbox.lock new file mode 100644 index 0000000..14600c5 --- /dev/null +++ b/devbox.lock @@ -0,0 +1,114 @@ +{ + "lockfile_version": "1", + "packages": { + "cpplint@2.0.0": { + "last_modified": "2024-11-03T14:18:04Z", + "resolved": "github:NixOS/nixpkgs/4ae2e647537bcdbb82265469442713d066675275#cpplint", + "source": "devbox-search", + "version": "2.0.0", + "systems": { + "aarch64-darwin": { + "outputs": [ + { + "name": "out", + "path": "/nix/store/70gc21bl3grda63djlks6x1f5g8h89xk-cpplint-2.0.0", + "default": true + }, + { + "name": "dist", + "path": "/nix/store/3sq5396dcbg3r06g9zci1w4rxql1xf0k-cpplint-2.0.0-dist" + } + ], + "store_path": "/nix/store/70gc21bl3grda63djlks6x1f5g8h89xk-cpplint-2.0.0" + }, + "aarch64-linux": { + "outputs": [ + { + "name": "out", + "path": "/nix/store/3fcbszvn75zwim2n3785bflrwww38l42-cpplint-2.0.0", + "default": true + }, + { + "name": "dist", + "path": "/nix/store/fqrmkx6q4q290hildy4l6gi2pzwzwinw-cpplint-2.0.0-dist" + } + ], + "store_path": "/nix/store/3fcbszvn75zwim2n3785bflrwww38l42-cpplint-2.0.0" + }, + "x86_64-darwin": { + "outputs": [ + { + "name": "out", + "path": "/nix/store/m675bxwgh3wmr0gix5g6xnhkidlql0ii-cpplint-2.0.0", + "default": true + }, + { + "name": "dist", + "path": "/nix/store/2bpdwa3r1kfzf4jkd4i24njxy3pr3xnv-cpplint-2.0.0-dist" + } + ], + "store_path": "/nix/store/m675bxwgh3wmr0gix5g6xnhkidlql0ii-cpplint-2.0.0" + }, + "x86_64-linux": { + "outputs": [ + { + "name": "out", + "path": "/nix/store/vrv52827v0b6h4k3nl7k9zg4ld182khg-cpplint-2.0.0", + "default": true + }, + { + "name": "dist", + "path": "/nix/store/132q2dd3wkkgdpybwzhybm7hbad0g097-cpplint-2.0.0-dist" + } + ], + "store_path": "/nix/store/vrv52827v0b6h4k3nl7k9zg4ld182khg-cpplint-2.0.0" + } + } + }, + "lcov@1.16": { + "last_modified": "2024-03-22T11:26:23Z", + "resolved": "github:NixOS/nixpkgs/a3ed7406349a9335cb4c2a71369b697cecd9d351#lcov", + "source": "devbox-search", + "version": "1.16", + "systems": { + "aarch64-darwin": { + "store_path": "/nix/store/cwjgl90nkg79za5gx41yg4663w7iyj0y-lcov-1.16" + }, + "aarch64-linux": { + "store_path": "/nix/store/81axjrgg3cjx09kdb25psiiyz60zacqw-lcov-1.16" + }, + "x86_64-darwin": { + "store_path": "/nix/store/5jir4n0q72z9p2h9sxwb2rcam3j165wp-lcov-1.16" + }, + "x86_64-linux": { + "store_path": "/nix/store/qfdwnjp77xlvg0jqfv1dl8b40xpv230k-lcov-1.16" + } + } + }, + "pipx": { + "resolved": "github:NixOS/nixpkgs/75a52265bda7fd25e06e3a67dee3f0354e73243c#pipx", + "source": "nixpkg" + }, + "python@3.11": { + "last_modified": "2024-03-22T11:26:23Z", + "plugin_version": "0.0.3", + "resolved": "github:NixOS/nixpkgs/a3ed7406349a9335cb4c2a71369b697cecd9d351#python3", + "source": "devbox-search", + "version": "3.11.8", + "systems": { + "aarch64-darwin": { + "store_path": "/nix/store/c05vbvkjxarxkws9zkwrcwrzlsx9nd68-python3-3.11.8" + }, + "aarch64-linux": { + "store_path": "/nix/store/pxzzyri1wbq7kc7pain665g94afkl4ww-python3-3.11.8" + }, + "x86_64-darwin": { + "store_path": "/nix/store/1zaap1xxxvw2ypsgh1mfxb3wzdd49873-python3-3.11.8" + }, + "x86_64-linux": { + "store_path": "/nix/store/7wz6hm9i8wljz0hgwz1wqmn2zlbgavrq-python3-3.11.8" + } + } + } + } +} diff --git a/library.json b/library.json index c9b7c1b..52dc8a0 100644 --- a/library.json +++ b/library.json @@ -1,6 +1,6 @@ { "name": "JLed", - "version": "4.13.1", + "version": "4.14", "description": "An embedded library to control LEDs", "license": "MIT", "frameworks": ["espidf", "arduino", "mbed"], diff --git a/library.properties b/library.properties index bb5b9cf..a11d63b 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=JLed -version=4.13.1 +version=4.14 author=Jan Delgado maintainer=Jan Delgado sentence=An Arduino library to control LEDs diff --git a/src/esp32_hal.h b/src/esp32_hal.h index 466fb77..7d235cb 100644 --- a/src/esp32_hal.h +++ b/src/esp32_hal.h @@ -133,7 +133,7 @@ class Esp32Hal { } uint32_t millis() const { - return (uint32_t)(esp_timer_get_time() / 1000ULL); + return static_cast(esp_timer_get_time() / 1000ULL); } PinType chan() const { return chan_; } diff --git a/src/jled_base.cpp b/src/jled_base.cpp index 4e95b59..6e59531 100644 --- a/src/jled_base.cpp +++ b/src/jled_base.cpp @@ -32,7 +32,7 @@ namespace jled { // fade-off and breath functions are all derived from fade-on, see // below. static constexpr uint8_t kFadeOnTable[] = {0, 3, 13, 33, 68, - 118, 179, 232, 255}; + 118, 179, 232, 255}; // NOLINT // https://www.wolframalpha.com/input/?i=plot+(exp(sin((x-100%2F2.)*PI%2F100))-0.36787944)*108.0++x%3D0+to+100 // The fade-on func is an approximation of @@ -70,7 +70,7 @@ uint8_t rand8() { // scale8(0, f) == 0 for all f // scale8(x, 255) == x for all x uint8_t scale8(uint8_t val, uint8_t factor) { - return ((uint16_t)val * (uint16_t)(1 + factor)) >> 8; + return (static_cast(val)*static_cast(1 + factor)) >> 8; } // interpolate a byte (val) to the interval [a,b]. diff --git a/src/jled_base.h b/src/jled_base.h index 8562879..60c492e 100644 --- a/src/jled_base.h +++ b/src/jled_base.h @@ -233,8 +233,6 @@ class TJLed { HalType& Hal() { return hal_; } - bool Update() { return Update(hal_.millis()); } - // Set physical LED polarity to be low active. This inverts every // signal physically output to a pin. B& LowActive() { @@ -378,14 +376,6 @@ class TJLed { // Returns current maximum brightness level. uint8_t MaxBrightness() const { return maxBrightness_; } - protected: - // test if time stored in last_update_time_ differs from provided timestamp. - bool inline timeChangedSinceLastUpdate(uint32_t now) { - return (now & 255) != last_update_time_; - } - - void trackLastUpdateTime(uint32_t t) { last_update_time_ = (t & 255); } - // update brightness of LED using the given brightness evaluator // (brightness) ________________ // on 255 | ¸-' @@ -395,6 +385,8 @@ class TJLed { // |<-delay before->|<--period-->|<-delay after-> (time) // | func(t) | // |<- num_repetitions times -> + bool Update() { return Update(hal_.millis()); } + bool Update(uint32_t now) { if (state_ == ST_STOPPED || !brightness_eval_) return false; @@ -408,18 +400,18 @@ class TJLed { trackLastUpdateTime(now); - if ((int32_t)(now - time_start_) < 0) return true; + if (static_cast(now - time_start_) < 0) return true; // t cycles in range [0..period+delay_after-1] const auto period = brightness_eval_->Period(); const auto t = (now - time_start_) % (period + delay_after_); if (!IsForever()) { - const auto time_end = - time_start_ + - (uint32_t)(period + delay_after_) * num_repetitions_ - 1; + const auto time_end = time_start_ + + static_cast(period + delay_after_) * + num_repetitions_ - 1; - if ((int32_t)(now - time_end) >= 0) { + if (static_cast(now - time_end) >= 0) { // make sure final value of t = (period-1) is set state_ = ST_STOPPED; const auto val = Eval(period - 1); @@ -442,6 +434,14 @@ class TJLed { return true; } + protected: + // test if time stored in last_update_time_ differs from provided timestamp. + bool inline timeChangedSinceLastUpdate(uint32_t now) { + return (now & 255) != last_update_time_; + } + + void trackLastUpdateTime(uint32_t t) { last_update_time_ = (t & 255); } + B& SetBrightnessEval(BrightnessEvaluator* be) { brightness_eval_ = be; // start over after the brightness evaluator changed @@ -504,8 +504,9 @@ class TJLedSequence { // active, else false bool UpdateParallel() { auto result = false; + uint32_t t = ptr(leds_[0])->Hal().millis(); for (auto i = 0u; i < n_; i++) { - result |= ptr(leds_[i])->Update(); + result |= ptr(leds_[i])->Update(t); } return result; } @@ -536,7 +537,7 @@ class TJLedSequence { : mode_{mode}, leds_{leds}, cur_{0}, n_{n} {} bool Update() { - if (!is_running_) { + if (!is_running_ || n_ < 1) { return false; } @@ -592,5 +593,5 @@ class TJLedSequence { bool is_running_ = true; }; -}; // namespace jled +}; // namespace jled #endif // SRC_JLED_BASE_H_ diff --git a/src/pico_hal.h b/src/pico_hal.h index 9b85173..9e2b68c 100644 --- a/src/pico_hal.h +++ b/src/pico_hal.h @@ -43,7 +43,8 @@ class PicoHal { uint32_t *top_) { // Set the frequency, making "top_" as large as possible for maximum // resolution. - *div = (uint32_t)(16 * clock_get_hz(clk_sys) / (uint32_t)freq); + *div = static_cast(16 * clock_get_hz(clk_sys) / + static_cast(freq)); *top_ = 1; for (;;) { // Try a few small prime factors to get close to the desired @@ -100,7 +101,7 @@ class PicoHal { void analogWrite(uint8_t val) const { set_pwm_duty(slice_num_, channel_, top_, - (uint32_t)(DUTY_100_PCT / 255) * val); + static_cast(DUTY_100_PCT / 255) * val); } uint32_t millis() const { return to_ms_since_boot(get_absolute_time()); } diff --git a/test/test_jled.cpp b/test/test_jled.cpp index 2298658..855b57d 100644 --- a/test/test_jled.cpp +++ b/test/test_jled.cpp @@ -297,14 +297,11 @@ TEST_CASE("dont evalute twice during one time tick", "[jled]") { auto eval = MockBrightnessEvaluator(ByteVec{0, 1, 2}); TestJLed jled = TestJLed(1).UserFunc(&eval); - jled.Hal().SetMillis(0); - jled.Update(); + jled.Update(0); CHECK(eval.Count() == 1); - jled.Update(); + jled.Update(0); CHECK(eval.Count() == 1); - - jled.Hal().SetMillis(1); - jled.Update(); + jled.Update(1); CHECK(eval.Count() == 2); } @@ -313,21 +310,18 @@ TEST_CASE("Handles millis overflow during effect", "[jled]") { TestJLed jled = TestJLed(10); // Set time close to overflow auto time = std::numeric_limits::max() - 25; - jled.Hal().SetMillis(time); - CHECK_FALSE(jled.Update()); + CHECK_FALSE(jled.Update(time)); // Start fade off jled.FadeOff(100); - CHECK(jled.Update()); + CHECK(jled.Update(time)); CHECK(jled.IsRunning()); CHECK(jled.Hal().Value() > 0); // Set time after overflow, before effect ends - jled.Hal().SetMillis(time + 50); - CHECK(jled.Update()); + CHECK(jled.Update(time+50)); CHECK(jled.IsRunning()); CHECK(jled.Hal().Value() > 0); // Set time after effect ends - jled.Hal().SetMillis(time + 150); - CHECK_FALSE(jled.Update()); + CHECK_FALSE(jled.Update(time+150)); CHECK_FALSE(jled.IsRunning()); CHECK(0 == jled.Hal().Value()); } @@ -382,11 +376,10 @@ TEST_CASE("LowActive() inverts signal", "[jled]") { CHECK(jled.IsLowActive()); - jled.Update(); + jled.Update(0); CHECK(255 == jled.Hal().Value()); - jled.Hal().SetMillis(1); - jled.Update(); + jled.Update(1); CHECK(0 == jled.Hal().Value()); } @@ -449,15 +442,11 @@ TEST_CASE("Update returns true while updating, else false", "[jled]") { TestJLed jled = TestJLed(10).UserFunc(&eval); // Update returns FALSE on last step and beyond, else TRUE - auto time = 0; - jled.Hal().SetMillis(time++); - CHECK(jled.Update()); + CHECK(jled.Update(0)); // when effect is done, we expect still false to be returned - jled.Hal().SetMillis(time++); - CHECK_FALSE(jled.Update()); - jled.Hal().SetMillis(time++); - CHECK_FALSE(jled.Update()); + CHECK_FALSE(jled.Update(1)); + CHECK_FALSE(jled.Update(2)); } TEST_CASE("After Reset() the effect can be restarted", "[jled]") {