v1.8.5 bug fixes

 * incorrect calculation of next task invocation in case callback changed the interval
 * Task::set() method calls setInterval() explicitly, therefore delaying the task in the same manner

1 files changed, 36 insertions, 31 deletions

diff --git a/src/TaskScheduler.h b/src/TaskScheduler.h
index dbe8dba..7ec6f0b 100644
--- a/src/TaskScheduler.h
+++ b/src/TaskScheduler.h
@@ -61,6 +61,10 @@
 // v1.8.4:
 //    2015-11-15 - bug fix: Task alignment with millis() for scheduling purposes should be done after OnEnable, not before. Especially since OnEnable method can change the interval
 //    2015-11-16 - further optimizations of the task scheduler execute loop
+//
+// v1.8.5:
+//    2015-11-23 - bug fix: incorrect calculation of next task invocation in case callback changed the interval
+//    2015-11-23 - bug fix: Task::set() method calls setInterval() explicitly, therefore delaying the task in the same manner
 
 
 /* ============================================
@@ -98,10 +102,10 @@ THE SOFTWARE.
  * The following "defines" control library functionality at compile time,
  * and should be used in the main sketch depending on the functionality required
  * 
- *	#define _TASK_TIMECRITICAL      // Enable monitoring scheduling overruns
- *	#define _TASK_SLEEP_ON_IDLE_RUN // Enable 1 ms SLEEP_IDLE powerdowns between tasks if no callback methods were invoked during the pass 
- *	#define _TASK_STATUS_REQUEST    // Compile with support for StatusRequest functionality - triggering tasks on status change events in addition to time only
- *	#define _TASK_WDT_IDS           // Compile with support for wdt control points and task ids
+ *  #define _TASK_TIMECRITICAL      // Enable monitoring scheduling overruns
+ *  #define _TASK_SLEEP_ON_IDLE_RUN // Enable 1 ms SLEEP_IDLE powerdowns between tasks if no callback methods were invoked during the pass 
+ *  #define _TASK_STATUS_REQUEST    // Compile with support for StatusRequest functionality - triggering tasks on status change events in addition to time only
+ *  #define _TASK_WDT_IDS           // Compile with support for wdt control points and task ids
  *  #define _TASK_LTS_POINTER       // Compile with support for local task storage pointer
  *  #define _TASK_ROLLOVER_FIX		// Compensate for millis() rollover once every 47 days
  */
@@ -134,8 +138,8 @@ class StatusRequest {
 		inline int getStatus() { return iStatus; }
 		
 	private:
-		unsigned int	iCount;  // waiting for more that 65000 events seems unreasonable: unsigned int should be sufficient
-		int				iStatus;  // negative = error;  zero = OK; >positive = OK with a specific status
+		unsigned int	iCount;  					// number of statuses to wait for. waiting for more that 65000 events seems unreasonable: unsigned int should be sufficient
+		int				iStatus;  				// status of the last completed request. negative = error;  zero = OK; >positive = OK with a specific status
 };
 #endif
 
@@ -192,31 +196,31 @@ class Task {
     private:
 		void reset();
 
-		volatile bool			iEnabled;
-		volatile bool			iInOnEnable;
-		volatile unsigned long	iInterval;
-		volatile unsigned long	iPreviousMillis;
+		volatile bool			iEnabled;			// indicates that task is enabled or not. @todo: combine iEnabled, iInOnEnable and iWaiting into one byte since those are bits really.
+		volatile bool			iInOnEnable;		// indicates that task execution is inside OnEnable method (preventing infinite loops)
+		volatile unsigned long	iInterval;			// execution interval in milliseconds. 0 - immediate
+		volatile unsigned long	iPreviousMillis;		// previous invocation time (millis).  Next invocation = iPreviousMillis + iInterval.  Delayed tasks will "catch up" 
 #ifdef _TASK_TIMECRITICAL
-		volatile long			iOverrun; 
+		volatile long			iOverrun; 		// negative if task is "catching up" to it's schedule (next invocation time is already in the past)
 #endif
-		volatile long			iIterations;
-		long					iSetIterations; 
-		unsigned long			iRunCounter;
-		void					(*iCallback)();
-		bool					(*iOnEnable)();
-		void					(*iOnDisable)();
-		Task					*iPrev, *iNext;
-		Scheduler				*iScheduler;
+		volatile long			iIterations;		// number of iterations left. 0 - last iteration. -1 - infinite iterations
+		long					iSetIterations; 		// number of iterations originally requested (for restarts)
+		unsigned long			iRunCounter;	// current number of iteration (starting with 1). Resets on enable. 
+		void					(*iCallback)();		// pointer to the void callback method
+		bool					(*iOnEnable)();	// pointer to the bolol OnEnable callback method
+		void					(*iOnDisable)();	// pointer to the void OnDisable method
+		Task					*iPrev, *iNext;		// pointers to the previous and next tasks in the chain
+		Scheduler				*iScheduler;		// pointer to the current scheduler
 #ifdef _TASK_STATUS_REQUEST
-		StatusRequest			*iStatusRequest;
-		byte					iWaiting;
+		StatusRequest			*iStatusRequest;	// pointer to the status request task is or was waiting on
+		byte					iWaiting;			// indication if task is waiting on the status request
 #endif
 #ifdef _TASK_WDT_IDS
-		unsigned int			iTaskID;
-		unsigned int			iControlPoint;
+		unsigned int			iTaskID;			// task ID (for debugging and watchdog identification)
+		unsigned int			iControlPoint;		// current control point within the callback method. Reset to 0 by scheduler at the beginning of each pass
 #endif
 #ifdef _TASK_LTS_POINTER
-		void					*iLTS;
+		void					*iLTS;			// pointer to task's local storage. Needs to be recast to appropriate type (usually a struct).
 #endif
 };
 
