diff --git a/.github/workflows/aunit_tests.yml b/.github/workflows/aunit_tests.yml index 9b613ee..78014b5 100644 --- a/.github/workflows/aunit_tests.yml +++ b/.github/workflows/aunit_tests.yml @@ -5,10 +5,10 @@ on: [push, pull_request] jobs: build: - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Build run: | diff --git a/.github/workflows/githubci.yml b/.github/workflows/githubci.yml index 913acae..90fd695 100644 --- a/.github/workflows/githubci.yml +++ b/.github/workflows/githubci.yml @@ -7,11 +7,11 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/setup-python@v1 + - uses: actions/setup-python@v4 with: python-version: '3.x' - - uses: actions/checkout@v2 - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 + - uses: actions/checkout@v3 with: repository: adafruit/ci-arduino path: ci @@ -20,8 +20,8 @@ jobs: run: bash ci/actions_install.sh - name: test platforms - #run: python3 ci/build_platform.py main_platforms - run: python3 ci/build_platform.py main_platforms esp32 esp8266 + run: python3 ci/build_platform.py main_platforms + #run: python3 ci/build_platform.py main_platforms esp32 esp8266 #run: python3 ci/build_platform.py main_platforms esp32 esp8266 nrf52840 # disabled for now. diff --git a/README.md b/README.md index fbd2d7b..d22fdd1 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Timer<>::Task every(unsigned long interval, handler_t handler, T opaque = T()); /* Cancel a timer task */ -void cancel(Timer<>::Task &task); +bool cancel(Timer<>::Task &task); /* Cancel all tasks */ void cancel(); diff --git a/extras/tests/timerMultiCancelTest/Makefile b/extras/tests/timerMultiCancelTest/Makefile new file mode 100644 index 0000000..95242c9 --- /dev/null +++ b/extras/tests/timerMultiCancelTest/Makefile @@ -0,0 +1,9 @@ +# See https://github.com/bxparks/UnixHostDuino for documentation about this +# Makefile to compile and run Arduino programs natively on Linux or MacOS. +# + +APP_NAME := timerMultiCancelTest +ARDUINO_LIBS := AUnit arduino-timer +CPPFLAGS += -Werror + +include $(UNIXHOSTDUINO) diff --git a/extras/tests/timerMultiCancelTest/timerMultiCancelTest.ino b/extras/tests/timerMultiCancelTest/timerMultiCancelTest.ino new file mode 100644 index 0000000..965e1e4 --- /dev/null +++ b/extras/tests/timerMultiCancelTest/timerMultiCancelTest.ino @@ -0,0 +1,173 @@ +// arduino-timer unit tests +// timerMultiCancelTest.ino unit test +// find regressions when: +// timer.cancel(...) is called twice and cancels an extra task +// timer.in/at/every(...) return task id 0 (should mean "not created") +// timer.in/at/every(...) return an in-use task id (1 id with 2 tasks) + +// Arduino "AUnit" library required + +// Required for UnixHostDuino emulation +#include + +#if defined(UNIX_HOST_DUINO) +#ifndef ARDUINO +#define ARDUINO 100 +#endif +#endif + +#include +#include + +//#define DEBUG +#ifdef DEBUG +#define DEBUG_PRINT(x) Serial.print(x) +#define DEBUG_PRINTLN(x) Serial.println(x) +#else +#define DEBUG_PRINT(x) +#define DEBUG_PRINTLN(x) +#endif + +auto timer = timer_create_default(); // create a timer with default settings + +// a generic task +bool dummyTask(void*) { + //digitalWrite(LED_BUILTIN, !digitalRead(LED_BUILTIN)); // toggle the LED + return true; // repeat? true +} + +struct TaskInfo { + Timer<>::Task id; // id to use to cancel + unsigned long instanceNumber; +}; + +static const int numTaskIds = 20; + +TaskInfo tasksToCancel[numTaskIds] = { { 0, 0 } }; + +unsigned long creationAttempts = 0; + +bool createTask(int index) { + bool successful = true; // OK so far + TaskInfo& taskInfo = tasksToCancel[index]; + + if (taskInfo.id != (Timer<>::Task)NULL) { + // cancel task in slot + auto staleId = taskInfo.id; + int beforeSize = (int)timer.size(); + successful &= timer.cancel(taskInfo.id); + int afterSize = (int)timer.size(); + successful &= (afterSize == beforeSize - 1); + if (!successful) { + Serial.println(F("could not cancel a task")); + } else { + + successful &= !timer.cancel(staleId); // double cancel should not hit another task + int afterSize2 = (int)timer.size(); + successful &= (afterSize2 == beforeSize - 1); + if (!successful) { + Serial.println(F("second cancel removed another task")); + } + } + } + + auto newId = timer.every(250, dummyTask); + ++creationAttempts; + + static size_t timerSize = 0; + auto newSize = timer.size(); + if (timerSize != newSize) { + timerSize = newSize; + DEBUG_PRINT(F("Timer now has ")); + DEBUG_PRINT(timerSize); + DEBUG_PRINTLN(F(" tasks")); + } + + if (newId == 0) { + Serial.print(F("timer task creation failure on creation number ")); + Serial.println(creationAttempts); + successful = false; + + } else { + + // check for collisions before saving taskInfo + for (int i = 0; i < numTaskIds; i++) { + const TaskInfo& ti = tasksToCancel[i]; + if (ti.id == newId) { + successful = false; + Serial.print(F("COLLISION FOUND! instance number: ")); + Serial.print(creationAttempts); + Serial.print(F(" hash ")); + Serial.print(F("0x")); + Serial.print((size_t)newId, HEX); + Serial.print(F(" ")); + Serial.print((size_t)newId, BIN); + + Serial.print(F(" matches hash for instance number: ")); + Serial.println(ti.instanceNumber); + } + } + taskInfo.id = newId; + taskInfo.instanceNumber = creationAttempts; + + static const unsigned long reportCountTime = 10000; + if (creationAttempts % reportCountTime == 0) { + DEBUG_PRINT(creationAttempts / 1000); + DEBUG_PRINTLN(F("k tasks created.")); + } + } + return successful; +} + +test(timerMultiCancel) { + timer.cancel(); // ensure timer starts empty + assertEqual((int)timer.size(), 0); + creationAttempts = 0; + + // timer capacity is 0x10 -- stay below + // load up some static tasks + for (int i = 0; i < 6; i++) { + assertTrue(createTask(i)); + } + + assertEqual((int)timer.size(), 6); + + // cancel/recreate tasks + //for (unsigned long groups = 0; groups < 30000UL; groups++) { + unsigned long groups = 0; + do { + //for (unsigned long groups = 0; creationAttempts < 0x10010; groups++) { // trouble over 64k tasks? + for (int i = 9; i < 0x10; i++) { + assertTrue(createTask(i)); + if (groups > 0) { + // should be steady-state task size now + assertEqual((int)timer.size(), 13); + } + } + groups++; + //} + } while (creationAttempts < 0x10010); // no trouble over 64K tasks? + + Serial.print(creationAttempts); + Serial.println(F(" tasks created.")); +} + +void sketch(void) { + Serial.println(); + Serial.println(F("Running " __FILE__ ", Built " __DATE__)); +} + +void setup() { + ::delay(1000UL); // wait for stability on some boards to prevent garbage Serial + Serial.begin(115200UL); // ESP8266 default of 74880 not supported on Linux + while (!Serial) + ; // for the Arduino Leonardo/Micro only + sketch(); +} + +void loop() { + // Should get: + // TestRunner summary: + // passed, failed, skipped, timed out, out of test(s). + aunit::TestRunner::run(); +} diff --git a/extras/tests/unitTest/unitTest.ino b/extras/tests/unitTest/unitTest.ino index 0c7a20c..dfb962d 100644 --- a/extras/tests/unitTest/unitTest.ino +++ b/extras/tests/unitTest/unitTest.ino @@ -262,7 +262,11 @@ test(timer_cancel) { // assert task has not run assertEqual(task.runs, 0UL); - timer.cancel(r); + auto stale_r = r; + const bool success = timer.cancel(r); + + // assert task found + assertEqual(success, true); // assert task cleared assertEqual((unsigned long)r, 0UL); @@ -275,6 +279,15 @@ test(timer_cancel) { // assert task did not run assertEqual(timer.tick(), 0UL); assertEqual(task.runs, 0UL); + + const bool fail = timer.cancel(r); + + // assert task not found + assertEqual(fail, false); + + // stale task pointer has nothing to cancel + const bool stale_cancel_fail = timer.cancel(stale_r); + assertEqual(stale_cancel_fail, false); } test(timer_cancel_all) { @@ -365,23 +378,23 @@ test(timer_size) { Timer<0x2, CLOCK::millis, Task *> timer; - assertEqual(timer.size(), 0UL); + assertEqual((unsigned long) timer.size(), 0UL); auto t = make_task(); auto r = timer.in(0UL, handler, &t); assertNotEqual((unsigned long)r, 0UL); - assertEqual(timer.size(), 1UL); + assertEqual((unsigned long) timer.size(), 1UL); r = timer.in(0UL, handler, &t); assertNotEqual((unsigned long)r, 0UL); - assertEqual(timer.size(), 2UL); + assertEqual((unsigned long) timer.size(), 2UL); timer.cancel(); - assertEqual(timer.size(), 0UL); + assertEqual((unsigned long) timer.size(), 0UL); } test(timer_empty) { diff --git a/library.properties b/library.properties index 8133441..ccb2f79 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=arduino-timer -version=2.3.0 +version=3.0.1 author=Michael Contreras maintainer=Michael Contreras diff --git a/src/arduino-timer.h b/src/arduino-timer.h index c87b06b..f9e6a76 100644 --- a/src/arduino-timer.h +++ b/src/arduino-timer.h @@ -35,11 +35,7 @@ #ifndef _CM_ARDUINO_TIMER_H__ #define _CM_ARDUINO_TIMER_H__ -#if defined(ARDUINO) && ARDUINO >= 100 #include -#else -#include -#endif #include @@ -64,14 +60,14 @@ template < class Timer { public: - typedef uintptr_t Task; /* public task handle */ + typedef void * Task; /* public task handle */ typedef bool (*handler_t)(T opaque); /* task handler func signature */ /* Calls handler with opaque as argument in delay units of time */ Task in(unsigned long delay, handler_t h, T opaque = T()) { - return task_id(add_task(time_func(), delay, h, opaque)); + return add_task(time_func(), delay, h, opaque); } /* Calls handler with opaque as argument at time */ @@ -79,30 +75,29 @@ class Timer { at(unsigned long time, handler_t h, T opaque = T()) { const unsigned long now = time_func(); - return task_id(add_task(now, time - now, h, opaque)); + return add_task(now, time - now, h, opaque); } /* Calls handler with opaque as argument every interval units of time */ Task every(unsigned long interval, handler_t h, T opaque = T()) { - return task_id(add_task(time_func(), interval, h, opaque, interval)); + return add_task(time_func(), interval, h, opaque, interval); } /* Cancel the timer task */ - void + bool cancel(Task &task) { - if (!task) return; + struct task * const t = static_cast(task); - timer_foreach_task(t) { - if (t->handler && task_id(t) == task) { - remove(t); - break; - } + if (t && (tasks <= t) && (t < tasks + max_tasks) && t->handler) { + remove(t); + task = static_cast(NULL); + return true; } - task = static_cast(NULL); + return false; } /* Cancel all timer tasks */ @@ -194,19 +189,16 @@ class Timer { return true; } - Timer() : ctr(0), tasks{} {} + Timer() : tasks{} {} private: - size_t ctr; - struct task { handler_t handler; /* task handler callback func */ T opaque; /* argument given to the callback handler */ unsigned long start, - expires; /* when the task expires */ - size_t repeat, /* repeat task */ - id; + expires, /* when the task expires */ + repeat; /* repeat task */ } tasks[max_tasks]; inline @@ -218,16 +210,6 @@ class Timer { task->start = 0; task->expires = 0; task->repeat = 0; - task->id = 0; - } - - inline - Task - task_id(const struct task * const t) - { - const Task id = reinterpret_cast(t); - - return id ? id ^ t->id : id; } inline @@ -250,9 +232,6 @@ class Timer { if (!slot) return NULL; - if (++ctr == 0) ++ctr; // overflow - - slot->id = ctr; slot->handler = h; slot->opaque = opaque; slot->start = start; diff --git a/src/timer.h b/src/timer.h index dbe114c..6e5a669 100644 --- a/src/timer.h +++ b/src/timer.h @@ -1,3 +1,3 @@ -#warning "Including this file is deprecated. Please #include instead." +#error "Including this file is deprecated. Please #include instead." #include