A supply nobody could read is unknown, not empty
Some checks failed
CI / backend (push) Failing after 7m21s
CI / frontend (push) Has been cancelled
CI / migrations-mysql (push) Has been cancelled
CI / naming (push) Has been cancelled

Reported as "CSF04 now reports all 0%", on the printer's own page as well as
the reports. Every supply on one host reading zero at the same instant is not
what a set of cartridges does; it is what a host that stopped answering looks
like.

getsuppliesbyip turned every unreadable item into 0:

    try:
        level = int(float(item.get("lastvalue", 0)))
    except (ValueError, TypeError):
        level = 0

An item that has never collected returns lastvalue as an EMPTY STRING, so
float('') raised and the handler substituted zero. lastclock was fetched in the
same request and read by nothing at all, so a reading from three weeks ago was
presented as current.

Zero is a legitimate level meaning spent, which is what made the substitution
dangerous rather than merely wrong. Downstream it reached `level <=
EMPTY_LEVEL`, which set daysleft to 0, which put the printer on the order list:
the report asked purchasing to buy a full set of cartridges for a printer
nobody had measured. On a waste container it was worse - waste is scored as
100 - level, so an unreadable one read as 100% full, the alarm state.

_readlevel now returns (level, lastseen) with level None for no usable reading:
empty, missing or unparseable lastvalue, or a lastclock older than 24h. A
lastclock that is absent is not treated as stale - a reading that cannot be
aged is not thereby wrong, and discarding it would lose real levels. A genuine
"0" is still 0, which is the half that keeps an actually-empty cartridge on the
order list.

classifysupply gains an 'unknown' status with remaining None. It is not
critical: critical is a claim someone acts on, and it may not be made about a
supply nobody measured.

The low-supplies report skips unknown rather than listing it as needing
replacement, and counts it in summary.unknown - a printer that has gone quiet
should be visible as that, not absent and not masquerading as empty.
_annotate_supply reads .get('level') with no 0 default, which would have put
the whole bug back.

PrinterDetail already rendered `level !== null ? ... : 'N/A'`, so it starts
telling the truth as soon as the backend sends null; the UI had been written
for a case the service never produced. It gains the unknown style - muted, not
a severity colour - and a tooltip separating "no reading at all" from "no
reading since <date>", which are different faults with different fixes.

Alerts needed no change: float(None) raises and that loop already continues,
so unknown supplies stop firing low-toner emails as a consequence.

12 of the 16 new tests fail on the old code. The 4 that pass either way pin
that real zeros still behave, which is the regression this fix could cause.

Not verified against CSF04's actual Zabbix data - dev has no reachable Zabbix.
This fixes the mechanism that turns unknown into 0%; /api/printers/<id>/supplies
now distinguishes them, with null for no reading and 0 for a measured zero.
This commit is contained in:
cproudlock
2026-08-21 11:43:56 -04:00
parent f9d298c7f0
commit b7cfd2b198
6 changed files with 277 additions and 12 deletions

View File

