mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-04 21:21:54 +02:00
Fix: AMS drying popover positioning + diagnostic logging (#1447)
Two bugs in one report, both shipped here.
(1) Popover positioning. The flame-icon onClick on PrintersPage
computed popover position as a fixed { top: rect.bottom + 4,
left: Math.max(8, rect.right - 240) } with no viewport-overflow
check. The flame icon sits at the bottom of the AMS info section
on the printer card, so on most realistic viewports
rect.bottom + 4 + popover_height (~320px) overruns viewport.height
and the popover renders partially or entirely off-screen with the
Start button unreachable. Reporter worked around it via DevTools to
confirm the popover was actually there, just clipped.
Extract a computePopoverPosition() helper in utils/popoverPosition.ts:
- defaults to below + right-aligned to the trigger (preserves the
original visual layout when there's room),
- flips ABOVE the trigger when below would overflow AND above fits,
- stays below in the degraded case (popover taller than viewport) —
at least the top is visible and the user can scroll inside; flipping
to a top-clipped position would lose the action buttons too,
- clamps the left coordinate so a trigger near either viewport edge
can't push the popover off-screen horizontally either.
Both PrintersPage callsites (compact AMS row at :3498 and dual-nozzle
layout at :4011) route through the helper.
(2) Diagnostic logging for the silent-drying-ignore. Reporter's
support bundle shows the printer receives every ams_filament_drying
command (P1S 01.10.00.00 firmware, AMS-HT at ams_id=128) and ACKs
each one, but the AMS info field never changes — drying neither
starts nor stops on Bambuddy's request, while pressing Start on the
printer's touchscreen works immediately. The command JSON matches
the format documented as working on H2D, all required fields present.
Diagnosing the silent rejection needs the printer's actual response
payload — result/reason — but bambu_mqtt.py:918 was only logging the
response command name, not the body. The existing extrusion_cali_* /
ams_filament_setting debug path at :919-920 was the template; this
PR extends it to ams_filament_drying at INFO level (not DEBUG like
its siblings) because drying responses are rare (user-initiated only)
and INFO ensures the body lands in support bundles by default without
the user having to bump log level first. Paired with an outgoing-side
INFO log inside send_drying_command that captures the full wire JSON,
so the next bundle has both halves of the conversation.
No guessing on the command-side. Mutating a field that matches the
documented-working H2D shape (e.g. flipping close_power_conflict)
could break currently-working installs. When the reporter retries on
this build and re-attaches a bundle, the rejection reason is visible
and the command-side fix follows from real data.
This commit is contained in:
maziggy
+1
-1
@@ -5,7 +5,7 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
## [0.2.5b1] - Unreleased
|
||||
|
||||
### Fixed
|
||||
- **AMS drying popover no longer renders off the bottom of the viewport (#1447 part 1, reported by @kleinweby)** — Reporter on P1S + AMS-HT couldn't see the Start button on the drying popover; he worked around it via DevTools to confirm the popover was actually there, just clipped below the fold. Root cause in `frontend/src/pages/PrintersPage.tsx:3489 / :4002` (two identical sites — one for the compact AMS row, one for the dual-nozzle layout): the flame-icon onClick computed popover position as a fixed `{ top: rect.bottom + 4, left: Math.max(8, rect.right - 240) }` with no viewport-overflow check. The flame icon sits at the bottom of the AMS info section on the printer card, so on most realistic viewports `rect.bottom + 4 + popover_height(~320px) > viewport.height` and the popover rendered partially or entirely off-screen. **Fix** extracts a small `computePopoverPosition()` helper in `frontend/src/utils/popoverPosition.ts` that defaults to placing the popover below + right-aligned to the trigger (preserving the original visual layout), flips ABOVE the trigger when below would overflow AND above would fit, stays below in the degraded case where neither fits (a popover taller than the viewport — at least the top is visible and the user can scroll inside), and clamps the left coordinate so a trigger near either viewport edge can't push the popover off-screen horizontally either. Both PrintersPage callsites now go through the helper. **NOTE — the underlying functional bug is NOT yet fixed**: kleinweby's support bundle shows the printer receives the `ams_filament_drying` MQTT command (multiple start / stop attempts on `ams_id=128`, P1S 01.10.00.00 firmware), the printer ACKs each one with `Received command response: ams_filament_drying`, but the AMS info field never changes — drying neither starts nor stops on Bambuddy's request, while pressing Start on the printer's own touchscreen worked immediately (so the hardware path is healthy). The Bambuddy command JSON matches the format documented as working on H2D, all required fields are present, types match BambuStudio. Diagnosing further needs the printer's actual response payload (whether `result: "fail"` and the specific `reason` code), and `bambu_mqtt.py:918` currently only logs the response *command name*, not the body — the existing `extrusion_cali_*` / `ams_filament_setting` debug path at `:919-920` is the template, just not extended to `ams_filament_drying`. Punted to a follow-up where I'll add the response-payload logging at INFO level (so it lands in support bundles), ask kleinweby to retry, and fix the actual command-side once we see what the printer is rejecting. The UI fix ships now because it's deterministic and unblocks the immediate "I can't see the button" symptom regardless of the MQTT outcome. **Tests** (8 new in `__tests__/utils/popoverPosition.test.ts`): below-has-room → places below; right-align to trigger; below overflows → flips above; degraded case (popover taller than viewport) → stays below; clamps right-edge and left-edge triggers; respects custom margin and gap. Frontend build clean.
|
||||
- **AMS drying popover no longer renders off the bottom of the viewport + diagnostic logging for the silent-drying-ignore bug (#1447, reported by @kleinweby)** — Two distinct bugs in the same report, both shipped in this PR. **(1) Popover positioning**: reporter on P1S + AMS-HT couldn't see the Start button on the drying popover and worked around it via DevTools to confirm the popover was actually there, just clipped below the fold. Root cause in `frontend/src/pages/PrintersPage.tsx:3498 / :4011` (two identical sites — one for the compact AMS row, one for the dual-nozzle layout): the flame-icon onClick computed popover position as a fixed `{ top: rect.bottom + 4, left: Math.max(8, rect.right - 240) }` with no viewport-overflow check. The flame icon sits at the bottom of the AMS info section on the printer card, so on most realistic viewports `rect.bottom + 4 + popover_height(~320px) > viewport.height` and the popover rendered partially or entirely off-screen. Fix extracts a `computePopoverPosition()` helper in `frontend/src/utils/popoverPosition.ts` that defaults to placing the popover below + right-aligned to the trigger (preserving the original visual layout), flips ABOVE the trigger when below would overflow AND above would fit, stays below in the degraded case where neither fits (popover taller than viewport — at least the top is visible and the user can scroll inside), and clamps the left coordinate so a trigger near either viewport edge can't push the popover off-screen horizontally either. Both PrintersPage callsites now go through the helper. **(2) Diagnostic logging for the silent-drying-ignore**: reporter's support bundle showed the printer receives every `ams_filament_drying` command (multiple start / stop attempts on `ams_id=128`, P1S 01.10.00.00 firmware), the printer ACKs each one, but the AMS info field never changes — drying neither starts nor stops on Bambuddy's request, while pressing Start on the printer's touchscreen worked immediately (so the hardware path is healthy and the LAN MQTT channel is delivering). The Bambuddy command JSON matches the format documented as working on H2D, all required fields are present, types match BambuStudio. Diagnosing the silent rejection needs the printer's actual response payload — whether `result: "fail"` and the specific `reason` code — but `bambu_mqtt.py:918` was only logging the response *command name*, not the body. The existing `extrusion_cali_*` / `ams_filament_setting` debug path at `:919-920` was the template; this PR extends it to `ams_filament_drying` at **INFO level** specifically (not DEBUG like its siblings) because drying responses are rare — user-initiated only — and INFO ensures the body lands in support bundles by default without needing the user to bump log level first. Paired with a matching outgoing-side INFO log inside `send_drying_command` that captures the full wire JSON, so the next support bundle has **both halves of the conversation**. The actual command-side fix can't happen without that data (no guessing — flipping `close_power_conflict: true` or otherwise mutating a field that matches the documented-working H2D shape could break currently-working installs). When kleinweby retries on this build and re-attaches a bundle, the rejection reason is visible and the command-side fix follows from real data. **Tests** (8 new in `__tests__/utils/popoverPosition.test.ts`): below-has-room places below; right-align to trigger; below overflows flips above; degraded case stays below; clamps right-edge and left-edge triggers; respects custom margin and gap. 276 backend service tests + frontend build clean.
|
||||
|
||||
- **Stats: Print Activity heatmap buckets prints by local date, not UTC date (#1446, reported and root-caused by @needo37)** — Reporter on CDT (UTC-5) noticed that prints finished in the local evening were jumping to "tomorrow's" cell on the GitHub-style contribution heatmap on the Stats page. He went through `frontend/src/components/PrintCalendar.tsx` and identified the root cause: line 30 split the raw ISO string on `'T'` to get a YYYY-MM-DD key, which always returns the **UTC** date — but the cell tooltip (line 161) rendered via `toLocaleDateString()`, which is **local-tz aware**. Same data, two renderers, only one was tz-correct. He confirmed with DB query: rows 29 and 30 stored as `2026-05-18 ... UTC` were both local `May 17` (20:46 CDT and 22:39 CDT), and the Archives → Print Log view formatted them correctly as May 17 via `toLocaleString()` while the heatmap split them onto May 18 via the raw-ISO shortcut. The component had two more instances of the same shape that I caught while applying the fix: line 152 built the per-cell lookup key via `day.toISOString().split('T')[0]` (the `day` Date objects produced by the calendar-generation loop are local-tz constructed via `new Date()` + `setDate`, so `toISOString()` shifted them back to UTC before the lookup — would have re-broken the join even after the bucketing fix), and line 154's "today" highlight comparison used `new Date().toISOString().split('T')[0]` too (so at e.g. 23:00 CDT the heatmap would have ringed UTC-tomorrow's cell instead of local-today's). **Fix** adds a `localDateKey(input: string | Date): string` helper in `frontend/src/utils/date.ts` that wraps `parseUTCDate()` and formats via the local-tz getters (`getFullYear` / `getMonth` / `getDate` with two-digit padding), returning a stable comparable YYYY-MM-DD string. PrintCalendar.tsx uses it in all three spots — bucket key for input ISO strings, grid-cell lookup key, and "today" highlight — so the bucketing, the cell join, and the today ring all live on the same local-tz axis as the user's tooltip label. Backend stays UTC (`PrintLogEntry.created_at` unchanged); bucketing is a presentation concern and the browser already knows the user's tz. The reporter's broader point ("same fix needed anywhere else the frontend buckets timestamps to days") still has stragglers — `StatsPage.tsx:55-84` (computeDateRange) builds the dateFrom/dateTo strings for backend stats queries using `getUTC*` getters everywhere, so a "this week" picked at 23:00 local on Sunday in CDT sends UTC-Monday-based ranges to the backend; that's a separate, deeper bug because it also requires the backend to filter on a tz-shifted UTC range, and Bambuddy has no user-tz setting model today. Punted with a `localDateKey` helper available for reuse when that work lands. **Tests** (5 new in `__tests__/utils/date.test.ts`): keys a local-evening Date to its local date (the bug repro), reproduces the reporter's row-30 case (a moment whose UTC date is "tomorrow" keys to local "today"), pads single-digit month/day, handles null / undefined / empty defensively, and accepts both Date and ISO-string inputs end-to-end via `parseUTCDate`. 74 date-util tests green; frontend build clean. Tests are written tz-independently — they construct `new Date(2026, 4, 17, 22, 0, 0)` via the local-time constructor form so they assert correctly regardless of which tz the CI runner happens to be in.
|
||||
|
||||
|
||||
@@ -918,6 +918,12 @@ class BambuMQTTClient:
|
||||
logger.debug("[%s] Received command response: %s", self.serial_number, cmd)
|
||||
if cmd in ("extrusion_cali_sel", "extrusion_cali_set", "extrusion_cali_del", "ams_filament_setting"):
|
||||
logger.debug("[%s] %s response: %s", self.serial_number, cmd, print_data)
|
||||
# AMS drying responses are rare (user-initiated only) and the
|
||||
# full payload — including `result` and any `reason` code —
|
||||
# is the only way to diagnose silent rejections like #1447.
|
||||
# INFO level so the body lands in support bundles by default.
|
||||
elif cmd == "ams_filament_drying":
|
||||
logger.info("[%s] ams_filament_drying response: %s", self.serial_number, print_data)
|
||||
# Check for developer mode probe response
|
||||
if (
|
||||
cmd == "ams_filament_setting"
|
||||
@@ -3703,14 +3709,17 @@ class BambuMQTTClient:
|
||||
"close_power_conflict": False,
|
||||
}
|
||||
}
|
||||
self._client.publish(self.topic_publish, json.dumps(command), qos=1)
|
||||
# Log the full wire JSON at INFO so support bundles capture exactly
|
||||
# what we sent — needed to diagnose silent rejections (#1447) where
|
||||
# the printer ACKs the command but never starts/stops drying.
|
||||
# Paired with the ams_filament_drying response-payload INFO log so
|
||||
# both halves of the conversation land in the bundle by default.
|
||||
wire_json = json.dumps(command)
|
||||
self._client.publish(self.topic_publish, wire_json, qos=1)
|
||||
logger.info(
|
||||
"[%s] Sent drying command: ams_id=%d, temp=%d, duration=%d, mode=%d",
|
||||
"[%s] Sent ams_filament_drying: %s",
|
||||
self.serial_number,
|
||||
ams_id,
|
||||
temp,
|
||||
duration,
|
||||
mode,
|
||||
wire_json,
|
||||
)
|
||||
return True
|
||||
|
||||
|
||||
Reference in New Issue
Block a user