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): https://github.com/OpenSprinkler/OpenSprinkler-Firmware/commit/bc4cf8456d632b4f503e0ae2b2d5f7fabe808a6f This issue will be closed when that branch is released.