Conversation
maziggy
left a comment
There was a problem hiding this comment.
A few items to address before we can merge:
1. Runtime crash — missing printer_manager import (blocker)
backend/app/api/routes/inventory.py — list_assignments calls printer_manager.get_status(pid) but never imports it. This will NameError on every GET
/inventory/assignments request. The pattern used elsewhere in this file is an inline import:
async def list_assignments(...):
from backend.app.services.printer_manager import printer_manager
# ... rest of function
2. N+1 queries in get_ams_labels fallback loop
backend/app/api/routes/printers.py — The synthetic-key backward-compat loop issues one SELECT per AMS unit. With dual-AMS on older firmware (no serial reporting), that's up to 8 separate queries. Collect all synthetic keys first and use a single IN clause — same pattern as the real-serial query block right above it.
3. AMS version changes don't broadcast without OTA module
backend/app/services/bambu_mqtt.py — state_changed is only set True inside the OTA block, so if get_version contains AMS modules but no OTA entry, the serial/firmware
values get written to raw_data but on_state_change never fires. Fix: also set state_changed = True when caching AMS version data.
4. PUT endpoint should use JSON body instead of query params
backend/app/api/routes/printers.py + frontend/src/api/client.ts — save_ams_label passes the label text as a URL query parameter (?label=...). This shows up in server
access logs and is unconventional for mutations. Recommend switching to a Pydantic JSON body model — this also lets you add min_length=1 to reject empty labels at the
API boundary (currently ?label= stores a blank row).
5. Minor: simplify onBlur hover detection
frontend/src/pages/PrintersPage.tsx — element.matches(':hover') in the onBlur handler is unreliable in Firefox during focus transitions. Since handleMouseLeave already
guards on !isInputFocused, the onBlur can just clear the flag and let the mouse-leave timeout handle closing naturally.
|
@maziggy Comments have been incorporated |
maziggy
left a comment
There was a problem hiding this comment.
Still a few items to address:
Bug — Hover card gets stuck open
When the user focuses the friendly name input and then clicks elsewhere, onBlur sets isInputFocused = false but doesn't check if the mouse already left the card. The popup stays open indefinitely until the user hovers back and leaves again.
Fix in AmsNameHoverCard:
onBlur={() => {
setIsInputFocused(false);
timeoutRef.current = setTimeout(() => setIsVisible(false), 200);
}}
"Clear" button UX
The "Clear" button only empties the input — the user still has to click "Save" to actually delete the label. Either rename it to "Reset" or have it call deleteAmsLabel directly and close the popup.
Warning logs may be noisy
_handle_version_info now logs warnings for every AMS unit missing serial/firmware after each get_version response. On older firmware that doesn't report these fields,
this will fire repeatedly. Consider debug level or warning once per AMS unit.
list_assignments performance
This endpoint now calls printer_manager.get_status() per unique printer + queries ams_labels on every Inventory page load. Not a blocker but worth keeping an eye on
with many printers.
Minor: Slot number rendering duplicated 4x
The filament circle + slot number JSX is copy-pasted across regular AMS, HT AMS, external spool, and printing state sections. A small FilamentSlotCircle component would
prevent drift between copies.
|
@maziggy Please re-review |
|
Please rebase to resolve conflict. |
|
@maziggy You all work so fast it's hard to keep up. ;). Isn't 0.2.2b1 the latest? |
|
@maziggy Are we good on changes for this one? |
Description
Adds two UX improvements to the AMS display on the Printers page: a hover popup on each AMS label exposing hardware details and a user-editable friendly name, and 1-based slot numbers centered inside each filament color circle with auto-inverted contrast.
AMS name hover popup
Hovering the AMS label (e.g. AMS-A) opens a popover showing:
Related Issue
Fixes #464
Type of Change
Changes Made
Backend
_handle_version_info in bambu_mqtt.py extended to parse ams/ modules and write sw_ver/sn back onto the stored AMS unit dict.
Frontend
Bug Fixes
Screenshots
Testing
Checklist
Additional Notes