@@ -2109,7 +2109,9 @@ def _annotate_supply(supply, vendor_name, modelnumberid):
Waste cartridge direction depends on vendor, so classification lives in
the supply_parts helper. Part numbers come from the modelsupplies table.
"""
level = supply.get('level', 0)
# .get('level') with no default: a supply with no reading carries None, and
# a 0 default would put back the very substitution this reports around.
level = supply.get('level')
name = supply.get('name', 'Unknown')
supplytype = derivesupplytype(name)
color = derivecolor(name, supply.get('color'))
@@ -2117,6 +2119,7 @@ def _annotate_supply(supply, vendor_name, modelnumberid):
return {
'name': name,
'level': level,
'lastseen': supply.get('lastseen'),
'color': color,
'supplytype': supplytype,
'status': cls['status'],
@@ -2160,6 +2163,7 @@ def _get_low_supplies_data():
results = []
total_checked = 0
unknown_printers = 0
for printer, asset, comm, vendor, model in unique_printers:
supplies = service.getsuppliesbyip_cached(comm.ipaddress)
@@ -2178,13 +2182,24 @@ def _get_low_supplies_data():
# shown it. The whole report exists to answer "what needs replacing".
annotated = []
has_low = False
has_unknown = False
for s in supplies:
item = _annotate_supply(s, vendor_name, modelnumberid)
if item['status'] == 'ok':
continue
# A supply nobody could read is not a supply that needs replacing,
# so it does not join the list of things to act on. It is counted
# instead: a printer that has gone quiet should be visible as that,
# not disappear from the report and not masquerade as empty.
if item['status'] == 'unknown':
has_unknown = True
continue
has_low = True
annotated.append(item)
if has_unknown:
unknown_printers += 1
if has_low:
# location name for the report row
# Via the relationship, the way the printers list does it. The
@@ -2222,7 +2237,12 @@ def _get_low_supplies_data():
'summary': {
'total_checked': total_checked,
'low': low_count,
'critical': critical_count
'critical': critical_count,
# Printers with at least one unreadable supply. They are NOT in
# 'printers' above - nothing about them needs ordering - but a
# count that only ever went up when a cartridge ran low would hide
# a fleet quietly going dark.
'unknown': unknown_printers
}
}

View File

@@ -215,8 +215,14 @@
<div v-for="supply in supplies" :key="supply.itemid || supply.name" class="supply-item">
<div class="supply-header">
<span class="supply-name">{{ supply.name }}</span>
<span class="supply-level" :class="supply.status">
{{ supply.level !== null ? `${supply.level}%` : 'N/A' }}
<!-- N/A is not the same as 0%. A supply Zabbix has no usable
reading for arrives with level null and status 'unknown';
the title says which of the two reasons it is, because
"no data" and "last seen three weeks ago" need different
people to fix them. -->
<span class="supply-level" :class="supply.status"
:title="supply.status === 'unknown' ? unknownReason(supply) : null">
{{ supply.level !== null && supply.level !== undefined ? `${supply.level}%` : 'N/A' }}
</span>
</div>
<div class="supply-bar">
@@ -227,7 +233,7 @@
></div>
</div>
<div class="supply-meta">
<span>{{ formatSupplyType(supply.supplytype) }}<template v-if="supply.iswaste"> ({{ supply.remaining }}% remaining)</template></span>
<span>{{ formatSupplyType(supply.supplytype) }}<template v-if="supply.iswaste && supply.remaining !== null"> ({{ supply.remaining }}% remaining)</template></span>
<span v-if="supply.partnumbers && supply.partnumbers.length" class="supply-parts">
<span
v-for="part in supply.partnumbers"
@@ -363,6 +369,15 @@ function formatSupplyType(supplytype) {
return supplytype.charAt(0).toUpperCase() + supplytype.slice(1)
}
// Why a supply reads N/A. lastseen is the epoch of the newest reading Zabbix
// holds: absent means the item has never collected, present means it has but
// the reading is too old to present as current. Those are different faults and
// the tooltip is the only place either one is stated.
function unknownReason(supply) {
if (!supply.lastseen) return 'Zabbix has no reading for this supply'
return `No reading since ${new Date(supply.lastseen * 1000).toLocaleString()}`
}
function formatDate(dateStr) {
if (!dateStr) return '-'
return new Date(dateStr).toLocaleString()
@@ -414,6 +429,10 @@ function formatDate(dateStr) {
.supply-level.ok { color: var(--success); }
.supply-level.low { color: var(--warning); }
.supply-level.critical { color: var(--danger); }
/* Muted on purpose. An unknown level is not a severity - colouring it like one
would put it in the same reading as critical, which is the confusion this
whole change exists to end. The help cursor points at the tooltip. */
.supply-level.unknown { color: var(--text-light); cursor: help; }
.supply-bar {
height: 10px;

View File

@@ -323,7 +323,9 @@ class PrintersPlugin(BasePlugin):
click.echo(f'Supply levels for {ip}:')
for supply in supplies:
click.echo(f" {supply['name']}: {supply['level']}%")
level = supply.get('level')
shown = 'unknown' if level is None else f'{level}%'
click.echo(f" {supply['name']}: {shown}")
@printerscli.command('seed-supplies')
def seedsuppliescommand():

View File

@@ -68,19 +68,34 @@ def derivecolor(name: str, tagcolor: Optional[str] = None) -> str:
return "none"
def classifysupply(level: float, name: str, vendor: Optional[str]) -> Dict:
"""Classify one supply item into ok/low/critical.
def classifysupply(level: Optional[float], name: str,
vendor: Optional[str]) -> Dict:
"""Classify one supply item into ok/low/critical, or unknown.
Waste cartridge fill is inverted vs a toner level: a full waste cartridge
is bad. Standard vendors report waste as percent FULL (high = bad). Xerox
EC/AltaLink series report waste as percent capacity REMAINING (low = bad),
same direction as toner. Normalise everything to percent remaining first.
A level of None means Zabbix has no usable reading; the status is 'unknown'
and remaining is None. Callers must not treat that as a low supply.
"""
lowername = (name or "").lower()
iswaste = "waste" in lowername
isdrum = "drum" in lowername or "imaging" in lowername
isxerox = bool(vendor) and "xerox" in vendor.lower()
# No reading is its own answer, not a level. It cannot be classified ok,
# low or critical without asserting something nobody measured - and every
# one of those three is a claim the reader would act on.
if level is None:
return {
"status": "unknown",
"remaining": None,
"iswaste": iswaste,
"isdrum": isdrum,
}
if iswaste and not isxerox:
remaining = 100 - level
else:

