From 617fdf0c1c9f09a1c3ceb9e3c73493f57497a9e3 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Thu, 30 May 2019 00:33:18 +0200 Subject: [PATCH 01/12] Use placement new for ETSTimer - no heap fragmentation, new/delete semantics unchanged. --- libraries/Ticker/Ticker.cpp | 4 ++-- libraries/Ticker/Ticker.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index b53b429dab..163f870112 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -50,7 +50,7 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t } else { - _timer = new ETSTimer; + _timer = new(_etsTimerMemory) ETSTimer; } os_timer_setfn(_timer, reinterpret_cast(callback), reinterpret_cast(arg)); @@ -63,7 +63,7 @@ void Ticker::detach() return; os_timer_disarm(_timer); - delete _timer; + _timer->~ETSTimer(); _timer = nullptr; _callback_function = nullptr; } diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 8ecc47581f..ae175b7b82 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -129,6 +129,7 @@ class Ticker protected: ETSTimer* _timer; + char _etsTimerMemory[sizeof(ETSTimer)]; callback_function_t _callback_function = nullptr; }; From e7af1c3ff30ea627633338164a542626a00e4e5b Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Thu, 30 May 2019 09:38:18 +0200 Subject: [PATCH 02/12] Make change completely invisible to derived classes at compile-time. --- libraries/Ticker/Ticker.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index ae175b7b82..ccd00ceefc 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -129,8 +129,10 @@ class Ticker protected: ETSTimer* _timer; - char _etsTimerMemory[sizeof(ETSTimer)]; callback_function_t _callback_function = nullptr; + +private: + char _etsTimerMemory[sizeof(ETSTimer)]; }; From 8dfb2fcfff42d863e7927f9ba9988d541f51afdc Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Fri, 31 May 2019 10:31:29 +0200 Subject: [PATCH 03/12] Fix "sizeof() incomplete type ETSTimer" error. --- libraries/Ticker/Ticker.cpp | 1 - libraries/Ticker/Ticker.h | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index 163f870112..915960c5d1 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -24,7 +24,6 @@ #include "c_types.h" #include "eagle_soc.h" -#include "ets_sys.h" #include "osapi.h" static const int ONCE = 0; diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index ccd00ceefc..5122b8041e 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -27,10 +27,7 @@ #include #include #include - -extern "C" { - typedef struct _ETSTIMER_ ETSTimer; -} +#include class Ticker { From c2bb84e69d5a22808271f359ae1514c44d967a7b Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Fri, 31 May 2019 10:38:12 +0200 Subject: [PATCH 04/12] C++ reinterpret_cast<> instead of C-style cast. void* instead of uint32_t - fixes x86_64 server compiles. --- libraries/Ticker/Ticker.cpp | 6 +++--- libraries/Ticker/Ticker.h | 30 +++++++++++++----------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index 915960c5d1..b21237f935 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -41,7 +41,7 @@ Ticker::~Ticker() detach(); } -void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg) +void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg) { if (_timer) { @@ -52,7 +52,7 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t _timer = new(_etsTimerMemory) ETSTimer; } - os_timer_setfn(_timer, reinterpret_cast(callback), reinterpret_cast(arg)); + os_timer_setfn(_timer, reinterpret_cast(callback), arg); os_timer_arm(_timer, milliseconds, (repeat)?REPEAT:ONCE); } @@ -74,7 +74,7 @@ bool Ticker::active() const void Ticker::_static_callback(void* arg) { - Ticker* _this = (Ticker*)arg; + Ticker* _this = reinterpret_cast(arg); if (_this == nullptr) { return; diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 5122b8041e..696b80f94d 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -46,7 +46,7 @@ class Ticker void attach(float seconds, callback_function_t callback) { _callback_function = callback; - attach(seconds, _static_callback, (void*)this); + attach(seconds, _static_callback, static_cast(this)); } void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -57,26 +57,24 @@ class Ticker void attach_ms(uint32_t milliseconds, callback_function_t callback) { _callback_function = callback; - attach_ms(milliseconds, _static_callback, (void*)this); + attach_ms(milliseconds, _static_callback, static_cast(this)); } template void attach(float seconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes"); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); // C-cast serves two purposes: // static_cast for smaller integer types, // reinterpret_cast + const_cast for pointer types - uint32_t arg32 = (uint32_t)arg; - _attach_ms(seconds * 1000, true, reinterpret_cast(callback), arg32); + _attach_ms(seconds * 1000, true, reinterpret_cast(callback), (void*)arg); } template void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes"); - uint32_t arg32 = (uint32_t)arg; - _attach_ms(milliseconds, true, reinterpret_cast(callback), arg32); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); + _attach_ms(milliseconds, true, reinterpret_cast(callback), (void*)arg); } void once_scheduled(float seconds, callback_function_t callback) @@ -87,7 +85,7 @@ class Ticker void once(float seconds, callback_function_t callback) { _callback_function = callback; - once(seconds, _static_callback, (void*)this); + once(seconds, _static_callback, static_cast(this)); } void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -98,30 +96,28 @@ class Ticker void once_ms(uint32_t milliseconds, callback_function_t callback) { _callback_function = callback; - once_ms(milliseconds, _static_callback, (void*)this); + once_ms(milliseconds, _static_callback, static_cast(this)); } template void once(float seconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes"); - uint32_t arg32 = (uint32_t)(arg); - _attach_ms(seconds * 1000, false, reinterpret_cast(callback), arg32); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); + _attach_ms(seconds * 1000, false, reinterpret_cast(callback), (void*)arg); } template void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes"); - uint32_t arg32 = (uint32_t)(arg); - _attach_ms(milliseconds, false, reinterpret_cast(callback), arg32); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); + _attach_ms(milliseconds, false, reinterpret_cast(callback), (void*)arg); } void detach(); bool active() const; protected: - void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg); + void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg); static void _static_callback (void* arg); protected: From c6c418363eff35469ab34367f54ec6539da88d2a Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sat, 1 Jun 2019 14:13:17 +0200 Subject: [PATCH 05/12] Simplify casts. --- libraries/Ticker/Ticker.cpp | 26 ++++++++++---------------- libraries/Ticker/Ticker.h | 37 ++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 35 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index b21237f935..17f8551a4e 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -1,9 +1,9 @@ -/* +/* Ticker.cpp - esp8266 library that calls functions periodically Copyright (c) 2014 Ivan Grokhotkov. All rights reserved. This file is part of the esp8266 core for Arduino environment. - + This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either @@ -26,13 +26,13 @@ #include "eagle_soc.h" #include "osapi.h" -static const int ONCE = 0; +static const int ONCE = 0; static const int REPEAT = 1; #include "Ticker.h" Ticker::Ticker() -: _timer(nullptr) + : _timer(nullptr) { } @@ -49,11 +49,11 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t } else { - _timer = new(_etsTimerMemory) ETSTimer; + _timer = new(&_etsTimer) ETSTimer; } - os_timer_setfn(_timer, reinterpret_cast(callback), arg); - os_timer_arm(_timer, milliseconds, (repeat)?REPEAT:ONCE); + os_timer_setfn(_timer, callback, arg); + os_timer_arm(_timer, milliseconds, (repeat) ? REPEAT : ONCE); } void Ticker::detach() @@ -62,8 +62,8 @@ void Ticker::detach() return; os_timer_disarm(_timer); - _timer->~ETSTimer(); _timer = nullptr; + _etsTimer.~ETSTimer(); _callback_function = nullptr; } @@ -75,12 +75,6 @@ bool Ticker::active() const void Ticker::_static_callback(void* arg) { Ticker* _this = reinterpret_cast(arg); - if (_this == nullptr) - { - return; - } - if (_this->_callback_function) - { - _this->_callback_function(); - } + if (!_this) return; + if (_this->_callback_function) _this->_callback_function(); } diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 696b80f94d..7c05837ffe 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -1,9 +1,9 @@ -/* +/* Ticker.h - esp8266 library that calls functions periodically Copyright (c) 2014 Ivan Grokhotkov. All rights reserved. This file is part of the esp8266 core for Arduino environment. - + This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either @@ -34,7 +34,7 @@ class Ticker public: Ticker(); ~Ticker(); - typedef void (*callback_t)(void); + typedef void (*callback_with_arg_t)(void*); typedef std::function callback_function_t; @@ -45,8 +45,8 @@ class Ticker void attach(float seconds, callback_function_t callback) { - _callback_function = callback; - attach(seconds, _static_callback, static_cast(this)); + _callback_function = std::move(callback); + _attach_ms(seconds * 1000, true, _static_callback, this); } void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -56,8 +56,8 @@ class Ticker void attach_ms(uint32_t milliseconds, callback_function_t callback) { - _callback_function = callback; - attach_ms(milliseconds, _static_callback, static_cast(this)); + _callback_function = std::move(callback); + _attach_ms(milliseconds, true, _static_callback, this); } template @@ -67,14 +67,14 @@ class Ticker // C-cast serves two purposes: // static_cast for smaller integer types, // reinterpret_cast + const_cast for pointer types - _attach_ms(seconds * 1000, true, reinterpret_cast(callback), (void*)arg); + _attach_ms(seconds * 1000, true, callback, arg); } template void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(milliseconds, true, reinterpret_cast(callback), (void*)arg); + _attach_ms(milliseconds, true, callback, arg); } void once_scheduled(float seconds, callback_function_t callback) @@ -84,8 +84,8 @@ class Ticker void once(float seconds, callback_function_t callback) { - _callback_function = callback; - once(seconds, _static_callback, static_cast(this)); + _callback_function = std::move(callback); + _attach_ms(seconds * 1000, false, _static_callback, this); } void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -95,37 +95,36 @@ class Ticker void once_ms(uint32_t milliseconds, callback_function_t callback) { - _callback_function = callback; - once_ms(milliseconds, _static_callback, static_cast(this)); + _callback_function = std::move(callback); + _attach_ms(milliseconds, false, _static_callback, this); } template void once(float seconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(seconds * 1000, false, reinterpret_cast(callback), (void*)arg); + _attach_ms(seconds * 1000, false, callback, arg); } template void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(milliseconds, false, reinterpret_cast(callback), (void*)arg); + _attach_ms(milliseconds, false, callback, arg); } void detach(); bool active() const; -protected: +protected: void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg); - static void _static_callback (void* arg); + static void _static_callback(void* arg); -protected: ETSTimer* _timer; callback_function_t _callback_function = nullptr; private: - char _etsTimerMemory[sizeof(ETSTimer)]; + ETSTimer _etsTimer; }; From 64b4d0fad92572be93d8be348d4534c33b3b79dd Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sat, 1 Jun 2019 23:41:23 +0200 Subject: [PATCH 06/12] Revert to complete placement new treatment of ETSTimer member. --- libraries/Ticker/Ticker.cpp | 4 ++-- libraries/Ticker/Ticker.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index 17f8551a4e..62dabe58aa 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -49,7 +49,7 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t } else { - _timer = new(&_etsTimer) ETSTimer; + _timer = new(_etsTimerMem) ETSTimer; } os_timer_setfn(_timer, callback, arg); @@ -62,8 +62,8 @@ void Ticker::detach() return; os_timer_disarm(_timer); + _timer->~ETSTimer(); _timer = nullptr; - _etsTimer.~ETSTimer(); _callback_function = nullptr; } diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 7c05837ffe..3c4020fc1c 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -124,7 +124,7 @@ class Ticker callback_function_t _callback_function = nullptr; private: - ETSTimer _etsTimer; + char _etsTimerMem[sizeof(ETSTimer)]; }; From 6dad1acbf21069bc274d3088e945550e0bc48279 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sun, 2 Jun 2019 01:04:20 +0200 Subject: [PATCH 07/12] Cleanup includes --- libraries/Ticker/Ticker.cpp | 5 +---- libraries/Ticker/Ticker.h | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index 62dabe58aa..fe0008a016 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -19,9 +19,6 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#include -#include - #include "c_types.h" #include "eagle_soc.h" #include "osapi.h" @@ -69,7 +66,7 @@ void Ticker::detach() bool Ticker::active() const { - return (bool)_timer; + return _timer; } void Ticker::_static_callback(void* arg) diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 3c4020fc1c..2bfdfde310 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -22,9 +22,6 @@ #ifndef TICKER_H #define TICKER_H -#include -#include -#include #include #include #include From 55c1c24da84ce7a3f07fe92469afd60dd0a95a4b Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sun, 2 Jun 2019 12:49:34 +0200 Subject: [PATCH 08/12] Fix omitted casts --- libraries/Ticker/Ticker.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 2bfdfde310..5ac814ee21 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -64,14 +64,14 @@ class Ticker // C-cast serves two purposes: // static_cast for smaller integer types, // reinterpret_cast + const_cast for pointer types - _attach_ms(seconds * 1000, true, callback, arg); + _attach_ms(seconds * 1000, true, reinterpret_cast(callback), (void*)arg); } template void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(milliseconds, true, callback, arg); + _attach_ms(milliseconds, true, reinterpret_cast(callback), (void*)arg); } void once_scheduled(float seconds, callback_function_t callback) @@ -100,14 +100,14 @@ class Ticker void once(float seconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(seconds * 1000, false, callback, arg); + _attach_ms(seconds * 1000, false, reinterpret_cast(callback), (void*)arg); } template void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(milliseconds, false, callback, arg); + _attach_ms(milliseconds, false, reinterpret_cast(callback), (void*)arg); } void detach(); From 3d8ad6dd62a86b9ddbb3e0581786815a320ece5e Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sun, 2 Jun 2019 18:50:25 +0200 Subject: [PATCH 09/12] Change per review https://github.com/esp8266/Arduino/pull/6164#pullrequestreview-243583458 --- libraries/Ticker/Ticker.cpp | 5 +++-- libraries/Ticker/Ticker.h | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index fe0008a016..920c277535 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -46,7 +46,8 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t } else { - _timer = new(_etsTimerMem) ETSTimer; + //_timer = new(_etsTimerMem) ETSTimer; + _timer = _etsTimer; } os_timer_setfn(_timer, callback, arg); @@ -59,7 +60,7 @@ void Ticker::detach() return; os_timer_disarm(_timer); - _timer->~ETSTimer(); + //_timer->~ETSTimer(); _timer = nullptr; _callback_function = nullptr; } diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 5ac814ee21..a5aee0cf74 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -121,7 +121,8 @@ class Ticker callback_function_t _callback_function = nullptr; private: - char _etsTimerMem[sizeof(ETSTimer)]; + //char _etsTimerMem[sizeof(ETSTimer)]; + ETSTimer _etsTimer; }; From d941a9f679df24763a569c0308e3652034b016fb Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Mon, 3 Jun 2019 11:59:48 +0200 Subject: [PATCH 10/12] wtf - local compile didn't catch this sloppy mistake --- libraries/Ticker/Ticker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index 920c277535..a764b3f5c8 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -47,7 +47,7 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t else { //_timer = new(_etsTimerMem) ETSTimer; - _timer = _etsTimer; + _timer = &_etsTimer; } os_timer_setfn(_timer, callback, arg); From 12805431e23cffc669ad783b32017de1b09aa552 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Tue, 4 Jun 2019 22:40:10 +0200 Subject: [PATCH 11/12] Resolves review https://github.com/esp8266/Arduino/pull/6164#discussion_r290388119 --- libraries/Ticker/Ticker.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index a764b3f5c8..e83c1f4694 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -23,11 +23,14 @@ #include "eagle_soc.h" #include "osapi.h" -static const int ONCE = 0; -static const int REPEAT = 1; - #include "Ticker.h" +namespace +{ + constexpr int ONCE = 0; + constexpr int REPEAT = 1; +} + Ticker::Ticker() : _timer(nullptr) { @@ -46,7 +49,6 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t } else { - //_timer = new(_etsTimerMem) ETSTimer; _timer = &_etsTimer; } @@ -60,7 +62,6 @@ void Ticker::detach() return; os_timer_disarm(_timer); - //_timer->~ETSTimer(); _timer = nullptr; _callback_function = nullptr; } @@ -73,6 +74,6 @@ bool Ticker::active() const void Ticker::_static_callback(void* arg) { Ticker* _this = reinterpret_cast(arg); - if (!_this) return; - if (_this->_callback_function) _this->_callback_function(); + if (_this && _this->_callback_function) + _this->_callback_function(); } From e82de3486adfa8a8fdaa7a974f7d1e862a837db8 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Tue, 4 Jun 2019 23:04:13 +0200 Subject: [PATCH 12/12] Reviewer stated that floating point operations are inlined, software operations - reduce number of code spots to one. --- libraries/Ticker/Ticker.cpp | 5 +++++ libraries/Ticker/Ticker.h | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index e83c1f4694..35ec21cdcd 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -41,6 +41,11 @@ Ticker::~Ticker() detach(); } +void Ticker::_attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg) +{ + _attach_ms(1000 * seconds, repeat, callback, arg); +} + void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg) { if (_timer) diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index a5aee0cf74..f8e71721c5 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -43,7 +43,7 @@ class Ticker void attach(float seconds, callback_function_t callback) { _callback_function = std::move(callback); - _attach_ms(seconds * 1000, true, _static_callback, this); + _attach_s(seconds, true, _static_callback, this); } void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -64,7 +64,7 @@ class Ticker // C-cast serves two purposes: // static_cast for smaller integer types, // reinterpret_cast + const_cast for pointer types - _attach_ms(seconds * 1000, true, reinterpret_cast(callback), (void*)arg); + _attach_s(seconds, true, reinterpret_cast(callback), (void*)arg); } template @@ -82,7 +82,7 @@ class Ticker void once(float seconds, callback_function_t callback) { _callback_function = std::move(callback); - _attach_ms(seconds * 1000, false, _static_callback, this); + _attach_s(seconds, false, _static_callback, this); } void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -100,7 +100,7 @@ class Ticker void once(float seconds, void (*callback)(TArg), TArg arg) { static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); - _attach_ms(seconds * 1000, false, reinterpret_cast(callback), (void*)arg); + _attach_s(seconds, false, reinterpret_cast(callback), (void*)arg); } template @@ -121,6 +121,7 @@ class Ticker callback_function_t _callback_function = nullptr; private: + void _attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg); //char _etsTimerMem[sizeof(ETSTimer)]; ETSTimer _etsTimer; };