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

6 files changed, 117 insertions, 47 deletions

diff --git a/README b/README
index 0505da1..b29b076 100644
--- a/README
+++ b/README
@@ -1,5 +1,5 @@
 Task Scheduler – cooperative multitasking for Arduino microcontrollers
-Version 1.8.4: 2015-11-17
+Version 1.8.5: 2015-11-23
   
 OVERVIEW:
 A lightweight implementation of cooperative multitasking (task scheduling) supporting:
@@ -13,6 +13,10 @@ A lightweight implementation of cooperative multitasking (task scheduling) suppo
 8. Support for Local Task Storage pointer (allowing use of same callback code for multiple tasks)
 
 Changelog:
+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
+	
 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
 
diff --git a/extras/TaskScheduler.doc b/extras/TaskScheduler.doc
index f73af96..ca503e1 100644
--- a/extras/TaskScheduler.doc
+++ b/extras/TaskScheduler.doc
diff --git a/extras/TaskScheduler.html b/extras/TaskScheduler.html
index b295af4..b2fb997 100644
--- a/extras/TaskScheduler.html
+++ b/extras/TaskScheduler.html
@@ -28,7 +28,7 @@ Scheduler</B></FONT></P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><B>cooperative
 multitasking for Arduino microcontrollers</B></P>
 <P CLASS="western" STYLE="margin-bottom: 0in; border-top: none; border-bottom: 1px solid #000000; border-left: none; border-right: none; padding-top: 0in; padding-bottom: 0.01in; padding-left: 0in; padding-right: 0in">
-<FONT SIZE=2 STYLE="font-size: 11pt"><B>Version 1.8.4: 2015-11-17</B></FONT></P>
+<FONT SIZE=2 STYLE="font-size: 11pt"><B>Version 1.8.5: 2015-11-23</B></FONT></P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><BR>
 </P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><B>OVERVIEW</B>:</P>
@@ -1120,18 +1120,40 @@ must return a value of <B>true</B> for task to be enabled. If
 regardless if task is already enabled or not. Alignment to current
 millis() is performed after <B>OnEnable</B> exits, so any changes to
 the interval inside <B>OnEnable</B> is taken into consideration.</P>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in">TaskScheduler
+allows tasks to be added to the a Scheduler and enabled at the time
+of creation. <B>Be very careful</B><SPAN STYLE="font-weight: normal">
+with such tasks – the </SPAN><B>OnEnable </B><SPAN STYLE="font-weight: normal">method
+will be executed immediately, while certain objects (i.e., other
+Tasks, libraries) are not yet ready (e.g., </SPAN><B>Wire.begin()</B><SPAN STYLE="font-weight: normal">
+was not yet called), or hardware not yet activated (pins not set to
+INPUT or OUTPUT). </SPAN>
+</P>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><SPAN STYLE="font-weight: normal">It
+is very much recommended to to enable all tasks at the end of </SPAN><B>setup()</B><SPAN STYLE="font-weight: normal">
+method after all initializations are done. </SPAN>
+</P>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><SPAN STYLE="font-weight: normal">If
+you require immediate execution of already enabled task, use
+</SPAN><B>forceNextIteratoin</B><SPAN STYLE="font-weight: normal">()
+method instead of </SPAN><B>enable</B><SPAN STYLE="font-weight: normal">():
+it achieves the result, but does not call </SPAN><B>OnEnable</B><SPAN STYLE="font-weight: normal">
+method. </SPAN>
+</P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><BR>
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><B>NOTE:</B>
-in the event enable() method is called inside the OnEnable callback
-method (thus basically creating indefinte loop), TaskScheduler will
-only call OnEnable once (thus protecting the Task against OnEnable
-infinite loop). 
+in the event <B>enable</B>() method is called inside the <B>OnEnable</B>
+callback method (thus basically creating indefinte loop),
+TaskScheduler will only call <B>OnEnable</B> once (thus protecting
+the Task against <B>OnEnable</B> infinite loop). 
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><BR>
 </P>
-<P CLASS="western" STYLE="margin-bottom: 0in"><B>bool enableIfNot();</B></P>
-<P CLASS="western" STYLE="margin-bottom: 0in"><BR>
+<P CLASS="western" STYLE="margin-bottom: 0in; page-break-before: auto; page-break-after: avoid">
+<B>bool enableIfNot();</B></P>
+<P CLASS="western" STYLE="margin-bottom: 0in; page-break-after: avoid">
+<BR>
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in">Enables
 the task only if it was previously disabled. Returns previous enable
@@ -1164,6 +1186,9 @@ forceNextIteration();</B></P>
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in">Schedules
 the task for execution during immediate next scheduling pass.</P>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in">The
+Task must be already <I>enabled</I> prior to this method. 
+</P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><BR>
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><B>Note:
@@ -1240,8 +1265,10 @@ dynamic control of task execution parameters in one method call.
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><B>Note</B><B>:
 </B>OnEnable and OnDisable parameters can be omitted. In that case
-they will be assigned to NULL and respective methods will no longer
-be called. 
+they will be assigned to <B>NULL</B> and respective methods will no
+longer be called. Therefore it is advisable to use either all five
+parameters explicitly, or employ individual “setter” methods
+below instead. 
 </P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><BR>
 </P>