View File

@@ -208,18 +208,64 @@ class ZabbixService:
continue
if str(item.get("state", "0")) != "0":
continue
try:
level = int(float(item.get("lastvalue", 0)))
except (ValueError, TypeError):
level = 0
level, lastseen = self._readlevel(item)
supplies.append({
"name": item.get("name", "Unknown"),
"level": level,
"lastseen": lastseen,
"color": self._extract_color(item),
"itemid": item.get("itemid"),
})
return supplies
# How old the newest reading may be and still be presented as the level.
# Generous on purpose: supply items are polled in hours, not seconds, and
# calling a slow poller "unknown" would be its own lie. This exists for the
# host that stopped answering, not for one that answers rarely.
STALE_AFTER_SECONDS = 24 * 3600
@classmethod
def _readlevel(cls, item):
"""(level, lastseen) for one supply item; level is None when unknown.
An item that has never collected returns lastvalue as an EMPTY STRING,
and one whose host went quiet keeps a lastvalue that is arbitrarily old.
Both used to become 0 - the empty string because float('') raises and
the handler substituted zero, the stale one because nothing ever read
lastclock, which was fetched and then ignored.
Zero is not a safe stand-in for "no reading". It is a legitimate value
meaning the cartridge is spent, so the substitution was indistinguishable
from a real empty - and it hit every item on the host at once, which is
what a printer that has gone dark looks like. Downstream that reached
`level <= EMPTY_LEVEL`, so the forecast gave it daysleft 0 and the order
list asked purchasing to buy a full set of cartridges for a printer
nobody had measured.
None says "unknown" and every consumer already handles it: the detail
page renders N/A, the alert loop skips it, and analyse() falls back to
the last real level in history rather than inventing one.
"""
raw = item.get("lastvalue")
if raw is None or str(raw).strip() == "":
return None, None
try:
level = int(float(raw))
except (ValueError, TypeError):
return None, None
try:
lastseen = int(item.get("lastclock") or 0)
except (ValueError, TypeError):
lastseen = 0
if lastseen <= 0:
# A value with no clock cannot be aged. Trust the value and say the
# age is unknown rather than discarding a reading that may be fine.
return level, None
if time.time() - lastseen > cls.STALE_AFTER_SECONDS:
return None, lastseen
return level, lastseen
# Per item, per call. Zabbix applies `limit` to the WHOLE result set, not
# per item, so the budget is multiplied by the number of items asked for.
HISTORY_PER_ITEM = 2000

View File

