mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 21:47:39 +02:00
fix(queue): tell a pinned queue item why it is waiting (issue #3074)
A job queued as "Any X1C" explains itself when it cannot start: the model-based branch builds a reason for every candidate printer and puts it on the row, so the queue shows "Busy: X1C-01" or "Waiting for filament: X1C-02 (needs PETG)". The same job pinned to one printer showed nothing. It sat at Pending with waiting_reason NULL for as long as that printer was busy, which from the outside is indistinguishable from a queue that has stopped working -- the reporter watched fourteen minutes of it while his X1C ran a print he had started from its own screen. The fixed-printer branch had six ways out and none of them wrote the field. The sensor interlock (#1148) was its only writer, and it cleared the field up front on every pass where no sensor was holding the printer, so NULL was not an oversight on those paths but a guarantee. Every exit now writes, through one helper. The reasons reuse the model-based branch's vocabulary so _is_busy_only() keeps deciding what is worth a notification: a printer that is printing, drying, or working through the item ahead of this one reads as "Busy: <printer>" and stays silent, because it resolves itself. A printer that is off with no Auto On plug, and one whose plug could not switch it on, are worth saying. A finished plate nobody has acknowledged is split out from plain busy and named as itself. _is_printer_idle() returns the same plain False for that and for a running print, but they are not the same thing to the person looking at the queue: one clears itself and the other needs somebody to walk over to the printer. That notification fires on the transition into asking, where a busy-only reason counts as not asking. Testing whether the item was waiting at all -- which is what the model-based branch does -- would never fire it here: nobody's queue goes straight from idle to an unconfirmed plate, it waits behind the print first. The cost is that a printer dropping offline, returning busy and dropping again asks twice rather than once. The interlock stays silent. It has never sent this notification, and a change about what the queue displays is not the place to start. Clearing the field up front is gone with it. It existed so a shut door could not leave "Waiting on Enclosure Door" standing while the printer stayed busy with something else, and the new rule carries that guarantee instead -- whichever exit runs next overwrites it, and the dispatch path clears it. Two paths clear it that the report did not mention. A staged item and a future-scheduled one skip before this branch and never reach it again, so anything written on an earlier pass would outlive its condition for the life of the row. That includes the filament-deficit check, which stages the item itself. The notification is wrapped: a queue that cannot say why it is waiting is the bug being fixed, and a queue that stops dispatching because a provider timed out would be a worse one. On the frontend, the queue timeline drops any pending item carrying a reason, on the grounds that such an item will not auto-dispatch. That held while only the model-based branch wrote the field; "Busy: <printer>" is now the commonest reason there is, and it describes the very chain the timeline forecasts, so the rule would have emptied the view for anyone whose queue is pinned. It now asks whether the reason needs the user, via a small shared reader of the same shape the scheduler encodes. Which job goes out, and when, is unchanged: running the previous scheduler and this one over the same 768 states dispatches the same items in the same order with the same statuses, across 1452 rows that now carry a reason.
This commit is contained in:
@@ -114,3 +114,60 @@ describe('QueueTimelineView job ordering', () => {
|
||||
expect(barsLeftToRight()).toEqual(['Running now', 'Long job', 'Short job']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('QueueTimelineView and the scheduler waiting reasons (#3074)', () => {
|
||||
/** The scheduler now puts a reason on a pinned item too, and the commonest
|
||||
* one by far -- "Busy: <printer>" -- describes the very chain this view
|
||||
* forecasts. Dropping every item that has a reason would empty the timeline
|
||||
* for anyone whose queue is pinned to specific printers. */
|
||||
function renderWith(items: PrintQueueItem[]) {
|
||||
render(
|
||||
<QueueTimelineView
|
||||
queueItems={items}
|
||||
printers={printers}
|
||||
printerStatuses={{ 1: { progress: 50, remaining_time: 30, state: 'RUNNING' } }}
|
||||
sjfEnabled={false}
|
||||
onItemClick={() => {}}
|
||||
t={(key: string) => key}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
it('still forecasts an item that is only waiting its turn', () => {
|
||||
renderWith([
|
||||
running,
|
||||
pending(1, 'Long job', 1, 2 * HOUR, { waiting_reason: 'Busy: X1C-01' }),
|
||||
pending(2, 'Medium job', 2, HOUR, { waiting_reason: 'Busy: X1C-01' }),
|
||||
]);
|
||||
expect(barsLeftToRight()).toEqual(['Running now', 'Long job', 'Medium job']);
|
||||
});
|
||||
|
||||
it('still forecasts one held behind a drying cycle', () => {
|
||||
renderWith([running, pending(1, 'Long job', 1, 2 * HOUR, { waiting_reason: 'Busy: X1C-01 (drying)' })]);
|
||||
expect(barsLeftToRight()).toEqual(['Running now', 'Long job']);
|
||||
});
|
||||
|
||||
it('drops one that is waiting for the user', () => {
|
||||
// These do not start on their own, so a bar would be a promise the queue
|
||||
// cannot keep.
|
||||
for (const reason of [
|
||||
'Waiting for plate confirmation: X1C-01',
|
||||
'Offline, no Auto On smart plug: X1C-01',
|
||||
'Waiting on Enclosure Door',
|
||||
'Waiting for filament: X1C-01 (needs PETG)',
|
||||
]) {
|
||||
const { unmount } = render(
|
||||
<QueueTimelineView
|
||||
queueItems={[running, pending(1, 'Long job', 1, 2 * HOUR, { waiting_reason: reason })]}
|
||||
printers={printers}
|
||||
printerStatuses={{ 1: { progress: 50, remaining_time: 30, state: 'RUNNING' } }}
|
||||
sjfEnabled={false}
|
||||
onItemClick={() => {}}
|
||||
t={(key: string) => key}
|
||||
/>,
|
||||
);
|
||||
expect(barsLeftToRight()).toEqual(['Running now']);
|
||||
unmount();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
/**
|
||||
* The frontend half of the `waiting_reason` shape contract (#3074).
|
||||
*
|
||||
* The scheduler encodes "starts by itself" as a reason made only of `Busy: ...`
|
||||
* clauses, joined with ` | `, and decides notifications on that basis. The UI
|
||||
* reads the same bit to decide what still belongs on a forecast. These cases
|
||||
* are the exact strings both branches of the scheduler emit, so a reword on
|
||||
* either side lands here first.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { isBusyOnlyWaitingReason } from '../../utils/waitingReason';
|
||||
|
||||
describe('isBusyOnlyWaitingReason', () => {
|
||||
it('treats nothing as not-busy rather than busy', () => {
|
||||
// An item with no reason is dispatchable, not "waiting its turn"; callers
|
||||
// check the reason's presence separately.
|
||||
expect(isBusyOnlyWaitingReason(null)).toBe(false);
|
||||
expect(isBusyOnlyWaitingReason(undefined)).toBe(false);
|
||||
expect(isBusyOnlyWaitingReason('')).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'Busy: X1C-01',
|
||||
'Busy: X1C-01 (drying)',
|
||||
'Busy: X1C-01, X1C-02',
|
||||
'Busy: X1C-01 | Busy: X1C-02',
|
||||
])('reads %s as waiting its turn', reason => {
|
||||
expect(isBusyOnlyWaitingReason(reason)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'Waiting for plate confirmation: X1C-01',
|
||||
'Offline, no Auto On smart plug: X1C-01',
|
||||
'Offline: X1C-01 — the smart plug could not power it on',
|
||||
'Waiting on Enclosure Door',
|
||||
'Waiting for filament: X1C-01 (needs PETG)',
|
||||
'No available X1C printers',
|
||||
'Every file for this job has been deleted — add a file back or remove the item',
|
||||
])('reads %s as waiting for the user', reason => {
|
||||
expect(isBusyOnlyWaitingReason(reason)).toBe(false);
|
||||
});
|
||||
|
||||
it('needs every clause to be busy, not just the first', () => {
|
||||
// The scheduler only joins with " | " when every clause is busy, but the
|
||||
// reader must not assume that -- one clause needing the user makes the
|
||||
// whole reason need the user.
|
||||
expect(isBusyOnlyWaitingReason('Busy: X1C-01 | Offline: X1C-02')).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -2533,7 +2533,7 @@ export interface PrintQueueItem {
|
||||
target_model: string | null; // Target printer model for model-based assignment
|
||||
target_location: string | null; // Target location filter for model-based assignment
|
||||
required_filament_types: string[] | null; // Required filament types for model-based assignment
|
||||
waiting_reason: string | null; // Why a model-based job hasn't started yet
|
||||
waiting_reason: string | null; // Why this job hasn't started yet (empty once it can)
|
||||
// Cross-model alternatives (#671), in priority order. Empty for ordinary
|
||||
// items. Present until dispatch resolves one, after which library_file_id and
|
||||
// target_model name the candidate that actually ran.
|
||||
|
||||
@@ -2,6 +2,7 @@ import { useState, useMemo, useEffect, useRef } from 'react';
|
||||
import { ChevronLeft, ChevronRight, Clock, Layers, Printer as PrinterIcon } from 'lucide-react';
|
||||
import { formatDuration, parseUTCDate } from '../utils/date';
|
||||
import { compareQueueOrder, queueLaneKey } from '../utils/queueOrder';
|
||||
import { isBusyOnlyWaitingReason } from '../utils/waitingReason';
|
||||
import type { PrintQueueItem, Printer } from '../api/client';
|
||||
import { api } from '../api/client';
|
||||
import { Button } from './Button';
|
||||
@@ -91,8 +92,11 @@ export function QueueTimelineView({
|
||||
// • pending with explicit scheduled_time → at that time
|
||||
// • pending ASAP that chain behind a print actually running on the same
|
||||
// lane → forecast
|
||||
// Staged (manual_start) and waiting (waiting_reason) items are not on the
|
||||
// timeline because they won't auto-dispatch — they'd be misleading bars.
|
||||
// Staged (manual_start) and blocked items are not on the timeline because
|
||||
// they won't auto-dispatch — they'd be misleading bars. "Blocked" is not the
|
||||
// same as "has a waiting_reason": since #3074 an item queued behind a running
|
||||
// print carries one too ("Busy: X1C-01"), and that is the chain this view
|
||||
// exists to forecast. Only a reason that needs the user takes an item off.
|
||||
// Idle-printer ASAP queues also stay off until something starts on them.
|
||||
const events = useMemo<ScheduleEvent[]>(() => {
|
||||
const result: ScheduleEvent[] = [];
|
||||
@@ -128,10 +132,12 @@ export function QueueTimelineView({
|
||||
lanesWithActive.add(lk);
|
||||
chainEndByLane.set(lk, Math.max(chainEndByLane.get(lk) ?? nowMs, endTime.getTime()));
|
||||
} else if (item.status === 'pending') {
|
||||
// Skip un-committed pending shapes — staged items and waiting items
|
||||
// won't auto-dispatch, so a bar would lie.
|
||||
// Skip un-committed pending shapes — staged items and blocked items
|
||||
// won't auto-dispatch, so a bar would lie. An item merely waiting its
|
||||
// turn behind a print does auto-dispatch, and is the whole point of the
|
||||
// chain forecast below.
|
||||
if (item.manual_start) continue;
|
||||
if (item.waiting_reason) continue;
|
||||
if (item.waiting_reason && !isBusyOnlyWaitingReason(item.waiting_reason)) continue;
|
||||
const lk = queueLaneKey(item);
|
||||
if (!pendingByLaneKey.has(lk)) pendingByLaneKey.set(lk, []);
|
||||
pendingByLaneKey.get(lk)!.push(item);
|
||||
|
||||
@@ -1836,10 +1836,12 @@ export function QueuePage() {
|
||||
// Queue items eligible for an "if started now" ETA (#2740).
|
||||
//
|
||||
// The ETA answers "when would this finish if it began right now", so it may
|
||||
// only appear on items that really could begin right now. Deriving that from
|
||||
// waiting_reason alone is not enough: the scheduler only writes that field on
|
||||
// the model-based assignment path (print_scheduler.py), so an item pinned to a
|
||||
// specific printer sits behind a running job with waiting_reason still NULL.
|
||||
// only appear on items that really could begin right now. waiting_reason now
|
||||
// covers the pinned-printer case too (#3074), but it is still not enough on
|
||||
// its own: it says whether the scheduler had a reason to hold the item on its
|
||||
// last pass, not whether this item is the one that printer takes next. Two
|
||||
// items pinned to the same free printer both come back with no reason, and
|
||||
// only one of them can start now — which is what the ordering below works out.
|
||||
//
|
||||
// Computed from the unfiltered queue on purpose — hiding a printer behind the
|
||||
// location filter must not make its printer look free.
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
/**
|
||||
* Reading the scheduler's `waiting_reason` (#3074).
|
||||
*
|
||||
* The backend writes one sentence per held queue item and encodes one bit in
|
||||
* its shape: a reason made only of `Busy: ...` clauses means the job starts by
|
||||
* itself once a printer frees up, and anything else means somebody has to do
|
||||
* something — load filament, switch a printer on, confirm a plate. The
|
||||
* scheduler uses that bit to decide whether a hold is worth a notification
|
||||
* (`PrintScheduler._is_busy_only`), and the UI needs the same distinction to
|
||||
* decide whether a held item still belongs on a forecast.
|
||||
*
|
||||
* Kept in one place on this side too, so the two halves of the contract are one
|
||||
* grep apart.
|
||||
*/
|
||||
|
||||
/** Does this reason mean "waiting its turn", rather than "waiting for you"? */
|
||||
export function isBusyOnlyWaitingReason(reason: string | null | undefined): boolean {
|
||||
if (!reason) return false;
|
||||
return reason
|
||||
.split(' | ')
|
||||
.map(part => part.trim())
|
||||
.every(part => part.startsWith('Busy:'));
|
||||
}
|
||||
Reference in New Issue
Block a user