@@ -1266,18 +1293,24 @@ setOnDisable (void (*aCallback)()) </B>
 </P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><BR>
 </P>
-<P CLASS="western" STYLE="margin-bottom: 0in"><B>Note: </B>Next
+<P CLASS="western" STYLE="margin-bottom: 0in"><B>NOTE: </B>Next
 execution time calculation takes place <B>after</B> the callback
 method is called, so new interval will be used immediately by the
 scheduler. For the situations when one task is changing the interval
 parameter for the other, <B>setInterval</B> method calls <B>delay
 </B>explicitly to guarantee schedule change, however it <B>does not
 </B>enable the task if task is disabled.</P>
-<P CLASS="western" STYLE="margin-bottom: 0in"><B>Note: </B>Tasks that
+<P CLASS="western" STYLE="margin-bottom: 0in"><B>NOTE: </B>Tasks that
 ran through all their allocated iterations are disabled.
 <B>SetIterations()</B> method <B>DOES NOT</B> enable the task. Either
 <B>enable</B> explicitly, or use <B>restart</B> methods. 
 </P>
+<P CLASS="western" STYLE="margin-bottom: 0in">Please note that as a
+result execution of the taks is <B>delayed</B><SPAN STYLE="font-weight: normal">
+by the provided interval. If immediate invocation is required, call
+</SPAN><B>forceNextIteration</B><SPAN STYLE="font-weight: normal">()
+method after setting a new interval. </SPAN>
+</P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><BR>
 </P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><B>STATUS REQUEST
@@ -1569,8 +1602,36 @@ task being enabled or disabled.
 one scheduling pass, including end-of-pass sleep. This method is
 typically placed inside the <B>loop()</B> method of the sketch. Since
 <B>execute</B> exits after every pass, you can put additional
-statements after <B>execute</B> inside the <B>loop()</B> 
+statements after <B>execute</B> inside the <B>loop().</B></P>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Generally,
+execute will perform the following steps:</SPAN></P>
+<OL>
+	<LI><P CLASS="western" STYLE="margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Ignore
+	task completely if it is disabled.</SPAN></P>
+	<LI><P CLASS="western" STYLE="margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Disable
+	task if it ran out of iterations (calling OnDesable, if necessary).</SPAN></P>
+	<LI><P CLASS="western" STYLE="margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Check
+	if task is waiting on a StatusRequest object, and make appropriate
+	scheduling arrangements</SPAN></P>
+	<LI><P CLASS="western" STYLE="margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Perform
+	necessary timing calculations (including millis() rollover fix, if
+	requested)</SPAN></P>
+	<LI><P CLASS="western" STYLE="margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Invoke
+	task's callback method, if it is time to do so, and one is provided.
+	</SPAN>
+	</P>
+	<LI><P CLASS="western" STYLE="margin-bottom: 0in"><SPAN STYLE="font-weight: normal">Put
+	microcontroller to sleep (if requested and supported) if none of the
+	tasks were invoked. </SPAN>
+	</P>
+</OL>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><BR>
 </P>
+<P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><B>Please
+NOTE:</B><SPAN STYLE="font-weight: normal"> schedule-related
+calculations are performed prior to task's callback method
+invocation. This allows tasks to manipulate their runtime parameters
+(like execution interval) directly.</SPAN></P>
 <P CLASS="western" STYLE="margin-left: 0.49in; margin-bottom: 0in"><BR>
 </P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><B>bool isOverrun()</B></P>
@@ -2749,7 +2810,7 @@ time examples of TaskScheduler are available here:</FONT></FONT></P>
 <P CLASS="western" STYLE="margin-bottom: 0in"><BR>
 </P>
 <DIV TYPE=FOOTER>
-	<P STYLE="margin-top: 0.35in; margin-bottom: 0in">	<SDFIELD TYPE=PAGE SUBTYPE=RANDOM FORMAT=ARABIC>5</SDFIELD></P>
+	<P STYLE="margin-top: 0.35in; margin-bottom: 0in">	<SDFIELD TYPE=PAGE SUBTYPE=RANDOM FORMAT=ARABIC>31</SDFIELD></P>
 </DIV>
 </BODY>
-</HTML>
+</HTML>
\ No newline at end of file
diff --git a/extras/TaskScheduler_html_m68472eb8.png b/extras/TaskScheduler_html_m68472eb8.png
new file mode 100644
index 0000000..febb8c7
--- /dev/null
+++ b/extras/TaskScheduler_html_m68472eb8.png
diff --git a/library.properties b/library.properties
index 67b7a7c..ac0d61f 100644
--- a/library.properties
+++ b/library.properties
@@ -1,5 +1,5 @@
 name=TaskScheduler
-version=1.8.4
+version=1.8.5
 author=Anatoli Arkhipenko <arkhipenko@hotmail.com>
 maintainer=Anatoli Arkhipenko <arkhipenko@hotmail.com>
 sentence=A light-weight cooperative multitasking library for arduino microcontrollers.
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;