From 9100bac5af7903aeab7fb36c2f9d367e2bf2521b Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Fri, 1 Jul 2022 00:54:04 +0300 Subject: [PATCH 01/13] Long delays in Ticker Run everything through a common callback 'variant', don't separate bound std::function and function pointers. Remeber original argument as part of the ticker. Because SDK does not support durations longer than ~6700seconds, split those into multiple ticks. By default, simply divide by 2 until we reach some reasonable duration and use repeating timer internally. --- libraries/Ticker/src/Ticker.cpp | 87 ++++++++++----- libraries/Ticker/src/Ticker.h | 189 ++++++++++++++++++++++---------- 2 files changed, 192 insertions(+), 84 deletions(-) diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp index dca4435dc2..ac4935cffb 100644 --- a/libraries/Ticker/src/Ticker.cpp +++ b/libraries/Ticker/src/Ticker.cpp @@ -23,49 +23,84 @@ #include "eagle_soc.h" #include "osapi.h" +#include #include "Ticker.h" -Ticker::Ticker() - : _timer(nullptr) {} - -Ticker::~Ticker() -{ - detach(); -} - -void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg) +void Ticker::_attach(Ticker::Milliseconds milliseconds, bool repeat) { - if (_timer) - { + if (_timer) { os_timer_disarm(_timer); + } else { + _timer = &_timer_internal; } - else - { - _timer = &_etsTimer; + + os_timer_setfn(_timer, + [](void* ptr) { + auto* ticker = reinterpret_cast(ptr); + ticker->_static_callback(); + }, this); + + // whenever duration excedes this limit, make timer repeatable N times + // in case it is really repeatable, it will reset itself and continue as usual + _tick = callback_tick_t{}; + + if (milliseconds > DurationMax) { + _tick.repeat = repeat; + _tick.total += 1; + while (milliseconds > DurationMax) { + _tick.total *= 2; + milliseconds /= 2; + } + repeat = true; } - os_timer_setfn(_timer, callback, arg); - os_timer_arm(_timer, milliseconds, repeat); + os_timer_arm(_timer, milliseconds.count(), repeat); } void Ticker::detach() { - if (!_timer) - return; + if (_timer) { + os_timer_disarm(_timer); + _timer = nullptr; - os_timer_disarm(_timer); - _timer = nullptr; - _callback_function = nullptr; + _tick = callback_tick_t{ + .total = 0, + .count = 0, + .repeat = false, + }; + + _callback = std::monostate{}; + } } bool Ticker::active() const { - return _timer; + return _timer != nullptr; } -void Ticker::_static_callback(void* arg) +void Ticker::_static_callback() { - Ticker* _this = reinterpret_cast(arg); - if (_this && _this->_callback_function) - _this->_callback_function(); + if (_tick.total) { + ++_tick.count; + if (_tick.count < _tick.total) { + return; + } + + if (_tick.repeat) { + _tick.count = 0; + } + } + + std::visit([](auto&& callback) { + using T = std::decay_t; + if constexpr (std::is_same_v) { + callback.func(callback.arg); + } else if constexpr (std::is_same_v) { + callback(); + } + }, _callback); + + if (_tick.total && _tick.repeat) { + _tick.count = 0; + } } diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 791ff94567..1a9c2a7954 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -19,140 +19,213 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#ifndef TICKER_H -#define TICKER_H +#pragma once +#include #include +#include + +#include #include #include class Ticker { public: - Ticker(); - ~Ticker(); + Ticker() = default; + + ~Ticker() + { + detach(); + } + + // TODO: should re-arm with the new _timer pointer + + Ticker(const Ticker&) = delete; + Ticker(Ticker&&) = delete; - typedef void (*callback_with_arg_t)(void*); - typedef std::function callback_function_t; + Ticker& operator=(const Ticker&) = delete; + Ticker& operator=(Ticker&&) = delete; + + // native SDK type, simple function with void* argument + using callback_with_arg_t = void(*)(void*); + + // our helper type to support any callable object + using callback_function_t = std::function; // callback will be called at following loop() after ticker fires void attach_scheduled(float seconds, callback_function_t callback) { - _callback_function = [callback]() { schedule_function(callback); }; - _attach_ms(1000UL * seconds, true); + _callback = [callback]() { + schedule_function(callback); + }; + _attach(Seconds(seconds), true); } // callback will be called in SYS ctx when ticker fires void attach(float seconds, callback_function_t callback) { - _callback_function = std::move(callback); - _attach_ms(1000UL * seconds, true); + _callback = std::move(callback); + _attach(Seconds(seconds), true); } // callback will be called at following loop() after ticker fires void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback) { - _callback_function = [callback]() { schedule_function(callback); }; - _attach_ms(milliseconds, true); + _callback = [callback]() { + schedule_function(callback); + }; + _attach(Milliseconds(milliseconds), true); } // callback will be called at following yield() after ticker fires void attach_ms_scheduled_accurate(uint32_t milliseconds, callback_function_t callback) { - _callback_function = [callback]() { schedule_recurrent_function_us([callback]() { callback(); return false; }, 0); }; - _attach_ms(milliseconds, true); + _callback = [callback]() { + schedule_recurrent_function_us([callback]() { + callback(); + return false; + }, 0); + }; + _attach(Milliseconds(milliseconds), true); } // callback will be called in SYS ctx when ticker fires void attach_ms(uint32_t milliseconds, callback_function_t callback) { - _callback_function = std::move(callback); - _attach_ms(milliseconds, true); + _callback = std::move(callback); + _attach(Milliseconds(milliseconds), true); } - // callback will be called in SYS ctx when ticker fires - template - void attach(float seconds, void (*callback)(TArg), TArg arg) + // instead of copying the callback function, store function pointer and it's argument + // callback will still be called in SYS ctx when ticker fires in N seconds + template + void attach(float seconds, Callback callback, TArg arg) { -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wcast-function-type" - static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(1000UL * seconds, true, reinterpret_cast(callback), reinterpret_cast(arg)); -#pragma GCC diagnostic pop + _callback = callback_data_t { + .arg = reinterpret_cast(arg), + .func = callback, + }; + _attach(Seconds(seconds), true); } - // callback will be called in SYS ctx when ticker fires - template - void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) + // instead of copying the callback function, store function pointer and it's argument + // (that may not be larger than the size of the pointer aka 4bytes) + // callback will still be called in SYS ctx when ticker fires in N milliseconds + template + void attach_ms(uint32_t milliseconds, Callback callback, TArg arg) { -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wcast-function-type" - static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(milliseconds, true, reinterpret_cast(callback), reinterpret_cast(arg)); -#pragma GCC diagnostic pop + _callback = callback_data_t { + .arg = reinterpret_cast(arg), + .func = callback, + }; + _attach(Milliseconds(milliseconds), true); } // callback will be called at following loop() after ticker fires void once_scheduled(float seconds, callback_function_t callback) { - _callback_function = [callback]() { schedule_function(callback); }; - _attach_ms(1000UL * seconds, false); + _callback = [callback]() { schedule_function(callback); }; + _attach(Seconds(seconds), false); } // callback will be called in SYS ctx when ticker fires void once(float seconds, callback_function_t callback) { - _callback_function = std::move(callback); - _attach_ms(1000UL * seconds, false); + _callback = std::move(callback); + _attach(Seconds(seconds), false); } // callback will be called at following loop() after ticker fires void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback) { - _callback_function = [callback]() { schedule_function(callback); }; - _attach_ms(milliseconds, false); + _callback = [callback]() { schedule_function(callback); }; + _attach(Milliseconds(milliseconds), false); } // callback will be called in SYS ctx when ticker fires void once_ms(uint32_t milliseconds, callback_function_t callback) { - _callback_function = std::move(callback); - _attach_ms(milliseconds, false); + _callback = std::move(callback); + _attach(Milliseconds(milliseconds), false); } // callback will be called in SYS ctx when ticker fires - template - void once(float seconds, void (*callback)(TArg), TArg arg) + template + void once(float seconds, Callback callback, TArg arg) { - static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(1000UL * seconds, false, reinterpret_cast(callback), reinterpret_cast(arg)); + _callback = callback_data_t { + .arg = reinterpret_cast(arg), + .func = callback, + }; + _attach(Seconds(seconds), false); } // callback will be called in SYS ctx when ticker fires - template - void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) + template + void once_ms(uint32_t milliseconds, Callback callback, TArg arg) { - static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(milliseconds, false, reinterpret_cast(callback), reinterpret_cast(arg)); + _callback = callback_data_t { + .arg = reinterpret_cast(arg), + .func = callback, + }; + _attach(Milliseconds(milliseconds), false); } void detach(); + bool active() const; + explicit operator bool() const { + return active(); + } + protected: - static void _static_callback(void* arg); - void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg); - void _attach_ms(uint32_t milliseconds, bool repeat) + // internals use this as duration + using Milliseconds = std::chrono::duration>; + + // however, we allow a floating point as input as well + using Seconds = std::chrono::duration>; + + // NONOS SDK timer object duration value range is 5...6870947 (0x68D7A3) + // when that's the case, we split execution into multiple 'ticks' + static constexpr auto DurationMin = Milliseconds(5); + static constexpr auto DurationMax = Milliseconds(6870947); + + struct callback_tick_t { - _attach_ms(milliseconds, repeat, _static_callback, this); + uint32_t total = 0; + uint32_t count = 0; + bool repeat = false; + }; + + void _static_callback(); + + void _attach(Milliseconds milliseconds, bool repeat); + void _attach(Seconds seconds, bool repeat) + { + _attach(std::chrono::duration_cast(seconds), repeat); } - ETSTimer* _timer; - callback_function_t _callback_function = nullptr; + ETSTimer* _timer = nullptr; private: - ETSTimer _etsTimer; + struct callback_ptr_t + { + void* arg = nullptr; + callback_with_arg_t func = nullptr; + }; + + using callback_data_t = std::variant< + std::monostate, + callback_ptr_t, + callback_function_t>; + + callback_data_t _callback = callback_ptr_t{ + .arg = nullptr, + .func = nullptr, + }; + callback_tick_t _tick; + + ETSTimer _timer_internal; }; - - -#endif //TICKER_H From 80e9503b5e7a7e9743d087500637d20cf2dd80ef Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Fri, 1 Jul 2022 01:18:20 +0300 Subject: [PATCH 02/13] detach when not repeating (both cases save original repeat value) --- libraries/Ticker/src/Ticker.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp index ac4935cffb..6540a2320a 100644 --- a/libraries/Ticker/src/Ticker.cpp +++ b/libraries/Ticker/src/Ticker.cpp @@ -43,9 +43,9 @@ void Ticker::_attach(Ticker::Milliseconds milliseconds, bool repeat) // whenever duration excedes this limit, make timer repeatable N times // in case it is really repeatable, it will reset itself and continue as usual _tick = callback_tick_t{}; + _tick.repeat = repeat; if (milliseconds > DurationMax) { - _tick.repeat = repeat; _tick.total += 1; while (milliseconds > DurationMax) { _tick.total *= 2; @@ -85,10 +85,6 @@ void Ticker::_static_callback() if (_tick.count < _tick.total) { return; } - - if (_tick.repeat) { - _tick.count = 0; - } } std::visit([](auto&& callback) { @@ -100,7 +96,11 @@ void Ticker::_static_callback() } }, _callback); - if (_tick.total && _tick.repeat) { + if (_tick.total) { _tick.count = 0; } + + if (!_tick.repeat) { + detach(); + } } From 0a203d9eede3e3b0e3c12c29829a84eecf3527d1 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Fri, 1 Jul 2022 01:31:10 +0300 Subject: [PATCH 03/13] rotate --- libraries/Ticker/src/Ticker.h | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 1a9c2a7954..05320430c3 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -102,9 +102,9 @@ class Ticker template void attach(float seconds, Callback callback, TArg arg) { - _callback = callback_data_t { + _callback = callback_ptr_t{ + .func = reinterpret_cast(callback), .arg = reinterpret_cast(arg), - .func = callback, }; _attach(Seconds(seconds), true); } @@ -115,9 +115,9 @@ class Ticker template void attach_ms(uint32_t milliseconds, Callback callback, TArg arg) { - _callback = callback_data_t { + _callback = callback_ptr_t{ + .func = reinterpret_cast(callback), .arg = reinterpret_cast(arg), - .func = callback, }; _attach(Milliseconds(milliseconds), true); } @@ -154,9 +154,9 @@ class Ticker template void once(float seconds, Callback callback, TArg arg) { - _callback = callback_data_t { + _callback = callback_ptr_t{ + .func = reinterpret_cast(callback), .arg = reinterpret_cast(arg), - .func = callback, }; _attach(Seconds(seconds), false); } @@ -165,9 +165,9 @@ class Ticker template void once_ms(uint32_t milliseconds, Callback callback, TArg arg) { - _callback = callback_data_t { + _callback = callback_ptr_t{ + .func = reinterpret_cast(callback), .arg = reinterpret_cast(arg), - .func = callback, }; _attach(Milliseconds(milliseconds), false); } @@ -212,8 +212,8 @@ class Ticker private: struct callback_ptr_t { - void* arg = nullptr; callback_with_arg_t func = nullptr; + void* arg = nullptr; }; using callback_data_t = std::variant< @@ -221,10 +221,7 @@ class Ticker callback_ptr_t, callback_function_t>; - callback_data_t _callback = callback_ptr_t{ - .arg = nullptr, - .func = nullptr, - }; + callback_data_t _callback; callback_tick_t _tick; ETSTimer _timer_internal; From 89fc3e84ffe31d8be70240fb5ba07c23e1ed6af3 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Fri, 1 Jul 2022 02:04:06 +0300 Subject: [PATCH 04/13] restore static_assert --- libraries/Ticker/src/Ticker.h | 50 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 05320430c3..078a5724bc 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -97,28 +97,26 @@ class Ticker _attach(Milliseconds(milliseconds), true); } + // use function pointer directly. SDK used for argument storage, so we don't support anything larger than 4 bytes + template + using callback_with_typed_arg_t = void(*)(T); + // instead of copying the callback function, store function pointer and it's argument // callback will still be called in SYS ctx when ticker fires in N seconds - template - void attach(float seconds, Callback callback, TArg arg) + template + void attach(float seconds, callback_with_typed_arg_t callback, T arg) { - _callback = callback_ptr_t{ - .func = reinterpret_cast(callback), - .arg = reinterpret_cast(arg), - }; + _callback = callback_ptr_t(callback, arg); _attach(Seconds(seconds), true); } // instead of copying the callback function, store function pointer and it's argument - // (that may not be larger than the size of the pointer aka 4bytes) + // (that **cannot** be larger than the size of the pointer - 4 bytes) // callback will still be called in SYS ctx when ticker fires in N milliseconds - template - void attach_ms(uint32_t milliseconds, Callback callback, TArg arg) + template + void attach_ms(uint32_t milliseconds, callback_with_typed_arg_t callback, T arg) { - _callback = callback_ptr_t{ - .func = reinterpret_cast(callback), - .arg = reinterpret_cast(arg), - }; + _callback = callback_ptr_t(callback, arg); _attach(Milliseconds(milliseconds), true); } @@ -151,24 +149,18 @@ class Ticker } // callback will be called in SYS ctx when ticker fires - template - void once(float seconds, Callback callback, TArg arg) + template + void once(float seconds, callback_with_typed_arg_t callback, T arg) { - _callback = callback_ptr_t{ - .func = reinterpret_cast(callback), - .arg = reinterpret_cast(arg), - }; + _callback = callback_ptr_t(callback, arg); _attach(Seconds(seconds), false); } // callback will be called in SYS ctx when ticker fires - template - void once_ms(uint32_t milliseconds, Callback callback, TArg arg) + template + void once_ms(uint32_t milliseconds, callback_with_typed_arg_t callback, T arg) { - _callback = callback_ptr_t{ - .func = reinterpret_cast(callback), - .arg = reinterpret_cast(arg), - }; + _callback = callback_ptr_t(callback, arg); _attach(Milliseconds(milliseconds), false); } @@ -212,6 +204,14 @@ class Ticker private: struct callback_ptr_t { + template + callback_ptr_t(callback_with_typed_arg_t func, T arg) : + func(reinterpret_cast(func)), + arg(reinterpret_cast(arg)) + { + static_assert(sizeof(T) <= sizeof(void*), ""); + } + callback_with_arg_t func = nullptr; void* arg = nullptr; }; From d06e4fca6c9e23077b88f84fc2a66ebf466c2f91 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sat, 2 Jul 2022 23:38:21 +0300 Subject: [PATCH 05/13] cleanup, types --- libraries/Ticker/src/Ticker.cpp | 10 ++-------- libraries/Ticker/src/Ticker.h | 31 ++++++++++++++++--------------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp index 6540a2320a..85518af4e7 100644 --- a/libraries/Ticker/src/Ticker.cpp +++ b/libraries/Ticker/src/Ticker.cpp @@ -46,7 +46,7 @@ void Ticker::_attach(Ticker::Milliseconds milliseconds, bool repeat) _tick.repeat = repeat; if (milliseconds > DurationMax) { - _tick.total += 1; + _tick.total = 1; while (milliseconds > DurationMax) { _tick.total *= 2; milliseconds /= 2; @@ -62,13 +62,7 @@ void Ticker::detach() if (_timer) { os_timer_disarm(_timer); _timer = nullptr; - - _tick = callback_tick_t{ - .total = 0, - .count = 0, - .repeat = false, - }; - + _tick = callback_tick_t{}; _callback = std::monostate{}; } } diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 078a5724bc..3be40939b4 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -39,18 +39,20 @@ class Ticker detach(); } - // TODO: should re-arm with the new _timer pointer + // TODO disable existing timr? =default to retain backwards compatibility + Ticker(const Ticker&) = default; + Ticker& operator=(const Ticker&) = default; - Ticker(const Ticker&) = delete; - Ticker(Ticker&&) = delete; + // TODO re-arm or disable the timer? =default to retain backwards compatibility + Ticker(Ticker&&) = default; + Ticker& operator=(Ticker&&) = default; - Ticker& operator=(const Ticker&) = delete; - Ticker& operator=(Ticker&&) = delete; - - // native SDK type, simple function with void* argument + // Native SDK type, simple function with void* argument using callback_with_arg_t = void(*)(void*); - // our helper type to support any callable object + // Our helper type to support any callable object + // In case of a lambda with bound variable(s), it will be destroyed + // either when the timer expires or detach() is called using callback_function_t = std::function; // callback will be called at following loop() after ticker fires @@ -97,12 +99,11 @@ class Ticker _attach(Milliseconds(milliseconds), true); } - // use function pointer directly. SDK used for argument storage, so we don't support anything larger than 4 bytes + // only works with a function pointer. argument *may not* be larger than the size of the `void*` template using callback_with_typed_arg_t = void(*)(T); - // instead of copying the callback function, store function pointer and it's argument - // callback will still be called in SYS ctx when ticker fires in N seconds + // callback will still be called in SYS ctx when ticker fires template void attach(float seconds, callback_with_typed_arg_t callback, T arg) { @@ -110,9 +111,7 @@ class Ticker _attach(Seconds(seconds), true); } - // instead of copying the callback function, store function pointer and it's argument - // (that **cannot** be larger than the size of the pointer - 4 bytes) - // callback will still be called in SYS ctx when ticker fires in N milliseconds + // callback will still be called in SYS ctx when ticker fires template void attach_ms(uint32_t milliseconds, callback_with_typed_arg_t callback, T arg) { @@ -164,6 +163,7 @@ class Ticker _attach(Milliseconds(milliseconds), false); } + // if active(), disables currently running timer void detach(); bool active() const; @@ -176,7 +176,8 @@ class Ticker // internals use this as duration using Milliseconds = std::chrono::duration>; - // however, we allow a floating point as input as well + // we allow a floating point as input as well + // float -> u32 has some precision issues, though using Seconds = std::chrono::duration>; // NONOS SDK timer object duration value range is 5...6870947 (0x68D7A3) From 30acb52f1a2c4fac2f9e341948dd2c63c0de2455 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sun, 3 Jul 2022 00:09:46 +0300 Subject: [PATCH 06/13] no min check --- libraries/Ticker/src/Ticker.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 3be40939b4..1adf5f4e72 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -180,9 +180,8 @@ class Ticker // float -> u32 has some precision issues, though using Seconds = std::chrono::duration>; - // NONOS SDK timer object duration value range is 5...6870947 (0x68D7A3) + // NONOS SDK timer object duration cannot be longer than 6870947 (0x68D7A3) // when that's the case, we split execution into multiple 'ticks' - static constexpr auto DurationMin = Milliseconds(5); static constexpr auto DurationMax = Milliseconds(6870947); struct callback_tick_t From e7f8699a3c23917295a0642e84edc783f12f7f1e Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sun, 3 Jul 2022 00:51:31 +0300 Subject: [PATCH 07/13] persist --- libraries/Ticker/src/Ticker.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 1adf5f4e72..b166cd1f67 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -204,12 +204,16 @@ class Ticker private: struct callback_ptr_t { + // XXX undefined behaviour? fix next pr +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-function-type" template callback_ptr_t(callback_with_typed_arg_t func, T arg) : func(reinterpret_cast(func)), arg(reinterpret_cast(arg)) +#pragma GCC diagnostic pop { - static_assert(sizeof(T) <= sizeof(void*), ""); + static_assert(sizeof(T) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); } callback_with_arg_t func = nullptr; From 151c63d90d9ef88d21975e67fa97036b1d052f19 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sun, 3 Jul 2022 01:08:29 +0300 Subject: [PATCH 08/13] syntax --- libraries/Ticker/src/Ticker.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index b166cd1f67..aac783a9b0 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -202,16 +202,15 @@ class Ticker ETSTimer* _timer = nullptr; private: - struct callback_ptr_t - { // XXX undefined behaviour? fix next pr #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wcast-function-type" + struct callback_ptr_t + { template callback_ptr_t(callback_with_typed_arg_t func, T arg) : func(reinterpret_cast(func)), arg(reinterpret_cast(arg)) -#pragma GCC diagnostic pop { static_assert(sizeof(T) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); } @@ -219,6 +218,7 @@ class Ticker callback_with_arg_t func = nullptr; void* arg = nullptr; }; +#pragma GCC diagnostic pop using callback_data_t = std::variant< std::monostate, From 4ad67816a353b879b1e80a6d626f2d8ab6e79a42 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Tue, 1 Nov 2022 17:45:45 +0300 Subject: [PATCH 09/13] optional tick instance --- libraries/Ticker/src/Ticker.cpp | 36 ++++++++++++++++++--------------- libraries/Ticker/src/Ticker.h | 6 ++++-- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp index 85518af4e7..1acddfea66 100644 --- a/libraries/Ticker/src/Ticker.cpp +++ b/libraries/Ticker/src/Ticker.cpp @@ -36,21 +36,24 @@ void Ticker::_attach(Ticker::Milliseconds milliseconds, bool repeat) os_timer_setfn(_timer, [](void* ptr) { - auto* ticker = reinterpret_cast(ptr); - ticker->_static_callback(); + reinterpret_cast(ptr)->_static_callback(); }, this); + _repeat = repeat; + // whenever duration excedes this limit, make timer repeatable N times // in case it is really repeatable, it will reset itself and continue as usual - _tick = callback_tick_t{}; - _tick.repeat = repeat; - + size_t total = 0; if (milliseconds > DurationMax) { - _tick.total = 1; + total = 1; while (milliseconds > DurationMax) { - _tick.total *= 2; + total *= 2; milliseconds /= 2; } + _tick.reset(new callback_tick_t{ + .total = total, + .count = 0, + }); repeat = true; } @@ -62,7 +65,7 @@ void Ticker::detach() if (_timer) { os_timer_disarm(_timer); _timer = nullptr; - _tick = callback_tick_t{}; + _tick.reset(nullptr); _callback = std::monostate{}; } } @@ -74,9 +77,9 @@ bool Ticker::active() const void Ticker::_static_callback() { - if (_tick.total) { - ++_tick.count; - if (_tick.count < _tick.total) { + if (_tick) { + ++_tick->count; + if (_tick->count < _tick->total) { return; } } @@ -90,11 +93,12 @@ void Ticker::_static_callback() } }, _callback); - if (_tick.total) { - _tick.count = 0; + if (_repeat) { + if (_tick) { + _tick->count = 0; + } + return; } - if (!_tick.repeat) { - detach(); - } + detach(); } diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index aac783a9b0..58389860ec 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -188,7 +189,6 @@ class Ticker { uint32_t total = 0; uint32_t count = 0; - bool repeat = false; }; void _static_callback(); @@ -226,7 +226,9 @@ class Ticker callback_function_t>; callback_data_t _callback; - callback_tick_t _tick; + + std::unique_ptr _tick; + bool _repeat = false; ETSTimer _timer_internal; }; From 0843cbfeaf2492a51dffc594dd3bc2234ba69f3b Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Tue, 1 Nov 2022 18:02:18 +0300 Subject: [PATCH 10/13] comments --- libraries/Ticker/src/Ticker.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 58389860ec..2428a65cf4 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -202,9 +202,11 @@ class Ticker ETSTimer* _timer = nullptr; private: - // XXX undefined behaviour? fix next pr #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wcast-function-type" + // original implementation inluded type coersion of integer values that would fit into uintptr_t + // to avoid writing these in our every method, use a generic type that automatically converts it + // (XXX it is a weird hack, though, consider removing this in the future and prever void* instead) struct callback_ptr_t { template From 8c0db60d2ee1ea30559e96c9a6c2afaffd543074 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Tue, 1 Nov 2022 18:57:58 +0300 Subject: [PATCH 11/13] less noisy --- libraries/Ticker/src/Ticker.cpp | 5 ++ libraries/Ticker/src/Ticker.h | 96 +++++++++++++++++---------------- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp index 1acddfea66..c0f9df8b1d 100644 --- a/libraries/Ticker/src/Ticker.cpp +++ b/libraries/Ticker/src/Ticker.cpp @@ -26,6 +26,11 @@ #include #include "Ticker.h" +Ticker::~Ticker() +{ + detach(); +} + void Ticker::_attach(Ticker::Milliseconds milliseconds, bool repeat) { if (_timer) { diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 2428a65cf4..5a335f1877 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -33,12 +33,25 @@ class Ticker { public: - Ticker() = default; + // Our helper type to support any callable object + // In case of a lambda with bound variable(s), it will be destroyed + // either when the timer expires or detach() is called + using callback_function_t = std::function; - ~Ticker() - { - detach(); - } + // Native SDK type, simple function with void* argument + using callback_with_arg_t = void(*)(void*); + + // Helper type to allow type coercion on function argument + // Only works with a function pointer. Argument *must not* be larger than the size of the `void*` + template + using remove_cvref_t = typename std::remove_cv_t< + typename std::remove_reference_t>; + + template > + using callback_with_typed_arg_t = void(*)(Y); + + Ticker() = default; + ~Ticker(); // TODO disable existing timr? =default to retain backwards compatibility Ticker(const Ticker&) = default; @@ -48,14 +61,6 @@ class Ticker Ticker(Ticker&&) = default; Ticker& operator=(Ticker&&) = default; - // Native SDK type, simple function with void* argument - using callback_with_arg_t = void(*)(void*); - - // Our helper type to support any callable object - // In case of a lambda with bound variable(s), it will be destroyed - // either when the timer expires or detach() is called - using callback_function_t = std::function; - // callback will be called at following loop() after ticker fires void attach_scheduled(float seconds, callback_function_t callback) { @@ -100,23 +105,19 @@ class Ticker _attach(Milliseconds(milliseconds), true); } - // only works with a function pointer. argument *may not* be larger than the size of the `void*` - template - using callback_with_typed_arg_t = void(*)(T); - // callback will still be called in SYS ctx when ticker fires - template - void attach(float seconds, callback_with_typed_arg_t callback, T arg) + template + void attach(float seconds, Func func, Arg arg) { - _callback = callback_ptr_t(callback, arg); + _callback = make_callback_ptr(func, arg); _attach(Seconds(seconds), true); } // callback will still be called in SYS ctx when ticker fires - template - void attach_ms(uint32_t milliseconds, callback_with_typed_arg_t callback, T arg) + template + void attach_ms(uint32_t milliseconds, Func func, Arg arg) { - _callback = callback_ptr_t(callback, arg); + _callback = make_callback_ptr(func, arg); _attach(Milliseconds(milliseconds), true); } @@ -149,18 +150,18 @@ class Ticker } // callback will be called in SYS ctx when ticker fires - template - void once(float seconds, callback_with_typed_arg_t callback, T arg) + template + void once(float seconds, Func func, Arg arg) { - _callback = callback_ptr_t(callback, arg); + _callback = make_callback_ptr(func, arg); _attach(Seconds(seconds), false); } // callback will be called in SYS ctx when ticker fires - template - void once_ms(uint32_t milliseconds, callback_with_typed_arg_t callback, T arg) + template + void once_ms(uint32_t milliseconds, Func func, Arg arg) { - _callback = callback_ptr_t(callback, arg); + _callback = make_callback_ptr(func, arg); _attach(Milliseconds(milliseconds), false); } @@ -199,27 +200,31 @@ class Ticker _attach(std::chrono::duration_cast(seconds), repeat); } + std::unique_ptr _tick; + bool _repeat = false; + ETSTimer* _timer = nullptr; private: -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wcast-function-type" - // original implementation inluded type coersion of integer values that would fit into uintptr_t - // to avoid writing these in our every method, use a generic type that automatically converts it - // (XXX it is a weird hack, though, consider removing this in the future and prever void* instead) struct callback_ptr_t { - template - callback_ptr_t(callback_with_typed_arg_t func, T arg) : - func(reinterpret_cast(func)), - arg(reinterpret_cast(arg)) - { - static_assert(sizeof(T) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - } - - callback_with_arg_t func = nullptr; - void* arg = nullptr; + callback_with_arg_t func; + void* arg; }; + + // original implementation inluded type coersion of integer values that would fit into uintptr_t + // to avoid writing these in our every method, use a generic type that automatically converts it + // (XXX it is a weird hack, though, consider removing this in the future and prever void* instead) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-function-type" + template > + static callback_ptr_t make_callback_ptr(callback_with_typed_arg_t func, T arg) { + static_assert(sizeof(Y) <= sizeof(void*), ""); + return callback_ptr_t{ + .func = reinterpret_cast(func), + .arg = reinterpret_cast(arg), + }; + } #pragma GCC diagnostic pop using callback_data_t = std::variant< @@ -229,8 +234,5 @@ class Ticker callback_data_t _callback; - std::unique_ptr _tick; - bool _repeat = false; - ETSTimer _timer_internal; }; From fc5b2a7832951d6d01c2953f969405249307352e Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Tue, 1 Nov 2022 19:24:47 +0300 Subject: [PATCH 12/13] dont copy ticker obj --- .../examples/ServerSentEvents/ServerSentEvents.ino | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/ESP8266WebServer/examples/ServerSentEvents/ServerSentEvents.ino b/libraries/ESP8266WebServer/examples/ServerSentEvents/ServerSentEvents.ino index 7c462295cd..ced3548b3c 100644 --- a/libraries/ESP8266WebServer/examples/ServerSentEvents/ServerSentEvents.ino +++ b/libraries/ESP8266WebServer/examples/ServerSentEvents/ServerSentEvents.ino @@ -146,7 +146,9 @@ void updateSensor(sensorType &sensor) { SSEBroadcastState(sensor.name, sensor.value, newVal); // only broadcast if state is different } sensor.value = newVal; - sensor.update.once(rand() % 20 + 10, std::bind(updateSensor, sensor)); // randomly update sensor + sensor.update.once(rand() % 20 + 10, [&]() { + updateSensor(sensor); + }); // randomly update sensor } void handleSubscribe() { From bd54953aa46597174db66323418b5b21a3be29d8 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Tue, 1 Nov 2022 19:45:26 +0300 Subject: [PATCH 13/13] detach() when copied or moved easier to implement than explaining =default failing the compilation. and it also did not make sense to copy at all, ETSTimer is unique and copy with an active timer will reference will still use the old instance --- .../TickerFunctional/TickerFunctional.ino | 8 +++--- libraries/Ticker/src/Ticker.cpp | 26 +++++++++++++++++++ libraries/Ticker/src/Ticker.h | 13 ++++------ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/libraries/Ticker/examples/TickerFunctional/TickerFunctional.ino b/libraries/Ticker/examples/TickerFunctional/TickerFunctional.ino index a023adfdd8..2d0673a17a 100644 --- a/libraries/Ticker/examples/TickerFunctional/TickerFunctional.ino +++ b/libraries/Ticker/examples/TickerFunctional/TickerFunctional.ino @@ -13,9 +13,11 @@ public: ExampleClass(int pin, int duration) : _pin(pin), _duration(duration) { pinMode(_pin, OUTPUT); - _myTicker.attach_ms(_duration, std::bind(&ExampleClass::classBlink, this)); + _myTicker.attach_ms(_duration, + [this]() { + classBlink(); + }); } - ~ExampleClass(){}; int _pin, _duration; Ticker _myTicker; @@ -53,7 +55,7 @@ void setup() { scheduledTicker.attach_ms_scheduled(100, scheduledBlink); pinMode(LED4, OUTPUT); - parameterTicker.attach_ms(100, std::bind(parameterBlink, LED4)); + parameterTicker.attach_ms(100, parameterBlink, LED4); pinMode(LED5, OUTPUT); lambdaTicker.attach_ms(100, []() { diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp index c0f9df8b1d..baac355ca5 100644 --- a/libraries/Ticker/src/Ticker.cpp +++ b/libraries/Ticker/src/Ticker.cpp @@ -26,11 +26,37 @@ #include #include "Ticker.h" +// ETSTimer is part of the instance, and we don't have any state besides +// the things required for the callback. Allow copies and moves, but +// disable any member copies and default-init + detach() instead. + Ticker::~Ticker() { detach(); } +Ticker::Ticker(const Ticker&) +{ +} + +Ticker& Ticker::operator=(const Ticker&) +{ + detach(); + return *this; +} + +Ticker::Ticker(Ticker&& other) noexcept +{ + other.detach(); +} + +Ticker& Ticker::operator=(Ticker&& other) noexcept +{ + other.detach(); + detach(); + return *this; +} + void Ticker::_attach(Ticker::Milliseconds milliseconds, bool repeat) { if (_timer) { diff --git a/libraries/Ticker/src/Ticker.h b/libraries/Ticker/src/Ticker.h index 5a335f1877..0b61487540 100644 --- a/libraries/Ticker/src/Ticker.h +++ b/libraries/Ticker/src/Ticker.h @@ -53,13 +53,11 @@ class Ticker Ticker() = default; ~Ticker(); - // TODO disable existing timr? =default to retain backwards compatibility - Ticker(const Ticker&) = default; - Ticker& operator=(const Ticker&) = default; + Ticker(const Ticker&); + Ticker& operator=(const Ticker&); - // TODO re-arm or disable the timer? =default to retain backwards compatibility - Ticker(Ticker&&) = default; - Ticker& operator=(Ticker&&) = default; + Ticker(Ticker&&) noexcept; + Ticker& operator=(Ticker&&) noexcept; // callback will be called at following loop() after ticker fires void attach_scheduled(float seconds, callback_function_t callback) @@ -233,6 +231,5 @@ class Ticker callback_function_t>; callback_data_t _callback; - - ETSTimer _timer_internal; + ETSTimer _timer_internal{}; };