Let a clear spool stay clear on the way to Spoolman (issue #2912) (#2924)

This commit is contained in:
Kouki Ojima
2026-08-25 13:45:57 +02:00
committed by GitHub
parent d9bc7ae47a
commit 73912d4f05
22 changed files with 866 additions and 179 deletions
+8 -3
View File
@@ -91,7 +91,10 @@ def assert_safe_spoolman_url(url: str) -> None:
assert_safe_lan_service_url(url, label="Spoolman URL")
_COLOR_HEX_RE = re.compile(r"^[0-9A-Fa-f]{6}$")
# Six characters, or eight when the filament carries an alpha byte. The write
# side stores eight only for genuinely translucent spools (#2912); rejecting
# them here turned every clear spool into neutral grey on read.
_COLOR_HEX_RE = re.compile(r"^[0-9A-Fa-f]{6}(?:[0-9A-Fa-f]{2})?$")
_TAG_HEX_RE = re.compile(r"^[0-9A-F]+$")
@@ -197,10 +200,12 @@ def _map_spoolman_spool(spool: dict) -> MappedSpoolFields:
else:
subtype = filament_name or None
# Colour: validate as 6-char hex; fall back to neutral grey for invalid values
# Colour: validate as 6- or 8-char hex; fall back to neutral grey for invalid
# values. An 8-char value already carries its alpha, so appending the opaque
# byte would push it to ten and lose the translucency it was stored to keep.
raw_color = (filament.get("color_hex") or "").upper().removeprefix("#")
color_hex: str = raw_color if _COLOR_HEX_RE.match(raw_color) else "808080"
rgba: str = color_hex + "FF"
rgba: str = color_hex if len(color_hex) == 8 else color_hex + "FF"
label_weight: int = _safe_int(filament.get("weight"), 1000)
real_used_weight: float = _safe_float(spool.get("used_weight"), 0.0)
+16 -5
View File
@@ -59,6 +59,7 @@ from backend.app.services.spoolman import (
init_spoolman_client,
)
from backend.app.services.spoolman_tracking import get_fallback_spool_tag_for_slot
from backend.app.utils.color_utils import spoolman_color_hex
from backend.app.utils.filament_ids import (
GENERIC_FILAMENT_IDS,
filament_id_to_setting_id,
@@ -502,7 +503,10 @@ async def _resolve_filament_id(data: SpoolmanInventoryCreate, client: SpoolmanCl
return data.spoolman_filament_id
# Validator guarantees material is non-None when spoolman_filament_id is None
assert data.material is not None # noqa: S101
color_hex = (data.rgba or "808080FF")[:6]
# `or "808080"` on the result rather than on the input: spoolman_color_hex
# returns None only for a missing value, so this is the same neutral grey the
# old inline default produced, without handing an Optional to a str parameter.
color_hex = spoolman_color_hex(data.rgba) or "808080"
async with _translate_spoolman_errors():
return await client.find_or_create_filament(
material=data.material,
@@ -708,7 +712,10 @@ async def update_spool(
else:
color_name = cur_filament.get("color_name") or None
cur_color = (cur_filament.get("color_hex") or "808080").upper().removeprefix("#")
rgba = data.rgba if data.rgba is not None else (cur_color + "FF")
# Handed over as stored. The opaque alpha this used to append was folded
# straight back off by `spoolman_color_hex` below, so the two paths landed on
# the same string and the append only obscured which shape was in hand (#2912).
rgba = data.rgba if data.rgba is not None else cur_color
label_weight = data.label_weight if data.label_weight is not None else int(cur_filament.get("weight") or 1000)
# Default weight_used from the synthetic mapping (label - remaining) so an
# edit that doesn't touch the weight field preserves Spoolman's real
@@ -736,7 +743,7 @@ async def update_spool(
except ValueError as exc:
raise HTTPException(status_code=400, detail=str(exc)) from exc
color_hex = rgba[:6]
color_hex = spoolman_color_hex(rgba) or rgba
# Resolve which filament this spool should be linked to AFTER the edit.
#
@@ -749,14 +756,18 @@ async def update_spool(
# filament in place when it's a singleton.
cur_filament_id = cur_filament.get("id")
desired_name = f"{material} {subtype}".strip() if subtype else material
cur_color_norm = (cur_filament.get("color_hex") or "").upper()[:6]
# Compare the stored shapes, not raw strings and not bare RGB prefixes. Raw
# strings make an opaque spool's six characters differ from an incoming eight
# and PATCH the filament on every no-op edit; bare prefixes make an
# alpha-only edit invisible so the change never lands (#2912).
cur_color_norm = spoolman_color_hex(cur_filament.get("color_hex")) or ""
cur_vendor_name = (cur_vendor.get("name") or "").strip()
cur_weight_int = int(cur_filament.get("weight") or 0)
metadata_unchanged = (
cur_filament_id
and (cur_filament.get("name") or "").strip() == desired_name
and (cur_filament.get("material") or "").upper() == material.upper()
and cur_color_norm == color_hex.upper()
and cur_color_norm == (color_hex or "").upper()
and cur_vendor_name.lower() == ((brand or "").strip().lower())
and cur_weight_int == int(label_weight)
)
+27 -14
View File
@@ -9,6 +9,8 @@ from typing import Literal
import httpx
from backend.app.utils.color_utils import color_match_key, spoolman_color_hex
logger = logging.getLogger(__name__)
BAMBU_RFID_TAG_LENGTH = 32
@@ -356,9 +358,10 @@ class SpoolmanClient:
if material:
data["material"] = material
if color_hex:
# Strip alpha channel if present (RRGGBBAA -> RRGGBB)
color_hex = color_hex[:6] if len(color_hex) >= 6 else color_hex
data["color_hex"] = color_hex
# Every create funnels through here, so this is where the stored shape
# is decided: six characters for an opaque spool, eight only when the
# alpha byte says the filament is translucent. See #2912.
data["color_hex"] = spoolman_color_hex(color_hex) or color_hex
if color_name:
data["color_name"] = color_name
if weight:
@@ -757,7 +760,12 @@ class SpoolmanClient:
) -> int:
"""Return the filament ID matching material/name/brand/color, creating it if absent."""
name = f"{material} {subtype}".strip() if subtype else material
color = color_hex[:6].upper() if len(color_hex) >= 6 else color_hex.upper()
# One value in both roles. `color_match_key` returns the shape the colour
# would be stored as, so the key the loop below compares on and the value
# a new filament is created with are the same string by construction: an
# opaque spool keys and stores as six characters, a translucent one as
# eight, and neither can be conflated with the other (#2912).
color = color_match_key(color_hex)
vendor_id: int | None = None
if brand:
@@ -772,7 +780,7 @@ class SpoolmanClient:
filaments = await self.get_filaments()
for f in filaments:
f_material = (f.get("material") or "").upper()
f_color = (f.get("color_hex") or "").upper()[:6]
f_color = color_match_key(f.get("color_hex"))
f_vendor = f.get("vendor") or {}
f_vendor_name = (f_vendor.get("name") or "").strip().lower()
@@ -1199,7 +1207,7 @@ class SpoolmanClient:
material=tray.tray_type,
subtype="",
brand=brand,
color_hex=tray.tray_color[:6],
color_hex=tray.tray_color,
label_weight=tray.tray_weight,
)
except (SpoolmanNotFoundError, SpoolmanUnavailableError, SpoolmanClientError):
@@ -1249,9 +1257,12 @@ class SpoolmanClient:
async def _find_or_create_filament(self, tray: AMSTray) -> dict | None:
"""Return a Bambu Lab filament matching the tray's material/color, creating it if absent."""
bambu_vendor_id = await self.ensure_bambu_vendor()
color_hex = tray.tray_color[:6] # Strip alpha channel
material_upper = tray.tray_type.upper()
color_upper = color_hex.upper()
# Same single value as the user-driven path: the match key is the stored
# shape. That is what lets an opaque tray still find the six-character
# filaments every existing instance is full of, while a clear tray keys
# to eight and gets its own record (#2912).
color = color_match_key(tray.tray_color)
# Search internal filaments - only match Bambu Lab vendor
filaments = await self.get_filaments()
@@ -1260,8 +1271,7 @@ class SpoolmanClient:
if fil_vendor_id != bambu_vendor_id:
continue
fil_material = filament.get("material") or ""
fil_color = filament.get("color_hex") or ""
if fil_material.upper() == material_upper and fil_color.upper() == color_upper:
if fil_material.upper() == material_upper and color_match_key(filament.get("color_hex")) == color:
return filament
# Search external filaments (SpoolmanDB) — restrict to Bambu Lab only.
@@ -1277,8 +1287,7 @@ class SpoolmanClient:
if manufacturer != "bambu lab" and not ext_id.startswith("bambulab_"):
continue
fil_material = filament.get("material") or ""
fil_color = filament.get("color_hex") or ""
if fil_material.upper() == material_upper and fil_color.upper() == color_upper:
if fil_material.upper() == material_upper and color_match_key(filament.get("color_hex")) == color:
bambu_candidates.append(filament)
if bambu_candidates:
@@ -1296,7 +1305,7 @@ class SpoolmanClient:
name=tray.tray_sub_brands or tray.tray_type,
vendor_id=bambu_vendor_id,
material=tray.tray_type,
color_hex=color_hex,
color_hex=color,
weight=tray.tray_weight,
)
@@ -1307,7 +1316,11 @@ class SpoolmanClient:
name=external.get("name", tray.tray_sub_brands),
vendor_id=vendor_id,
material=external.get("material", tray.tray_type),
color_hex=external.get("color_hex", tray.tray_color[:6]),
# `or`, not a two-argument get: an entry that carries the key with an
# explicit null would hand None to create_filament rather than reach
# the tray fallback. Only a candidate when the tray colour is empty
# too, so this is a correctness tidy, not a fix for a live path.
color_hex=external.get("color_hex") or color_match_key(tray.tray_color),
weight=external.get("weight", tray.tray_weight),
density=external.get("density"),
)
+54
View File
@@ -2,6 +2,60 @@
import math
# Alpha byte that means "fully opaque". Bambu's firmware reports every opaque
# spool as RRGGBBFF, so this is the overwhelmingly common value.
_OPAQUE_ALPHA = "FF"
def spoolman_color_hex(rgba: str | None) -> str | None:
"""Normalise an RRGGBB(AA) value to what Spoolman's ``color_hex`` should hold.
Eight characters only when the spool is genuinely translucent. Bambuddy used
to truncate to six unconditionally, which turned a clear spool's ``00000000``
into opaque black (#2912); passing everything through instead would rewrite
the ``color_hex`` of every opaque spool on its next touch, churning records in
people's Spoolman for no benefit. Keeping the opaque case at six characters
leaves existing data byte-identical.
Returns ``None`` for a missing value. A value shorter than six characters is
passed through unchanged so a malformed colour is not reshaped into something
that looks valid; a value between six and eight is truncated to six, which is
what the pre-#2912 behaviour did and what the six-character path still means.
Neither is reachable through ``_validate_rgba``, which admits only 6 or 8.
"""
if not rgba:
return None
clean = rgba.strip().removeprefix("#").upper()
if len(clean) < 6:
return clean or None
if len(clean) >= 8 and clean[6:8] != _OPAQUE_ALPHA:
return clean[:8]
return clean[:6]
def color_match_key(color_hex: str | None) -> str:
"""Return the key two colours are compared on: **the shape they would be stored as**.
Deliberately the same rule as :func:`spoolman_color_hex`, so two colours match
exactly when storing them would produce the same value. That settles both
directions of the alpha question at once (#2912):
============== ============ ==================================================
value key consequence
============== ============ ==================================================
``000000`` ``000000`` existing six-character data
``000000FF`` ``000000`` still matches it — the upgrade guard, without which
the next AMS sync mints a duplicate filament for
every spool on the instance
``00000000`` ``00000000`` a clear spool gets its own filament and is never
conflated with the black one, in either direction
============== ============ ==================================================
Returns ``""`` rather than ``None`` for a missing value so callers can compare
without guarding, which is the only reason this is not simply an alias.
"""
return spoolman_color_hex(color_hex) or ""
def colors_similar(hex_a: str, hex_b: str, threshold: int = 50) -> bool:
"""Compare two RRGGBB(AA) hex colors with tolerance for RFID/firmware variations.
@@ -378,6 +378,88 @@ class TestSpoolmanInventoryCRUD:
assert call_args.args[0] == 7
assert call_args.args[1]["name"] == "PLA Matte"
@pytest.mark.asyncio
@pytest.mark.integration
async def test_create_spool_keeps_a_clear_colour_translucent(
self,
async_client: AsyncClient,
spoolman_settings,
mock_spoolman_client,
):
"""#2912: the create route truncated rgba to six characters, so entering
"fully transparent" by hand landed on the same opaque black as the AMS
case in the report."""
payload = {
"material": "PLA",
"rgba": "00000000",
"label_weight": 1000,
"weight_used": 0,
}
response = await async_client.post("/api/v1/spoolman/inventory/spools", json=payload)
assert response.status_code == 200
assert mock_spoolman_client.find_or_create_filament.call_args.kwargs["color_hex"] == "00000000"
@pytest.mark.asyncio
@pytest.mark.integration
async def test_create_spool_keeps_an_opaque_colour_at_six(
self,
async_client: AsyncClient,
spoolman_settings,
mock_spoolman_client,
):
"""The opaque case has to stay six characters or every create starts
writing a shape the rest of the instance does not hold."""
payload = {
"material": "PLA",
"rgba": "FF0000FF",
"label_weight": 1000,
"weight_used": 0,
}
response = await async_client.post("/api/v1/spoolman/inventory/spools", json=payload)
assert response.status_code == 200
assert mock_spoolman_client.find_or_create_filament.call_args.kwargs["color_hex"] == "FF0000"
@pytest.mark.asyncio
@pytest.mark.integration
async def test_update_alpha_only_edit_reaches_the_filament(
self,
async_client: AsyncClient,
spoolman_settings,
mock_spoolman_client,
):
"""#2912: making a spool translucent is a real change to the filament's
colour. Comparing bare RGB prefixes would call it a no-op and the edit
would never land."""
# Sample filament is FF0000; make it half-transparent.
payload = {"rgba": "FF000080"}
response = await async_client.patch("/api/v1/spoolman/inventory/spools/42", json=payload)
assert response.status_code == 200
mock_spoolman_client.patch_filament.assert_called_once()
assert mock_spoolman_client.patch_filament.call_args.args[1]["color_hex"] == "FF000080"
@pytest.mark.asyncio
@pytest.mark.integration
async def test_update_with_the_round_tripped_opaque_rgba_is_a_no_op(
self,
async_client: AsyncClient,
spoolman_settings,
mock_spoolman_client,
):
"""#2912: the read side hands the frontend FF0000FF for a filament stored
as FF0000, and the edit form sends it straight back. Comparing raw strings
would make metadata_unchanged permanently False and PATCH the filament on
every no-op edit.
"""
payload = {"rgba": "FF0000FF", "note": "unrelated change"}
response = await async_client.patch("/api/v1/spoolman/inventory/spools/42", json=payload)
assert response.status_code == 200
mock_spoolman_client.patch_filament.assert_not_called()
mock_spoolman_client.find_or_create_filament.assert_not_called()
@pytest.mark.asyncio
@pytest.mark.integration
async def test_update_shared_filament_falls_back_to_find_or_create(
@@ -900,3 +900,208 @@ class TestFindOrCreateFilament:
mock_create.assert_called_once()
assert mock_create.call_args.kwargs["density"] == 1.31
class TestColorHexAlphaHandling:
"""#2912 — a clear spool must not be stored as opaque black, and widening the
stored value must not mint duplicates against inventories that hold six
characters everywhere.
"""
@pytest.fixture
def client(self):
return SpoolmanClient("http://localhost:7912")
def _tray(self, tray_color: str) -> AMSTray:
return AMSTray(
ams_id=0,
tray_id=0,
tray_type="PLA",
tray_sub_brands="PLA Basic",
tray_color=tray_color,
remain=100,
tag_uid="",
tray_uuid="A1B2C3D4E5F6A1B2C3D4E5F6A1B2C3D4",
tray_info_idx="GFA00",
tray_weight=1000,
)
async def _posted_payload(self, client, color_hex: str) -> dict:
"""Run create_filament and return the JSON body it actually sent."""
with patch.object(client, "_get_client") as mock_get_client:
mock_http_client = AsyncMock()
mock_response = Mock()
mock_response.status_code = 200
mock_response.raise_for_status = Mock()
mock_response.json = Mock(return_value={"id": 99})
mock_http_client.post = AsyncMock(return_value=mock_response)
mock_get_client.return_value = mock_http_client
await client.create_filament(name="PLA Basic", material="PLA", color_hex=color_hex)
return mock_http_client.post.call_args.kwargs["json"]
@pytest.mark.asyncio
async def test_create_filament_stores_alpha_for_a_translucent_spool(self, client):
"""create_filament is the chokepoint every create funnels through — it
truncated to six characters unconditionally, which is what turned a clear
spool into opaque black."""
payload = await self._posted_payload(client, "00000000")
assert payload["color_hex"] == "00000000"
@pytest.mark.asyncio
async def test_create_filament_keeps_an_opaque_spool_at_six(self, client):
"""Passing everything through would rewrite the color_hex of every opaque
spool on its next touch. Existing data has to stay byte-identical."""
payload = await self._posted_payload(client, "FF0000FF")
assert payload["color_hex"] == "FF0000"
@pytest.mark.asyncio
async def test_clear_tray_creates_a_translucent_filament(self, client):
"""End-to-end through the AMS auto-create path with nothing to match."""
with (
patch.object(client, "ensure_bambu_vendor", AsyncMock(return_value=2)),
patch.object(client, "get_filaments", AsyncMock(return_value=[])),
patch.object(client, "get_external_filaments", AsyncMock(return_value=[])),
patch.object(client, "create_filament", AsyncMock(return_value={"id": 99})) as mock_create,
):
await client._find_or_create_filament(self._tray("00000000"))
assert mock_create.call_args.kwargs["color_hex"] == "00000000"
@pytest.mark.asyncio
async def test_opaque_tray_still_matches_an_existing_six_char_filament(self, client):
"""The upgrade hazard neither the report nor the original patch mentioned.
Every filament already in a user's Spoolman is stored six characters. If
the match compared full strings, an 8-char tray colour would stop matching
them and the next AMS sync would mint a duplicate filament for every spool
on the instance. An opaque tray keys to six characters and still matches.
"""
existing = {"id": 6, "name": "Black", "material": "PLA", "color_hex": "000000", "vendor_id": 2}
with (
patch.object(client, "ensure_bambu_vendor", AsyncMock(return_value=2)),
patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
patch.object(client, "get_external_filaments", AsyncMock()) as mock_external,
patch.object(client, "create_filament", AsyncMock()) as mock_create,
):
result = await client._find_or_create_filament(self._tray("000000FF"))
assert result is existing
mock_external.assert_not_called()
mock_create.assert_not_called()
@pytest.mark.asyncio
async def test_clear_tray_does_not_attach_to_the_black_filament(self, client):
"""A translucent tray keys to eight characters, so it does not match the
opaque filament of the same RGB and gets its own record instead."""
black = {"id": 6, "name": "Black", "material": "PLA", "color_hex": "000000", "vendor_id": 2}
with (
patch.object(client, "ensure_bambu_vendor", AsyncMock(return_value=2)),
patch.object(client, "get_filaments", AsyncMock(return_value=[black])),
patch.object(client, "get_external_filaments", AsyncMock(return_value=[])),
patch.object(client, "create_filament", AsyncMock(return_value={"id": 99})) as mock_create,
):
await client._find_or_create_filament(self._tray("00000000"))
assert mock_create.call_args.kwargs["color_hex"] == "00000000"
@pytest.mark.asyncio
async def test_black_tray_does_not_attach_to_a_clear_filament(self, client):
"""The inverse direction, which only became possible once 8-char values
were storable at all: without the alpha in the key, an opaque black roll
would match the clear filament, then render as the transparency
checkerboard and be named Clear. Whichever roll synced first would decide
and the other would be mislabelled.
"""
clear = {"id": 6, "name": "Clear", "material": "PLA", "color_hex": "00000000", "vendor_id": 2}
with (
patch.object(client, "ensure_bambu_vendor", AsyncMock(return_value=2)),
patch.object(client, "get_filaments", AsyncMock(return_value=[clear])),
patch.object(client, "get_external_filaments", AsyncMock(return_value=[])),
patch.object(client, "create_filament", AsyncMock(return_value={"id": 99})) as mock_create,
):
await client._find_or_create_filament(self._tray("000000FF"))
assert mock_create.call_args.kwargs["color_hex"] == "000000"
@pytest.mark.asyncio
async def test_clear_tray_does_not_take_a_same_rgb_external_entry(self, client):
"""The reported path on a fresh Spoolman with the external library
reachable. Candidates are built with the same key, so SpoolmanDB's opaque
"PLA Basic Black" is no longer a candidate for a clear tray and the
filament is created from the tray data with its alpha intact.
"""
external = [
{
"id": "bambulab_pla_black_1000_175_n",
"manufacturer": "Bambu Lab",
"name": "PLA Basic Black",
"material": "PLA",
"color_hex": "000000",
},
]
with (
patch.object(client, "ensure_bambu_vendor", AsyncMock(return_value=2)),
patch.object(client, "get_filaments", AsyncMock(return_value=[])),
patch.object(client, "get_external_filaments", AsyncMock(return_value=external)),
patch.object(client, "create_filament", AsyncMock(return_value={"id": 99})) as mock_create,
):
await client._find_or_create_filament(self._tray("00000000"))
assert mock_create.call_args.kwargs["color_hex"] == "00000000"
@pytest.mark.asyncio
async def test_find_or_create_filament_creates_a_clear_filament_with_its_alpha(self, client):
"""The user-driven path, with nothing to match. There is no split to pin
here: the key and the created value are the same string, and for a clear
spool that string is eight characters."""
with (
patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
patch.object(client, "get_filaments", AsyncMock(return_value=[])),
patch.object(client, "create_filament", AsyncMock(return_value={"id": 99})) as mock_create,
):
await client.find_or_create_filament(
material="PLA",
subtype="Basic",
brand="Bambu Lab",
color_hex="00000000",
label_weight=1000,
)
assert mock_create.call_args.kwargs["color_hex"] == "00000000"
@pytest.mark.asyncio
async def test_find_or_create_filament_matches_an_existing_six_char_filament(self, client):
"""The upgrade guard on the *other* match loop.
`test_opaque_tray_still_matches_an_existing_six_char_filament` pins it for
the AMS path. The public `find_or_create_filament` has its own loop, and
the non-BL RFID auto-create (spoolman.py, `_sync_tray_to_spoolman`) now
hands it `tray.tray_color` whole where it used to hand over
`tray.tray_color[:6]`. If the opaque fold ever came off this key, every
non-Bambu RFID spool on an instance would mint a duplicate filament on the
next sync and nothing would go red.
"""
existing = {
"id": 7,
"name": "PLA Basic",
"material": "PLA",
"color_hex": "FF0000",
"vendor": {"id": 3, "name": "Bambu Lab"},
}
with (
patch.object(client, "find_or_create_vendor", AsyncMock(return_value=3)),
patch.object(client, "get_filaments", AsyncMock(return_value=[existing])),
patch.object(client, "create_filament", AsyncMock()) as mock_create,
):
result = await client.find_or_create_filament(
material="PLA",
subtype="Basic",
brand="Bambu Lab",
color_hex="FF0000FF",
label_weight=1000,
)
assert result == 7
mock_create.assert_not_called()
+65 -1
View File
@@ -1,6 +1,6 @@
"""Unit tests for color_utils — hex color similarity comparison."""
from backend.app.utils.color_utils import colors_similar
from backend.app.utils.color_utils import color_match_key, colors_similar, spoolman_color_hex
class TestColorsSimilar:
@@ -52,3 +52,67 @@ class TestColorsSimilar:
def test_white_and_off_white(self):
assert colors_similar("FFFFFF", "F0F0F0") is True
class TestSpoolmanColorHex:
"""#2912 — what a colour is stored as in Spoolman's color_hex."""
def test_opaque_value_stays_six_characters(self):
"""The common case must be byte-identical to what is already stored, or
every opaque spool gets rewritten on its next touch."""
assert spoolman_color_hex("FF0000FF") == "FF0000"
def test_translucent_value_keeps_its_alpha(self):
assert spoolman_color_hex("FF000080") == "FF000080"
def test_fully_transparent_keeps_its_alpha(self):
"""The reported case: a clear spool reads as 00000000 and must not be
stored as opaque black."""
assert spoolman_color_hex("00000000") == "00000000"
def test_six_character_input_passes_through(self):
assert spoolman_color_hex("00FF00") == "00FF00"
def test_normalises_case_and_hash_prefix(self):
assert spoolman_color_hex("#ff000080") == "FF000080"
def test_none_and_empty_return_none(self):
assert spoolman_color_hex(None) is None
assert spoolman_color_hex("") is None
def test_short_value_passes_through_rather_than_being_padded(self):
"""A malformed value is reported as it is, not reshaped into something
that looks valid."""
assert spoolman_color_hex("FFF") == "FFF"
class TestColorMatchKey:
"""#2912 — two colours match exactly when storing them would give the same value."""
def test_opaque_value_matches_its_six_character_twin(self):
"""The upgrade hazard: a user's existing filaments are all stored six
characters. If an opaque 8-char value stopped matching them, the next AMS
sync would mint a duplicate filament for every spool on the instance."""
assert color_match_key("FF0000FF") == color_match_key("FF0000")
def test_translucent_value_does_not_match_its_opaque_twin(self):
"""Both directions. A clear roll must not attach to the black filament,
and — the case that only exists once 8-char values are storable — a black
roll must not attach to a clear one and inherit its swatch and name."""
assert color_match_key("00000000") != color_match_key("000000")
assert color_match_key("00000000") != color_match_key("000000FF")
def test_differs_when_the_rgb_differs(self):
assert color_match_key("FF0000FF") != color_match_key("00FF00FF")
def test_is_the_stored_shape(self):
"""Stated as an invariant because three separate comparisons rely on it."""
for value in ("FF0000", "FF0000FF", "FF000080", "00000000"):
assert color_match_key(value) == spoolman_color_hex(value)
def test_normalises_case_and_hash_prefix(self):
assert color_match_key("#ff0000") == "FF0000"
def test_missing_value_is_empty_string(self):
assert color_match_key(None) == ""
assert color_match_key("") == ""
@@ -192,9 +192,37 @@ class TestMapSpoolmanSpool:
result = _map_spoolman_spool(spool)
assert result["rgba"] == "808080FF"
def test_eight_char_color_hex_falls_back(self):
# Only 6-char hex is valid from Spoolman; 8-char (RGBA) should fall back
spool = {**MINIMAL_SPOOL, "filament": {**MINIMAL_SPOOL["filament"], "color_hex": "FF0000FF"}}
def test_eight_char_color_hex_is_read_back_with_its_alpha(self):
"""#2912: 8-char color_hex is a value the write side stores on purpose for a
translucent spool, so the read must return it rather than grey it out.
This test previously asserted the opposite, on the premise that only 6-char
hex was valid from Spoolman. Spoolman stores whatever it is given, and
Bambuddy's own rgba fields advertise RRGGBBAA — the read was what turned a
clear spool into neutral grey.
"""
spool = {**MINIMAL_SPOOL, "filament": {**MINIMAL_SPOOL["filament"], "color_hex": "FF000080"}}
result = _map_spoolman_spool(spool)
assert result["rgba"] == "FF000080"
def test_fully_transparent_color_hex_survives_the_read(self):
"""The reported case: a clear spool stored as 00000000 must not come back
as opaque black."""
spool = {**MINIMAL_SPOOL, "filament": {**MINIMAL_SPOOL["filament"], "color_hex": "00000000"}}
result = _map_spoolman_spool(spool)
assert result["rgba"] == "00000000"
def test_six_char_color_hex_still_gains_the_opaque_alpha(self):
"""Existing data is 6-char and must keep round-tripping unchanged — the
opaque byte is appended, not doubled onto an alpha that is already there."""
spool = {**MINIMAL_SPOOL, "filament": {**MINIMAL_SPOOL["filament"], "color_hex": "FF0000"}}
result = _map_spoolman_spool(spool)
assert result["rgba"] == "FF0000FF"
def test_seven_char_color_hex_falls_back(self):
"""Only 6 or 8 are valid lengths; a 7-char value is malformed and still
greys out rather than being padded into something plausible."""
spool = {**MINIMAL_SPOOL, "filament": {**MINIMAL_SPOOL["filament"], "color_hex": "FF0000F"}}
result = _map_spoolman_spool(spool)
assert result["rgba"] == "808080FF"
@@ -109,6 +109,27 @@ describe('AssignSpoolModal', () => {
expect(screen.getByText(/Jade White/)).toBeInTheDocument();
});
it('renders a clear tray as the transparency checkerboard, not an invisible circle (#2912)', async () => {
render(
<AssignSpoolModal
{...defaultProps}
trayInfo={{ type: 'PETG', color: '00000000', location: 'AMS 1 - Slot 1' }}
/>,
);
await waitFor(() => {
expect(screen.getByText(/PETG/)).toBeInTheDocument();
});
// The tray swatch used to be painted `#00000000` directly, which is a valid
// colour the browser renders as nothing at all — a clear spool showed an
// empty circle. getSwatchStyle draws the checkerboard instead.
const swatches = Array.from(document.querySelectorAll<HTMLElement>('[style]'));
expect(
swatches.some((el) => el.style.backgroundImage.includes('repeating-conic-gradient')),
).toBe(true);
});
it('filters out spools already assigned to other slots', async () => {
(api.getAssignments as ReturnType<typeof vi.fn>).mockResolvedValue([
{ id: 1, spool_id: 3, printer_id: 1, ams_id: 0, tray_id: 1 }, // spool 3 assigned to different slot
@@ -96,6 +96,23 @@ describe('ConfigureAmsSlotModal', () => {
expect(screen.queryByText('Configure AMS Slot')).not.toBeInTheDocument();
});
it('renders a clear tray as the transparency checkerboard rather than solid black (#2912)', () => {
render(
<ConfigureAmsSlotModal
{...defaultProps}
slotInfo={{ ...defaultProps.slotInfo, trayType: 'PETG', trayColor: '00000000' }}
/>,
);
const styled = Array.from(document.querySelectorAll<HTMLElement>('[style]'));
expect(
styled.some((el) => el.style.backgroundImage.includes('repeating-conic-gradient')),
).toBe(true);
// The regression this pins: the alpha byte was sliced off before painting,
// so a clear tray rendered as opaque black — the reported symptom, in the UI.
expect(styled.some((el) => el.style.backgroundColor === 'rgb(0, 0, 0)')).toBe(false);
});
it('renders modal when open', async () => {
render(<ConfigureAmsSlotModal {...defaultProps} />);
await waitFor(() => {
@@ -100,6 +100,25 @@ describe('LinkSpoolModal', () => {
});
});
it('paints a clear spool as the checkerboard, not an invisible swatch', async () => {
// Same swatch bug as the filament picker: this row built its background as
// `#${filament_color_hex}`, so a clear spool — which only reaches the list
// with its alpha intact because of #2912 — rendered as an empty circle.
vi.mocked(api.getUnlinkedSpools).mockResolvedValue([
{ ...mockSpools[0], id: 3, filament_name: 'Bambu PLA Clear', filament_color_hex: '00000000' },
]);
render(<LinkSpoolModal {...defaultProps} />);
await waitFor(() => {
expect(screen.getByText(/Bambu PLA Clear/)).toBeInTheDocument();
});
const swatch = document.querySelector('span.rounded-full') as HTMLElement;
expect(swatch.style.backgroundImage).toContain('repeating-conic-gradient');
expect(swatch.style.backgroundColor).toBe('');
});
it('displays unlinked spools from Spoolman', async () => {
render(<LinkSpoolModal {...defaultProps} />);
@@ -703,6 +703,18 @@ vi.mock('../../components/spool-form/SpoolmanFilamentPicker', () => ({
})}>
Select Filament
</button>
<button data-testid="picker-select-clear-btn" onClick={() => onSelect({
id: 8,
name: 'PLA Basic Clear',
material: 'PLA',
color_hex: '00000000',
color_name: 'Clear',
weight: 1000,
spool_weight: 196,
vendor: { id: 1, name: 'Bambu Lab' },
})}>
Select Clear Filament
</button>
</div>
);
},
@@ -765,6 +777,66 @@ describe('SpoolFormModal — SpoolmanFilamentPicker integration (T2)', () => {
});
});
it('prefills a translucent filament with its own alpha, not 808080FF (#2912)', async () => {
// The guard here required exactly 6 hex chars and then appended FF. That was
// unreachable while Bambuddy never wrote 8 characters; once a clear filament
// is storable, picking it out of the Spoolman catalogue prefilled the form
// with neutral grey — the frontend twin of the read-side regex.
render(
<SpoolFormModal
isOpen={true}
onClose={vi.fn()}
currencySymbol="$"
spoolmanMode={true}
spoolsQueryKey={['spoolman-spools']}
/>
);
await waitFor(() => {
expect(screen.getByTestId('picker-select-clear-btn')).toBeInTheDocument();
});
fireEvent.click(screen.getByTestId('picker-select-clear-btn'));
const saveButton = screen.getByRole('button', { name: /save|add spool/i });
fireEvent.click(saveButton);
await waitFor(() => {
expect(api.createSpoolmanInventorySpool).toHaveBeenCalledTimes(1);
});
const payload = vi.mocked(api.createSpoolmanInventorySpool).mock.calls[0][0] as Record<string, unknown>;
expect(payload.rgba).toBe('00000000');
});
it('still appends the opaque alpha to a 6-char catalogue colour', async () => {
render(
<SpoolFormModal
isOpen={true}
onClose={vi.fn()}
currencySymbol="$"
spoolmanMode={true}
spoolsQueryKey={['spoolman-spools']}
/>
);
await waitFor(() => {
expect(screen.getByTestId('picker-select-btn')).toBeInTheDocument();
});
fireEvent.click(screen.getByTestId('picker-select-btn'));
const saveButton = screen.getByRole('button', { name: /save|add spool/i });
fireEvent.click(saveButton);
await waitFor(() => {
expect(api.createSpoolmanInventorySpool).toHaveBeenCalledTimes(1);
});
const payload = vi.mocked(api.createSpoolmanInventorySpool).mock.calls[0][0] as Record<string, unknown>;
expect(payload.rgba).toBe('FF0000FF');
});
it('includes spoolman_filament_id in the submit payload when a filament is pre-selected', async () => {
render(
<SpoolFormModal
@@ -185,4 +185,24 @@ describe('SpoolmanFilamentPicker', () => {
// PLA Basic has color_name 'Red' — should be filtered out
expect(screen.queryByText(/PLA Basic/)).toBeNull();
});
it('paints a clear filament as the checkerboard, not an invisible swatch', () => {
// Storing the alpha is what puts 00000000 into this list in the first place,
// and the hand-rolled `#${hex}` background this swatch used to build renders
// that as nothing at all — a fully transparent circle (#2912).
const clear: SpoolmanFilamentEntry[] = [
{ ...FILAMENTS[0], id: 4, name: 'PLA Basic Clear', color_hex: '00000000', color_name: 'Clear' },
];
render(
<SpoolmanFilamentPicker
filaments={clear}
isLoading={false}
selectedId={4}
onSelect={vi.fn()}
/>
);
const swatch = screen.getByLabelText('inventory.spoolmanFilamentColorSwatch');
expect(swatch.style.backgroundImage).toContain('repeating-conic-gradient');
expect(swatch.style.backgroundColor).toBe('');
});
});
@@ -1,6 +1,7 @@
import { describe, it, expect, beforeEach } from 'vitest';
import {
disambiguateColorNames,
getSwatchStyle,
colorFamily,
colorSortKey,
hexToColorName,
@@ -343,3 +344,48 @@ describe('disambiguateColorNames', () => {
expect(disambiguateColorNames({}, {})).toEqual(['', '']);
});
});
describe('getSwatchStyle (#1545, #2912)', () => {
const CHECKERBOARD = 'repeating-conic-gradient(#979797 0% 25%, #f5f5f5 0% 50%)';
it('falls back to neutral grey for missing or unparseable input', () => {
expect(getSwatchStyle(null)).toEqual({ backgroundColor: '#808080' });
expect(getSwatchStyle(undefined)).toEqual({ backgroundColor: '#808080' });
expect(getSwatchStyle('')).toEqual({ backgroundColor: '#808080' });
expect(getSwatchStyle('ABC')).toEqual({ backgroundColor: '#808080' });
});
it('paints an opaque colour flat, with or without the FF byte', () => {
expect(getSwatchStyle('FF0000')).toEqual({ backgroundColor: '#FF0000' });
expect(getSwatchStyle('FF0000FF')).toEqual({ backgroundColor: '#FF0000' });
expect(getSwatchStyle('#FF0000FF')).toEqual({ backgroundColor: '#FF0000' });
});
it('shows the checkerboard alone for a fully transparent colour', () => {
expect(getSwatchStyle('00000000')).toEqual({
backgroundImage: CHECKERBOARD,
backgroundSize: '8px 8px',
});
});
it('layers a partly translucent colour over the checkerboard (#2912)', () => {
// Regression: this used to fall through to the RGB prefix, so a 50%-alpha
// spool rendered identically to an opaque one. Spoolman mode can now store
// any non-FF alpha, so the in-between case is reachable in normal use.
const style = getSwatchStyle('FF000080');
expect(style.backgroundColor).toBeUndefined();
expect(style.backgroundImage).toBe(
`linear-gradient(#FF000080, #FF000080), ${CHECKERBOARD}`,
);
expect(style.backgroundSize).toBe('100% 100%, 8px 8px');
});
it('treats the alpha byte case-insensitively', () => {
expect(getSwatchStyle('ff0000ff')).toEqual({ backgroundColor: '#ff0000' });
expect(getSwatchStyle('ff000000')).toEqual({
backgroundImage: CHECKERBOARD,
backgroundSize: '8px 8px',
});
});
});
+1 -1
View File
@@ -381,7 +381,7 @@ export function AssignSpoolModal({ isOpen, onClose, printerId, amsId, trayId, tr
{trayInfo.color && (
<span
className="w-4 h-4 rounded-full border border-black/20"
style={{ backgroundColor: `#${trayInfo.color}` }}
style={getSwatchStyle(trayInfo.color)}
/>
)}
<span className="text-white font-medium">{trayInfo.type || t('ams.emptySlot')}</span>
@@ -8,6 +8,7 @@ import { matchesPrinterModelSuffix, presetCompatibility, buildCompatibilityIndex
import { toFilamentId } from './spool-form/utils';
import { Button } from './Button';
import { getAmsLabel } from '../utils/amsHelpers';
import { getSwatchStyle } from '../utils/colors';
import { useCancellableTimeout } from '../hooks/useCancellableTimeout';
interface SlotInfo {
@@ -1179,7 +1180,10 @@ export function ConfigureAmsSlotModal({
const canSave = selectedPresetId && !configureMutation.isPending;
// Get display color (custom or slot default)
const displayColor = colorHex || slotInfo.trayColor?.slice(0, 6) || 'FFFFFF';
// Not sliced to six: a clear tray reports RRGGBB00, and cutting the alpha off
// here previewed it as solid black. `colorHex` is the edited form value and is
// always six characters, so only the tray fallback ever carries an alpha (#2912).
const displayColor = colorHex || slotInfo.trayColor || 'FFFFFF';
return (
<div className={`fixed inset-0 z-50 flex ${fullScreen ? '' : 'items-center justify-center'}`}>
@@ -1208,7 +1212,7 @@ export function ConfigureAmsSlotModal({
{slotInfo.trayColor && (
<span
className="w-4 h-4 rounded-full border border-black/20"
style={{ backgroundColor: `#${slotInfo.trayColor.slice(0, 6)}` }}
style={getSwatchStyle(slotInfo.trayColor)}
/>
)}
<span className="text-white/70">
@@ -1249,7 +1253,7 @@ export function ConfigureAmsSlotModal({
{slotInfo.trayColor && (
<span
className="w-4 h-4 rounded-full border border-black/20"
style={{ backgroundColor: `#${slotInfo.trayColor.slice(0, 6)}` }}
style={getSwatchStyle(slotInfo.trayColor)}
/>
)}
<span className="text-white font-medium">
@@ -1468,7 +1472,7 @@ export function ConfigureAmsSlotModal({
<div className="flex gap-2 items-center">
<div
className="w-10 h-10 rounded-lg border-2 border-white/20 flex-shrink-0"
style={{ backgroundColor: `#${displayColor}` }}
style={getSwatchStyle(displayColor)}
/>
<input
type="text"
@@ -1717,7 +1721,7 @@ export function ConfigureAmsSlotModal({
<div className="flex gap-2 items-center">
<div
className="w-10 h-10 rounded-lg border-2 border-white/20 flex-shrink-0"
style={{ backgroundColor: `#${displayColor}` }}
style={getSwatchStyle(displayColor)}
/>
<input
type="text"
+2 -1
View File
@@ -6,6 +6,7 @@ import { api } from '../api/client';
import type { UnlinkedSpool } from '../api/client';
import { Button } from './Button';
import { useToast } from '../contexts/ToastContext';
import { getSwatchStyle } from '../utils/colors';
interface LinkSpoolModalProps {
isOpen: boolean;
@@ -126,7 +127,7 @@ export function LinkSpoolModal({ isOpen, onClose, tagUid, trayUuid, printerId, a
>
<span
className="w-6 h-6 rounded-full border border-black/20 flex-shrink-0"
style={{ backgroundColor: spool.filament_color_hex ? `#${spool.filament_color_hex}` : '#808080' }}
style={getSwatchStyle(spool.filament_color_hex)}
/>
<div className="flex-1 min-w-0">
<div className="text-sm text-white font-medium truncate">
+6 -3
View File
@@ -450,15 +450,18 @@ export function SpoolFormModal({
const name = filament.name || '';
const subtype = material && name.startsWith(material) ? name.slice(material.length).trim() : name;
const rawHex = (filament.color_hex ?? '').replace('#', '').toUpperCase();
// Guard against short/malformed hex values — must be exactly 6 hex chars
const colorHex = /^[0-9A-F]{6}$/.test(rawHex) ? rawHex : '808080';
// Guard against short/malformed hex values — 6 chars (RRGGBB), or 8 when the
// filament is translucent and carries its own alpha (#2912). Rejecting 8 here
// prefilled a clear filament picked from the Spoolman catalogue as 808080FF.
const colorHex = /^[0-9A-F]{6}(?:[0-9A-F]{2})?$/.test(rawHex) ? rawHex : '808080';
const prefillRgba = colorHex.length === 8 ? colorHex : `${colorHex}FF`;
setFormData(prev => ({
...prev,
spoolman_filament_id: filament.id,
material,
subtype,
brand: filament.vendor?.name || '',
rgba: `${colorHex}FF`,
rgba: prefillRgba,
color_name: filament.color_name || '',
label_weight: filament.weight ?? prev.label_weight,
}));
@@ -2,6 +2,7 @@ import { useState, useRef, useEffect, useMemo } from 'react';
import { ChevronDown, Loader2, Package } from 'lucide-react';
import { useTranslation } from 'react-i18next';
import type { SpoolmanFilamentEntry } from '../../api/client';
import { getSwatchStyle } from '../../utils/colors';
interface SpoolmanFilamentPickerProps {
filaments: SpoolmanFilamentEntry[];
@@ -65,9 +66,6 @@ export function SpoolmanFilamentPicker({
setSearch('');
};
const colorStyle = (hex: string | null): string =>
hex ? `#${hex.replace('#', '')}` : '#808080';
return (
<div ref={containerRef} className="relative">
<label className="block text-sm font-medium text-bambu-gray mb-1">
@@ -86,7 +84,7 @@ export function SpoolmanFilamentPicker({
<>
<span
className="w-4 h-4 rounded-full shrink-0 border border-white/20"
style={{ backgroundColor: colorStyle(selected.color_hex) }}
style={getSwatchStyle(selected.color_hex)}
aria-label={t('inventory.spoolmanFilamentColorSwatch')}
/>
<span className="text-white text-sm truncate flex-1">
@@ -143,7 +141,7 @@ export function SpoolmanFilamentPicker({
>
<span
className="w-4 h-4 rounded-full shrink-0 border border-white/20"
style={{ backgroundColor: colorStyle(f.color_hex) }}
style={getSwatchStyle(f.color_hex)}
aria-label={t('inventory.spoolmanFilamentColorSwatch')}
/>
<span className="flex-1 min-w-0">
+31 -7
View File
@@ -334,13 +334,20 @@ export function spoolColorString(rgba: string | null | undefined): string {
return `#${clean.substring(0, 6)}`;
}
// The transparency checkerboard, shared by every swatch that has to show a
// translucent colour. One definition so the fully-transparent and partly-
// transparent branches below cannot drift apart.
const CHECKERBOARD = 'repeating-conic-gradient(#979797 0% 25%, #f5f5f5 0% 50%)';
/**
* Build an inline-style object for a simple filament swatch (a div / button
* background) given a spool's rgba. Opaque colours return a plain
* `backgroundColor`; transparent (alpha=00) returns a small checkerboard
* pattern so the user can see the swatch instead of an invisible element
* (#1545). Null / unparseable input falls back to the neutral `#808080` used
* elsewhere in the codebase.
* (#1545); anything in between returns the colour at its real alpha layered
* over that checkerboard, so a half-translucent spool reads as neither opaque
* nor blank (#2912). Null / unparseable input falls back to the neutral
* `#808080` used elsewhere in the codebase.
*
* Use this anywhere a quick swatch was previously painted via
* `style={{ backgroundColor: '#' + rgba.slice(0, 6) }}` — alpha-stripping
@@ -358,11 +365,28 @@ export function getSwatchStyle(rgba: string | null | undefined): {
if (!rgba) return { backgroundColor: '#808080' };
const clean = rgba.replace(/^#/, '');
if (clean.length < 6) return { backgroundColor: '#808080' };
if (clean.length >= 8 && clean.substring(6, 8).toLowerCase() === '00') {
return {
backgroundImage: 'repeating-conic-gradient(#979797 0% 25%, #f5f5f5 0% 50%)',
backgroundSize: '8px 8px',
};
if (clean.length >= 8) {
const alpha = clean.substring(6, 8).toLowerCase();
if (alpha === '00') {
return {
backgroundImage: CHECKERBOARD,
backgroundSize: '8px 8px',
};
}
if (alpha !== 'ff') {
// Partly translucent: paint the colour at its real alpha *over* the
// checkerboard, so the swatch shows both the tint and that it is
// see-through. Dropping to the RGB prefix here would render a 10%-alpha
// spool identically to an opaque one, and painting it alone would leave
// it near-invisible against the panel behind it. Two background layers
// rather than backgroundColor: a background colour paints *under* the
// image, which would put the checkerboard on top of the tint (#2912).
const translucent = `#${clean.substring(0, 8)}`;
return {
backgroundImage: `linear-gradient(${translucent}, ${translucent}), ${CHECKERBOARD}`,
backgroundSize: '100% 100%, 8px 8px',
};
}
}
return { backgroundColor: `#${clean.substring(0, 6)}` };
}
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -26,7 +26,7 @@
<!-- Splash screens for iOS -->
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
<script type="module" crossorigin src="/assets/index-Bo6-nt-q.js"></script>
<script type="module" crossorigin src="/assets/index-CgFeKbCm.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-C7cOM7tZ.css">
</head>
<body>