@@ -0,0 +1,163 @@
"""A supply nobody could read is unknown, not empty.
Reported as "CSF04 now reports all 0%", on the printer's own page as well as the
reports. Every supply on one host reading zero at the same moment is not what a
set of cartridges does; it is what a host that stopped answering looks like.
`getsuppliesbyip` turned every unreadable item into 0:
try:
level = int(float(item.get("lastvalue", 0)))
except (ValueError, TypeError):
level = 0
An item that has never collected returns lastvalue as an EMPTY STRING, so
float('') raised and the handler substituted zero. `lastclock` was fetched in
the same request and never read by anything, so a reading from three weeks ago
was presented as current.
Zero is a legitimate level meaning "spent", which is what made the substitution
dangerous rather than merely wrong: it reached `level <= EMPTY_LEVEL` in the
forecast, which set daysleft to 0, which put the printer on the order list. The
report asked purchasing to buy a full set of cartridges for a printer nobody had
measured.
None means unknown, and it has to stay None the whole way out.
"""
import time
import pytest
from plugins.printers.services.zabbix_service import ZabbixService
from plugins.printers.services.supply_parts import classifysupply
from plugins.printers.services.supply_history import analyse, EMPTY_LEVEL
def item(lastvalue, lastclock=None, name='Black Cartridge'):
"""One Zabbix item.get row, shaped as the API returns it."""
if lastclock is None:
lastclock = int(time.time()) - 600
return {'itemid': '1', 'name': name,
'lastvalue': lastvalue, 'lastclock': str(lastclock)}
# --- reading one item -------------------------------------------------------
def test_an_item_that_never_collected_is_unknown_not_zero():
"""The reported case. Zabbix returns '' for an item with no data."""
level, lastseen = ZabbixService._readlevel(item(''))
assert level is None
assert lastseen is None
def test_a_missing_lastvalue_is_unknown_not_zero():
level, _ = ZabbixService._readlevel({'lastvalue': None, 'lastclock': '0'})
assert level is None
def test_an_unparseable_lastvalue_is_unknown_not_zero():
level, _ = ZabbixService._readlevel(item('n/a'))
assert level is None
def test_a_real_zero_is_still_zero():
"""The whole point of the distinction: an empty cartridge must survive it.
If this returned None, an actually-spent cartridge would stop being ordered.
"""
level, _ = ZabbixService._readlevel(item('0'))
assert level == 0
def test_a_normal_reading_comes_back_with_its_clock():
when = int(time.time()) - 300
level, lastseen = ZabbixService._readlevel(item('47', lastclock=when))
assert level == 47
assert lastseen == when
def test_a_float_reading_is_truncated_to_an_int():
level, _ = ZabbixService._readlevel(item('47.8'))
assert level == 47
# --- staleness --------------------------------------------------------------
def test_a_reading_older_than_the_window_is_unknown():
"""lastclock was fetched and then ignored, so a host that went quiet weeks
ago kept presenting its last level as current."""
stale = int(time.time()) - (ZabbixService.STALE_AFTER_SECONDS + 60)
level, lastseen = ZabbixService._readlevel(item('47', lastclock=stale))
assert level is None
assert lastseen == stale, 'the age must survive, so the UI can say how old'
def test_a_reading_inside_the_window_is_kept():
recent = int(time.time()) - (ZabbixService.STALE_AFTER_SECONDS - 3600)
level, _ = ZabbixService._readlevel(item('47', lastclock=recent))
assert level == 47
def test_a_value_with_no_clock_is_trusted():
"""A reading that cannot be aged is not thereby wrong. Discarding it would
lose real levels wherever lastclock is absent."""
level, lastseen = ZabbixService._readlevel(
{'lastvalue': '47', 'lastclock': '0'})
assert level == 47
assert lastseen is None
# --- classification ---------------------------------------------------------
def test_an_unknown_level_classifies_as_unknown():
result = classifysupply(None, 'Black Cartridge', 'HP')
assert result['status'] == 'unknown'
assert result['remaining'] is None
def test_an_unknown_level_is_not_critical():
"""'critical' is a claim someone acts on. It may not be made about a supply
nobody measured."""
assert classifysupply(None, 'Black Cartridge', 'HP')['status'] != 'critical'
def test_a_real_zero_is_still_critical():
assert classifysupply(0, 'Black Cartridge', 'HP')['status'] == 'critical'
def test_an_unknown_waste_cartridge_does_not_invert_into_full():
"""Waste is scored as 100 - level. With level 0 standing in for unknown,
an unreadable waste cartridge read as 100% full, which is the alarm state."""
result = classifysupply(None, 'Waste Toner Container', 'HP')
assert result['status'] == 'unknown'
assert result['remaining'] is None
# --- the forecast -----------------------------------------------------------
def test_an_unknown_level_with_no_history_gets_no_countdown():
"""0 reached `level <= EMPTY_LEVEL` and became daysleft 0 - the step that
put CSF04 on the order list."""
result = analyse([], currentlevel=None)
assert result['daysleft'] is None
assert result['reason'] == 'no history'
def test_a_real_zero_with_no_history_still_counts_as_due_now():
assert analyse([], currentlevel=0)['daysleft'] == 0
assert EMPTY_LEVEL >= 0
def test_an_unknown_level_falls_back_to_the_last_real_reading():
"""History is evidence the live read lacks. Preferring it beats inventing a
level, and beats throwing the cartridge off the report.
Clock is unix seconds and level falls over time, which is the only direction
toner goes; a rising series would read as a cartridge change.
"""
now = int(time.time())
points = [(now - n * 86400, 50 + n * 2) for n in range(20, -1, -1)]
result = analyse(points, currentlevel=None)
assert result['currentlevel'] == 50, 'the newest stored reading'
assert result['daysleft'] is not None, 'a forecast is still possible'