@@ -242,16 +246,16 @@ class Scheduler {
 #endif
 
 	private:
-		Task	*iFirst, *iLast, *iCurrent;
+		Task	*iFirst, *iLast, *iCurrent;			// pointers to first, last and current tasks in the chain
 #ifdef _TASK_SLEEP_ON_IDLE_RUN
-		bool	iAllowSleep;
+		bool	iAllowSleep;						// indication if putting avr to IDLE_SLEEP mode is allowed by the program at this time. 
 #endif
 };
 
 
 // ------------------ TaskScheduler implementation --------------------
 #ifdef _TASK_WDT_IDS
-	static unsigned int __task_id_counter = 0;
+	static unsigned int __task_id_counter = 0;		// global task ID counter for assiging task IDs automatically. 
 #endif
 /** Constructor, uses default values for the parameters
  * so could be called with no parameters.
@@ -361,7 +365,7 @@ void Task::reset() {
  * @param aOnDisable - pointer to the callback method which is called on disable()
  */
 void Task::set(unsigned long aInterval, long aIterations, void (*aCallback)(),bool (*aOnEnable)(), void (*aOnDisable)()) {
-	iInterval = aInterval;
+	setInterval(aInterval); 
 	iSetIterations = iIterations = aIterations;
 	iCallback = aCallback;
 	iOnEnable = aOnEnable;
@@ -591,9 +595,8 @@ void Scheduler::execute() {
 					break;
 				}
 				m = millis();
-				p = iCurrent->iPreviousMillis;
 				i = iCurrent->iInterval;
-				#ifdef  _TASK_STATUS_REQUEST
+	#ifdef  _TASK_STATUS_REQUEST
 	// If StatusRequest object was provided, and still pending, and task is waiting, this task should not run
 	// Otherwise, continue with execution as usual.  Tasks waiting to StatusRequest need to be rescheduled according to 
 	// how they were placed into waiting state (waitFor or waitForDelayed)
@@ -603,6 +606,8 @@ void Scheduler::execute() {
 					iCurrent->iWaiting = 0;
 				}
 	#endif
+				p = iCurrent->iPreviousMillis;
+
 	// Determine when current task is supposed to run
 	// Once every 47 days there is a rollover execution which will occur due to millis and targetMillis rollovers
 	// That is why there is an option to compile with rollover fix
@@ -631,13 +636,13 @@ void Scheduler::execute() {
 	#endif
 				if ( iCurrent->iIterations > 0 ) iCurrent->iIterations--;  // do not decrement (-1) being a signal of never-ending task
 				iCurrent->iRunCounter++;
+				iCurrent->iPreviousMillis = targetMillis; //p + i
 				if ( iCurrent->iCallback ) {
 					( *(iCurrent->iCallback) )();
 	#ifdef _TASK_SLEEP_ON_IDLE_RUN
 					idleRun = false;
 	#endif
 				}
-				iCurrent->iPreviousMillis = targetMillis;
 			}
 		} while (0); //guaranteed single run - allows use of "break" to exit 
 		iCurrent = iCurrent->iNext;