Bug in Scheduler when using Groups and negative master_adjustments

[Andi-Fwd]

While testing the firmware 2.2.0(2) I came across a bug in the station scheduler which happens when the following settings are used:

• >=2 stations are configured and sheduled to run at the same time
• the stations have a Master activated and the Master is configured with a negative "on adjustment"
• at least 2 stations are assigned to the same sequential group
  In this case the first two stations of each group have an overlapping runtime equivalent to the negative "on adjustment" value.

The problem is caused by the function void schedule_all_stations(ulong curr_time) in main.cpp.
After each station is scheduled the handle_master_adjustments(curr_time, q) is performed. This updates correctly the station start time but the seq_start_times[gid] array is not updated. Hence the 2nd station in the same group is scheduled with an overlapping runtime to the 1st station.

Solution: update void handle_master_adjustments to correct also the seq_start_times[ ] array for the subsequent stations.

Proposed code change:

void handle_master_adjustments(ulong curr_time, RuntimeQueueStruct *q, byte gid, ulong *seq_start_times) {

	int16_t start_adj = 0;
	int16_t dequeue_adj = 0;

	for (byte mas = MASTER_1; mas < NUM_MASTER_ZONES; mas++) {

		byte masid = os.masters[mas][MASOPT_SID];

		if (masid && os.bound_to_master(q->sid, mas)) {

			int16_t mas_on_adj = os.get_on_adj(mas);
			int16_t mas_off_adj = os.get_off_adj(mas);

			start_adj = min(start_adj, mas_on_adj);
			dequeue_adj = max(dequeue_adj, mas_off_adj);
		}
	}

	// in case of negative master on adjustment
	// push back station's start time to allow sufficient time to turn on master
	if (q->st - curr_time < abs(start_adj)) {
		q->st += abs(start_adj);
		seq_start_times[gid] += abs(start_adj);
	}

	q->deque_time = q->st + q->dur + dequeue_adj;
}

/** Scheduler
 * This function loops through the queue
 * and schedules the start time of each station
 */
void schedule_all_stations(ulong curr_time) {
	ulong con_start_time = curr_time + 1;   // concurrent start time
	// if the queue is paused, make sure the start time is after the scheduled pause ends
	if (os.status.pause_state) {
		con_start_time += os.pause_timer;
	}
	int16_t station_delay = water_time_decode_signed(os.iopts[IOPT_STATION_DELAY_TIME]);
	ulong seq_start_times[NUM_SEQ_GROUPS];  // sequential start times
	for(byte i=0;i<NUM_SEQ_GROUPS;i++) {
		seq_start_times[i] = con_start_time;
		// if the sequential queue already has stations running
		if (pd.last_seq_stop_times[i] > curr_time) {
			seq_start_times[i] = pd.last_seq_stop_times[i] + station_delay;
		}
	}
	RuntimeQueueStruct *q = pd.queue;
	byte re = os.iopts[IOPT_REMOTE_EXT_MODE];
	byte gid;

	// go through runtime queue and calculate start time of each station
	for(;q<pd.queue+pd.nqueue;q++) {
		if(q->st) continue; // if this queue element has already been scheduled, skip
		if(!q->dur) continue; // if the element has been marked to reset, skip
		gid = os.get_station_gid(q->sid);

		// use sequential scheduling per sequential group
		// apply station delay time
		if (os.is_sequential_station(q->sid) && !re) {
			q->st = seq_start_times[gid];
			seq_start_times[gid] += q->dur;
			seq_start_times[gid] += station_delay; // add station delay time
		} else {
			// otherwise, concurrent scheduling
			q->st = con_start_time;
			// stagger concurrent stations by 1 second
			con_start_time++;
		}

		handle_master_adjustments(curr_time, q, gid, seq_start_times);

		if (!os.status.program_busy) {
			os.status.program_busy = 1;  // set program busy bit
			// start flow count
			if(os.iopts[IOPT_SENSOR1_TYPE] == SENSOR_TYPE_FLOW) {  // if flow sensor is connected
				os.flowcount_log_start = flow_count;
				os.sensor1_active_lasttime = curr_time;
			}
		}
	}
}

[trbom5c]

I can confirm this issue.

Thank you for your work in discovering the problem. I will update our code to match your proposal, and report back my results.

[trbom5c]

Deployed and confirmed.

@Andi-Fwd Your code revisions appear to have resolved this bug for me.

Thank you for solving a problem I identified 3 months ago, and invested zero time into finxing myself!

[rayshobby]

Thanks for reporting the issue and proposing the solution. I've verified it and integrated your change to the dev/221_1 branch so it will be released in the next firmware 2.2.1(1):
bc4cf84
This issue will be closed when that branch is released.